[alsa-devel] [PATCH v5 1/2] ALSA: hda: add Intel DSP configuration / probe code

[Jaroslav Kysela]

Dne 24. 10. 19 v 22:33 Pierre-Louis Bossart napsal(a):
> 
> 
> On 10/22/19 12:43 PM, 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.
> 
> Jaroslav, I found a case where the logic doesn't seem to work as intended.
> 
> the code currently scans through the table looking for a PCI ID match,
> and a DMI match for quirks.
> 
> On a KBL NUC device with the DSP enabled, we go through the entire list
> without finding a match, which means we return the _ANY flag. so if we
> try to probe the snd-soc-skl driver, the checks will allow the driver to
> continue, despite this configuration not being supported.
> 
> It was my understanding that we would always load the legacy driver
> *unless* there is a match in the table? If yes, there we can change the
> default this way:
> 
> @@ -333,7 +355,7 @@ int snd_intel_dsp_driver_probe(struct pci_dev *pci)
>           /* find the configuration for the specific device */
>           cfg = snd_intel_dsp_find_config(pci, config_table,
> ARRAY_SIZE(config_table));
>           if (!cfg)
> -               return SND_INTEL_DSP_DRIVER_ANY;
> +               return SND_INTEL_DSP_DRIVER_LEGACY;
> 
> Or we need to add a case where we force the legacy in the table for each
> platform.

I would prefer to add another entry to the configuration table without .flags 
(or .flags = 0) with a comment, why the legacy driver is selected by default 
even if the DSP is present.

				Thanks,
					Jaroslav

-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.