diff mbox

[2/4] drm/i915: add intel_dp_power_get/put

Message ID 1398207345-1865-2-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni April 22, 2014, 10:55 p.m. UTC 
From: Paulo Zanoni <paulo.r.zanoni@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 :)

The downside of this patch is that in a few places we will call
intel_display_port_power_domain() twice instead of once.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 61 ++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 32 deletions(-)

Comments

Ville Syrjälä April 23, 2014, 9:45 a.m. UTC | #1 
On Tue, Apr 22, 2014 at 07:55:43PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@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 :)
> 
> The downside of this patch is that in a few places we will call
> intel_display_port_power_domain() twice instead of once.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 61 ++++++++++++++++++++---------------------
>  1 file changed, 29 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 44df493..b385b03 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1063,13 +1063,30 @@ static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
>  	return control;
>  }
>  
> +static void intel_dp_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);
> +}
> +
> +static void intel_dp_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);
> +}

There's nothing DP specific about these functions, so they could be
moved somewhere else and used for other output types as well.

> +
>  static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
>  {
>  	struct drm_device *dev = intel_dp_to_dev(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 +1099,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_dp_power_get(intel_encoder);
>  
>  	DRM_DEBUG_KMS("Turning eDP VDD on\n");
>  
> @@ -1133,7 +1149,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 +1168,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_dp_power_put(intel_encoder);
>  	}
>  }
>  
> @@ -1242,7 +1256,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 +1285,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_dp_power_put(intel_encoder);
>  }
>  
>  void intel_edp_backlight_on(struct intel_dp *intel_dp)
> @@ -3161,13 +3173,10 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>  	struct drm_device *dev = connector->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	enum drm_connector_status status;
> -	enum intel_display_power_domain power_domain;
>  	struct edid *edid = NULL;
>  
>  	intel_runtime_pm_get(dev_priv);
> -
> -	power_domain = intel_display_port_power_domain(intel_encoder);
> -	intel_display_power_get(dev_priv, power_domain);
> +	intel_dp_power_get(intel_encoder);
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>  		      connector->base.id, drm_get_connector_name(connector));
> @@ -3199,8 +3208,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>  	status = connector_status_connected;
>  
>  out:
> -	intel_display_power_put(dev_priv, power_domain);
> -
> +	intel_dp_power_put(intel_encoder);
>  	intel_runtime_pm_put(dev_priv);
>  
>  	return status;
> @@ -3213,18 +3221,14 @@ static int intel_dp_get_modes(struct drm_connector *connector)
>  	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
>  	struct drm_device *dev = connector->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	enum intel_display_power_domain power_domain;
>  	int ret;
>  
>  	/* We should parse the EDID data and find out if it has an audio sink
>  	 */
>  
> -	power_domain = intel_display_port_power_domain(intel_encoder);
> -	intel_display_power_get(dev_priv, power_domain);
> -
> +	intel_dp_power_get(intel_encoder);
>  	ret = intel_dp_get_edid_modes(connector, &intel_dp->aux.ddc);
> -	intel_display_power_put(dev_priv, power_domain);
> +	intel_dp_power_put(intel_encoder);
>  	if (ret)
>  		return ret;
>  
> @@ -3247,14 +3251,10 @@ intel_dp_detect_audio(struct drm_connector *connector)
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> -	struct drm_device *dev = connector->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	enum intel_display_power_domain power_domain;
>  	struct edid *edid;
>  	bool has_audio = false;
>  
> -	power_domain = intel_display_port_power_domain(intel_encoder);
> -	intel_display_power_get(dev_priv, power_domain);
> +	intel_dp_power_get(intel_encoder);
>  
>  	edid = intel_dp_get_edid(connector, &intel_dp->aux.ddc);
>  	if (edid) {
> @@ -3262,7 +3262,7 @@ intel_dp_detect_audio(struct drm_connector *connector)
>  		kfree(edid);
>  	}
>  
> -	intel_display_power_put(dev_priv, power_domain);
> +	intel_dp_power_put(intel_encoder);
>  
>  	return has_audio;
>  }
> @@ -3801,11 +3801,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_dp_power_get(intel_encoder);
>  
>  	/* Cache DPCD and EDID for edp. */
>  	intel_edp_panel_vdd_on(intel_dp);
> -- 
> 1.9.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 44df493..b385b03 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1063,13 +1063,30 @@  static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
 	return control;
 }
 
+static void intel_dp_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);
+}
+
+static void intel_dp_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 bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(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 +1099,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_dp_power_get(intel_encoder);
 
 	DRM_DEBUG_KMS("Turning eDP VDD on\n");
 
@@ -1133,7 +1149,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 +1168,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_dp_power_put(intel_encoder);
 	}
 }
 
@@ -1242,7 +1256,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 +1285,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_dp_power_put(intel_encoder);
 }
 
 void intel_edp_backlight_on(struct intel_dp *intel_dp)
@@ -3161,13 +3173,10 @@  intel_dp_detect(struct drm_connector *connector, bool force)
 	struct drm_device *dev = connector->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum drm_connector_status status;
-	enum intel_display_power_domain power_domain;
 	struct edid *edid = NULL;
 
 	intel_runtime_pm_get(dev_priv);
-
-	power_domain = intel_display_port_power_domain(intel_encoder);
-	intel_display_power_get(dev_priv, power_domain);
+	intel_dp_power_get(intel_encoder);
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, drm_get_connector_name(connector));
@@ -3199,8 +3208,7 @@  intel_dp_detect(struct drm_connector *connector, bool force)
 	status = connector_status_connected;
 
 out:
-	intel_display_power_put(dev_priv, power_domain);
-
+	intel_dp_power_put(intel_encoder);
 	intel_runtime_pm_put(dev_priv);
 
 	return status;
@@ -3213,18 +3221,14 @@  static int intel_dp_get_modes(struct drm_connector *connector)
 	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct intel_connector *intel_connector = to_intel_connector(connector);
 	struct drm_device *dev = connector->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum intel_display_power_domain power_domain;
 	int ret;
 
 	/* We should parse the EDID data and find out if it has an audio sink
 	 */
 
-	power_domain = intel_display_port_power_domain(intel_encoder);
-	intel_display_power_get(dev_priv, power_domain);
-
+	intel_dp_power_get(intel_encoder);
 	ret = intel_dp_get_edid_modes(connector, &intel_dp->aux.ddc);
-	intel_display_power_put(dev_priv, power_domain);
+	intel_dp_power_put(intel_encoder);
 	if (ret)
 		return ret;
 
@@ -3247,14 +3251,10 @@  intel_dp_detect_audio(struct drm_connector *connector)
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct intel_encoder *intel_encoder = &intel_dig_port->base;
-	struct drm_device *dev = connector->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum intel_display_power_domain power_domain;
 	struct edid *edid;
 	bool has_audio = false;
 
-	power_domain = intel_display_port_power_domain(intel_encoder);
-	intel_display_power_get(dev_priv, power_domain);
+	intel_dp_power_get(intel_encoder);
 
 	edid = intel_dp_get_edid(connector, &intel_dp->aux.ddc);
 	if (edid) {
@@ -3262,7 +3262,7 @@  intel_dp_detect_audio(struct drm_connector *connector)
 		kfree(edid);
 	}
 
-	intel_display_power_put(dev_priv, power_domain);
+	intel_dp_power_put(intel_encoder);
 
 	return has_audio;
 }
@@ -3801,11 +3801,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_dp_power_get(intel_encoder);
 
 	/* Cache DPCD and EDID for edp. */
 	intel_edp_panel_vdd_on(intel_dp);