[7/8] drm/i915: Move toggling planes out of crtc enable/disable.

Submitted by Maarten Lankhorst on April 15, 2015, 2:34 p.m.

Details

Message ID 1429108486-22008-7-git-send-email-maarten.lankhorst@linux.intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Maarten Lankhorst April 15, 2015, 2:34 p.m.
This makes disabling planes more explicit.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  4 ++++
 drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 3 files changed, 24 insertions(+), 18 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 07a71c0ff775..2d63e15a8669 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3594,8 +3594,10 @@  static void hsw_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
 		intel_display_power_get(dev_priv,
 					POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A));
 
+		intel_crtc_disable_planes(&crtc->base);
 		dev_priv->display.crtc_disable(&crtc->base);
 		dev_priv->display.crtc_enable(&crtc->base);
+		intel_crtc_enable_planes(&crtc->base);
 	}
 	drm_modeset_unlock_all(dev);
 }
@@ -3616,8 +3618,10 @@  static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
 	if (crtc->config->pch_pfit.force_thru) {
 		crtc->config->pch_pfit.force_thru = false;
 
+		intel_crtc_disable_planes(&crtc->base);
 		dev_priv->display.crtc_disable(&crtc->base);
 		dev_priv->display.crtc_enable(&crtc->base);
+		intel_crtc_enable_planes(&crtc->base);
 
 		intel_display_power_put(dev_priv,
 					POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A));
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 47fbdb5c71cb..cb677aff4245 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3123,8 +3123,8 @@  void intel_prepare_reset(struct drm_device *dev)
 	 * g33 docs say we should at least disable all the planes.
 	 */
 	for_each_intel_crtc(dev, crtc) {
-		if (crtc->active)
-			dev_priv->display.crtc_disable(&crtc->base);
+		intel_crtc_disable_planes(&crtc->base);
+		dev_priv->display.crtc_disable(&crtc->base);
 	}
 }
 
@@ -4711,10 +4711,13 @@  intel_pre_disable_primary(struct drm_crtc *crtc)
 	hsw_disable_ips(intel_crtc);
 }
 
-static void intel_crtc_enable_planes(struct drm_crtc *crtc)
+void intel_crtc_enable_planes(struct drm_crtc *crtc)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
+	if (intel_crtc->active)
+		return;
+
 	intel_enable_primary_hw_plane(crtc->primary, crtc);
 	intel_enable_sprite_planes(crtc);
 	intel_crtc_update_cursor(crtc, true);
@@ -4722,13 +4725,16 @@  static void intel_crtc_enable_planes(struct drm_crtc *crtc)
 	intel_post_enable_primary(crtc);
 }
 
-static void intel_crtc_disable_planes(struct drm_crtc *crtc)
+void intel_crtc_disable_planes(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_plane *intel_plane;
 	int pipe = intel_crtc->pipe;
 
+	if (!intel_crtc->active)
+		return;
+
 	intel_crtc_wait_for_pending_flips(crtc);
 
 	intel_pre_disable_primary(crtc);
@@ -4820,8 +4826,6 @@  static void ironlake_crtc_enable(struct drm_crtc *crtc)
 
 	if (HAS_PCH_CPT(dev))
 		cpt_verify_modeset(dev, intel_crtc->pipe);
-
-	intel_crtc_enable_planes(crtc);
 }
 
 /* IPS only exists on ULT machines and is tied to pipe A. */
@@ -4943,7 +4947,6 @@  static void haswell_crtc_enable(struct drm_crtc *crtc)
 	/* If we change the relative order between pipe/planes enabling, we need
 	 * to change the workaround. */
 	haswell_mode_set_planes_workaround(intel_crtc);
-	intel_crtc_enable_planes(crtc);
 }
 
 static void ironlake_pfit_disable(struct intel_crtc *crtc)
@@ -4973,8 +4976,6 @@  static void ironlake_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->active)
 		return;
 
-	intel_crtc_disable_planes(crtc);
-
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->disable(encoder);
 
@@ -5037,8 +5038,6 @@  static void haswell_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->active)
 		return;
 
-	intel_crtc_disable_planes(crtc);
-
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
 		intel_opregion_notify_encoder(encoder, false);
 		encoder->disable(encoder);
@@ -5578,8 +5577,6 @@  static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->enable(encoder);
-
-	intel_crtc_enable_planes(crtc);
 }
 
 static void i9xx_set_pll_dividers(struct intel_crtc *crtc)
@@ -5636,8 +5633,6 @@  static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->enable(encoder);
-
-	intel_crtc_enable_planes(crtc);
 }
 
 static void i9xx_pfit_disable(struct intel_crtc *crtc)
@@ -5666,8 +5661,6 @@  static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->active)
 		return;
 
-	intel_crtc_disable_planes(crtc);
-
 	/*
 	 * On gen2 planes are double buffered but the pipe isn't, so we must
 	 * wait for planes to fully turn off before disabling the pipe.
@@ -5731,9 +5724,11 @@  void intel_crtc_control(struct drm_crtc *crtc, bool enable)
 			intel_crtc->enabled_power_domains = domains;
 
 			dev_priv->display.crtc_enable(crtc);
+			intel_crtc_enable_planes(crtc);
 		}
 	} else {
 		if (intel_crtc->active) {
+			intel_crtc_disable_planes(crtc);
 			dev_priv->display.crtc_disable(crtc);
 
 			domains = intel_crtc->enabled_power_domains;
@@ -5768,6 +5763,7 @@  static void intel_crtc_disable(struct drm_crtc *crtc)
 	/* crtc should still be enabled when we disable it. */
 	WARN_ON(!crtc->state->enable);
 
+	intel_crtc_disable_planes(crtc);
 	dev_priv->display.crtc_disable(crtc);
 	dev_priv->display.off(crtc);
 
@@ -11991,8 +11987,10 @@  static int __intel_set_mode(struct drm_crtc *crtc,
 		intel_crtc_disable(&intel_crtc->base);
 
 	for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) {
-		if (intel_crtc->base.state->enable)
+		if (intel_crtc->base.state->enable) {
+			intel_crtc_disable_planes(&intel_crtc->base);
 			dev_priv->display.crtc_disable(&intel_crtc->base);
+		}
 	}
 
 	/* crtc->mode is already used by the ->mode_set callbacks, hence we need
@@ -12040,6 +12038,7 @@  static int __intel_set_mode(struct drm_crtc *crtc,
 		update_scanline_offset(intel_crtc);
 
 		dev_priv->display.crtc_enable(&intel_crtc->base);
+		intel_crtc_enable_planes(&intel_crtc->base);
 	}
 
 	/* FIXME: add subpixel order */
@@ -14433,6 +14432,7 @@  static void intel_sanitize_crtc(struct intel_crtc *crtc)
 		plane = crtc->plane;
 		to_intel_plane_state(crtc->base.primary->state)->visible = true;
 		crtc->plane = !plane;
+		intel_crtc_disable_planes(&crtc->base);
 		dev_priv->display.crtc_disable(&crtc->base);
 		crtc->plane = plane;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index eb1195a1de99..f521d21d04e9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -987,6 +987,8 @@  void intel_mark_busy(struct drm_device *dev);
 void intel_mark_idle(struct drm_device *dev);
 void intel_crtc_restore_mode(struct drm_crtc *crtc);
 void intel_crtc_control(struct drm_crtc *crtc, bool enable);
+void intel_crtc_enable_planes(struct drm_crtc *crtc);
+void intel_crtc_disable_planes(struct drm_crtc *crtc);
 void intel_crtc_update_dpms(struct drm_crtc *crtc);
 void intel_encoder_destroy(struct drm_encoder *encoder);
 int intel_connector_init(struct intel_connector *);

Comments

On Wed, 2015-04-15 at 16:34 +0200, Maarten Lankhorst wrote:
> This makes disabling planes more explicit.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  4 ++++
>  drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  3 files changed, 24 insertions(+), 18 deletions(-)

[...]

> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 47fbdb5c71cb..cb677aff4245 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4711,10 +4711,13 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
>  	hsw_disable_ips(intel_crtc);
>  }
>  
> -static void intel_crtc_enable_planes(struct drm_crtc *crtc)
> +void intel_crtc_enable_planes(struct drm_crtc *crtc)
>  {
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
> +	if (intel_crtc->active)
> +		return;
> +
>  	intel_enable_primary_hw_plane(crtc->primary, crtc);
>  	intel_enable_sprite_planes(crtc);
>  	intel_crtc_update_cursor(crtc, true);

[...]

> @@ -12040,6 +12038,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>  		update_scanline_offset(intel_crtc);
>  
>  		dev_priv->display.crtc_enable(&intel_crtc->base);
> +		intel_crtc_enable_planes(&intel_crtc->base);
>  	}

The crtc_enable hooks set intel_crtc->active, so the call to
intel_crtc_enable_planes() doesn't actually do anything because of the
new check in the hunk above.

Ander
Op 16-04-15 om 09:32 schreef Ander Conselvan De Oliveira:
> On Wed, 2015-04-15 at 16:34 +0200, Maarten Lankhorst wrote:
>> This makes disabling planes more explicit.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c  |  4 ++++
>>  drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++------------------
>>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>>  3 files changed, 24 insertions(+), 18 deletions(-)
> [...]
>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 47fbdb5c71cb..cb677aff4245 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4711,10 +4711,13 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
>>  	hsw_disable_ips(intel_crtc);
>>  }
>>  
>> -static void intel_crtc_enable_planes(struct drm_crtc *crtc)
>> +void intel_crtc_enable_planes(struct drm_crtc *crtc)
>>  {
>>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>  
>> +	if (intel_crtc->active)
>> +		return;
>> +
>>  	intel_enable_primary_hw_plane(crtc->primary, crtc);
>>  	intel_enable_sprite_planes(crtc);
>>  	intel_crtc_update_cursor(crtc, true);
> [...]
>
>> @@ -12040,6 +12038,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>>  		update_scanline_offset(intel_crtc);
>>  
>>  		dev_priv->display.crtc_enable(&intel_crtc->base);
>> +		intel_crtc_enable_planes(&intel_crtc->base);
>>  	}
> The crtc_enable hooks set intel_crtc->active, so the call to
> intel_crtc_enable_planes() doesn't actually do anything because of the
> new check in the hunk above.
Argh, you're right. I'll respin this patch.

~Maarten