diff mbox series

ALSA: hda: Avoid unsol event during RPM suspending

Message ID 20220329155940.26331-1-mkumard@nvidia.com (mailing list archive)
State Accepted
Commit 6ddc2f749621d5d45ca03edc9f0616bcda136d29
Headers show
Series ALSA: hda: Avoid unsol event during RPM suspending | expand

Commit Message

Mohan Kumar March 29, 2022, 3:59 p.m. UTC
There is a corner case with unsol event handling during codec runtime
suspending state. When the codec runtime suspend call initiated, the
codec->in_pm atomic variable would be 0, currently the codec runtime
suspend function calls snd_hdac_enter_pm() which will just increments
the codec->in_pm atomic variable. Consider unsol event happened just
after this step and before snd_hdac_leave_pm() in the codec runtime
suspend function. The snd_hdac_power_up_pm() in the unsol event
flow in hdmi_present_sense_via_verbs() function would just increment
the codec->in_pm atomic variable without calling pm_runtime_get_sync
function.

As codec runtime suspend flow is already in progress and in parallel
unsol event is also accessing the codec verbs, as soon as codec
suspend flow completes and clocks are  switched off before completing
the unsol event handling as both functions doesn't wait for each other.
This will result in below errors

[  589.428020] tegra-hda 3510000.hda: azx_get_response timeout, switching
to polling mode: last cmd=0x505f2f57
[  589.428344] tegra-hda 3510000.hda: spurious response 0x80000074:0x5,
last cmd=0x505f2f57
[  589.428547] tegra-hda 3510000.hda: spurious response 0x80000065:0x5,
last cmd=0x505f2f57

To avoid this, the unsol event flow should not perform any codec verb
related operations during RPM_SUSPENDING state.

Signed-off-by: Mohan Kumar <mkumard@nvidia.com>
---
 sound/pci/hda/patch_hdmi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Takashi Iwai March 30, 2022, 8:21 a.m. UTC | #1
On Tue, 29 Mar 2022 17:59:40 +0200,
Mohan Kumar wrote:
> 
> There is a corner case with unsol event handling during codec runtime
> suspending state. When the codec runtime suspend call initiated, the
> codec->in_pm atomic variable would be 0, currently the codec runtime
> suspend function calls snd_hdac_enter_pm() which will just increments
> the codec->in_pm atomic variable. Consider unsol event happened just
> after this step and before snd_hdac_leave_pm() in the codec runtime
> suspend function. The snd_hdac_power_up_pm() in the unsol event
> flow in hdmi_present_sense_via_verbs() function would just increment
> the codec->in_pm atomic variable without calling pm_runtime_get_sync
> function.
> 
> As codec runtime suspend flow is already in progress and in parallel
> unsol event is also accessing the codec verbs, as soon as codec
> suspend flow completes and clocks are  switched off before completing
> the unsol event handling as both functions doesn't wait for each other.
> This will result in below errors
> 
> [  589.428020] tegra-hda 3510000.hda: azx_get_response timeout, switching
> to polling mode: last cmd=0x505f2f57
> [  589.428344] tegra-hda 3510000.hda: spurious response 0x80000074:0x5,
> last cmd=0x505f2f57
> [  589.428547] tegra-hda 3510000.hda: spurious response 0x80000065:0x5,
> last cmd=0x505f2f57
> 
> To avoid this, the unsol event flow should not perform any codec verb
> related operations during RPM_SUSPENDING state.
> 
> Signed-off-by: Mohan Kumar <mkumard@nvidia.com>

Thanks, applied now.


Takashi
diff mbox series

Patch

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index c85ed7bc121e..3e086eebf88d 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1625,6 +1625,7 @@  static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
 	struct hda_codec *codec = per_pin->codec;
 	struct hdmi_spec *spec = codec->spec;
 	struct hdmi_eld *eld = &spec->temp_eld;
+	struct device *dev = hda_codec_dev(codec);
 	hda_nid_t pin_nid = per_pin->pin_nid;
 	int dev_id = per_pin->dev_id;
 	/*
@@ -1638,8 +1639,13 @@  static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
 	int present;
 	int ret;
 
+#ifdef	CONFIG_PM
+	if (dev->power.runtime_status == RPM_SUSPENDING)
+		return;
+#endif
+
 	ret = snd_hda_power_up_pm(codec);
-	if (ret < 0 && pm_runtime_suspended(hda_codec_dev(codec)))
+	if (ret < 0 && pm_runtime_suspended(dev))
 		goto out;
 
 	present = snd_hda_jack_pin_sense(codec, pin_nid, dev_id);