| Message ID | s5hpoyvsgto.wl-tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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@intel.com> > Fixes: 25adc137c546 ('ALSA: hda - Wake the codec up on pin/ELD notify events') > Cc: <stable@vger.kernel.org> # v4.3+ > Signed-off-by: Takashi Iwai <tiwai@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); > } > >
On Fri, 27 Nov 2015 14:45:31 +0100, David Henningsson wrote: > > > > 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? Do you mean the hotplug handling runtime PM? I tested it locally, but wider ranged tests would be appreciated. In theory, it should work as mentioned in the changelog. Takashi > > > > > thanks, > > > > Takashi > > > > -- 8< -- > > From: Takashi Iwai <tiwai@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@intel.com> > > Fixes: 25adc137c546 ('ALSA: hda - Wake the codec up on pin/ELD notify events') > > Cc: <stable@vger.kernel.org> # v4.3+ > > Signed-off-by: Takashi Iwai <tiwai@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 >
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); }