diff mbox

[52/58] drm/i915: push commit_output_state past crtc disabling

Message ID 1345403595-9678-53-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State Accepted
Headers show

Commit Message

Daniel Vetter Aug. 19, 2012, 7:13 p.m. UTC
This requires a few changes
- We still need a noop function for crtc->disable, becuase the fb
  helper is a bit too intimate with the crtc helper.
- We need to clear crtc->fb ourselves in intel_crtc_disable now that
  we no longer rely on the helper's disable_unused_functions to do
  that.
- We need to split out the sare update code, becuase the crtc code
  can't call update_dpms any more, it needs to disable the crtc
  unconditionally. This is because we now keep onto the encoder ->
  crtc mapping of the (still) active output pipe configuration.
- To check that we really disable a crtc that still has encoders,
  insert a WARN_ON(!enabled) in the crtc disable function.
- Lastly, we need to walk over all crtcs to update their enabled state
  after having called commit_output_state - for all disabled crtc the
  crtc helper code has done that for us previously.

v2: Update connector dpms and encoder->connectors_active after
disabling the crtc, too.

v3: Noop-out intel_encoder_disable. Similarly to the crtc disable
callback used by the crtc helper code we can't simply remove all these
encoder callbacks: The fb helper (which we still use) has a rather
incetious relationship with the crtc helper code ...

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 81 +++++++++++++++++++++++-------------
 1 file changed, 53 insertions(+), 28 deletions(-)

Comments

Jesse Barnes Sept. 5, 2012, 6:17 p.m. UTC | #1
On Sun, 19 Aug 2012 21:13:09 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> This requires a few changes
> - We still need a noop function for crtc->disable, becuase the fb
>   helper is a bit too intimate with the crtc helper.
> - We need to clear crtc->fb ourselves in intel_crtc_disable now that
>   we no longer rely on the helper's disable_unused_functions to do
>   that.
> - We need to split out the sare update code, becuase the crtc code
>   can't call update_dpms any more, it needs to disable the crtc
>   unconditionally. This is because we now keep onto the encoder ->
>   crtc mapping of the (still) active output pipe configuration.
> - To check that we really disable a crtc that still has encoders,
>   insert a WARN_ON(!enabled) in the crtc disable function.
> - Lastly, we need to walk over all crtcs to update their enabled state
>   after having called commit_output_state - for all disabled crtc the
>   crtc helper code has done that for us previously.
> 
> v2: Update connector dpms and encoder->connectors_active after
> disabling the crtc, too.
> 
> v3: Noop-out intel_encoder_disable. Similarly to the crtc disable
> callback used by the crtc helper code we can't simply remove all these
> encoder callbacks: The fb helper (which we still use) has a rather
> incetious relationship with the crtc helper code ...
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 81 +++++++++++++++++++++++-------------
>  1 file changed, 53 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3d99522..48d763d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3462,26 +3462,13 @@ static void i9xx_crtc_off(struct drm_crtc *crtc)
>  {
>  }
>  
> -/**
> - * Sets the power management mode of the pipe and plane.
> - */
> -void intel_crtc_update_dpms(struct drm_crtc *crtc)
> +static void intel_crtc_update_sarea(struct drm_crtc *crtc,
> +				    bool enabled)
>  {
>  	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_i915_master_private *master_priv;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct intel_encoder *intel_encoder;
>  	int pipe = intel_crtc->pipe;
> -	bool enabled, enable = false;
> -
> -	for_each_encoder_on_crtc(dev, crtc, intel_encoder)
> -		enable |= intel_encoder->connectors_active;
> -
> -	if (enable)
> -		dev_priv->display.crtc_enable(crtc);
> -	else
> -		dev_priv->display.crtc_disable(crtc);
>  
>  	if (!dev->primary->master)
>  		return;
> @@ -3490,8 +3477,6 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
>  	if (!master_priv->sarea_priv)
>  		return;
>  
> -	enabled = crtc->enabled && enable;
> -
>  	switch (pipe) {
>  	case 0:
>  		master_priv->sarea_priv->pipeA_w = enabled ? crtc->mode.hdisplay : 0;
> @@ -3507,14 +3492,42 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
>  	}
>  }
>  
> +/**
> + * Sets the power management mode of the pipe and plane.
> + */
> +void intel_crtc_update_dpms(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_encoder *intel_encoder;
> +	bool enable = false;
> +
> +	for_each_encoder_on_crtc(dev, crtc, intel_encoder)
> +		enable |= intel_encoder->connectors_active;
> +
> +	if (enable)
> +		dev_priv->display.crtc_enable(crtc);
> +	else
> +		dev_priv->display.crtc_disable(crtc);
> +
> +	intel_crtc_update_sarea(crtc, enable);
> +}
> +
> +static void intel_crtc_noop(struct drm_crtc *crtc)
> +{
> +}
> +
>  static void intel_crtc_disable(struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->dev;
> +	struct drm_connector *connector;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	/* crtc->disable is only called when we have no encoders, hence this
> -	 * will disable the pipe. */
> -	intel_crtc_update_dpms(crtc);
> +	/* crtc should still be enabled when we disable it. */
> +	WARN_ON(!crtc->enabled);
> +
> +	dev_priv->display.crtc_disable(crtc);
> +	intel_crtc_update_sarea(crtc, false);
>  	dev_priv->display.off(crtc);
>  
>  	assert_plane_disabled(dev->dev_private, to_intel_crtc(crtc)->plane);
> @@ -3524,14 +3537,24 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
>  		mutex_lock(&dev->struct_mutex);
>  		intel_unpin_fb_obj(to_intel_framebuffer(crtc->fb)->obj);
>  		mutex_unlock(&dev->struct_mutex);
> +		crtc->fb = NULL;
> +	}
> +
> +	/* Update computed state. */
> +	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> +		if (!connector->encoder || !connector->encoder->crtc)
> +			continue;
> +
> +		if (connector->encoder->crtc != crtc)
> +			continue;
> +
> +		connector->dpms = DRM_MODE_DPMS_OFF;
> +		to_intel_encoder(connector->encoder)->connectors_active = false;
>  	}
>  }
>  
>  void intel_encoder_disable(struct drm_encoder *encoder)
>  {
> -	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
> -
> -	intel_encoder->disable(intel_encoder);
>  }
>  
>  void intel_encoder_destroy(struct drm_encoder *encoder)
> @@ -6560,7 +6583,7 @@ free_work:
>  static struct drm_crtc_helper_funcs intel_helper_funcs = {
>  	.mode_set_base_atomic = intel_pipe_set_base_atomic,
>  	.load_lut = intel_crtc_load_lut,
> -	.disable = intel_crtc_disable,
> +	.disable = intel_crtc_noop,
>  };
>  
>  bool intel_encoder_check_non_cloned(struct intel_encoder *encoder)
> @@ -6816,12 +6839,14 @@ bool intel_set_mode(struct drm_crtc *crtc,
>  	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
>  		      modeset_pipes, prepare_pipes, disable_pipes);
>  
> -	intel_modeset_commit_output_state(dev);
> +	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
> +		intel_crtc_disable(&intel_crtc->base);
>  
> -	crtc->enabled = drm_helper_crtc_in_use(crtc);
> +	intel_modeset_commit_output_state(dev);
>  
> -	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
> -		drm_helper_disable_unused_functions(dev);
> +	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
> +			    base.head)
> +		intel_crtc->base.enabled = drm_helper_crtc_in_use(crtc);
>  
>  	saved_hwmode = crtc->hwmode;
>  	saved_mode = crtc->mode;

Starting to look much better.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3d99522..48d763d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3462,26 +3462,13 @@  static void i9xx_crtc_off(struct drm_crtc *crtc)
 {
 }
 
-/**
- * Sets the power management mode of the pipe and plane.
- */
-void intel_crtc_update_dpms(struct drm_crtc *crtc)
+static void intel_crtc_update_sarea(struct drm_crtc *crtc,
+				    bool enabled)
 {
 	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_master_private *master_priv;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_encoder *intel_encoder;
 	int pipe = intel_crtc->pipe;
-	bool enabled, enable = false;
-
-	for_each_encoder_on_crtc(dev, crtc, intel_encoder)
-		enable |= intel_encoder->connectors_active;
-
-	if (enable)
-		dev_priv->display.crtc_enable(crtc);
-	else
-		dev_priv->display.crtc_disable(crtc);
 
 	if (!dev->primary->master)
 		return;
@@ -3490,8 +3477,6 @@  void intel_crtc_update_dpms(struct drm_crtc *crtc)
 	if (!master_priv->sarea_priv)
 		return;
 
-	enabled = crtc->enabled && enable;
-
 	switch (pipe) {
 	case 0:
 		master_priv->sarea_priv->pipeA_w = enabled ? crtc->mode.hdisplay : 0;
@@ -3507,14 +3492,42 @@  void intel_crtc_update_dpms(struct drm_crtc *crtc)
 	}
 }
 
+/**
+ * Sets the power management mode of the pipe and plane.
+ */
+void intel_crtc_update_dpms(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_encoder *intel_encoder;
+	bool enable = false;
+
+	for_each_encoder_on_crtc(dev, crtc, intel_encoder)
+		enable |= intel_encoder->connectors_active;
+
+	if (enable)
+		dev_priv->display.crtc_enable(crtc);
+	else
+		dev_priv->display.crtc_disable(crtc);
+
+	intel_crtc_update_sarea(crtc, enable);
+}
+
+static void intel_crtc_noop(struct drm_crtc *crtc)
+{
+}
+
 static void intel_crtc_disable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
+	struct drm_connector *connector;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	/* crtc->disable is only called when we have no encoders, hence this
-	 * will disable the pipe. */
-	intel_crtc_update_dpms(crtc);
+	/* crtc should still be enabled when we disable it. */
+	WARN_ON(!crtc->enabled);
+
+	dev_priv->display.crtc_disable(crtc);
+	intel_crtc_update_sarea(crtc, false);
 	dev_priv->display.off(crtc);
 
 	assert_plane_disabled(dev->dev_private, to_intel_crtc(crtc)->plane);
@@ -3524,14 +3537,24 @@  static void intel_crtc_disable(struct drm_crtc *crtc)
 		mutex_lock(&dev->struct_mutex);
 		intel_unpin_fb_obj(to_intel_framebuffer(crtc->fb)->obj);
 		mutex_unlock(&dev->struct_mutex);
+		crtc->fb = NULL;
+	}
+
+	/* Update computed state. */
+	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+		if (!connector->encoder || !connector->encoder->crtc)
+			continue;
+
+		if (connector->encoder->crtc != crtc)
+			continue;
+
+		connector->dpms = DRM_MODE_DPMS_OFF;
+		to_intel_encoder(connector->encoder)->connectors_active = false;
 	}
 }
 
 void intel_encoder_disable(struct drm_encoder *encoder)
 {
-	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
-
-	intel_encoder->disable(intel_encoder);
 }
 
 void intel_encoder_destroy(struct drm_encoder *encoder)
@@ -6560,7 +6583,7 @@  free_work:
 static struct drm_crtc_helper_funcs intel_helper_funcs = {
 	.mode_set_base_atomic = intel_pipe_set_base_atomic,
 	.load_lut = intel_crtc_load_lut,
-	.disable = intel_crtc_disable,
+	.disable = intel_crtc_noop,
 };
 
 bool intel_encoder_check_non_cloned(struct intel_encoder *encoder)
@@ -6816,12 +6839,14 @@  bool intel_set_mode(struct drm_crtc *crtc,
 	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
 		      modeset_pipes, prepare_pipes, disable_pipes);
 
-	intel_modeset_commit_output_state(dev);
+	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
+		intel_crtc_disable(&intel_crtc->base);
 
-	crtc->enabled = drm_helper_crtc_in_use(crtc);
+	intel_modeset_commit_output_state(dev);
 
-	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
-		drm_helper_disable_unused_functions(dev);
+	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
+			    base.head)
+		intel_crtc->base.enabled = drm_helper_crtc_in_use(crtc);
 
 	saved_hwmode = crtc->hwmode;
 	saved_mode = crtc->mode;