[Intel-gfx] [PATCH 2/4] drm/i915: add intel_encoder_power_get/put

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni at intel.com>

This was suggested by Chris on his review to the first version of
"drm/i915: get power domain in case the BIOS enabled eDP VDD". Well,
at least that's what I understood from his comment :)

v2: - Make it intel_encoder_power_get/put
    - Don't call the new functions on functions where we need both
      get() and put(), so we don't end up calling
      intel_display_port_power_domain() more than needed.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 19 +++++--------------
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 drivers/gpu/drm/i915/intel_pm.c  | 24 ++++++++++++++++++++++++
 3 files changed, 31 insertions(+), 14 deletions(-)


I am not really sure if this patch is worth it... I certainly won't be mad if we
drop it. The diffstat is not good.


diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a421c81..8201b60 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1069,7 +1069,6 @@ static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum intel_display_power_domain power_domain;
 	u32 pp;
 	u32 pp_stat_reg, pp_ctrl_reg;
 	bool need_to_disable = !intel_dp->want_panel_vdd;
@@ -1082,8 +1081,7 @@ static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
 	if (edp_have_panel_vdd(intel_dp))
 		return need_to_disable;
 
-	power_domain = intel_display_port_power_domain(intel_encoder);
-	intel_display_power_get(dev_priv, power_domain);
+	intel_encoder_power_get(intel_encoder);
 
 	DRM_DEBUG_KMS("Turning eDP VDD on\n");
 
@@ -1133,7 +1131,6 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
 		struct intel_digital_port *intel_dig_port =
 						dp_to_dig_port(intel_dp);
 		struct intel_encoder *intel_encoder = &intel_dig_port->base;
-		enum intel_display_power_domain power_domain;
 
 		DRM_DEBUG_KMS("Turning eDP VDD off\n");
 
@@ -1153,8 +1150,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
 		if ((pp & POWER_TARGET_ON) == 0)
 			intel_dp->last_power_cycle = jiffies;
 
-		power_domain = intel_display_port_power_domain(intel_encoder);
-		intel_display_power_put(dev_priv, power_domain);
+		intel_encoder_power_put(intel_encoder);
 	}
 }
 
@@ -1242,7 +1238,6 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
 	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum intel_display_power_domain power_domain;
 	u32 pp;
 	u32 pp_ctrl_reg;
 
@@ -1272,8 +1267,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
 	wait_panel_off(intel_dp);
 
 	/* We got a reference when we enabled the VDD. */
-	power_domain = intel_display_port_power_domain(intel_encoder);
-	intel_display_power_put(dev_priv, power_domain);
+	intel_encoder_power_put(intel_encoder);
 }
 
 void intel_edp_backlight_on(struct intel_dp *intel_dp)
@@ -3798,11 +3792,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 
 	/* The VDD bit needs a power domain reference, so if the bit is already
 	 * enabled when we boot, grab this reference. */
-	if (edp_have_panel_vdd(intel_dp)) {
-		enum intel_display_power_domain power_domain;
-		power_domain = intel_display_port_power_domain(intel_encoder);
-		intel_display_power_get(dev_priv, power_domain);
-	}
+	if (edp_have_panel_vdd(intel_dp))
+		intel_encoder_power_get(intel_encoder);
 
 	/* Cache DPCD and EDID for edp. */
 	intel_edp_panel_vdd_on(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b885df1..5f27b51 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -920,6 +920,8 @@ void intel_display_power_get(struct drm_i915_private *dev_priv,
 			     enum intel_display_power_domain domain);
 void intel_display_power_put(struct drm_i915_private *dev_priv,
 			     enum intel_display_power_domain domain);
+void intel_encoder_power_get(struct intel_encoder *encoder);
+void intel_encoder_power_put(struct intel_encoder *encoder);
 void intel_power_domains_init_hw(struct drm_i915_private *dev_priv);
 void intel_init_gt_powersave(struct drm_device *dev);
 void intel_cleanup_gt_powersave(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 75c1c76..8ed8d53 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5734,6 +5734,30 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 	intel_runtime_pm_put(dev_priv);
 }
 
+/*
+ * Short version for when you don't need to use both get() and put() on the same
+ * function. If you need both, consider calling intel_display_power_get/put()
+ * directly so you will save a call to intel_display_port_power_domain().
+ */
+void intel_encoder_power_get(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	enum intel_display_power_domain power_domain;
+
+	power_domain = intel_display_port_power_domain(encoder);
+	intel_display_power_get(dev_priv, power_domain);
+}
+
+/* See the comment above intel_encoder_power_get(). */
+void intel_encoder_power_put(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	enum intel_display_power_domain power_domain;
+
+	power_domain = intel_display_port_power_domain(encoder);
+	intel_display_power_put(dev_priv, power_domain);
+}
+
 static struct i915_power_domains *hsw_pwr;
 
 /* Display audio driver power well request */
-- 
1.9.0