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

From: "Zhang, Xiong Y" <xiong.y.zhang@intel.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Vetter, Daniel" <daniel.vetter@intel.com>,
	David Henningsson <david.henningsson@canonical.com>
Subject: Re: [PATCH 4/4] ALSA: hda - Wake the codec up on pin/ELD	notify events
Date: Thu, 26 Nov 2015 06:06:56 +0000	[thread overview]
Message-ID: <8082FF9BCB2B054996454E47167FF4EC0B0F2834@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <s5hbnaiiazu.wl-tiwai@suse.de>

> On Wed, 25 Nov 2015 11:57:13 +0100,
> Zhang, Xiong Y wrote:
> >
> > > On Wed, 25 Nov 2015 10:56:51 +0100,
> > > Zhang, Xiong Y wrote:
> > > >
> > > > Recently I always see the following error message during S4 or S3 resume
> > > with drm-intel-nightly.
> > > > [   97.778063] PM: Syncing filesystems ... done.
> > > > [   97.801550] Freezing user space processes ... (elapsed 0.002 seconds)
> > > done.
> > > > [   97.804297] PM: Marking nosave pages: [mem
> 0x00000000-0x00000fff]
> > > > [   97.804302] PM: Marking nosave pages: [mem
> 0x00058000-0x00058fff]
> > > > [   97.804305] PM: Marking nosave pages: [mem 0x0009e000-0x000fffff]
> > > > [   97.804310] PM: Marking nosave pages: [mem
> 0x84c11000-0x84c12fff]
> > > > [   97.804312] PM: Marking nosave pages: [mem
> 0x876fc000-0x87746fff]
> > > > [   97.804317] PM: Marking nosave pages: [mem
> 0x8785e000-0x87fe9fff]
> > > > [   97.804387] PM: Marking nosave pages: [mem 0x88000000-0xffffffff]
> > > > [   97.806363] PM: Basic memory bitmaps created
> > > > [   97.806409] PM: Preallocating image memory... done (allocated
> 321557
> > > pages)
> > > > [   98.150475] PM: Allocated 1286228 kbytes in 0.34 seconds (3783.02
> > > MB/s)
> > > > [   98.150476] Freezing remaining freezable tasks ... (elapsed 0.001
> seconds)
> > > done.
> > > > [   98.151998] Suspending console(s) (use no_console_suspend to
> debug)
> > > > [   98.173485] hdmi_present_sense: snd_hda_codec_hdmi
> hdaudioC0D2:
> > > HDMI status: Codec=2 Pin=6 Presence_Detect=1 ELD_Valid=1
> > > > [   99.178150] snd_hda_intel 0000:00:1f.3: spurious response 0x0:0x2,
> last
> > > cmd=0x206f2e08
> > > > [   99.178151] snd_hda_intel 0000:00:1f.3: spurious response 0x0:0x2,
> last
> > > cmd=0x206f2e08
> > > > [   99.178151] snd_hda_intel 0000:00:1f.3: spurious response 0x0:0x2,
> last
> > > cmd=0x206f2e08
> > > > [   99.178152] snd_hda_intel 0000:00:1f.3: spurious response 0x0:0x2,
> last
> > > cmd=0x206f2e08
> > > > [   99.178152] snd_hda_intel 0000:00:1f.3: spurious response 0x0:0x2,
> last
> > > cmd=0x206f2e08
> > > > [   99.178153] snd_hda_intel 0000:00:1f.3: spurious response 0x0:0x2,
> last
> > > cmd=0x206f2e08
> > > > [   99.178153] snd_hda_intel 0000:00:1f.3: spurious response 0x0:0x2,
> last
> > > cmd=0x206f2e08
> > > > [   99.178154] snd_hda_intel 0000:00:1f.3: spurious response 0x0:0x2,
> last
> > > cmd=0x206f2e08
> > > > [   99.178154] snd_hda_intel 0000:00:1f.3: spurious response 0x0:0x2,
> last
> > > cmd=0x206f2e08
> > > > [   99.178155] snd_hda_intel 0000:00:1f.3: spurious response 0x0:0x2,
> last
> > > cmd=0x206f2e08
> > > > [   99.178162] snd_hda_codec_hdmi hdaudioC0D2: HDMI: ELD buf size
> is 0,
> > > force 128
> > > > [  101.189709] snd_hda_intel 0000:00:1f.3: azx_get_response timeout,
> > > switching to polling mode: last cmd=0x206f2f00
> > > > [  102.195492] snd_hda_intel 0000:00:1f.3: No response from codec,
> > > disabling MSI: last cmd=0x206f2f00
> > > > [  103.201275] snd_hda_intel 0000:00:1f.3: azx_get_response timeout,
> > > switching to single_cmd mode: last cmd=0x206f2f00
> > > > [  103.201396] azx_single_wait_for_response: 42 callbacks suppressed
> > > >
> > > > The bisect result points to this commit.
> > > > I checked this patch and had one question: if i915 driver wake up ahead of
> > > snd_hda_intel driver during resume,  i915 driver will call audio driver's
> > > hdmi_present_sense() function through this patch, but the audio interrupt
> is
> > > disabled at this moment, how could hdmi_present_sense() get the
> response
> > > from codec ?
> > >
> > > Yeah, a bad thing happens there.  The fix should be simple like below,
> > > though.  Basically the pins are checked in the resume callback in
> > > anyway, so there is no need to handle the notification during PM.
> > >
> > > Could you check whether it works?
> > >
> > >
> > > thanks,
> > >
> > > Takashi
> >
> > Strange, your patch couldn't get away the error message.
> 
> OK, then it's not about the race *during* the hd-audio driver
> resuming or such.  I guess it's the other way round: the i915 driver
> notifies it unnecessarily while it's getting off.  The audio part of
> HDMI controller relies on the i915 power state, so it's screwed up.
> 
> Check with drm.debug option to see in which code path it's triggered.
> If it's a call in i915 suspend, the call should be removed not to wake
> up the audio part unnecessarily.

[Zhang, Xiong Y] it is called in intel_audio_codec_disable() from i915 suspend.
---

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index bdb6f226d006..b468fe0e6195 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2352,7 +2352,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);
+	/* don't call notifier during PM; will be checked in resume callback */
+	if (atomic_read(&codec->core.in_pm))
+		check_presence_and_report(codec, pin_nid);
 }
 
 static int patch_generic_hdmi(struct hda_codec *codec)
> 
This could remove the error message. Go through the audio driver, I found in_pm isn't zero only during call codec_suspend() and codec_resume(). atomic_inc_not_zero(&codec->in_pm) in snd_hdac_power_up_pm() isn't inc in_pm since in_pm is zero. 
Is this right ?

thanks
> 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.
> 
> 
> Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
