[Intel-gfx] [PATCH 4/4] ALSA: hda - Wake the codec up on pin/ELD notify events

[Daniel Vetter]

On Thu, Nov 26, 2015 at 04:23:16PM +0100, Takashi Iwai wrote:
> On Thu, 26 Nov 2015 16:08:06 +0100,
> David Henningsson wrote:
> > 
> > 
> > 
> > On 2015-11-26 10:24, Takashi Iwai wrote:
> > > On Thu, 26 Nov 2015 10:16:17 +0100,
> > > Zhang, Xiong Y wrote:
> > >>
> > >>
> > >>> On Thu, 26 Nov 2015 08:57:30 +0100,
> > >>> Zhang, Xiong Y wrote:
> > >>>>
> > >>>>>>> BTW, I have a patchset to avoid the audio h/w wakeup by a new
> > >>>>>>> component ops to get ELD and connection states.  It was posted to
> > >>>>>>> alsa-devel shortly ago just as a reference, but this should work well
> > >>>>>>> in such a case, too.  The test patches are found in test/hdmi-jack
> > >>>>>>> branch of my sound git tree.
> > >>>>>
> > >>>>> Did you try this branch (merge onto intel-test-nightly)?
> > >>>>>
> > >>>> [Zhang, Xiong Y] Yes, this branch could fix my issue.
> > >>>
> > >>> OK, good to hear.  But this isn't for 4.4 or backport, as it's more
> > >>> radical changes.
> > >>>
> > >>> Could you check the patch below instead?  This suppresses the notifier
> > >>> handling during suspend.  It's bad, but the hotplug should be still
> > >>> handled via normal unsol event, so it should keep working, good enough
> > >>> as a stop gap.
> > >>>
> > >>>
> > >>> Takashi
> > >>>
> > >>> ---
> > >>> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> > >>> index bdb6f226d006..f7c70e2ae65c 100644
> > >>> --- a/sound/pci/hda/patch_hdmi.c
> > >>> +++ b/sound/pci/hda/patch_hdmi.c
> > >>> @@ -33,6 +33,7 @@
> > >>>   #include <linux/delay.h>
> > >>>   #include <linux/slab.h>
> > >>>   #include <linux/module.h>
> > >>> +#include <linux/pm_runtime.h>
> > >>>   #include <sound/core.h>
> > >>>   #include <sound/jack.h>
> > >>>   #include <sound/asoundef.h>
> > >>> @@ -2352,7 +2353,9 @@ static void intel_pin_eld_notify(void *audio_ptr, int
> > >>> port)
> > >>>   	struct hda_codec *codec = audio_ptr;
> > >>>   	int pin_nid = port + 0x04;
> > >>>
> > >>> -	check_presence_and_report(codec, pin_nid);
> > >>> +	if (!atomic_read(&codec->core.in_pm) &&
> > >>> +	    !pm_runtime_suspended(hda_codec_dev(codec)))
> > >>> +		check_presence_and_report(codec, pin_nid);
> > >>>   }
> > >>>
> > >>>   static int patch_generic_hdmi(struct hda_codec *codec)
> > >> [Zhang, Xiong Y]  this patch couldn't remove the error message. So audio controller isn't in Runtime D3 after azx_suspend().
> > >
> > > OK, then the problem isn't about the HD-audio driver but rather i915
> > > driver.  As already mentioned, i915 driver shouldn't issue eld_notify
> > > callback in the suspend code path.
> > 
> > We don't have a complete drm log so I'm only speculating here; but the 
> > HDA log seems to indicate the power well is off when we try to talk to 
> > the HDA controller. That means either the i915 shut it down while we 
> > were holding a lock on it, or HDA tried to lock it and that didn't make 
> > it through; in which case the HDA side should handle an error from 
> > get_power more gracefully...?
> 
> My understanding is that it's triggered *during* i915 suspend.  Then
> the path calls the HDA callback, and HDA controller tries to get power
> and proceeds as it has no idea that it's being shut off.
> 
> Unfortunately, neither get_power callback or the corresponding HDA
> code has a capability to check that state.  So, changing / fixing this
> there would be nice but very hard.
> 
> So I believe it's easier to avoid calling the eld_notify callback from
> i915 side during its suspend code.

Not calling eld_notify doesn't really help since when we suspend we do a
normal modeset. And on platforms where the eld notify interrupt stuff
works that will happen. This needs to be solved somewhere else I think.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch