Skip to content

Fix/legacy sst coexistence #480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
plbossart wants to merge 5 commits into from
Closed

Fix/legacy sst coexistence #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2bf7e58
ALSA: HD-Audio: SKL+: abort probe if DSP is present and Skylake drive…
plbossart Dec 15, 2018
ff215d0
ALSA: HD-Audio: SKL+: force HDaudio legacy or SKL+ driver selection
plbossart Dec 15, 2018
5510151
ASoC: SOF: Intel: add placeholder for CoffeeLake
plbossart Dec 21, 2018
50a6888
ASoC: SOF: Intel: enable HDaudio legacy DSP detection
plbossart Dec 21, 2018
a74e976
ASoC: Intel: Add mutual exclusion with SOF drivers
plbossart Dec 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions include/sound/hdaudio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,12 @@ enum {
HDA_DEV_ASOC,
};

enum {
SND_SKL_PCI_BIND_AUTO, /* automatic selection based on pci class */
SND_SKL_PCI_BIND_LEGACY,/* bind only with legacy driver */
SND_SKL_PCI_BIND_ASOC /* bind only with ASoC driver */
};

/* direction */
enum {
HDA_INPUT, HDA_OUTPUT
Expand Down
62 changes: 62 additions & 0 deletions sound/pci/hda/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,68 @@ config SND_HDA_POWER_SAVE_DEFAULT
The default time-out value in seconds for HD-audio automatic
power-save mode. 0 means to disable the power-save mode.

if SND_HDA_INTEL

# The options below should not be enabled by distributions or
# users. They are selected by Intel/Skylake or SOF drivers when they
# register for a PCI ID which is also handled by the HDAudio legacy
# driver. When this option is selected and the DSP is detected based on
# the PCI class/subclass/prog-if, the probe of the HDAudio legacy
# aborts. This mechanism removes the need for distributions to use
# blacklists. It can be bypassed with module parameters should the
# Intel/Skylake or SOF drivers fail to handle a specific platform.

config SND_HDA_INTEL_DSP_DETECTION_SKL
bool
help

@lyakh lyakh Jan 4, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it be better to hide these Kconfig entries by removing their help entries, if they are anyway not for humans to toggle?

@plbossart plbossart Jan 4, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are different opinions on this. I used comments in the past but was asked to add a help message.

@plbossart plbossart Jan 4, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don't like the fact that commit messages are out of scope in github. Thanks for the typo report.

This option is selected by SOF or SST drivers, not users or distros.
It enables DSP detection based on PCI class information for
Skylake machines.

config SND_HDA_INTEL_DSP_DETECTION_APL
bool
help
This option is selected by SOF or SST drivers, not users or distros.
It enables DSP detection based on PCI class information for
Broxton/ApolloLake machines

config SND_HDA_INTEL_DSP_DETECTION_KBL
bool
help
This option is selected by SOF or SST drivers, not users or distros.
It enables DSP detection based on PCI class information for
KabyLake machines

config SND_HDA_INTEL_DSP_DETECTION_GLK
bool
help
This option is selected by SOF or SST drivers, not users or distros.
It enables DSP detection based on PCI class information for
GeminiLake machines

config SND_HDA_INTEL_DSP_DETECTION_CNL
bool
help
This option is selected by SOF or SST drivers, not users or distros.
It enables DSP detection based on PCI class information for
CannonLake machines

config SND_HDA_INTEL_DSP_DETECTION_CFL
bool
help
This option is selected by SOF or SST drivers, not users or distros.
It enables DSP detection based on PCI class information for
CoffeeLake machines

config SND_HDA_INTEL_DSP_DETECTION_ICL
bool
help
This option is selected by SOF or SST drivers, not users or distros.
It enables DSP detection based on PCI class information for
IceLake machines

endif ## SND_HDA_INTEL

endif

endmenu
2 changes: 1 addition & 1 deletion sound/pci/hda/hda_controller.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
#else
#define AZX_DCAPS_I915_COMPONENT 0 /* NOP */
#endif
/* 14 unused */
#define AZX_DCAPS_INTEL_SHARED (1 << 14) /* shared with ASoC */
#define AZX_DCAPS_CTX_WORKAROUND (1 << 15) /* X-Fi workaround */
#define AZX_DCAPS_POSFIX_LPIB (1 << 16) /* Use LPIB as default */
/* 17 unused */
Expand Down
54 changes: 47 additions & 7 deletions sound/pci/hda/hda_intel.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ module_param_array(beep_mode, bool, NULL, 0444);
MODULE_PARM_DESC(beep_mode, "Select HDA Beep registration mode "
"(0=off, 1=on) (default=1).");
#endif
static int skl_pci_binding;
module_param_named(pci_binding, skl_pci_binding, int, 0444);
MODULE_PARM_DESC(pci_binding, "PCI binding (0=auto, 1=only legacy, 2=only asoc");

#ifdef CONFIG_PM
static int param_set_xint(const char *val, const struct kernel_param *kp);
Expand Down Expand Up @@ -357,6 +360,7 @@ enum {
AZX_DCAPS_NO_64BIT |\
AZX_DCAPS_4K_BDLE_BOUNDARY | AZX_DCAPS_SNOOP_OFF)

#define AZX_DCAPS_INTEL_DSP_DETECTION(conf) (IS_ENABLED(CONFIG_SND_HDA_INTEL_DSP_DETECTION_##conf) ? AZX_DCAPS_INTEL_SHARED : 0)
/*
* vga_switcheroo support
*/
Expand Down Expand Up @@ -2130,6 +2134,28 @@ static int azx_probe(struct pci_dev *pci,
bool schedule_probe;
int err;

/* check if this driver can be used on SKL+ Intel platforms */
if (pci_id->driver_data & AZX_DCAPS_INTEL_SHARED) {
switch (skl_pci_binding) {
case SND_SKL_PCI_BIND_AUTO:
if (pci->class != 0x040300) {
dev_info(&pci->dev, "The DSP is enabled on this platform, aborting probe\n");
return -ENODEV;
}
dev_info(&pci->dev, "No DSP detected, continuing HDaudio legacy probe\n");
break;
case SND_SKL_PCI_BIND_LEGACY:
dev_info(&pci->dev, "Module parameter forced binding with HDaudio legacy, bypassed detection logic\n");
break;
case SND_SKL_PCI_BIND_ASOC:
dev_info(&pci->dev, "Module parameter forced binding with SKL+ ASoC driver, aborting probe\n");
return -ENODEV;
default:
dev_err(&pci->dev, "invalid value for skl_pci_binding module parameter, ignored\n");

@lyakh lyakh Jan 4, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't ignoring an invalid value be equivalent to automatic detection, to also fall back to the pre-patch behaviour? Similar for the ASoC code below.

@plbossart plbossart Jan 4, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get your point. In both cases an invalid module parameter is ignored and the previous behavior is used.

break;
}
}

if (dev >= SNDRV_CARDS)
return -ENODEV;
if (!enable[dev]) {
Expand Down Expand Up @@ -2430,34 +2456,48 @@ static const struct pci_device_id azx_ids[] = {
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
/* Sunrise Point-LP */
{ PCI_DEVICE(0x8086, 0x9d70),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
AZX_DCAPS_INTEL_DSP_DETECTION(SKL)
},
/* Kabylake */
{ PCI_DEVICE(0x8086, 0xa171),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
/* Kabylake-LP */
{ PCI_DEVICE(0x8086, 0x9d71),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
AZX_DCAPS_INTEL_DSP_DETECTION(KBL)
},
/* Kabylake-H */
{ PCI_DEVICE(0x8086, 0xa2f0),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
/* Coffelake */
{ PCI_DEVICE(0x8086, 0xa348),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE},
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
AZX_DCAPS_INTEL_DSP_DETECTION(CFL)
},
/* Cannonlake */
{ PCI_DEVICE(0x8086, 0x9dc8),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE},
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
AZX_DCAPS_INTEL_DSP_DETECTION(CNL)
},
/* Icelake */
{ PCI_DEVICE(0x8086, 0x34c8),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE},
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
AZX_DCAPS_INTEL_DSP_DETECTION(ICL)
},
/* Broxton-P(Apollolake) */
{ PCI_DEVICE(0x8086, 0x5a98),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON |
AZX_DCAPS_INTEL_DSP_DETECTION(APL)
},
/* Broxton-T */
{ PCI_DEVICE(0x8086, 0x1a98),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
/* Gemini-Lake */
{ PCI_DEVICE(0x8086, 0x3198),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON |
AZX_DCAPS_INTEL_DSP_DETECTION(GLK)
},
/* Haswell */
{ PCI_DEVICE(0x8086, 0x0a0c),
.driver_data = AZX_DRIVER_HDMI | AZX_DCAPS_INTEL_HASWELL },
Expand Down
12 changes: 12 additions & 0 deletions sound/soc/intel/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ config SND_SOC_INTEL_SKYLAKE
config SND_SOC_INTEL_SKL
tristate "Skylake Platforms"
depends on PCI && ACPI
depends on SND_SOC_SOF_SKYLAKE=n
select SND_SOC_INTEL_SKYLAKE_FAMILY
help
If you have a Intel Skylake platform with the DSP enabled
Expand All @@ -128,6 +129,7 @@ config SND_SOC_INTEL_SKL
config SND_SOC_INTEL_APL
tristate "Broxton/ApolloLake Platforms"
depends on PCI && ACPI
depends on SND_SOC_SOF_APOLLOLAKE=n
select SND_SOC_INTEL_SKYLAKE_FAMILY
help
If you have a Intel Broxton/ApolloLake platform with the DSP
Expand All @@ -136,6 +138,7 @@ config SND_SOC_INTEL_APL
config SND_SOC_INTEL_KBL
tristate "Kabylake Platforms"
depends on PCI && ACPI
depends on SND_SOC_SOF_KABYLAKE=n
select SND_SOC_INTEL_SKYLAKE_FAMILY
help
If you have a Intel Kabylake platform with the DSP
Expand All @@ -144,6 +147,7 @@ config SND_SOC_INTEL_KBL
config SND_SOC_INTEL_GLK
tristate "GeminiLake Platforms"
depends on PCI && ACPI
depends on SND_SOC_SOF_GEMINILAKE=n
select SND_SOC_INTEL_SKYLAKE_FAMILY
help
If you have a Intel GeminiLake platform with the DSP
Expand All @@ -152,6 +156,7 @@ config SND_SOC_INTEL_GLK
config SND_SOC_INTEL_CNL
tristate "CannonLake/WhiskyLake Platforms"
depends on PCI && ACPI
depends on SND_SOC_SOF_CANNONLAKE=n
select SND_SOC_INTEL_SKYLAKE_FAMILY
help
If you have a Intel CNL/WHL platform with the DSP
Expand All @@ -160,6 +165,7 @@ config SND_SOC_INTEL_CNL
config SND_SOC_INTEL_CFL
tristate "CoffeeLake Platforms"
depends on PCI && ACPI
depends on SND_SOC_SOF_COFFEELAKE=n
select SND_SOC_INTEL_SKYLAKE_FAMILY
help
If you have a Intel CoffeeLake platform with the DSP
Expand Down Expand Up @@ -188,6 +194,12 @@ config SND_SOC_INTEL_SKYLAKE_COMMON
select SND_SOC_TOPOLOGY
select SND_SOC_INTEL_SST
select SND_SOC_HDAC_HDA if SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC
select SND_HDA_INTEL_DSP_DETECTION_SKL if SND_SOC_INTEL_SKL
select SND_HDA_INTEL_DSP_DETECTION_APL if SND_SOC_INTEL_APL
select SND_HDA_INTEL_DSP_DETECTION_KBL if SND_SOC_INTEL_KBL
select SND_HDA_INTEL_DSP_DETECTION_GLK if SND_SOC_INTEL_GLK
select SND_HDA_INTEL_DSP_DETECTION_CNL if SND_SOC_INTEL_CNL
select SND_HDA_INTEL_DSP_DETECTION_CFL if SND_SOC_INTEL_CFL
select SND_SOC_ACPI_INTEL_MATCH
help
If you have a Intel Skylake/Broxton/ApolloLake/KabyLake/
Expand Down
48 changes: 33 additions & 15 deletions sound/soc/intel/skylake/skl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
#if IS_ENABLED(CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC)
#include "../../../soc/codecs/hdac_hda.h"
#endif
static int skl_pci_binding;
module_param_named(pci_binding, skl_pci_binding, int, 0444);
MODULE_PARM_DESC(pci_binding, "PCI binding (0=auto, 1=only legacy, 2=only asoc");

/*
* initialize the PCI registers
Expand Down Expand Up @@ -896,21 +899,6 @@ static int skl_first_init(struct hdac_bus *bus)
unsigned short gcap;
int cp_streams, pb_streams, start_idx;

/*
* detect DSP by checking class/subclass/prog-id information
* class=04 subclass 03 prog-if 00: no DSP, legacy driver needs to be used
* class=04 subclass 01 prog-if 00: DSP is present (and may be required e.g. for DMIC or SSP support)
* class=04 subclass 03 prog-if 80: either of DSP or legacy mode can be used
*/
if (pci->class == 0x040300) {
dev_err(bus->dev, "The DSP is not enabled on this platform, aborting probe\n");
return -ENODEV;
} else if (pci->class != 0x040100 && pci->class != 0x040380) {
dev_err(bus->dev, "Unknown PCI class/subclass/prog-if information (0x%06x) found, aborting probe\n", pci->class);
return -ENODEV;
}
dev_info(bus->dev, "DSP detected with PCI class/subclass/prog-if info 0x%06x\n", pci->class);

err = pci_request_regions(pci, "Skylake HD audio");
if (err < 0)
return err;
Expand Down Expand Up @@ -984,6 +972,36 @@ static int skl_probe(struct pci_dev *pci,
struct hdac_bus *bus = NULL;
int err;

switch (skl_pci_binding) {
case SND_SKL_PCI_BIND_AUTO:
/*
* detect DSP by checking class/subclass/prog-id information
* class=04 subclass 03 prog-if 00: no DSP, use legacy driver
* class=04 subclass 01 prog-if 00: DSP is present
* (and may be required e.g. for DMIC or SSP support)
* class=04 subclass 03 prog-if 80: use DSP or legacy mode
*/
if (pci->class == 0x040300) {
dev_info(&pci->dev, "The DSP is not enabled on this platform, aborting probe\n");
return -ENODEV;
}
if (pci->class != 0x040100 && pci->class != 0x040380) {
dev_err(&pci->dev, "Unknown PCI class/subclass/prog-if information (0x%06x) found, aborting probe\n", pci->class);
return -ENODEV;
}
dev_info(&pci->dev, "DSP detected with PCI class/subclass/prog-if info 0x%06x\n", pci->class);
break;
case SND_SKL_PCI_BIND_LEGACY:
dev_info(&pci->dev, "Module parameter forced binding with HDaudio legacy, aborting probe\n");
return -ENODEV;
case SND_SKL_PCI_BIND_ASOC:
dev_info(&pci->dev, "Module parameter forced binding with SKL driver, bypassed detection logic\n");
break;
default:
dev_err(&pci->dev, "invalid value for skl_pci_binding module parameter, ignored\n");
break;
}

/* we use ext core ops, so provide NULL for ops here */
err = skl_create(pci, NULL, &skl);
if (err < 0)
Expand Down
23 changes: 23 additions & 0 deletions sound/soc/sof/intel/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ config SND_SOC_SOF_INTEL_PCI
select SND_SOC_SOF_APOLLOLAKE if SND_SOC_SOF_APOLLOLAKE_SUPPORT
select SND_SOC_SOF_GEMINILAKE if SND_SOC_SOF_GEMINILAKE_SUPPORT
select SND_SOC_SOF_CANNONLAKE if SND_SOC_SOF_CANNONLAKE_SUPPORT
select SND_SOC_SOF_COFFEELAKE if SND_SOC_SOF_COFFEELAKE_SUPPORT
select SND_SOC_SOF_KABYLAKE if SND_SOC_SOF_KABYLAKE_SUPPORT
select SND_SOC_SOF_SKYLAKE if SND_SOC_SOF_SKYLAKE_SUPPORT
select SND_SOC_SOF_ICELAKE if SND_SOC_SOF_ICELAKE_SUPPORT
Expand Down Expand Up @@ -124,6 +125,7 @@ config SND_SOC_SOF_APOLLOLAKE_SUPPORT
config SND_SOC_SOF_APOLLOLAKE
tristate
select SND_SOC_SOF_HDA_COMMON
select SND_HDA_INTEL_DSP_DETECTION_APL if SND_HDA_INTEL
help
This option is not user-selectable but automagically handled by
'select' statements at a higher level
Expand All @@ -139,6 +141,7 @@ config SND_SOC_SOF_GEMINILAKE_SUPPORT
config SND_SOC_SOF_GEMINILAKE
tristate
select SND_SOC_SOF_HDA_COMMON
select SND_HDA_INTEL_DSP_DETECTION_GLK if SND_HDA_INTEL
help
This option is not user-selectable but automagically handled by
'select' statements at a higher level
Expand All @@ -154,6 +157,23 @@ config SND_SOC_SOF_CANNONLAKE_SUPPORT
config SND_SOC_SOF_CANNONLAKE
tristate
select SND_SOC_SOF_HDA_COMMON
select SND_HDA_INTEL_DSP_DETECTION_CNL if SND_HDA_INTEL
help
This option is not user-selectable but automagically handled by
'select' statements at a higher level

config SND_SOC_SOF_COFFEELAKE_SUPPORT
bool "SOF support for Coffeelake"
help
This adds support for Sound Open Firmware for Intel(R) platforms
using the Coffeelake processors.
Say Y if you have such a device.
If unsure select "N".

config SND_SOC_SOF_COFFEELAKE
tristate
select SND_SOC_SOF_HDA_COMMON
select SND_HDA_INTEL_DSP_DETECTION_CFL if SND_HDA_INTEL
help
This option is not user-selectable but automagically handled by
'select' statements at a higher level
Expand All @@ -169,6 +189,7 @@ config SND_SOC_SOF_KABYLAKE_SUPPORT
config SND_SOC_SOF_KABYLAKE
tristate
select SND_SOC_SOF_HDA_COMMON
select SND_HDA_INTEL_DSP_DETECTION_KBL if SND_HDA_INTEL
help
This option is not user-selectable but automagically handled by
'select' statements at a higher level
Expand All @@ -184,6 +205,7 @@ config SND_SOC_SOF_SKYLAKE_SUPPORT
config SND_SOC_SOF_SKYLAKE
tristate
select SND_SOC_SOF_HDA_COMMON
select SND_HDA_INTEL_DSP_DETECTION_SKL if SND_HDA_INTEL
help
This option is not user-selectable but automagically handled by
'select' statements at a higher level
Expand All @@ -199,6 +221,7 @@ config SND_SOC_SOF_ICELAKE_SUPPORT
config SND_SOC_SOF_ICELAKE
tristate
select SND_SOC_SOF_HDA_COMMON
select SND_HDA_INTEL_DSP_DETECTION_ICL if SND_HDA_INTEL
help
This option is not user-selectable but automagically handled by
'select' statements at a higher level
Expand Down
Loading