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 -- Jaroslav Kysela <perex@xxxxxxxx> Linux Sound Maintainer; ALSA Project; Red Hat, Inc. _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
