Dne 07. 10. 19 v 16:05 Pierre-Louis Bossart napsal(a):
On 10/6/19 10:22 AM, Jaroslav Kysela wrote:
For distributions, we need one place where we can decide which driver will be activated for the auto-configation of the Intel's HDA hardware with DSP. Actually, we cover three drivers:
- Legacy HDA
- Intel SST
- Intel Sound Open Firmware (SOF)
All those drivers registers similar PCI IDs, so the first driver probed from the PCI stack can win. But... it is not guaranteed that the correct driver wins.
This commit changes Intel's NHLT ACPI module to a common DSP probe module for the Intel's hardware. All above sound drivers calls this code. The user can force another behaviour using the module parameter 'dsp_driver' located in the 'snd-intel-dspcfg' module.
This change allows to add specific dmi checks for the specific systems. The examples are taken from the pull request:
https://github.com/thesofproject/linux/pull/927
Tested on Lenovo Carbon X1 7th gen.
Thanks Jaroslav, I like the ideas in the patch, the flags+DMI table is quite elegant.
We will need to do additional checks for the quirks, e.g. I know there is a 'Phaser' Chromebook with HDaudio and I don't recall if they use DMICs. I also don't know if we always have the NHTL information when the legacy BIOS is used.
It's likely that we will have multiple iterations before getting this right. And we'll have to add SoundWire support as well (which isn't that hard, I already have all the ACPI parsing needed to detect if a link is enabled).
Some additional comments below.
+module_param(dsp_driver, int, 0444); +MODULE_PARM_DESC(dsp_driver, "Force the DSP driver for Intel DSP (0=auto, 1=legacy, 2=SST, 3=SOF)");
+#define FLAG_SST (1<<0) +#define FLAG_SOF (1<<1) +#define FLAG_SOF_ONLY_IF_DMIC (1<<16)
+struct config_entry {
- u32 flags;
- u16 device;
- const struct dmi_system_id *dmi_table;
+};
+/*
- configuration table - the order of entries is important!
It's not really the order but the exclusion between SST and SOF for the same PCI ID?
Yes, I'll rephrase this as:
* configuration table * - the order of similar PCI ID entries is important! * - the first successful match will win
- */
+static const struct config_entry config_table[] = { +/* Cometlake-LP */ +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CML_LP)
- {
/* prefer SST */.flags = FLAG_SST,.device = 0x02c8,- },
+#elif IS_ENABLED(CONFIG_SND_SOC_SOF_COMETLAKE_LP)
- {
.flags = FLAG_SOF,need to add FLAG_SOF_ONLY_IF_DMIC
Aaghrr. We have 0x02C8 not 0x02c8 in hda_intel.c so I didn't found them using the case-sensitive grep. I will fix that.
.device = 0x02c8,- },
+#endif +/* Cometlake-H */ +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CML_H)
- {
.flags = FLAG_SST,.device = 0x06c8,- },
+#elif IS_ENABLED(CONFIG_SND_SOC_SOF_COMETLAKE_H)
- {
.flags = FLAG_SOF,| FLAG_SOF_ONLY_IF_DMIC
.device = 0x06c8,- },
+#endif +/* Merrifield */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_MERRIFIELD)
- {
.flags = FLAG_SOF,.device = 0x119a,- },
+#endif +/* Broxton-T */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_APOLLOLAKE)
- {
.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC,.device = 0x1a98,- },
+#endif +/* Geminilake */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_GEMINILAKE)
- {
.flags = FLAG_SOF,can we have more than one table per PCI ID? e.g. in this case it'd be good to have the DMIC case separate from Google.
Yes, first match wins. So we need to add flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC entry bellow the dmi exceptions for device == 0x3198, too?
.device = 0x3198,.dmi_table = (const struct dmi_system_id []) {{.ident = "Google Chromebooks",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "Google"),}},{}}- },
+#endif +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_GLK)
should it be elif, as done for CometLake/CML?
I though that the SST driver is the default for 0x3198. Or the legacy driver is in the game, too? If yes, we need to add conditional SST entries.
- {
.flags = FLAG_SST,.device = 0x3198,- },
+#endif +/* Icelake */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_ICELAKE)
- {
.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC,.device = 0x34c8,- },
+#endif +/* Elkhart Lake */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_ELKHARTLAKE)
- {
.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC,.device = 0x4b55,- },
+#endif +/* Appololake (Broxton-P) */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_APOLLOLAKE)
- {
.flags = FLAG_SOF,.device = 0x5a98,.dmi_table = (const struct dmi_system_id []) {{.ident = "Up Squared",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "AAEON"),DMI_MATCH(DMI_BOARD_NAME, "UP-APL01"),}},{}}- },
+#endif +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_APL)
elif?
Same. What's the default driver for APL?
- {
.flags = FLAG_SST,.device = 0x5a98,- },
+#endif +/* Cannonlake */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_CANNONLAKE)
- {
.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC,.device = 0x9dc8,- },
+#endif +/* Sunrise Point-LP */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_SKYLAKE)
- {
.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC,.device = 0x9d70,- },
+#endif +/* Kabylake-LP */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_KABYLAKE)
- {
.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC,.device = 0x9d71,- },
+#endif
SKL and SKL are not supported by SOF for now, and SST doesn't support HDaudio+DMIC combinations
This should be FLAG_SST but only for Google Chromebooks, e.g.
.flags = FLAG_SST, .device = 0x9d70 or 0x9d71 .dmi_table = (const struct dmi_system_id []) { { .ident = "Google Chromebooks", .matches = { DMI_MATCH(DMI_SYS_VENDOR, "Google"), } }, {} }},
Added. Will be in v4. Thanks for all of the input.
Jaroslav