ASoC: Skylake SST driver - blacklist the PCI device IDs for the auto probe
diff mbox series

Message ID 20190923165739.3975-1-perex@perex.cz
State New
Headers show
Series
  • ASoC: Skylake SST driver - blacklist the PCI device IDs for the auto probe
Related show

Commit Message

Jaroslav Kysela Sept. 23, 2019, 4:57 p.m. UTC
There are basically three drivers for the PCI devices for
the recent Intel hardware with the build-in DSPs. The legacy HDA
driver has dmic_detect module option for the auto detection
of the platforms with the digital microphone. Because the SOF
driver is preferred, just skip PCI probe in the Skylake SST
driver when the PCI device ID clashes by default. The user
can override the auto behaviour with the pci_binding
module parameter.

Boot log from Lenovo Carbon X1 (7th gen) with the default settings:

  snd_hda_intel 0000:00:1f.3: Digital mics found on Skylake+ platform, aborting probe
  snd_soc_skl 0000:00:1f.3: SOF driver is preferred on this platform, aborting probe
  sof-audio-pci 0000:00:1f.3: warning: No matching ASoC machine driver found
  sof-audio-pci 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if 0x040380
  ....

Perhaps, it may be more wise to create one shared module and all
three drivers should call the driver detection routine(s) from one
place.

Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/skylake/skl.c | 37 +++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

Comments

Pierre-Louis Bossart Sept. 23, 2019, 6:24 p.m. UTC | #1
On 9/23/19 11:57 AM, Jaroslav Kysela wrote:
> There are basically three drivers for the PCI devices for
> the recent Intel hardware with the build-in DSPs. The legacy HDA
> driver has dmic_detect module option for the auto detection
> of the platforms with the digital microphone. Because the SOF
> driver is preferred, just skip PCI probe in the Skylake SST
> driver when the PCI device ID clashes by default. The user
> can override the auto behaviour with the pci_binding
> module parameter.

Thanks Jaroslav for re-opening this mutual-exclusion issue.

I think we want to deal with this in two alternate ways
1. static built-time exclusion based on Kconfigs
2. probe-time exclusion based on quirks (CPU ID + DMI)

For example with a SKL/KBL/APL chromebook w/ DMIC we'd want to use the 
SST driver and for GLK+ we want to use SOF. For any device with 
HDAudio+DMIC we'd want SOF, same for any device with SoundWire when it's 
fully supported.

I can't recall if I shared the patches I worked on a couple of months 
ago, but they are still at https://github.com/thesofproject/linux/pull/927

the first part essentially does the same thing as this patch, the second 
relies on quirks. I've been busy with other things but indeed it's high 
time we closed this for distributions.

> Boot log from Lenovo Carbon X1 (7th gen) with the default settings:
> 
>    snd_hda_intel 0000:00:1f.3: Digital mics found on Skylake+ platform, aborting probe
>    snd_soc_skl 0000:00:1f.3: SOF driver is preferred on this platform, aborting probe
>    sof-audio-pci 0000:00:1f.3: warning: No matching ASoC machine driver found
>    sof-audio-pci 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if 0x040380
>    ....
> 
> Perhaps, it may be more wise to create one shared module and all
> three drivers should call the driver detection routine(s) from one
> place.

We did look into this and it's a bit complicated in terms of plumbing.

> 
> Signed-off-by: Jaroslav Kysela <perex@perex.cz>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>   sound/soc/intel/skylake/skl.c | 37 +++++++++++++++++++++++++++++++++++
>   1 file changed, 37 insertions(+)
> 
> diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
> index 141dbbf975ac..cace55ca8d55 100644
> --- a/sound/soc/intel/skylake/skl.c
> +++ b/sound/soc/intel/skylake/skl.c
> @@ -38,6 +38,39 @@ 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");
>   
> +/*
> + *
> + */
> +static int skl_sof_support(struct pci_dev *pci)
> +{
> +	/* the SOF driver has same PCI IDs */
> +	if (pci->vendor == 0x8086) {
> +		switch (pci->device) {
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMETLAKE_LP)
> +		case 0x02c8: /* CML-LP */
> +#endif
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMETLAKE_H)
> +		case 0x06c8: /* CML-H */
> +#endif
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_GEMINILAKE)
> +		case 0x3198: /* GLK */
> +#endif
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_APOLLOLAKE)
> +		case 0x5a98: /* BXT-P */
> +#endif
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_CANNONLAKE)
> +		case 0x9dc8: /* CNL */
> +#endif
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_COFFEELAKE)
> +		case 0xa348: /* CFL */
> +#endif
> +		case 0x0000: /* a dummy value when no SOF driver enabled */
> +			return 1;
> +		}
> +	}
> +	return 0;
> +}
> +
>   /*
>    * initialize the PCI registers
>    */
> @@ -1002,6 +1035,10 @@ static int skl_probe(struct pci_dev *pci,
>   			dev_err(&pci->dev, "Unknown PCI class/subclass/prog-if information (0x%06x) found, aborting probe\n", pci->class);
>   			return -ENODEV;
>   		}
> +		if (skl_sof_support(pci)) {
> +			dev_info(&pci->dev, "SOF driver is preferred on this platform, aborting probe\n");
> +			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:
>
Jaroslav Kysela Sept. 23, 2019, 8:35 p.m. UTC | #2
Dne 23. 09. 19 v 20:24 Pierre-Louis Bossart napsal(a):
> On 9/23/19 11:57 AM, Jaroslav Kysela wrote:
>> There are basically three drivers for the PCI devices for
>> the recent Intel hardware with the build-in DSPs. The legacy HDA
>> driver has dmic_detect module option for the auto detection
>> of the platforms with the digital microphone. Because the SOF
>> driver is preferred, just skip PCI probe in the Skylake SST
>> driver when the PCI device ID clashes by default. The user
>> can override the auto behaviour with the pci_binding
>> module parameter.
> 
> Thanks Jaroslav for re-opening this mutual-exclusion issue.
> 
> I think we want to deal with this in two alternate ways
> 1. static built-time exclusion based on Kconfigs

Unfortunately, that's really an issue for the universal distros.

> 2. probe-time exclusion based on quirks (CPU ID + DMI)
> 
> For example with a SKL/KBL/APL chromebook w/ DMIC we'd want to use the 
> SST driver and for GLK+ we want to use SOF. For any device with 
> HDAudio+DMIC we'd want SOF, same for any device with SoundWire when it's 
> fully supported.
> 
> I can't recall if I shared the patches I worked on a couple of months 
> ago, but they are still at https://github.com/thesofproject/linux/pull/927

Thanks for pointing me to this. It does not address the legacy HDA, but it's a
step forward.

> the first part essentially does the same thing as this patch, the second 
> relies on quirks. I've been busy with other things but indeed it's high 
> time we closed this for distributions.

Yes, and I have to say, it's too late for the hardware vendors right now. I
will probably apply my patch to our distribution (I don't care too much about
chromebooks - the user can change the module/driver behaviour manually) until
we have a better code.

>> Boot log from Lenovo Carbon X1 (7th gen) with the default settings:
>>
>>    snd_hda_intel 0000:00:1f.3: Digital mics found on Skylake+ platform, aborting probe
>>    snd_soc_skl 0000:00:1f.3: SOF driver is preferred on this platform, aborting probe
>>    sof-audio-pci 0000:00:1f.3: warning: No matching ASoC machine driver found
>>    sof-audio-pci 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if 0x040380
>>    ....
>>
>> Perhaps, it may be more wise to create one shared module and all
>> three drivers should call the driver detection routine(s) from one
>> place.
> 
> We did look into this and it's a bit complicated in terms of plumbing.

Could you elaborate more here? I believe that for the runtime environment
where all drivers are compiled in the kernel, it might make sense to have this
code at one place and installed only once for all three (or may be four in the
soundwire future) drivers.

We should have one straight way which driver/module is used. The separate
conditions in the mentioned drivers will cause problems. Also, it will
simplify things for the end user. One module parameter (in the driver selector
library) is better than three or four to make things working (if the DMI /
whatever table is not preset correctly for the new hardware).

				Thanks,
					Jaroslav
Takashi Iwai Sept. 23, 2019, 9:21 p.m. UTC | #3
On Mon, 23 Sep 2019 22:35:14 +0200,
Jaroslav Kysela wrote:
> 
> Dne 23. 09. 19 v 20:24 Pierre-Louis Bossart napsal(a):
> > On 9/23/19 11:57 AM, Jaroslav Kysela wrote:
> >> There are basically three drivers for the PCI devices for
> >> the recent Intel hardware with the build-in DSPs. The legacy HDA
> >> driver has dmic_detect module option for the auto detection
> >> of the platforms with the digital microphone. Because the SOF
> >> driver is preferred, just skip PCI probe in the Skylake SST
> >> driver when the PCI device ID clashes by default. The user
> >> can override the auto behaviour with the pci_binding
> >> module parameter.
> > 
> > Thanks Jaroslav for re-opening this mutual-exclusion issue.
> > 
> > I think we want to deal with this in two alternate ways
> > 1. static built-time exclusion based on Kconfigs
> 
> Unfortunately, that's really an issue for the universal distros.

Right.  The Kconfig of Intel audio is already too messy even for now.
We don't want more complexity just for covering some very corner
case.

Practically seen, if SOF Kconfig is enabled, we may assume that SOF is
preferred in general.  I don't think of any big need of yet another
static configuration.

> > 2. probe-time exclusion based on quirks (CPU ID + DMI)
> > 
> > For example with a SKL/KBL/APL chromebook w/ DMIC we'd want to use the 
> > SST driver and for GLK+ we want to use SOF. For any device with 
> > HDAudio+DMIC we'd want SOF, same for any device with SoundWire when it's 
> > fully supported.
> > 
> > I can't recall if I shared the patches I worked on a couple of months 
> > ago, but they are still at https://github.com/thesofproject/linux/pull/927
> 
> Thanks for pointing me to this. It does not address the legacy HDA, but it's a
> step forward.

The legacy HD-audio stuff is resolved with the recent DMIC detection
on 5.4, I suppose?

> > the first part essentially does the same thing as this patch, the second 
> > relies on quirks. I've been busy with other things but indeed it's high 
> > time we closed this for distributions.
> 
> Yes, and I have to say, it's too late for the hardware vendors right now. I
> will probably apply my patch to our distribution (I don't care too much about
> chromebooks - the user can change the module/driver behaviour manually) until
> we have a better code.
> 
> >> Boot log from Lenovo Carbon X1 (7th gen) with the default settings:
> >>
> >>    snd_hda_intel 0000:00:1f.3: Digital mics found on Skylake+ platform, aborting probe
> >>    snd_soc_skl 0000:00:1f.3: SOF driver is preferred on this platform, aborting probe
> >>    sof-audio-pci 0000:00:1f.3: warning: No matching ASoC machine driver found
> >>    sof-audio-pci 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if 0x040380
> >>    ....
> >>
> >> Perhaps, it may be more wise to create one shared module and all
> >> three drivers should call the driver detection routine(s) from one
> >> place.
> > 
> > We did look into this and it's a bit complicated in terms of plumbing.
> 
> Could you elaborate more here? I believe that for the runtime environment
> where all drivers are compiled in the kernel, it might make sense to have this
> code at one place and installed only once for all three (or may be four in the
> soundwire future) drivers.
> 
> We should have one straight way which driver/module is used. The separate
> conditions in the mentioned drivers will cause problems. Also, it will
> simplify things for the end user. One module parameter (in the driver selector
> library) is better than three or four to make things working (if the DMI /
> whatever table is not preset correctly for the new hardware).

Well, one question is where to put this option.  I thought of HD-audio
core in the past, but it's not always the common place any longer.
We may introduce yet another common module just for an option, but it
sounds little appealing to me in comparison with the needed
resources.

Basically the deployment of SST is only for the already existing
devices, and all newer should go for SOF.  And, the pattern Pierre
mentioned should cover almost all use cases.  This made me believing
that a simple switch is no mandatory request.

In anyway, Jaroslav's patch looks like a good starting point.  We can
build up a few more exceptions (SKl/KBl/APL Chromebooks with DMIC) on
top of it, then we've done mostly, right?


thanks,

Takashi
Pierre-Louis Bossart Sept. 23, 2019, 11:34 p.m. UTC | #4
On 9/23/19 4:21 PM, Takashi Iwai wrote:
> On Mon, 23 Sep 2019 22:35:14 +0200,
> Jaroslav Kysela wrote:
>>
>> Dne 23. 09. 19 v 20:24 Pierre-Louis Bossart napsal(a):
>>> On 9/23/19 11:57 AM, Jaroslav Kysela wrote:
>>>> There are basically three drivers for the PCI devices for
>>>> the recent Intel hardware with the build-in DSPs. The legacy HDA
>>>> driver has dmic_detect module option for the auto detection
>>>> of the platforms with the digital microphone. Because the SOF
>>>> driver is preferred, just skip PCI probe in the Skylake SST
>>>> driver when the PCI device ID clashes by default. The user
>>>> can override the auto behaviour with the pci_binding
>>>> module parameter.
>>>
>>> Thanks Jaroslav for re-opening this mutual-exclusion issue.
>>>
>>> I think we want to deal with this in two alternate ways
>>> 1. static built-time exclusion based on Kconfigs
>>
>> Unfortunately, that's really an issue for the universal distros.
> 
> Right.  The Kconfig of Intel audio is already too messy even for now.
> We don't want more complexity just for covering some very corner
> case.
> 
> Practically seen, if SOF Kconfig is enabled, we may assume that SOF is
> preferred in general.  I don't think of any big need of yet another
> static configuration.
> 
>>> 2. probe-time exclusion based on quirks (CPU ID + DMI)
>>>
>>> For example with a SKL/KBL/APL chromebook w/ DMIC we'd want to use the
>>> SST driver and for GLK+ we want to use SOF. For any device with
>>> HDAudio+DMIC we'd want SOF, same for any device with SoundWire when it's
>>> fully supported.
>>>
>>> I can't recall if I shared the patches I worked on a couple of months
>>> ago, but they are still at https://github.com/thesofproject/linux/pull/927
>>
>> Thanks for pointing me to this. It does not address the legacy HDA, but it's a
>> step forward.
> 
> The legacy HD-audio stuff is resolved with the recent DMIC detection
> on 5.4, I suppose?
> 
>>> the first part essentially does the same thing as this patch, the second
>>> relies on quirks. I've been busy with other things but indeed it's high
>>> time we closed this for distributions.
>>
>> Yes, and I have to say, it's too late for the hardware vendors right now. I
>> will probably apply my patch to our distribution (I don't care too much about
>> chromebooks - the user can change the module/driver behaviour manually) until
>> we have a better code.
>>
>>>> Boot log from Lenovo Carbon X1 (7th gen) with the default settings:
>>>>
>>>>     snd_hda_intel 0000:00:1f.3: Digital mics found on Skylake+ platform, aborting probe
>>>>     snd_soc_skl 0000:00:1f.3: SOF driver is preferred on this platform, aborting probe
>>>>     sof-audio-pci 0000:00:1f.3: warning: No matching ASoC machine driver found
>>>>     sof-audio-pci 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if 0x040380
>>>>     ....
>>>>
>>>> Perhaps, it may be more wise to create one shared module and all
>>>> three drivers should call the driver detection routine(s) from one
>>>> place.
>>>
>>> We did look into this and it's a bit complicated in terms of plumbing.
>>
>> Could you elaborate more here? I believe that for the runtime environment
>> where all drivers are compiled in the kernel, it might make sense to have this
>> code at one place and installed only once for all three (or may be four in the
>> soundwire future) drivers.
>>
>> We should have one straight way which driver/module is used. The separate
>> conditions in the mentioned drivers will cause problems. Also, it will
>> simplify things for the end user. One module parameter (in the driver selector
>> library) is better than three or four to make things working (if the DMI /
>> whatever table is not preset correctly for the new hardware).
> 
> Well, one question is where to put this option.  I thought of HD-audio
> core in the past, but it's not always the common place any longer.
> We may introduce yet another common module just for an option, but it
> sounds little appealing to me in comparison with the needed
> resources.
> 
> Basically the deployment of SST is only for the already existing
> devices, and all newer should go for SOF.  And, the pattern Pierre
> mentioned should cover almost all use cases.  This made me believing
> that a simple switch is no mandatory request.
> 
> In anyway, Jaroslav's patch looks like a good starting point.  We can
> build up a few more exceptions (SKl/KBl/APL Chromebooks with DMIC) on
> top of it, then we've done mostly, right?

Yes, there are only a handful of quirks really.
Jaroslav Kysela Sept. 24, 2019, 6:46 a.m. UTC | #5
Dne 24. 09. 19 v 1:34 Pierre-Louis Bossart napsal(a):
> On 9/23/19 4:21 PM, Takashi Iwai wrote:
>> On Mon, 23 Sep 2019 22:35:14 +0200,
>> Jaroslav Kysela wrote:
>>>
>>> Dne 23. 09. 19 v 20:24 Pierre-Louis Bossart napsal(a):
>>>> On 9/23/19 11:57 AM, Jaroslav Kysela wrote:
>>>>> There are basically three drivers for the PCI devices for
>>>>> the recent Intel hardware with the build-in DSPs. The legacy HDA
>>>>> driver has dmic_detect module option for the auto detection
>>>>> of the platforms with the digital microphone. Because the SOF
>>>>> driver is preferred, just skip PCI probe in the Skylake SST
>>>>> driver when the PCI device ID clashes by default. The user
>>>>> can override the auto behaviour with the pci_binding
>>>>> module parameter.
>>>>
>>>> Thanks Jaroslav for re-opening this mutual-exclusion issue.
>>>>
>>>> I think we want to deal with this in two alternate ways
>>>> 1. static built-time exclusion based on Kconfigs
>>>
>>> Unfortunately, that's really an issue for the universal distros.
>>
>> Right.  The Kconfig of Intel audio is already too messy even for now.
>> We don't want more complexity just for covering some very corner
>> case.
>>
>> Practically seen, if SOF Kconfig is enabled, we may assume that SOF is
>> preferred in general.  I don't think of any big need of yet another
>> static configuration.
>>
>>>> 2. probe-time exclusion based on quirks (CPU ID + DMI)
>>>>
>>>> For example with a SKL/KBL/APL chromebook w/ DMIC we'd want to use the
>>>> SST driver and for GLK+ we want to use SOF. For any device with
>>>> HDAudio+DMIC we'd want SOF, same for any device with SoundWire when it's
>>>> fully supported.
>>>>
>>>> I can't recall if I shared the patches I worked on a couple of months
>>>> ago, but they are still at https://github.com/thesofproject/linux/pull/927
>>>
>>> Thanks for pointing me to this. It does not address the legacy HDA, but it's a
>>> step forward.
>>
>> The legacy HD-audio stuff is resolved with the recent DMIC detection
>> on 5.4, I suppose?

Unfortunately not completely. The Broxton and Coffelake PCI IDs are also
shared by the SST / Legacy HDA drivers, so the universal kernel will be
confused (at least snd_hda_intel will be loaded as first).

>>>> the first part essentially does the same thing as this patch, the second
>>>> relies on quirks. I've been busy with other things but indeed it's high
>>>> time we closed this for distributions.
>>>
>>> Yes, and I have to say, it's too late for the hardware vendors right now. I
>>> will probably apply my patch to our distribution (I don't care too much about
>>> chromebooks - the user can change the module/driver behaviour manually) until
>>> we have a better code.
>>>
>>>>> Boot log from Lenovo Carbon X1 (7th gen) with the default settings:
>>>>>
>>>>>     snd_hda_intel 0000:00:1f.3: Digital mics found on Skylake+ platform, aborting probe
>>>>>     snd_soc_skl 0000:00:1f.3: SOF driver is preferred on this platform, aborting probe
>>>>>     sof-audio-pci 0000:00:1f.3: warning: No matching ASoC machine driver found
>>>>>     sof-audio-pci 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if 0x040380
>>>>>     ....
>>>>>
>>>>> Perhaps, it may be more wise to create one shared module and all
>>>>> three drivers should call the driver detection routine(s) from one
>>>>> place.
>>>>
>>>> We did look into this and it's a bit complicated in terms of plumbing.
>>>
>>> Could you elaborate more here? I believe that for the runtime environment
>>> where all drivers are compiled in the kernel, it might make sense to have this
>>> code at one place and installed only once for all three (or may be four in the
>>> soundwire future) drivers.
>>>
>>> We should have one straight way which driver/module is used. The separate
>>> conditions in the mentioned drivers will cause problems. Also, it will
>>> simplify things for the end user. One module parameter (in the driver selector
>>> library) is better than three or four to make things working (if the DMI /
>>> whatever table is not preset correctly for the new hardware).
>>
>> Well, one question is where to put this option.  I thought of HD-audio
>> core in the past, but it's not always the common place any longer.
>> We may introduce yet another common module just for an option, but it
>> sounds little appealing to me in comparison with the needed
>> resources.
>>
>> Basically the deployment of SST is only for the already existing
>> devices, and all newer should go for SOF.  And, the pattern Pierre
>> mentioned should cover almost all use cases.  This made me believing
>> that a simple switch is no mandatory request.
>>
>> In anyway, Jaroslav's patch looks like a good starting point.  We can
>> build up a few more exceptions (SKl/KBl/APL Chromebooks with DMIC) on
>> top of it, then we've done mostly, right?
> 
> Yes, there are only a handful of quirks really.

It seems like a not very rubust solution for me. If you don't like to have
a standalone module, the code might be included to all drivers, but we losethe
possibility to control the auto detection behaviour from the one place
(one module parameter).

Perhaps, we can rename snd_intel_nhlt() module and put the auto-detection code
there. It's required by all mentioned drivers, so the extra resources required
for this new code are minimal.

				Jaroslav
Takashi Iwai Sept. 24, 2019, 7:31 a.m. UTC | #6
On Tue, 24 Sep 2019 08:46:44 +0200,
Jaroslav Kysela wrote:
> 
> Dne 24. 09. 19 v 1:34 Pierre-Louis Bossart napsal(a):
> > On 9/23/19 4:21 PM, Takashi Iwai wrote:
> >> On Mon, 23 Sep 2019 22:35:14 +0200,
> >> Jaroslav Kysela wrote:
> >>>
> >>> Dne 23. 09. 19 v 20:24 Pierre-Louis Bossart napsal(a):
> >>>> On 9/23/19 11:57 AM, Jaroslav Kysela wrote:
> >>>>> There are basically three drivers for the PCI devices for
> >>>>> the recent Intel hardware with the build-in DSPs. The legacy HDA
> >>>>> driver has dmic_detect module option for the auto detection
> >>>>> of the platforms with the digital microphone. Because the SOF
> >>>>> driver is preferred, just skip PCI probe in the Skylake SST
> >>>>> driver when the PCI device ID clashes by default. The user
> >>>>> can override the auto behaviour with the pci_binding
> >>>>> module parameter.
> >>>>
> >>>> Thanks Jaroslav for re-opening this mutual-exclusion issue.
> >>>>
> >>>> I think we want to deal with this in two alternate ways
> >>>> 1. static built-time exclusion based on Kconfigs
> >>>
> >>> Unfortunately, that's really an issue for the universal distros.
> >>
> >> Right.  The Kconfig of Intel audio is already too messy even for now.
> >> We don't want more complexity just for covering some very corner
> >> case.
> >>
> >> Practically seen, if SOF Kconfig is enabled, we may assume that SOF is
> >> preferred in general.  I don't think of any big need of yet another
> >> static configuration.
> >>
> >>>> 2. probe-time exclusion based on quirks (CPU ID + DMI)
> >>>>
> >>>> For example with a SKL/KBL/APL chromebook w/ DMIC we'd want to use the
> >>>> SST driver and for GLK+ we want to use SOF. For any device with
> >>>> HDAudio+DMIC we'd want SOF, same for any device with SoundWire when it's
> >>>> fully supported.
> >>>>
> >>>> I can't recall if I shared the patches I worked on a couple of months
> >>>> ago, but they are still at https://github.com/thesofproject/linux/pull/927
> >>>
> >>> Thanks for pointing me to this. It does not address the legacy HDA, but it's a
> >>> step forward.
> >>
> >> The legacy HD-audio stuff is resolved with the recent DMIC detection
> >> on 5.4, I suppose?
> 
> Unfortunately not completely. The Broxton and Coffelake PCI IDs are also
> shared by the SST / Legacy HDA drivers, so the universal kernel will be
> confused (at least snd_hda_intel will be loaded as first).

Yes, that's a missing piece.  OTOH, the situation wrt these platforms
hasn't been changed, so it's no regression, at least.


> >>>> the first part essentially does the same thing as this patch, the second
> >>>> relies on quirks. I've been busy with other things but indeed it's high
> >>>> time we closed this for distributions.
> >>>
> >>> Yes, and I have to say, it's too late for the hardware vendors right now. I
> >>> will probably apply my patch to our distribution (I don't care too much about
> >>> chromebooks - the user can change the module/driver behaviour manually) until
> >>> we have a better code.
> >>>
> >>>>> Boot log from Lenovo Carbon X1 (7th gen) with the default settings:
> >>>>>
> >>>>>     snd_hda_intel 0000:00:1f.3: Digital mics found on Skylake+ platform, aborting probe
> >>>>>     snd_soc_skl 0000:00:1f.3: SOF driver is preferred on this platform, aborting probe
> >>>>>     sof-audio-pci 0000:00:1f.3: warning: No matching ASoC machine driver found
> >>>>>     sof-audio-pci 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if 0x040380
> >>>>>     ....
> >>>>>
> >>>>> Perhaps, it may be more wise to create one shared module and all
> >>>>> three drivers should call the driver detection routine(s) from one
> >>>>> place.
> >>>>
> >>>> We did look into this and it's a bit complicated in terms of plumbing.
> >>>
> >>> Could you elaborate more here? I believe that for the runtime environment
> >>> where all drivers are compiled in the kernel, it might make sense to have this
> >>> code at one place and installed only once for all three (or may be four in the
> >>> soundwire future) drivers.
> >>>
> >>> We should have one straight way which driver/module is used. The separate
> >>> conditions in the mentioned drivers will cause problems. Also, it will
> >>> simplify things for the end user. One module parameter (in the driver selector
> >>> library) is better than three or four to make things working (if the DMI /
> >>> whatever table is not preset correctly for the new hardware).
> >>
> >> Well, one question is where to put this option.  I thought of HD-audio
> >> core in the past, but it's not always the common place any longer.
> >> We may introduce yet another common module just for an option, but it
> >> sounds little appealing to me in comparison with the needed
> >> resources.
> >>
> >> Basically the deployment of SST is only for the already existing
> >> devices, and all newer should go for SOF.  And, the pattern Pierre
> >> mentioned should cover almost all use cases.  This made me believing
> >> that a simple switch is no mandatory request.
> >>
> >> In anyway, Jaroslav's patch looks like a good starting point.  We can
> >> build up a few more exceptions (SKl/KBl/APL Chromebooks with DMIC) on
> >> top of it, then we've done mostly, right?
> > 
> > Yes, there are only a handful of quirks really.
> 
> It seems like a not very rubust solution for me. If you don't like to have
> a standalone module, the code might be included to all drivers, but we losethe
> possibility to control the auto detection behaviour from the one place
> (one module parameter).
> 
> Perhaps, we can rename snd_intel_nhlt() module and put the auto-detection code
> there. It's required by all mentioned drivers, so the extra resources required
> for this new code are minimal.

I don't mind either way, as long as we don't need to introduce too
complex new stuff just for that.


thanks,

Takashi
Pierre-Louis Bossart Sept. 24, 2019, 1:41 p.m. UTC | #7
On 9/24/19 1:46 AM, Jaroslav Kysela wrote:
> Dne 24. 09. 19 v 1:34 Pierre-Louis Bossart napsal(a):
>> On 9/23/19 4:21 PM, Takashi Iwai wrote:
>>> On Mon, 23 Sep 2019 22:35:14 +0200,
>>> Jaroslav Kysela wrote:
>>>>
>>>> Dne 23. 09. 19 v 20:24 Pierre-Louis Bossart napsal(a):
>>>>> On 9/23/19 11:57 AM, Jaroslav Kysela wrote:
>>>>>> There are basically three drivers for the PCI devices for
>>>>>> the recent Intel hardware with the build-in DSPs. The legacy HDA
>>>>>> driver has dmic_detect module option for the auto detection
>>>>>> of the platforms with the digital microphone. Because the SOF
>>>>>> driver is preferred, just skip PCI probe in the Skylake SST
>>>>>> driver when the PCI device ID clashes by default. The user
>>>>>> can override the auto behaviour with the pci_binding
>>>>>> module parameter.
>>>>>
>>>>> Thanks Jaroslav for re-opening this mutual-exclusion issue.
>>>>>
>>>>> I think we want to deal with this in two alternate ways
>>>>> 1. static built-time exclusion based on Kconfigs
>>>>
>>>> Unfortunately, that's really an issue for the universal distros.
>>>
>>> Right.  The Kconfig of Intel audio is already too messy even for now.
>>> We don't want more complexity just for covering some very corner
>>> case.
>>>
>>> Practically seen, if SOF Kconfig is enabled, we may assume that SOF is
>>> preferred in general.  I don't think of any big need of yet another
>>> static configuration.
>>>
>>>>> 2. probe-time exclusion based on quirks (CPU ID + DMI)
>>>>>
>>>>> For example with a SKL/KBL/APL chromebook w/ DMIC we'd want to use the
>>>>> SST driver and for GLK+ we want to use SOF. For any device with
>>>>> HDAudio+DMIC we'd want SOF, same for any device with SoundWire when it's
>>>>> fully supported.
>>>>>
>>>>> I can't recall if I shared the patches I worked on a couple of months
>>>>> ago, but they are still at https://github.com/thesofproject/linux/pull/927
>>>>
>>>> Thanks for pointing me to this. It does not address the legacy HDA, but it's a
>>>> step forward.
>>>
>>> The legacy HD-audio stuff is resolved with the recent DMIC detection
>>> on 5.4, I suppose?
> 
> Unfortunately not completely. The Broxton and Coffelake PCI IDs are also
> shared by the SST / Legacy HDA drivers, so the universal kernel will be
> confused (at least snd_hda_intel will be loaded as first).

I don't see any issues with this. The use of the DSP is only required 
when DMICs/I2S/SoundWire are used, or the firmware contains processing. 
Using the firmware is passthrough mode brings no added value.

If the only thing that's done is manage an HDaudio link, the legacy is 
just fine.

> 
>>>>> the first part essentially does the same thing as this patch, the second
>>>>> relies on quirks. I've been busy with other things but indeed it's high
>>>>> time we closed this for distributions.
>>>>
>>>> Yes, and I have to say, it's too late for the hardware vendors right now. I
>>>> will probably apply my patch to our distribution (I don't care too much about
>>>> chromebooks - the user can change the module/driver behaviour manually) until
>>>> we have a better code.
>>>>
>>>>>> Boot log from Lenovo Carbon X1 (7th gen) with the default settings:
>>>>>>
>>>>>>      snd_hda_intel 0000:00:1f.3: Digital mics found on Skylake+ platform, aborting probe
>>>>>>      snd_soc_skl 0000:00:1f.3: SOF driver is preferred on this platform, aborting probe
>>>>>>      sof-audio-pci 0000:00:1f.3: warning: No matching ASoC machine driver found
>>>>>>      sof-audio-pci 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if 0x040380
>>>>>>      ....
>>>>>>
>>>>>> Perhaps, it may be more wise to create one shared module and all
>>>>>> three drivers should call the driver detection routine(s) from one
>>>>>> place.
>>>>>
>>>>> We did look into this and it's a bit complicated in terms of plumbing.
>>>>
>>>> Could you elaborate more here? I believe that for the runtime environment
>>>> where all drivers are compiled in the kernel, it might make sense to have this
>>>> code at one place and installed only once for all three (or may be four in the
>>>> soundwire future) drivers.
>>>>
>>>> We should have one straight way which driver/module is used. The separate
>>>> conditions in the mentioned drivers will cause problems. Also, it will
>>>> simplify things for the end user. One module parameter (in the driver selector
>>>> library) is better than three or four to make things working (if the DMI /
>>>> whatever table is not preset correctly for the new hardware).
>>>
>>> Well, one question is where to put this option.  I thought of HD-audio
>>> core in the past, but it's not always the common place any longer.
>>> We may introduce yet another common module just for an option, but it
>>> sounds little appealing to me in comparison with the needed
>>> resources.
>>>
>>> Basically the deployment of SST is only for the already existing
>>> devices, and all newer should go for SOF.  And, the pattern Pierre
>>> mentioned should cover almost all use cases.  This made me believing
>>> that a simple switch is no mandatory request.
>>>
>>> In anyway, Jaroslav's patch looks like a good starting point.  We can
>>> build up a few more exceptions (SKl/KBl/APL Chromebooks with DMIC) on
>>> top of it, then we've done mostly, right?
>>
>> Yes, there are only a handful of quirks really.
> 
> It seems like a not very rubust solution for me. If you don't like to have
> a standalone module, the code might be included to all drivers, but we losethe
> possibility to control the auto detection behaviour from the one place
> (one module parameter).
> 
> Perhaps, we can rename snd_intel_nhlt() module and put the auto-detection code
> there. It's required by all mentioned drivers, so the extra resources required
> for this new code are minimal.

it's kinda what my patches were about, there was a central set of quirks 
in sound/soc/intel/common called by SOF/SST drivers.
Jaroslav Kysela Sept. 24, 2019, 5:29 p.m. UTC | #8
Dne 24. 09. 19 v 15:41 Pierre-Louis Bossart napsal(a):
> 
> 
> On 9/24/19 1:46 AM, Jaroslav Kysela wrote:
>> Dne 24. 09. 19 v 1:34 Pierre-Louis Bossart napsal(a):
>>> On 9/23/19 4:21 PM, Takashi Iwai wrote:
>>>> On Mon, 23 Sep 2019 22:35:14 +0200,
>>>> Jaroslav Kysela wrote:
>>>>>
>>>>> Dne 23. 09. 19 v 20:24 Pierre-Louis Bossart napsal(a):
>>>>>> On 9/23/19 11:57 AM, Jaroslav Kysela wrote:
>>>>>>> There are basically three drivers for the PCI devices for
>>>>>>> the recent Intel hardware with the build-in DSPs. The legacy HDA
>>>>>>> driver has dmic_detect module option for the auto detection
>>>>>>> of the platforms with the digital microphone. Because the SOF
>>>>>>> driver is preferred, just skip PCI probe in the Skylake SST
>>>>>>> driver when the PCI device ID clashes by default. The user
>>>>>>> can override the auto behaviour with the pci_binding
>>>>>>> module parameter.
>>>>>>
>>>>>> Thanks Jaroslav for re-opening this mutual-exclusion issue.
>>>>>>
>>>>>> I think we want to deal with this in two alternate ways
>>>>>> 1. static built-time exclusion based on Kconfigs
>>>>>
>>>>> Unfortunately, that's really an issue for the universal distros.
>>>>
>>>> Right.  The Kconfig of Intel audio is already too messy even for now.
>>>> We don't want more complexity just for covering some very corner
>>>> case.
>>>>
>>>> Practically seen, if SOF Kconfig is enabled, we may assume that SOF is
>>>> preferred in general.  I don't think of any big need of yet another
>>>> static configuration.
>>>>
>>>>>> 2. probe-time exclusion based on quirks (CPU ID + DMI)
>>>>>>
>>>>>> For example with a SKL/KBL/APL chromebook w/ DMIC we'd want to use the
>>>>>> SST driver and for GLK+ we want to use SOF. For any device with
>>>>>> HDAudio+DMIC we'd want SOF, same for any device with SoundWire when it's
>>>>>> fully supported.
>>>>>>
>>>>>> I can't recall if I shared the patches I worked on a couple of months
>>>>>> ago, but they are still at https://github.com/thesofproject/linux/pull/927
>>>>>
>>>>> Thanks for pointing me to this. It does not address the legacy HDA, but it's a
>>>>> step forward.
>>>>
>>>> The legacy HD-audio stuff is resolved with the recent DMIC detection
>>>> on 5.4, I suppose?
>>
>> Unfortunately not completely. The Broxton and Coffelake PCI IDs are also
>> shared by the SST / Legacy HDA drivers, so the universal kernel will be
>> confused (at least snd_hda_intel will be loaded as first).
> 
> I don't see any issues with this. The use of the DSP is only required 
> when DMICs/I2S/SoundWire are used, or the firmware contains processing. 
> Using the firmware is passthrough mode brings no added value.
> 
> If the only thing that's done is manage an HDaudio link, the legacy is 
> just fine.

Yes, but the dependancy on the PCI probe order specified just by the module
name is not really nice at all. We are just lucky that the legacy
snd_hda_intel is first.

>>>>>> the first part essentially does the same thing as this patch, the second
>>>>>> relies on quirks. I've been busy with other things but indeed it's high
>>>>>> time we closed this for distributions.
>>>>>
>>>>> Yes, and I have to say, it's too late for the hardware vendors right now. I
>>>>> will probably apply my patch to our distribution (I don't care too much about
>>>>> chromebooks - the user can change the module/driver behaviour manually) until
>>>>> we have a better code.
>>>>>
>>>>>>> Boot log from Lenovo Carbon X1 (7th gen) with the default settings:
>>>>>>>
>>>>>>>      snd_hda_intel 0000:00:1f.3: Digital mics found on Skylake+ platform, aborting probe
>>>>>>>      snd_soc_skl 0000:00:1f.3: SOF driver is preferred on this platform, aborting probe
>>>>>>>      sof-audio-pci 0000:00:1f.3: warning: No matching ASoC machine driver found
>>>>>>>      sof-audio-pci 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if 0x040380
>>>>>>>      ....
>>>>>>>
>>>>>>> Perhaps, it may be more wise to create one shared module and all
>>>>>>> three drivers should call the driver detection routine(s) from one
>>>>>>> place.
>>>>>>
>>>>>> We did look into this and it's a bit complicated in terms of plumbing.
>>>>>
>>>>> Could you elaborate more here? I believe that for the runtime environment
>>>>> where all drivers are compiled in the kernel, it might make sense to have this
>>>>> code at one place and installed only once for all three (or may be four in the
>>>>> soundwire future) drivers.
>>>>>
>>>>> We should have one straight way which driver/module is used. The separate
>>>>> conditions in the mentioned drivers will cause problems. Also, it will
>>>>> simplify things for the end user. One module parameter (in the driver selector
>>>>> library) is better than three or four to make things working (if the DMI /
>>>>> whatever table is not preset correctly for the new hardware).
>>>>
>>>> Well, one question is where to put this option.  I thought of HD-audio
>>>> core in the past, but it's not always the common place any longer.
>>>> We may introduce yet another common module just for an option, but it
>>>> sounds little appealing to me in comparison with the needed
>>>> resources.
>>>>
>>>> Basically the deployment of SST is only for the already existing
>>>> devices, and all newer should go for SOF.  And, the pattern Pierre
>>>> mentioned should cover almost all use cases.  This made me believing
>>>> that a simple switch is no mandatory request.
>>>>
>>>> In anyway, Jaroslav's patch looks like a good starting point.  We can
>>>> build up a few more exceptions (SKl/KBl/APL Chromebooks with DMIC) on
>>>> top of it, then we've done mostly, right?
>>>
>>> Yes, there are only a handful of quirks really.
>>
>> It seems like a not very rubust solution for me. If you don't like to have
>> a standalone module, the code might be included to all drivers, but we losethe
>> possibility to control the auto detection behaviour from the one place
>> (one module parameter).
>>
>> Perhaps, we can rename snd_intel_nhlt() module and put the auto-detection code
>> there. It's required by all mentioned drivers, so the extra resources required
>> for this new code are minimal.
> 
> it's kinda what my patches were about, there was a central set of quirks 
> in sound/soc/intel/common called by SOF/SST drivers.

Yes, but no legacy HDA... I can expect that we want to use DSP to enable some
effects or decoders on hardware which has this co-processor in the near future.

					Jaroslav
Pierre-Louis Bossart Sept. 24, 2019, 7:27 p.m. UTC | #9
On 9/24/19 12:29 PM, Jaroslav Kysela wrote:
> Dne 24. 09. 19 v 15:41 Pierre-Louis Bossart napsal(a):
>>
>>
>> On 9/24/19 1:46 AM, Jaroslav Kysela wrote:
>>> Dne 24. 09. 19 v 1:34 Pierre-Louis Bossart napsal(a):
>>>> On 9/23/19 4:21 PM, Takashi Iwai wrote:
>>>>> On Mon, 23 Sep 2019 22:35:14 +0200,
>>>>> Jaroslav Kysela wrote:
>>>>>>
>>>>>> Dne 23. 09. 19 v 20:24 Pierre-Louis Bossart napsal(a):
>>>>>>> On 9/23/19 11:57 AM, Jaroslav Kysela wrote:
>>>>>>>> There are basically three drivers for the PCI devices for
>>>>>>>> the recent Intel hardware with the build-in DSPs. The legacy HDA
>>>>>>>> driver has dmic_detect module option for the auto detection
>>>>>>>> of the platforms with the digital microphone. Because the SOF
>>>>>>>> driver is preferred, just skip PCI probe in the Skylake SST
>>>>>>>> driver when the PCI device ID clashes by default. The user
>>>>>>>> can override the auto behaviour with the pci_binding
>>>>>>>> module parameter.
>>>>>>>
>>>>>>> Thanks Jaroslav for re-opening this mutual-exclusion issue.
>>>>>>>
>>>>>>> I think we want to deal with this in two alternate ways
>>>>>>> 1. static built-time exclusion based on Kconfigs
>>>>>>
>>>>>> Unfortunately, that's really an issue for the universal distros.
>>>>>
>>>>> Right.  The Kconfig of Intel audio is already too messy even for now.
>>>>> We don't want more complexity just for covering some very corner
>>>>> case.
>>>>>
>>>>> Practically seen, if SOF Kconfig is enabled, we may assume that SOF is
>>>>> preferred in general.  I don't think of any big need of yet another
>>>>> static configuration.
>>>>>
>>>>>>> 2. probe-time exclusion based on quirks (CPU ID + DMI)
>>>>>>>
>>>>>>> For example with a SKL/KBL/APL chromebook w/ DMIC we'd want to use the
>>>>>>> SST driver and for GLK+ we want to use SOF. For any device with
>>>>>>> HDAudio+DMIC we'd want SOF, same for any device with SoundWire when it's
>>>>>>> fully supported.
>>>>>>>
>>>>>>> I can't recall if I shared the patches I worked on a couple of months
>>>>>>> ago, but they are still at https://github.com/thesofproject/linux/pull/927
>>>>>>
>>>>>> Thanks for pointing me to this. It does not address the legacy HDA, but it's a
>>>>>> step forward.
>>>>>
>>>>> The legacy HD-audio stuff is resolved with the recent DMIC detection
>>>>> on 5.4, I suppose?
>>>
>>> Unfortunately not completely. The Broxton and Coffelake PCI IDs are also
>>> shared by the SST / Legacy HDA drivers, so the universal kernel will be
>>> confused (at least snd_hda_intel will be loaded as first).
>>
>> I don't see any issues with this. The use of the DSP is only required
>> when DMICs/I2S/SoundWire are used, or the firmware contains processing.
>> Using the firmware is passthrough mode brings no added value.
>>
>> If the only thing that's done is manage an HDaudio link, the legacy is
>> just fine.
> 
> Yes, but the dependancy on the PCI probe order specified just by the module
> name is not really nice at all. We are just lucky that the legacy
> snd_hda_intel is first.
> 
>>>>>>> the first part essentially does the same thing as this patch, the second
>>>>>>> relies on quirks. I've been busy with other things but indeed it's high
>>>>>>> time we closed this for distributions.
>>>>>>
>>>>>> Yes, and I have to say, it's too late for the hardware vendors right now. I
>>>>>> will probably apply my patch to our distribution (I don't care too much about
>>>>>> chromebooks - the user can change the module/driver behaviour manually) until
>>>>>> we have a better code.
>>>>>>
>>>>>>>> Boot log from Lenovo Carbon X1 (7th gen) with the default settings:
>>>>>>>>
>>>>>>>>       snd_hda_intel 0000:00:1f.3: Digital mics found on Skylake+ platform, aborting probe
>>>>>>>>       snd_soc_skl 0000:00:1f.3: SOF driver is preferred on this platform, aborting probe
>>>>>>>>       sof-audio-pci 0000:00:1f.3: warning: No matching ASoC machine driver found
>>>>>>>>       sof-audio-pci 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if 0x040380
>>>>>>>>       ....
>>>>>>>>
>>>>>>>> Perhaps, it may be more wise to create one shared module and all
>>>>>>>> three drivers should call the driver detection routine(s) from one
>>>>>>>> place.
>>>>>>>
>>>>>>> We did look into this and it's a bit complicated in terms of plumbing.
>>>>>>
>>>>>> Could you elaborate more here? I believe that for the runtime environment
>>>>>> where all drivers are compiled in the kernel, it might make sense to have this
>>>>>> code at one place and installed only once for all three (or may be four in the
>>>>>> soundwire future) drivers.
>>>>>>
>>>>>> We should have one straight way which driver/module is used. The separate
>>>>>> conditions in the mentioned drivers will cause problems. Also, it will
>>>>>> simplify things for the end user. One module parameter (in the driver selector
>>>>>> library) is better than three or four to make things working (if the DMI /
>>>>>> whatever table is not preset correctly for the new hardware).
>>>>>
>>>>> Well, one question is where to put this option.  I thought of HD-audio
>>>>> core in the past, but it's not always the common place any longer.
>>>>> We may introduce yet another common module just for an option, but it
>>>>> sounds little appealing to me in comparison with the needed
>>>>> resources.
>>>>>
>>>>> Basically the deployment of SST is only for the already existing
>>>>> devices, and all newer should go for SOF.  And, the pattern Pierre
>>>>> mentioned should cover almost all use cases.  This made me believing
>>>>> that a simple switch is no mandatory request.
>>>>>
>>>>> In anyway, Jaroslav's patch looks like a good starting point.  We can
>>>>> build up a few more exceptions (SKl/KBl/APL Chromebooks with DMIC) on
>>>>> top of it, then we've done mostly, right?
>>>>
>>>> Yes, there are only a handful of quirks really.
>>>
>>> It seems like a not very rubust solution for me. If you don't like to have
>>> a standalone module, the code might be included to all drivers, but we losethe
>>> possibility to control the auto detection behaviour from the one place
>>> (one module parameter).
>>>
>>> Perhaps, we can rename snd_intel_nhlt() module and put the auto-detection code
>>> there. It's required by all mentioned drivers, so the extra resources required
>>> for this new code are minimal.
>>
>> it's kinda what my patches were about, there was a central set of quirks
>> in sound/soc/intel/common called by SOF/SST drivers.
> 
> Yes, but no legacy HDA... I can expect that we want to use DSP to enable some
> effects or decoders on hardware which has this co-processor in the near future.

I don't disagree, but I have more modest goals for the near future. 
There are quite a few intermediate steps to make before enabling fancy 
processing, with the transition to HDAudio+DMICs there are multiple gaps 
in the stack, e.g. the dependency on UCM, support for mute buttons and 
LEDs, hardware volumes handled by PulseAudio, firmware/topologies pushed 
to distributions with packages, etc.
Cezary Rojewski Sept. 27, 2019, 3:31 p.m. UTC | #10
On 2019-09-23 18:57, Jaroslav Kysela wrote:
> There are basically three drivers for the PCI devices for
> the recent Intel hardware with the build-in DSPs. The legacy HDA
> driver has dmic_detect module option for the auto detection
> of the platforms with the digital microphone. Because the SOF
> driver is preferred, just skip PCI probe in the Skylake SST
> driver when the PCI device ID clashes by default. The user
> can override the auto behaviour with the pci_binding
> module parameter.
> 

Honestly, the SKL/ SOF/ legacy enumeration is complicated enough and I 
don't think it needs yet enough if or else.

On top of that, some of platforms listed here are shared by both and you 
cannot just enable SOF interface by default there.

> Boot log from Lenovo Carbon X1 (7th gen) with the default settings:
> 
>    snd_hda_intel 0000:00:1f.3: Digital mics found on Skylake+ platform, aborting probe
>    snd_soc_skl 0000:00:1f.3: SOF driver is preferred on this platform, aborting probe
>    sof-audio-pci 0000:00:1f.3: warning: No matching ASoC machine driver found
>    sof-audio-pci 0000:00:1f.3: DSP detected with PCI class/subclass/prog-if 0x040380
>    ....

> 
> Perhaps, it may be more wise to create one shared module and all
> three drivers should call the driver detection routine(s) from one
> place.
> 

This.
It is the only right path to follow.

-

In general there are greater plans ahead as cAVS interface is actually 
the preferred one. That has been already committed by Intel. The details 
of actual merge of both drivers to solve the enumeration-and-other 
issues are still being worked on as that is quite a task : )

Czarek

Patch
diff mbox series

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 141dbbf975ac..cace55ca8d55 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -38,6 +38,39 @@  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");
 
+/*
+ *
+ */
+static int skl_sof_support(struct pci_dev *pci)
+{
+	/* the SOF driver has same PCI IDs */
+	if (pci->vendor == 0x8086) {
+		switch (pci->device) {
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMETLAKE_LP)
+		case 0x02c8: /* CML-LP */
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMETLAKE_H)
+		case 0x06c8: /* CML-H */
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_GEMINILAKE)
+		case 0x3198: /* GLK */
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_APOLLOLAKE)
+		case 0x5a98: /* BXT-P */
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_CANNONLAKE)
+		case 0x9dc8: /* CNL */
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_COFFEELAKE)
+		case 0xa348: /* CFL */
+#endif
+		case 0x0000: /* a dummy value when no SOF driver enabled */
+			return 1;
+		}
+	}
+	return 0;
+}
+
 /*
  * initialize the PCI registers
  */
@@ -1002,6 +1035,10 @@  static int skl_probe(struct pci_dev *pci,
 			dev_err(&pci->dev, "Unknown PCI class/subclass/prog-if information (0x%06x) found, aborting probe\n", pci->class);
 			return -ENODEV;
 		}
+		if (skl_sof_support(pci)) {
+			dev_info(&pci->dev, "SOF driver is preferred on this platform, aborting probe\n");
+			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: