[alsa-devel] [PATCH] ASoC: Skylake SST driver - blacklist the PCI device IDs for the auto probe

[Takashi Iwai]

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