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

[David Henningsson]

On 2015-11-27 14:38, Takashi Iwai wrote:
> On Fri, 27 Nov 2015 03:55:28 +0100,
> Zhang, Xiong Y wrote:
>>
>>> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
>>> index bdb6f226d006..0cd7bb30b045 100644
>>> --- a/sound/pci/hda/patch_hdmi.c
>>> +++ b/sound/pci/hda/patch_hdmi.c
>>> @@ -2352,6 +2352,10 @@ static void intel_pin_eld_notify(void *audio_ptr, int
>>> port)
>>>   	struct hda_codec *codec = audio_ptr;
>>>   	int pin_nid = port + 0x04;
>>>
>>> +	/* skip notification during suspend */
>>> +	if (snd_power_get_state(codec->card) != SNDRV_CTL_POWER_D0)
>>> +		return;
>>> +
>>>   	check_presence_and_report(codec, pin_nid);
>>>   }
>>>
>> [Zhang, Xiong Y] yes, this patch could remove the error message.
>
> Alright, then it's indeed a failure in the sound driver side.
> Below is the official patch I'm going to merge.

Just a quick question; have you checked that this does not bring back 
the original bug the entire patch set was supposed to fix?

>
> thanks,
>
> Takashi
>
> -- 8< --
> From: Takashi Iwai <tiwai at suse.de>
> Subject: [PATCH] ALSA: hda - Skip ELD notification during system suspend
>
> The recent addition of ELD notifier for Intel HDMI/DP codec may lead
> the bad codec connection found as kernel messages like below:
>   Suspending console(s) (use no_console_suspend to debug)
>   hdmi_present_sense: snd_hda_codec_hdmi hdaudioC0D2: HDMI status: Codec=2 Pin=6 Presence_Detect=1 ELD_Valid=1
>   snd_hda_intel 0000:00:1f.3: spurious response 0x0:0x2, last cmd=0x206f2e08
>   snd_hda_intel 0000:00:1f.3: spurious response 0x0:0x2, last cmd=0x206f2e08
>   ....
>    snd_hda_codec_hdmi hdaudioC0D2: HDMI: ELD buf size is 0, force 128
>    snd_hda_intel 0000:00:1f.3: azx_get_response timeout, switching to polling mode: last cmd=0x206f2f00
>   snd_hda_intel 0000:00:1f.3: No response from codec, disabling MSI: last cmd=0x206f2f00
>   snd_hda_intel 0000:00:1f.3: azx_get_response timeout, switching to single_cmd mode: last cmd=0x206f2f00
>   azx_single_wait_for_response: 42 callbacks suppressed
>
> This seems appearing when the sound driver went to suspend before i915
> driver.  Then i915 driver disables HDMI/DP audio bit and calls the
> registered notifier, and the HDA codec tries to handle it as a
> hot(un)plug.  But since the driver is already in the suspended state,
> it fails miserably.
>
> As this is a sort of spurious wakeup, it can be ignored safely, as
> long as it's delivered during the system suspend.  OTOH, if a
> notification comes during the runtime suspend, the situation is
> different: we need to wake up.  But during the system suspend, such a
> notification can't be the reason for a wakeup.
>
> This patch addresses it by a simple check of the current sound card
> status.  The skipped notification doesn't matter because the HDA
> driver will check the plugged status forcibly at the resume in
> return.
>
> Then, why the card status, not a runtime PM status or else?  The HDA
> controller driver is supposed to set the card status to D3 at the
> system suspend but not at the runtime suspend.  So we can see it as a
> flag that is set only for the system suspend.  Admittedly, it's a bit
> ugly, but it should work well for now.
>
> Reported-and-tested-by: "Zhang, Xiong Y" <xiong.y.zhang at intel.com>
> Fixes: 25adc137c546 ('ALSA: hda - Wake the codec up on pin/ELD notify events')
> Cc: <stable at vger.kernel.org> # v4.3+
> Signed-off-by: Takashi Iwai <tiwai at suse.de>
> ---
>   sound/pci/hda/patch_hdmi.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index bdb6f226d006..4b6fb668c91c 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -2352,6 +2352,12 @@ static void intel_pin_eld_notify(void *audio_ptr, int port)
>   	struct hda_codec *codec = audio_ptr;
>   	int pin_nid = port + 0x04;
>
> +	/* skip notification during system suspend (but not in runtime PM);
> +	 * the state will be updated at resume
> +	 */
> +	if (snd_power_get_state(codec->card) != SNDRV_CTL_POWER_D0)
> +		return;
> +
>   	check_presence_and_report(codec, pin_nid);
>   }
>
>

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic