[PATCH] snd/hda, drm/i915: Track the display_power_status using a cookie

* [PATCH] snd/hda, drm/i915: Track the display_power_status using a cookie
@ 2019-02-13 15:21 Chris Wilson
  2019-02-13 15:58 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Chris Wilson @ 2019-02-13 15:21 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, Jani Nikula

drm/i915 is tracking all wakeref owners with a cookie in order to
identify leaks. To that end, each rpm acquisition ops->get_power is
assigned a cookie which should be passed to ops->put_power to signify
its release (and removal from the list of wakeref owners). As snd/hda is
already using a bool to track current status of display_power extending
that to an unsigned long to hold the boolean cookie is a trivial
extension, and will quell all doubt that snd/hda is the cause of the
device runtime pm leaks.

v2: Keep using the power abstraction for local wakeref tracking.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 20 +++++++++++---------
 include/drm/drm_audio_component.h  |  7 +++++--
 include/sound/hdaudio.h            |  2 +-
 sound/hda/hdac_component.c         | 18 ++++++++++++------
 4 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 5104c6bbd66f..a1e60370eb34 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -741,27 +741,28 @@ void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
 	}
 }
 
-static void i915_audio_component_get_power(struct device *kdev)
+static unsigned long i915_audio_component_get_power(struct device *kdev)
 {
-	intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
+	return intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
 }
 
-static void i915_audio_component_put_power(struct device *kdev)
+static void i915_audio_component_put_power(struct device *kdev,
+					   unsigned long cookie)
 {
-	intel_display_power_put_unchecked(kdev_to_i915(kdev),
-					  POWER_DOMAIN_AUDIO);
+	intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO, cookie);
 }
 
 static void i915_audio_component_codec_wake_override(struct device *kdev,
 						     bool enable)
 {
 	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+	unsigned long wakeref;
 	u32 tmp;
 
 	if (!IS_GEN(dev_priv, 9))
 		return;
 
-	i915_audio_component_get_power(kdev);
+	wakeref = i915_audio_component_get_power(kdev);
 
 	/*
 	 * Enable/disable generating the codec wake signal, overriding the
@@ -779,7 +780,7 @@ static void i915_audio_component_codec_wake_override(struct device *kdev,
 		usleep_range(1000, 1500);
 	}
 
-	i915_audio_component_put_power(kdev);
+	i915_audio_component_put_power(kdev, wakeref);
 }
 
 /* Get CDCLK in kHz  */
@@ -850,12 +851,13 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
 	struct i915_audio_component *acomp = dev_priv->audio_component;
 	struct intel_encoder *encoder;
 	struct intel_crtc *crtc;
+	unsigned long wakeref;
 	int err = 0;
 
 	if (!HAS_DDI(dev_priv))
 		return 0;
 
-	i915_audio_component_get_power(kdev);
+	wakeref = i915_audio_component_get_power(kdev);
 	mutex_lock(&dev_priv->av_mutex);
 
 	/* 1. get the pipe */
@@ -875,7 +877,7 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
 
  unlock:
 	mutex_unlock(&dev_priv->av_mutex);
-	i915_audio_component_put_power(kdev);
+	i915_audio_component_put_power(kdev, wakeref);
 	return err;
 }
 
diff --git a/include/drm/drm_audio_component.h b/include/drm/drm_audio_component.h
index 4923b00328c1..d0c7444319f5 100644
--- a/include/drm/drm_audio_component.h
+++ b/include/drm/drm_audio_component.h
@@ -18,14 +18,17 @@ struct drm_audio_component_ops {
 	 * @get_power: get the POWER_DOMAIN_AUDIO power well
 	 *
 	 * Request the power well to be turned on.
+	 *
+	 * Returns a wakeref cookie to be passed back to the corresponding
+	 * call to @put_power.
 	 */
-	void (*get_power)(struct device *);
+	unsigned long (*get_power)(struct device *);
 	/**
 	 * @put_power: put the POWER_DOMAIN_AUDIO power well
 	 *
 	 * Allow the power well to be turned off.
 	 */
-	void (*put_power)(struct device *);
+	void (*put_power)(struct device *, unsigned long);
 	/**
 	 * @codec_wake_override: Enable/disable codec wake signal
 	 */
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 45f944d57982..06f504c10b80 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -367,7 +367,7 @@ struct hdac_bus {
 	/* DRM component interface */
 	struct drm_audio_component *audio_component;
 	long display_power_status;
-	bool display_power_active;
+	unsigned long display_power_active;
 
 	/* parameters required for enhanced capabilities */
 	int num_streams;
diff --git a/sound/hda/hdac_component.c b/sound/hda/hdac_component.c
index 5c95933e739a..13915fdc6a54 100644
--- a/sound/hda/hdac_component.c
+++ b/sound/hda/hdac_component.c
@@ -79,17 +79,23 @@ void snd_hdac_display_power(struct hdac_bus *bus, unsigned int idx, bool enable)
 
 	if (bus->display_power_status) {
 		if (!bus->display_power_active) {
+			unsigned long cookie = -1;
+
 			if (acomp->ops->get_power)
-				acomp->ops->get_power(acomp->dev);
+				cookie = acomp->ops->get_power(acomp->dev);
+
 			snd_hdac_set_codec_wakeup(bus, true);
 			snd_hdac_set_codec_wakeup(bus, false);
-			bus->display_power_active = true;
+			bus->display_power_active = cookie;
 		}
 	} else {
 		if (bus->display_power_active) {
+			unsigned long cookie = bus->display_power_active;
+
 			if (acomp->ops->put_power)
-				acomp->ops->put_power(acomp->dev);
-			bus->display_power_active = false;
+				acomp->ops->put_power(acomp->dev, cookie);
+
+			bus->display_power_active = 0;
 		}
 	}
 }
@@ -325,9 +331,9 @@ int snd_hdac_acomp_exit(struct hdac_bus *bus)
 		return 0;
 
 	if (WARN_ON(bus->display_power_active) && acomp->ops)
-		acomp->ops->put_power(acomp->dev);
+		acomp->ops->put_power(acomp->dev, bus->display_power_active);
 
-	bus->display_power_active = false;
+	bus->display_power_active = 0;
 	bus->display_power_status = 0;
 
 	component_master_del(dev, &hdac_component_master_ops);
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread