[PATCH] drm/i915: Track ownership of display powerwells on each crtc_state

Chris Wilson chris at chris-wilson.co.uk
Sat Feb 16 09:26:45 UTC 2019


Make each crtc_state own the powerwell references it requires, making
each atomic state truly independent and responsible for managing its own
power requirements.

The primary motivation for this is precise wakeref tracking so that we
can see which powerwells remain enabled and thus identify any potential
leaks, or even see a snapshot of the current users for debugging.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_atomic.c  |  1 +
 drivers/gpu/drm/i915/intel_display.c | 66 +++++++++++++---------------
 drivers/gpu/drm/i915/intel_drv.h     |  6 ++-
 3 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 7cf9290ea34a..9bf288228964 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -186,6 +186,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
 	crtc_state->wm.need_postvbl_update = false;
 	crtc_state->fb_bits = 0;
 	crtc_state->update_planes = 0;
+	crtc_state->power_domains = 0;
 
 	return &crtc_state->base;
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index afa21daaae51..9792fae071a0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6234,34 +6234,42 @@ static u64 get_crtc_power_domains(struct drm_crtc *crtc,
 	return mask;
 }
 
-static u64
+static void
 modeset_get_crtc_power_domains(struct drm_crtc *crtc,
 			       struct intel_crtc_state *crtc_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	enum intel_display_power_domain domain;
-	u64 domains, new_domains, old_domains;
-
-	old_domains = intel_crtc->enabled_power_domains;
-	intel_crtc->enabled_power_domains = new_domains =
-		get_crtc_power_domains(crtc, crtc_state);
-
-	domains = new_domains & ~old_domains;
+	u64 domains;
 
-	for_each_power_domain(domain, domains)
-		intel_display_power_get(dev_priv, domain);
+	domains = get_crtc_power_domains(crtc, crtc_state);
+	for_each_power_domain(domain, domains & ~crtc_state->power_domains) {
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
+		crtc_state->wakeref[domain] =
+#endif
+			intel_display_power_get(to_i915(crtc->dev), domain);
+	}
+	crtc_state->power_domains |= domains;
+}
 
-	return old_domains & ~new_domains;
+static inline intel_wakeref_t
+__get_wakeref(struct intel_crtc_state *crtc_state,
+	      enum intel_display_power_domain domain)
+{
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
+		return crtc_state->wakeref[domain];
+#else
+		return -1;
+#endif
 }
 
-static void modeset_put_power_domains(struct drm_i915_private *dev_priv,
-				      u64 domains)
+static void modeset_put_power_domains(struct drm_crtc *crtc,
+				      struct intel_crtc_state *crtc_state)
 {
 	enum intel_display_power_domain domain;
 
-	for_each_power_domain(domain, domains)
-		intel_display_power_put_unchecked(dev_priv, domain);
+	for_each_power_domain(domain, crtc_state->power_domains)
+		intel_display_power_put(to_i915(crtc->dev), domain,
+					__get_wakeref(crtc_state, domain));
 }
 
 static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
@@ -6456,9 +6464,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
 	struct intel_encoder *encoder;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
-	enum intel_display_power_domain domain;
 	struct intel_plane *plane;
-	u64 domains;
 	struct drm_atomic_state *state;
 	struct intel_crtc_state *crtc_state;
 	int ret;
@@ -6510,11 +6516,6 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
 	intel_update_watermarks(intel_crtc);
 	intel_disable_shared_dpll(to_intel_crtc_state(crtc->state));
 
-	domains = intel_crtc->enabled_power_domains;
-	for_each_power_domain(domain, domains)
-		intel_display_power_put_unchecked(dev_priv, domain);
-	intel_crtc->enabled_power_domains = 0;
-
 	dev_priv->active_crtcs &= ~(1 << intel_crtc->pipe);
 	dev_priv->min_cdclk[intel_crtc->pipe] = 0;
 	dev_priv->min_voltage_level[intel_crtc->pipe] = 0;
@@ -13217,7 +13218,6 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 	struct intel_crtc_state *new_intel_crtc_state, *old_intel_crtc_state;
 	struct drm_crtc *crtc;
 	struct intel_crtc *intel_crtc;
-	u64 put_domains[I915_MAX_PIPES] = {};
 	intel_wakeref_t wakeref = 0;
 	int i;
 
@@ -13235,10 +13235,8 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 
 		if (needs_modeset(new_crtc_state) ||
 		    to_intel_crtc_state(new_crtc_state)->update_pipe) {
-
-			put_domains[intel_crtc->pipe] =
-				modeset_get_crtc_power_domains(crtc,
-					new_intel_crtc_state);
+			modeset_get_crtc_power_domains(crtc,
+						       new_intel_crtc_state);
 		}
 
 		if (!needs_modeset(new_crtc_state))
@@ -13351,8 +13349,8 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		intel_post_plane_update(to_intel_crtc_state(old_crtc_state));
 
-		if (put_domains[i])
-			modeset_put_power_domains(dev_priv, put_domains[i]);
+		modeset_put_power_domains(crtc,
+					  to_intel_crtc_state(old_crtc_state));
 
 		intel_modeset_verify_crtc(crtc, state, old_crtc_state, new_crtc_state);
 	}
@@ -16217,12 +16215,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
 	}
 
 	for_each_intel_crtc(dev, crtc) {
-		u64 put_domains;
-
 		crtc_state = to_intel_crtc_state(crtc->base.state);
-		put_domains = modeset_get_crtc_power_domains(&crtc->base, crtc_state);
-		if (WARN_ON(put_domains))
-			modeset_put_power_domains(dev_priv, put_domains);
+		modeset_get_crtc_power_domains(&crtc->base, crtc_state);
 	}
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT, wakeref);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index eec4ed93c335..82f3fbc4ae67 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -982,6 +982,11 @@ struct intel_crtc_state {
 
 	/* Forward Error correction State */
 	bool fec_enable;
+
+	u64 power_domains;
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
+	intel_wakeref_t wakeref[POWER_DOMAIN_NUM];
+#endif
 };
 
 struct intel_crtc {
@@ -994,7 +999,6 @@ struct intel_crtc {
 	 */
 	bool active;
 	u8 plane_ids_mask;
-	unsigned long long enabled_power_domains;
 	struct intel_overlay *overlay;
 
 	struct intel_crtc_state *config;
-- 
2.20.1



More information about the Intel-gfx-trybot mailing list