ALSA: hda: Refactor display power management

The current HD-audio code manages the DRM audio power via too complex
redirections, and this seems even still unbalanced in a corner case as
Intel DRM CI has been intermittently reporting.  This patch is a big
surgery for addressing the complexity and the possible unbalance.

Basically the patch changes the display PM in the following ways:

- Both HD-audio controller and codec drivers call a single helper,
  snd_hdac_display_power().  (Formerly, the display power control from
  a codec was done indirectly via link_power bus ops.)

- snd_hdac_display_power() receives the codec address index.  For
  turning on/off from the controller, pass HDA_CODEC_IDX_CONTROLLER.

- snd_hdac_display_power() doesn't manage refcounts any longer, but
  keeps the power status in bitmap.  If any of controller or codecs is
  turned on, the function updates the DRM power state via get_power()
  or put_power().

Also this refactor allows us more cleanup:

- The link_power bus ops is dropped, so there is no longer indirect
  management, as mentioned in the above.

- hdac_device link_power_control flag is moved to hda_codec
  display_power_control flag, as it's only for HDA legacy.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106525
Signed-off-by: Takashi Iwai <tiwai@suse.de>

4 files changed, 22 insertions, 31 deletions

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 0957813939e5..9f8d59e7e89f 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -36,6 +36,7 @@
 #include "hda_beep.h"
 #include "hda_jack.h"
 #include <sound/hda_hwdep.h>
+#include <sound/hda_component.h>
 
 #define codec_in_pm(codec)		snd_hdac_is_in_pm(&codec->core)
 #define hda_codec_is_power_on(codec)	snd_hdac_is_power_on(&codec->core)
@@ -799,6 +800,13 @@ void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec)
 static unsigned int hda_set_power_state(struct hda_codec *codec,
 				unsigned int power_state);
 
+/* enable/disable display power per codec */
+static void codec_display_power(struct hda_codec *codec, bool enable)
+{
+	if (codec->display_power_control)
+		snd_hdac_display_power(&codec->bus->core, codec->addr, enable);
+}
+
 /* also called from hda_bind.c */
 void snd_hda_codec_register(struct hda_codec *codec)
 {
@@ -806,7 +814,7 @@ void snd_hda_codec_register(struct hda_codec *codec)
 		return;
 	if (device_is_registered(hda_codec_dev(codec))) {
 		snd_hda_register_beep_device(codec);
-		snd_hdac_link_power(&codec->core, true);
+		codec_display_power(codec, true);
 		pm_runtime_enable(hda_codec_dev(codec));
 		/* it was powered up in snd_hda_codec_new(), now all done */
 		snd_hda_power_down(codec);
@@ -834,7 +842,7 @@ static int snd_hda_codec_dev_free(struct snd_device *device)
 
 	codec->in_freeing = 1;
 	snd_hdac_device_unregister(&codec->core);
-	snd_hdac_link_power(&codec->core, false);
+	codec_display_power(codec, false);
 	put_device(hda_codec_dev(codec));
 	return 0;
 }
@@ -2926,7 +2934,7 @@ static int hda_codec_runtime_suspend(struct device *dev)
 	    (codec_has_clkstop(codec) && codec_has_epss(codec) &&
 	     (state & AC_PWRST_CLK_STOP_OK)))
 		snd_hdac_codec_link_down(&codec->core);
-	snd_hdac_link_power(&codec->core, false);
+	codec_display_power(codec, false);
 	return 0;
 }
 
@@ -2934,7 +2942,7 @@ static int hda_codec_runtime_resume(struct device *dev)
 {
 	struct hda_codec *codec = dev_to_hda_codec(dev);
 
-	snd_hdac_link_power(&codec->core, true);
+	codec_display_power(codec, true);
 	snd_hdac_codec_link_up(&codec->core);
 	hda_call_codec_resume(codec);
 	pm_runtime_mark_last_busy(dev);
diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index fe2506672a72..532e081f8b8a 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -989,20 +989,9 @@ static int azx_get_response(struct hdac_bus *bus, unsigned int addr,
 		return azx_rirb_get_response(bus, addr, res);
 }
 
-static int azx_link_power(struct hdac_bus *bus, bool enable)
-{
-	struct azx *chip = bus_to_azx(bus);
-
-	if (chip->ops->link_power)
-		return chip->ops->link_power(chip, enable);
-	else
-		return -EINVAL;
-}
-
 static const struct hdac_bus_ops bus_core_ops = {
 	.command = azx_send_cmd,
 	.get_response = azx_get_response,
-	.link_power = azx_link_power,
 };
 
 #ifdef CONFIG_SND_HDA_DSP_LOADER
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index cc06a323c817..9f67425d5039 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -667,13 +667,8 @@ static int azx_position_check(struct azx *chip, struct azx_dev *azx_dev)
 	return 0;
 }
 
-/* Enable/disable i915 display power for the link */
-static int azx_intel_link_power(struct azx *chip, bool enable)
-{
-	struct hdac_bus *bus = azx_bus(chip);
-
-	return snd_hdac_display_power(bus, enable);
-}
+#define display_power(chip, enable) \
+	snd_hdac_display_power(azx_bus(chip), HDA_CODEC_IDX_CONTROLLER, enable)
 
 /*
  * Check whether the current DMA position is acceptable for updating
@@ -957,7 +952,7 @@ static void __azx_runtime_suspend(struct azx *chip)
 	azx_clear_irq_pending(chip);
 	if ((chip->driver_caps & AZX_DCAPS_I915_POWERWELL) &&
 	    hda->need_i915_power)
-		snd_hdac_display_power(azx_bus(chip), false);
+		display_power(chip, false);
 }
 
 static void __azx_runtime_resume(struct azx *chip)
@@ -968,7 +963,7 @@ static void __azx_runtime_resume(struct azx *chip)
 	int status;
 
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
-		snd_hdac_display_power(bus, true);
+		display_power(chip, true);
 		if (hda->need_i915_power)
 			snd_hdac_i915_set_bclk(bus);
 	}
@@ -989,7 +984,7 @@ static void __azx_runtime_resume(struct azx *chip)
 	/* 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);
+		display_power(chip, false);
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -1355,7 +1350,7 @@ static int azx_free(struct azx *chip)
 
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
 		if (hda->need_i915_power)
-			snd_hdac_display_power(bus, false);
+			display_power(chip, false);
 	}
 	if (chip->driver_caps & AZX_DCAPS_I915_COMPONENT)
 		snd_hdac_i915_exit(bus);
@@ -2056,7 +2051,6 @@ static const struct hda_controller_ops pci_hda_ops = {
 	.disable_msi_reset_irq = disable_msi_reset_irq,
 	.pcm_mmap_prepare = pcm_mmap_prepare,
 	.position_check = azx_position_check,
-	.link_power = azx_intel_link_power,
 };
 
 static int azx_probe(struct pci_dev *pci,
@@ -2239,7 +2233,7 @@ static int azx_probe_continue(struct azx *chip)
 		if (CONTROLLER_IN_GPU(pci))
 			hda->need_i915_power = 1;
 
-		err = snd_hdac_display_power(bus, true);
+		err = display_power(chip, true);
 		if (err < 0) {
 			dev_err(chip->card->dev,
 				"Cannot turn on display power on i915\n");
@@ -2295,7 +2289,7 @@ static int azx_probe_continue(struct azx *chip)
 out_free:
 	if ((chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
 		&& !hda->need_i915_power)
-		snd_hdac_display_power(bus, false);
+		display_power(chip, false);
 
 i915_power_fail:
 	if (err < 0)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 67099cbb6be2..30fe4dbdb0ae 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2620,7 +2620,7 @@ static int intel_hsw_common_init(struct hda_codec *codec, hda_nid_t vendor_nid)
 	 * can cover the codec power request, and so need not set this flag.
 	 */
 	if (!is_haswell(codec) && !is_broadwell(codec))
-		codec->core.link_power_control = 1;
+		codec->display_power_control = 1;
 
 	codec->patch_ops.set_power_state = haswell_set_power_state;
 	codec->depop_delay = 0;
@@ -2656,7 +2656,7 @@ static int patch_i915_byt_hdmi(struct hda_codec *codec)
 	/* For Valleyview/Cherryview, only the display codec is in the display
 	 * power well and can use link_power ops to request/release the power.
 	 */
-	codec->core.link_power_control = 1;
+	codec->display_power_control = 1;
 
 	codec->depop_delay = 0;
 	codec->auto_runtime_pm = 1;