[alsa-devel] [PATCH 1/2] ALSA: HD-Audio: SKL+: abort probe if DSP is present and Skylake driver selected

[Takashi Iwai]

On Mon, 10 Dec 2018 17:05:00 +0100,
Takashi Iwai wrote:
> 
> On Mon, 10 Dec 2018 16:57:49 +0100,
> Pierre-Louis Bossart wrote:
> > 
> > 
> > On 12/10/18 9:08 AM, Takashi Iwai wrote:
> > > On Mon, 10 Dec 2018 15:31:08 +0100,
> > > Pierre-Louis Bossart wrote:
> > >>
> > >> On 12/8/18 1:56 AM, Takashi Iwai wrote:
> > >>> On Sat, 08 Dec 2018 01:00:38 +0100,
> > >>> Pierre-Louis Bossart wrote:
> > >>>> Now that the SST/Skylake driver supports per platform selectors, we
> > >>>> can add logic to automatically select the right driver.
> > >>>>
> > >>>> If the Skylake driver is selected, and the DSP is enable, the legacy
> > >>>> HDaudio driver aborts the probe. This will result in a single driver
> > >>>> probing and remove the need for modprobe blacklists.
> > >>>>
> > >>>> Follow-up patches will add a module parameter to bypass the logic if
> > >>>> this automatic detection fails, or if the Skylake driver is unable to
> > >>>> actually support the platform (firmware authentication, missing
> > >>>> topology file, hardware issue, etc).
> > >>>>
> > >>>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> > >>>> ---
> > >>>>    sound/pci/hda/Kconfig          | 46 ++++++++++++++++++++++++++++++++++
> > >>>>    sound/pci/hda/hda_controller.h |  2 +-
> > >>>>    sound/pci/hda/hda_intel.c      | 34 +++++++++++++++++++------
> > >>>>    sound/soc/intel/Kconfig        |  6 +++++
> > >>>>    4 files changed, 80 insertions(+), 8 deletions(-)
> > >>>>
> > >>>> diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
> > >>>> index 4235907b7858..634b7fe6a936 100644
> > >>>> --- a/sound/pci/hda/Kconfig
> > >>>> +++ b/sound/pci/hda/Kconfig
> > >>>> @@ -226,6 +226,52 @@ config SND_HDA_POWER_SAVE_DEFAULT
> > >>>>    	  The default time-out value in seconds for HD-audio automatic
> > >>>>    	  power-save mode.  0 means to disable the power-save mode.
> > >>>>    +if SND_HDA_INTEL
> > >>>> +
> > >>>> +config SND_HDA_INTEL_DISABLE_SKL
> > >>>> +	bool
> > >>>> +	help
> > >>>> +	  This option disables HD-audio legacy for
> > >>>> +	  Skylake machines
> > >>> I'm not sure whether we need the selection of this disablement for
> > >>> each model.  Distros would choose these unlikely, and individual users
> > >>> don't have to select multiple of them but only for their machine's
> > >>> model.  So, in the end, the choice would be either yes or no.
> > >> Ah yes, maybe I wasn't clear. This wasn't intended to be selected by
> > >> the user, but selected when when the SND_SOC_INTEL_KBL or
> > >> SND_SOC_SOF_CNL options are set. See the conditions below.
> > >>
> > >> The main idea what to only deal with the conflict resolution when we
> > >> indeed have a conflict. I also introduced this option on the
> > >> sound/pci/hda side so that SOF can use the same mechanisms, i.e. it's
> > >> the legacy driver doesn't need to know if the conflict happens with
> > >> the SST/Skylake or SOF driver.
> > > OK, that makes sense.
> > >
> > > But then better to rephrase the help texts there for avoiding
> > > confusion.  Currently it sounds as if the kconfig always disables the
> > > support of the given chipset.  But the actual behavior is to disable
> > > the binding with the legacy driver *only if* the PCI device class is
> > > declared for Intel DSP.
> > 
> > ok, will respin the help text.
> > 
> > I was wondering if my email client ate your answers, was is the only
> > change you wanted? In reply to the cover letter you mentioned "some
> > comments" but I only see this one that needs an update, and no
> > comments for the initial series of Skylake-specific patches.
> 
> Maybe you missed my comments for the second and later hunks of
> patch#2?  It was about some dev_warn() and dev_err() usages, which I 
> suggested to degrade to dev_info().

Oh, BTW, did the fallback mechanism work properly with your patches on
the actual machines?

At the last time I tried on SKL, the fallback failed by some reason,
the driver core didn't try to load and bind two drivers.


Takashi