[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 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().


thanks,

Takashi