[Intel-gfx] [PATCH] Revert "ALSA: hda - Fix intermittent CORB/RIRB stall on Intel chips"

[Takashi Iwai]

On Thu, 25 Jul 2019 14:50:18 +0200,
Chris Wilson wrote:
> 
> Quoting Takashi Iwai (2019-07-25 11:44:08)
> > On Thu, 25 Jul 2019 12:21:11 +0200,
> > Chris Wilson wrote:
> > > 
> > > Quoting Chris Wilson (2019-07-25 09:30:25)
> > > > Quoting Takashi Iwai (2019-07-25 09:26:56)
> > > > > On Thu, 25 Jul 2019 10:16:07 +0200,
> > > > > Takashi Iwai wrote:
> > > > > > 
> > > > > > On Thu, 25 Jul 2019 10:03:00 +0200,
> > > > > > Chris Wilson wrote:
> > > > > > > 
> > > > > > > Just a heads up that icl is consistently showing
> > > > > > > 
> > > > > > > <4> [315.478830] snd_hda_intel 0000:00:1f.3: azx_get_response timeout, switching to polling mode: last cmd=0x202f8100
> > > > > > > <4> [316.482799] snd_hda_intel 0000:00:1f.3: No response from codec, disabling MSI: last cmd=0x202f8100
> > > > > > > <3> [508.412915] snd_hda_codec_hdmi hdaudioC0D2: Unable to sync register 0x2f8100. -11
> > > > > > > 
> > > > > > > following commits 2756d9143aa5 ("ALSA: hda - Fix intermittent CORB/RIRB
> > > > > > > stall on Intel chips") and a30f1743e4f5 ("ALSA: line6: sizeof (byte) is
> > > > > > > always 1, use that fact.")
> > > > > > 
> > > > > > The verb that stalls (0x202f8100) is a read verb (0xf81, Intel
> > > > > > vendor-specific verb for HDMI), so it shouldn't matter whether with or
> > > > > > without write sync, because it needs to read the response in anyway.
> > > > > > 
> > > > > > If that patch broke anything, it means that something else was already
> > > > > > broken.  Oh well, that ICL crap...
> > > > > > 
> > > > > > Is it about the runtime PM, or S3 or S4?  The only case we need to
> > > > > > re-issue this verb is only S4, I suppose, so we may skip that in most
> > > > > > cases.
> > > > > 
> > > > > Now checking the code, and I believe the workaround applied there can
> > > > > be skipped for non-Haswell chips.  Could you try the patch below in
> > > > > addition?
> > > > 
> > > > Due to the way patchwork works, this patch will now be tested instead of
> > > > the revert. So watch this space.
> > > 
> > > Sadly, no change. Patchwork definitely lists this patch as being the one
> > > tested, but maybe send it separately just in case.
> > 
> > Hm, does the error indicate the same message ("last cmd=0x202f8100")?
> > If yes, we might need another workaround.  This is a special verb for
> > Intel with some black magic to communicate with GPU.  The tweak via
> > this verb is needed for other platforms to assure the enablement of
> > all pins and DP 1.2, but it might be incorrect for ICL.
> > 
> > Can anyone at Intel check whether the verb (0x781/0xf81) is still
> > valid for ICL?
> > 
> > Anyways below is a patch to leave the verb access for ICL.
> > Let's see...
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> > -- 8< --
> > From: Takashi Iwai <tiwai at suse.de>
> > Subject: [PATCH] ALSA: hda/hdmi - Don't apply black magic to Icelake HDMI
> >  codecs
> > 
> > The Intel-specific verb to enable all pins and DP 1.2 seems causing a
> > problem on ICL by some reason.  Skip applying the verb for ICL as a
> > workaround.
> 
> Hmm. Nothing with icl-hda managed to boot. Coincidence?

No idea, honestly speaking, but it's a bad sign.  Let's scratch.


Takashi


> 
> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > ---
> >  sound/pci/hda/patch_hdmi.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> > index 2096993eaf28..7e8236e5eac0 100644
> > --- a/sound/pci/hda/patch_hdmi.c
> > +++ b/sound/pci/hda/patch_hdmi.c
> > @@ -2804,12 +2804,15 @@ static int intel_hsw_common_init(struct hda_codec *codec, hda_nid_t vendor_nid,
> >         spec->port_map = port_map;
> >         spec->port_num = port_num;
> >  
> > -       intel_haswell_enable_all_pins(codec, true);
> > -       intel_haswell_fixup_enable_dp12(codec);
> > +       if (!is_icelake(codec)) {
> > +               intel_haswell_enable_all_pins(codec, true);
> > +               intel_haswell_fixup_enable_dp12(codec);
> > +       }
> >  
> >         codec->display_power_control = 1;
> >  
> > -       codec->patch_ops.set_power_state = haswell_set_power_state;
> > +       if (!is_icelake(codec))
> > +               codec->patch_ops.set_power_state = haswell_set_power_state;
> >         codec->depop_delay = 0;
> >         codec->auto_runtime_pm = 1;
> >  
> > -- 
> > 2.16.4
> > 
> > 
>