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

[Takashi Iwai <tiwai@suse.de>]

On Thu, 26 Nov 2015 07:06:56 +0100,
Zhang, Xiong Y wrote:
> 
> > 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.

So the call of eld_notifier should be suppressed at 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 ?

It's the flag that is set only during the sound driver's PM, not i915
driver's PM.  I gave the patch from the wrong assumption.  It's not
the race between i915 and audio PM, thus the patch doesn't help.

> > 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)?


Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx