Re: [alsa-devel] [PATCH v2] ALSA: hda: add Intel DSP configuration / probe code

[Jaroslav Kysela <perex@perex.cz>]

Dne 02. 10. 19 v 19:06 Pierre-Louis Bossart napsal(a):
> 
>> +module_param(dsp_driver, int, 0444);
>> +MODULE_PARM_DESC(dsp_driver, "Force the DSP driver for Intel DSP (0=auto, 1=noDSP, 2=legacy, 3=SST, 4=SOF)");
> 
> remove noDSP?
> 
>> +
>> +static const u16 sof_skl_table[] = {
>> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMETLAKE_LP)
>> +	0x02c8,	/* Cometlake-LP */
>> +#endif
>> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMETLAKE_H)
>> +	0x06c8,	/* Cometlake-H */
>> +#endif
>> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_GEMINILAKE)
>> +	0x3198,	/* Geminilake */
>> +#endif
>> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_CANNONLAKE)
>> +	0x9dc8, /* Cannonlake */
>> +#endif
>> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_COFFEELAKE)
>> +	0xa348, /* Coffelake */
>> +#endif
> 
> I am lost here, is this to handle the cases between legacy and SOF? If 
> yes, we need to add all PCI IDs after gemini lake.
> 
>> +};
>> +
>> +static int snd_intel_dsp_check_device(u16 device, const u16 *table, u32 len)
>> +{
>> +	for (; len > 0; len--, table++) {
>> +		if (*table == device)
>> +			return 1;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static int snd_intel_dsp_check_dmic(struct pci_dev *pci)
>> +{
>> +	struct nhlt_acpi_table *nhlt;
>> +	int ret = 0;
>> +
>> +	if (snd_intel_dsp_check_device(pci->device, sof_skl_table, ARRAY_SIZE(sof_skl_table))) {
>> +		nhlt = intel_nhlt_init(&pci->dev);
>> +		if (nhlt) {
>> +			if (intel_nhlt_get_dmic_geo(&pci->dev, nhlt))
>> +				ret = 1;
>> +			intel_nhlt_free(nhlt);
>> +		}
>> +	}
>> +	return ret;
>> +}
>> +
>> +int snd_intel_dsp_driver_probe(struct pci_dev *pci)
>> +{
>> +	if (dsp_driver > 0 && dsp_driver <= SND_INTEL_DSP_DRIVER_LAST)
>> +		return dsp_driver;
>> +
>> +	/* Intel vendor only */
>> +	if (snd_BUG_ON(pci->vendor != 0x8086))
>> +		return SND_INTEL_DSP_DRIVER_ANY;
>> +
>> +	/*
>> +	 * 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)
>> +		return SND_INTEL_DSP_DRIVER_LEGACY;
>> +	if (pci->class != 0x040100 && pci->class != 0x040380) {
>> +		dev_err(&pci->dev, "Unknown PCI class/subclass/prog-if information (0x%06x) found, selecting HDA legacy driver\n", pci->class);
>> +		return SND_INTEL_DSP_DRIVER_LEGACY;
>> +	}
>> +
>> +	dev_info(&pci->dev, "DSP detected with PCI class/subclass/prog-if info 0x%06x\n", pci->class);
>> +
>> +	/* Prefer SST driver for Broxton-P (Appololake) */
>> +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_APL) || IS_ENABLED(CONFIG_SND_SOC_SOF_APOLLOLAKE)
>> +	if (pci->device == 0x5a98)
>> +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_APL)
>> +		return SND_INTEL_DSP_DRIVER_SST;
>> +#else
>> +		return SND_INTEL_DSP_DRIVER_SOF;
>> +#endif
>> +#endif
> 
> This should be done with quirks, where we select SST for Chromebooks for 
> APL, SKL, KBL.
> 
> Using the PCI ID + class only will break at least one device, I remotely 
> debugged Hans de Goede's APL platform and saw the same issues with HDMI 
> probe as on Linus' laptop.
> 
> I am running out of time for this week, so will check updates next week.

I reworked the logic in the code (v3 of the patch) to use one table with the
configuration entries for all platforms including dmi based quirks. Please,
review it, if you see any problems there. I believe that we are reaching
really nice and clean solution for this problem.

				Thank you,
					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel