Re: [GIT PULL] sound updates for 4.21

[Takashi Iwai]

On Fri, 28 Dec 2018 13:43:03 +0100,
Ingo Molnar wrote:
> 
> 
> * Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> 
> > On Thu, Dec 20, 2018 at 7:38 AM Takashi Iwai <tiwai@xxxxxxx> wrote:
> > >
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git tags/sound-4.21-rc1
> > 
> > Hmm.
> > 
> > It turns out that commit c337104b1a16 ("ALSA: HD-Audio: SKL+: abort
> > probe if DSP is present and Skylake driver selected") causes my laptop
> > (XPS13 9350) to no longer suspend.
> 
> Just a wild guess, I can see two ways in which that commit could make a 
> difference on your setup:
> 
> 1)
> 
> If any of these is not set in your .config:
> 
> +       select SND_HDA_INTEL_DSP_DETECTION_SKL if SND_SOC_INTEL_SKL
> +       select SND_HDA_INTEL_DSP_DETECTION_APL if SND_SOC_INTEL_APL
> +       select SND_HDA_INTEL_DSP_DETECTION_KBL if SND_SOC_INTEL_KBL
> +       select SND_HDA_INTEL_DSP_DETECTION_GLK if SND_SOC_INTEL_GLK
> +       select SND_HDA_INTEL_DSP_DETECTION_CNL if SND_SOC_INTEL_CNL
> +       select SND_HDA_INTEL_DSP_DETECTION_CFL if SND_SOC_INTEL_CFL
> 
> I.e. I'd enable all of the SND_SOC_INTEL_* options to cover this angle.
> 
> 2)
> 
> There's the added logic of checking whether the DSP is enabled:
> 
> +       /* check if this driver can be used on SKL+ Intel platforms */
> +       if ((pci_id->driver_data & AZX_DCAPS_INTEL_SHARED) &&
> +           pci->class != 0x040300)
> +               return -ENODEV;
> +
> 
> if pci->class is not 0x040300 the driver could end up not detecting the 
> device while previously it would.
> 
> That code goes through several transformations later on - but the hack 
> below should make the commit an invariant. I think. Totally untested 
> though.

Yes, Linus pointed out a similar "fix" to make things working again in
a later thread without Cc to ML.

The problem seems to be that this laptop doesn't work with ASoC Intel
SST driver now we're trying to bind when DSP is available.  From
heuristics, it's identified from PCI class number, but this doesn't
seem enough on some models, unfortunately.

And, the old behavior (bind with "legacy" HDA-Intel driver) should be
achieved simply by passing pci_binding=1 option to snd-hda-intel
module, i.e. a patch like below would be enough:

--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -172,7 +172,7 @@ module_param_array(beep_mode, bool, NULL, 0444);
 MODULE_PARM_DESC(beep_mode, "Select HDA Beep registration mode "
 			    "(0=off, 1=on) (default=1).");
 #endif
-static int skl_pci_binding;
+static int skl_pci_binding = 1;
 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");
 
===

Actually, there are two aspects wrt this problem:

1) Whether ASoC driver cannot work with these Dell machines at all

2) Whether we want to keep binding with the legacy HDA driver even if
   DSP is available

AFAIK, the problem seems specific to some Dell models (XPS13 or such),
and I thought Intel already tested with these laptops.  But maybe the
behavior differs depending on the model number or BIOS.  IIRC, XPS13
already showed a drastic change of the HD-audio binding by BIOS
upgrades in the past, too.  So (1) might be some machine-specific
fixes in the end.

But, even if (1) is fixed somehow, the user-visible behavior may be
slightly different from the earlier kernels (e.g. routing setup
required, etc), and this might annoy existing users.  That's the
question in (2).

If we prefer being conservative and keeping the binding with the
legacy HDA driver as-is, it'd be reasonable to provide (yet) another
Kconfig to choose the default option value above.

Or, we may add some blacklisting (e.g. via DMI matching) in HDA-Intel
driver side to skip the faulty PCI class check.

I'm not sure which solution we'll take in the end, but certainly it's
a bug that has to be fixed soonish.


thanks,

Takashi