FAILED: patch "[PATCH] ALSA: hda - Manage power well properly for resume" failed to apply to 4.4-stable tree

FAILED: patch "[PATCH] ALSA: hda - Manage power well properly for resume" failed to apply to 4.4-stable tree

[gregkh]

The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

>From a52ff34e5ec61749c62c6618b76a9d6dbecee450 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Thu, 4 Aug 2016 22:38:36 +0200
Subject: [PATCH] ALSA: hda - Manage power well properly for resume

For SKL and later Intel chips, we control the power well per codec
basis via link_power callback since the commit [03b135cebc47: ALSA:
hda - remove dependency on i915 power well for SKL].
However, there are a few exceptional cases where the gfx registers are
accessed from the audio driver: namely the wakeup override bit
toggling at (both system and runtime) resume.  This seems causing a
kernel warning when accessed during the power well down (and likely
resulting in the bogus register accesses).

This patch puts the proper power up / down sequence around the resume
code so that the wakeup bit is fiddled properly while the power is
up.  (The other callback, sync_audio_rate, is used only in the PCM
callback, so it's guaranteed in the power-on.)

Also, by this proper power up/down, the instantaneous flip of wakeup
bit in the resume callback that was introduced by the commit
[033ea349a7cd: ALSA: hda - Fix Skylake codec timeout] becomes
superfluous, as snd_hdac_display_power() already does it.  So we can
clean it up together.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96214
Fixes: 03b135cebc47 ('ALSA: hda - remove dependency on i915 power well for SKL')
Cc: <stable@vger.kernel.org> # v4.2+
Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 89dacf9b4e6c..160c7f713722 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -906,20 +906,23 @@ static int azx_resume(struct device *dev)
 	struct snd_card *card = dev_get_drvdata(dev);
 	struct azx *chip;
 	struct hda_intel *hda;
+	struct hdac_bus *bus;
 
 	if (!card)
 		return 0;
 
 	chip = card->private_data;
 	hda = container_of(chip, struct hda_intel, chip);
+	bus = azx_bus(chip);
 	if (chip->disabled || hda->init_failed || !chip->running)
 		return 0;
 
-	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL
-		&& hda->need_i915_power) {
-		snd_hdac_display_power(azx_bus(chip), true);
-		snd_hdac_i915_set_bclk(azx_bus(chip));
+	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
+		snd_hdac_display_power(bus, true);
+		if (hda->need_i915_power)
+			snd_hdac_i915_set_bclk(bus);
 	}
+
 	if (chip->msi)
 		if (pci_enable_msi(pci) < 0)
 			chip->msi = 0;
@@ -929,6 +932,11 @@ static int azx_resume(struct device *dev)
 
 	hda_intel_init_chip(chip, true);
 
+	/* power down again for link-controlled chips */
+	if ((chip->driver_caps & AZX_DCAPS_I915_POWERWELL) &&
+	    !hda->need_i915_power)
+		snd_hdac_display_power(bus, false);
+
 	snd_power_change_state(card, SNDRV_CTL_POWER_D0);
 
 	trace_azx_resume(chip);
@@ -1008,6 +1016,7 @@ static int azx_runtime_resume(struct device *dev)
 
 	chip = card->private_data;
 	hda = container_of(chip, struct hda_intel, chip);
+	bus = azx_bus(chip);
 	if (chip->disabled || hda->init_failed)
 		return 0;
 
@@ -1015,15 +1024,9 @@ static int azx_runtime_resume(struct device *dev)
 		return 0;
 
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
-		bus = azx_bus(chip);
-		if (hda->need_i915_power) {
-			snd_hdac_display_power(bus, true);
+		snd_hdac_display_power(bus, true);
+		if (hda->need_i915_power)
 			snd_hdac_i915_set_bclk(bus);
-		} else {
-			/* toggle codec wakeup bit for STATESTS read */
-			snd_hdac_set_codec_wakeup(bus, true);
-			snd_hdac_set_codec_wakeup(bus, false);
-		}
 	}
 
 	/* Read STATESTS before controller reset */
@@ -1043,6 +1046,11 @@ static int azx_runtime_resume(struct device *dev)
 	azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) &
 			~STATESTS_INT_MASK);
 
+	/* power down again for link-controlled chips */
+	if ((chip->driver_caps & AZX_DCAPS_I915_POWERWELL) &&
+	    !hda->need_i915_power)
+		snd_hdac_display_power(bus, false);
+
 	trace_azx_runtime_resume(chip);
 	return 0;
 }

Re: FAILED: patch "[PATCH] ALSA: hda - Manage power well properly for resume" failed to apply to 4.4-stable tree

[Takashi Iwai]

On Tue, 30 Aug 2016 10:48:23 +0200,
<gregkh@linuxfoundation.org> wrote:
> 
> 
> The patch below does not apply to the 4.4-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.

It failed due to a slight API change.  Will submit the rebased 4.4.x
version.


thanks,

Takashi

> 
> thanks,
> 
> greg k-h
> 
> ------------------ original commit in Linus's tree ------------------
> 
> >From a52ff34e5ec61749c62c6618b76a9d6dbecee450 Mon Sep 17 00:00:00 2001
> From: Takashi Iwai <tiwai@suse.de>
> Date: Thu, 4 Aug 2016 22:38:36 +0200
> Subject: [PATCH] ALSA: hda - Manage power well properly for resume
> 
> For SKL and later Intel chips, we control the power well per codec
> basis via link_power callback since the commit [03b135cebc47: ALSA:
> hda - remove dependency on i915 power well for SKL].
> However, there are a few exceptional cases where the gfx registers are
> accessed from the audio driver: namely the wakeup override bit
> toggling at (both system and runtime) resume.  This seems causing a
> kernel warning when accessed during the power well down (and likely
> resulting in the bogus register accesses).
> 
> This patch puts the proper power up / down sequence around the resume
> code so that the wakeup bit is fiddled properly while the power is
> up.  (The other callback, sync_audio_rate, is used only in the PCM
> callback, so it's guaranteed in the power-on.)
> 
> Also, by this proper power up/down, the instantaneous flip of wakeup
> bit in the resume callback that was introduced by the commit
> [033ea349a7cd: ALSA: hda - Fix Skylake codec timeout] becomes
> superfluous, as snd_hdac_display_power() already does it.  So we can
> clean it up together.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96214
> Fixes: 03b135cebc47 ('ALSA: hda - remove dependency on i915 power well for SKL')
> Cc: <stable@vger.kernel.org> # v4.2+
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index 89dacf9b4e6c..160c7f713722 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -906,20 +906,23 @@ static int azx_resume(struct device *dev)
>  	struct snd_card *card = dev_get_drvdata(dev);
>  	struct azx *chip;
>  	struct hda_intel *hda;
> +	struct hdac_bus *bus;
>  
>  	if (!card)
>  		return 0;
>  
>  	chip = card->private_data;
>  	hda = container_of(chip, struct hda_intel, chip);
> +	bus = azx_bus(chip);
>  	if (chip->disabled || hda->init_failed || !chip->running)
>  		return 0;
>  
> -	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL
> -		&& hda->need_i915_power) {
> -		snd_hdac_display_power(azx_bus(chip), true);
> -		snd_hdac_i915_set_bclk(azx_bus(chip));
> +	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
> +		snd_hdac_display_power(bus, true);
> +		if (hda->need_i915_power)
> +			snd_hdac_i915_set_bclk(bus);
>  	}
> +
>  	if (chip->msi)
>  		if (pci_enable_msi(pci) < 0)
>  			chip->msi = 0;
> @@ -929,6 +932,11 @@ static int azx_resume(struct device *dev)
>  
>  	hda_intel_init_chip(chip, true);
>  
> +	/* power down again for link-controlled chips */
> +	if ((chip->driver_caps & AZX_DCAPS_I915_POWERWELL) &&
> +	    !hda->need_i915_power)
> +		snd_hdac_display_power(bus, false);
> +
>  	snd_power_change_state(card, SNDRV_CTL_POWER_D0);
>  
>  	trace_azx_resume(chip);
> @@ -1008,6 +1016,7 @@ static int azx_runtime_resume(struct device *dev)
>  
>  	chip = card->private_data;
>  	hda = container_of(chip, struct hda_intel, chip);
> +	bus = azx_bus(chip);
>  	if (chip->disabled || hda->init_failed)
>  		return 0;
>  
> @@ -1015,15 +1024,9 @@ static int azx_runtime_resume(struct device *dev)
>  		return 0;
>  
>  	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
> -		bus = azx_bus(chip);
> -		if (hda->need_i915_power) {
> -			snd_hdac_display_power(bus, true);
> +		snd_hdac_display_power(bus, true);
> +		if (hda->need_i915_power)
>  			snd_hdac_i915_set_bclk(bus);
> -		} else {
> -			/* toggle codec wakeup bit for STATESTS read */
> -			snd_hdac_set_codec_wakeup(bus, true);
> -			snd_hdac_set_codec_wakeup(bus, false);
> -		}
>  	}
>  
>  	/* Read STATESTS before controller reset */
> @@ -1043,6 +1046,11 @@ static int azx_runtime_resume(struct device *dev)
>  	azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) &
>  			~STATESTS_INT_MASK);
>  
> +	/* power down again for link-controlled chips */
> +	if ((chip->driver_caps & AZX_DCAPS_I915_POWERWELL) &&
> +	    !hda->need_i915_power)
> +		snd_hdac_display_power(bus, false);
> +
>  	trace_azx_runtime_resume(chip);
>  	return 0;
>  }
>