[alsa-devel] [PATCH] ALSA: hda - Fix runtime PM
Before commit 3b5b899ca67d ("ALSA: hda: Make use of core codec functions to sync power state"), hda_set_power_state() returned the response to the Get Power State verb, a 32-bit unsigned integer whose expected value is 0x233 after transitioning a codec to D3, and 0x0 after transitioning it to D0.
The response value is significant because hda_codec_runtime_suspend() does not clear the codec's bit in the codec_powered bitmask unless the AC_PWRST_CLK_STOP_OK bit (0x200) is set in the response value. That in turn prevents the HDA controller from runtime suspending because azx_runtime_idle() checks that the codec_powered bitmask is zero.
Since commit 3b5b899ca67d, hda_set_power_state() only returns 0x0 or 0x1, thereby breaking runtime PM for any HDA controller. That's because an inline function introduced by the commit returns a bool instead of a 32-bit unsigned int. The change was likely erroneous and resulted from copying and pasting snd_hda_check_power_state(), which is immediately preceding the newly introduced inline function. Fix it.
Link: https://bugs.freedesktop.org/show_bug.cgi?id=106597 Fixes: 3b5b899ca67d ("ALSA: hda: Make use of core codec functions to sync power state") Cc: Alex Deucher alexander.deucher@amd.com Cc: Abhijeet Kumar abhijeet.kumar@intel.com Reported-and-tested-by: Gunnar Krüger taijian@posteo.de Signed-off-by: Lukas Wunner lukas@wunner.de --- sound/pci/hda/hda_local.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index 321e78baa63c..9bd935216c18 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -622,8 +622,10 @@ snd_hda_check_power_state(struct hda_codec *codec, hda_nid_t nid, { return snd_hdac_check_power_state(&codec->core, nid, target_state); } -static inline bool snd_hda_sync_power_state(struct hda_codec *codec, - hda_nid_t nid, unsigned int target_state) + +static inline unsigned int snd_hda_sync_power_state(struct hda_codec *codec, + hda_nid_t nid, + unsigned int target_state) { return snd_hdac_sync_power_state(&codec->core, nid, target_state); }
-----Original Message----- From: Lukas Wunner [mailto:lukas@wunner.de] Sent: Thursday, May 24, 2018 1:01 PM To: Takashi Iwai tiwai@suse.com Cc: Jaroslav Kysela perex@perex.cz; Abhijeet Kumar abhijeet.kumar@intel.com; Deucher, Alexander Alexander.Deucher@amd.com; Gunnar Krueger taijian@posteo.de; alsa-devel@alsa-project.org Subject: [PATCH] ALSA: hda - Fix runtime PM
Before commit 3b5b899ca67d ("ALSA: hda: Make use of core codec functions to sync power state"), hda_set_power_state() returned the response to the Get Power State verb, a 32-bit unsigned integer whose expected value is 0x233 after transitioning a codec to D3, and 0x0 after transitioning it to D0.
The response value is significant because hda_codec_runtime_suspend() does not clear the codec's bit in the codec_powered bitmask unless the AC_PWRST_CLK_STOP_OK bit (0x200) is set in the response value. That in turn prevents the HDA controller from runtime suspending because azx_runtime_idle() checks that the codec_powered bitmask is zero.
Since commit 3b5b899ca67d, hda_set_power_state() only returns 0x0 or 0x1, thereby breaking runtime PM for any HDA controller. That's because an inline function introduced by the commit returns a bool instead of a 32-bit unsigned int. The change was likely erroneous and resulted from copying and pasting snd_hda_check_power_state(), which is immediately preceding the newly introduced inline function. Fix it.
Link: https://bugs.freedesktop.org/show_bug.cgi?id=106597 Fixes: 3b5b899ca67d ("ALSA: hda: Make use of core codec functions to sync power state") Cc: Alex Deucher alexander.deucher@amd.com Cc: Abhijeet Kumar abhijeet.kumar@intel.com Reported-and-tested-by: Gunnar Krüger taijian@posteo.de Signed-off-by: Lukas Wunner lukas@wunner.de
Acked-by: Alex Deucher alexander.deucher@amd.com
sound/pci/hda/hda_local.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index 321e78baa63c..9bd935216c18 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -622,8 +622,10 @@ snd_hda_check_power_state(struct hda_codec *codec, hda_nid_t nid, { return snd_hdac_check_power_state(&codec->core, nid, target_state); } -static inline bool snd_hda_sync_power_state(struct hda_codec *codec,
hda_nid_t nid, unsigned int target_state)
+static inline unsigned int snd_hda_sync_power_state(struct hda_codec *codec,
hda_nid_t nid,unsigned int target_state){ return snd_hdac_sync_power_state(&codec->core, nid, target_state); } -- 2.17.0
On Thu, 24 May 2018 19:01:07 +0200, Lukas Wunner wrote:
Before commit 3b5b899ca67d ("ALSA: hda: Make use of core codec functions to sync power state"), hda_set_power_state() returned the response to the Get Power State verb, a 32-bit unsigned integer whose expected value is 0x233 after transitioning a codec to D3, and 0x0 after transitioning it to D0.
The response value is significant because hda_codec_runtime_suspend() does not clear the codec's bit in the codec_powered bitmask unless the AC_PWRST_CLK_STOP_OK bit (0x200) is set in the response value. That in turn prevents the HDA controller from runtime suspending because azx_runtime_idle() checks that the codec_powered bitmask is zero.
Since commit 3b5b899ca67d, hda_set_power_state() only returns 0x0 or 0x1, thereby breaking runtime PM for any HDA controller. That's because an inline function introduced by the commit returns a bool instead of a 32-bit unsigned int. The change was likely erroneous and resulted from copying and pasting snd_hda_check_power_state(), which is immediately preceding the newly introduced inline function. Fix it.
Link: https://bugs.freedesktop.org/show_bug.cgi?id=106597 Fixes: 3b5b899ca67d ("ALSA: hda: Make use of core codec functions to sync power state") Cc: Alex Deucher alexander.deucher@amd.com Cc: Abhijeet Kumar abhijeet.kumar@intel.com Reported-and-tested-by: Gunnar Krüger taijian@posteo.de Signed-off-by: Lukas Wunner lukas@wunner.de
Gah, that one was a silly mistake. Applied now. Thanks for spotting out!
Takashi
participants (3)
-
Deucher, Alexander -
Lukas Wunner -
Takashi Iwai