[alsa-devel] [PATCH] ALSA: hda - Fix runtime PM

[Deucher, Alexander]

> -----Original Message-----
> From: Lukas Wunner [mailto:lukas at wunner.de]
> Sent: Thursday, May 24, 2018 1:01 PM
> To: Takashi Iwai <tiwai at suse.com>
> Cc: Jaroslav Kysela <perex at perex.cz>; Abhijeet Kumar
> <abhijeet.kumar at intel.com>; Deucher, Alexander
> <Alexander.Deucher at amd.com>; Gunnar Krueger <taijian at posteo.de>;
> alsa-devel at 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 at amd.com>
> Cc: Abhijeet Kumar <abhijeet.kumar at intel.com>
> Reported-and-tested-by: Gunnar Krüger <taijian at posteo.de>
> Signed-off-by: Lukas Wunner <lukas at wunner.de>
> ---

Acked-by: Alex Deucher <alexander.deucher at 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