diff mbox

[2/4] drm/i915: Move sprite/cursor plane disable to intel_sanitize_crtc()

Message ID 1441900750-25599-2-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjala Sept. 10, 2015, 3:59 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Move the sprite/cursor plane disabling to occur in intel_sanitize_crtc()
where it belongs instead of doing it in intel_modeset_readout_hw_state().

The plane disabling was first added in
4cf0ebbd4fafbdf8e6431dbb315e5511c3efdc3b drm/i915: Rework plane readout.

I got the idea from some patches from Partik and/or Maarten but those
moved also the plane state readout to intel_sanitize_crtc() which isn't
quite right in my opinion.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 44 ++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 24 deletions(-)

Comments

Patrik Jakobsson Sept. 14, 2015, 11:48 a.m. UTC | #1 
On Thu, Sep 10, 2015 at 06:59:08PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move the sprite/cursor plane disabling to occur in intel_sanitize_crtc()
> where it belongs instead of doing it in intel_modeset_readout_hw_state().
> 
> The plane disabling was first added in
> 4cf0ebbd4fafbdf8e6431dbb315e5511c3efdc3b drm/i915: Rework plane readout.
> 
> I got the idea from some patches from Partik and/or Maarten but those
> moved also the plane state readout to intel_sanitize_crtc() which isn't
> quite right in my opinion.

I agree, this is better.

Probably related bug report:
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91910

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 44 ++++++++++++++++--------------------
>  1 file changed, 20 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f5673c8..3707212 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14878,9 +14878,19 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
>  	/* restore vblank interrupts to correct state */
>  	drm_crtc_vblank_reset(&crtc->base);
>  	if (crtc->active) {
> +		struct intel_plane *plane;
> +
>  		drm_calc_timestamping_constants(&crtc->base, &crtc->base.hwmode);
>  		update_scanline_offset(crtc);
>  		drm_crtc_vblank_on(&crtc->base);
> +
> +		/* Disable everything but the primary plane */
> +		for_each_intel_plane_on_crtc(dev, crtc, plane) {
> +			if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)
> +				continue;
> +
> +			plane->disable_plane(&plane->base, &crtc->base);
> +		}
>  	}
>  
>  	/* We need to sanitize the plane -> pipe mapping first because this will
> @@ -15043,35 +15053,21 @@ void i915_redisable_vga(struct drm_device *dev)
>  	i915_redisable_vga_power_on(dev);
>  }
>  
> -static bool primary_get_hw_state(struct intel_crtc *crtc)
> +static bool primary_get_hw_state(struct intel_plane *plane)
>  {
> -	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  
> -	return !!(I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE);
> +	return I915_READ(DSPCNTR(plane->plane)) & DISPLAY_PLANE_ENABLE;
>  }
>  
> -static void readout_plane_state(struct intel_crtc *crtc,
> -				struct intel_crtc_state *crtc_state)
> +/* FIXME read out full plane state for all planes */
> +static void readout_plane_state(struct intel_crtc *crtc)
>  {
> -	struct intel_plane *p;
> -	struct intel_plane_state *plane_state;
> -	bool active = crtc_state->base.active;
> +	struct intel_plane_state *plane_state =
> +		to_intel_plane_state(crtc->base.primary->state);
>  
> -	for_each_intel_plane(crtc->base.dev, p) {
> -		if (crtc->pipe != p->pipe)
> -			continue;
> -
> -		plane_state = to_intel_plane_state(p->base.state);
> -
> -		if (p->base.type == DRM_PLANE_TYPE_PRIMARY)
> -			plane_state->visible = primary_get_hw_state(crtc);
> -		else {
> -			if (active)
> -				p->disable_plane(&p->base, &crtc->base);
> -
> -			plane_state->visible = false;
> -		}
> -	}
> +	plane_state->visible =
> +		primary_get_hw_state(to_intel_plane(crtc->base.primary));
>  }
>  
>  static void intel_modeset_readout_hw_state(struct drm_device *dev)
> @@ -15094,7 +15090,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  		crtc->base.state->active = crtc->active;
>  		crtc->base.enabled = crtc->active;
>  
> -		readout_plane_state(crtc, to_intel_crtc_state(crtc->base.state));
> +		readout_plane_state(crtc);
>  
>  		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
>  			      crtc->base.base.id,
> -- 
> 2.4.6
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f5673c8..3707212 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14878,9 +14878,19 @@  static void intel_sanitize_crtc(struct intel_crtc *crtc)
 	/* restore vblank interrupts to correct state */
 	drm_crtc_vblank_reset(&crtc->base);
 	if (crtc->active) {
+		struct intel_plane *plane;
+
 		drm_calc_timestamping_constants(&crtc->base, &crtc->base.hwmode);
 		update_scanline_offset(crtc);
 		drm_crtc_vblank_on(&crtc->base);
+
+		/* Disable everything but the primary plane */
+		for_each_intel_plane_on_crtc(dev, crtc, plane) {
+			if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)
+				continue;
+
+			plane->disable_plane(&plane->base, &crtc->base);
+		}
 	}
 
 	/* We need to sanitize the plane -> pipe mapping first because this will
@@ -15043,35 +15053,21 @@  void i915_redisable_vga(struct drm_device *dev)
 	i915_redisable_vga_power_on(dev);
 }
 
-static bool primary_get_hw_state(struct intel_crtc *crtc)
+static bool primary_get_hw_state(struct intel_plane *plane)
 {
-	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 
-	return !!(I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE);
+	return I915_READ(DSPCNTR(plane->plane)) & DISPLAY_PLANE_ENABLE;
 }
 
-static void readout_plane_state(struct intel_crtc *crtc,
-				struct intel_crtc_state *crtc_state)
+/* FIXME read out full plane state for all planes */
+static void readout_plane_state(struct intel_crtc *crtc)
 {
-	struct intel_plane *p;
-	struct intel_plane_state *plane_state;
-	bool active = crtc_state->base.active;
+	struct intel_plane_state *plane_state =
+		to_intel_plane_state(crtc->base.primary->state);
 
-	for_each_intel_plane(crtc->base.dev, p) {
-		if (crtc->pipe != p->pipe)
-			continue;
-
-		plane_state = to_intel_plane_state(p->base.state);
-
-		if (p->base.type == DRM_PLANE_TYPE_PRIMARY)
-			plane_state->visible = primary_get_hw_state(crtc);
-		else {
-			if (active)
-				p->disable_plane(&p->base, &crtc->base);
-
-			plane_state->visible = false;
-		}
-	}
+	plane_state->visible =
+		primary_get_hw_state(to_intel_plane(crtc->base.primary));
 }
 
 static void intel_modeset_readout_hw_state(struct drm_device *dev)
@@ -15094,7 +15090,7 @@  static void intel_modeset_readout_hw_state(struct drm_device *dev)
 		crtc->base.state->active = crtc->active;
 		crtc->base.enabled = crtc->active;
 
-		readout_plane_state(crtc, to_intel_crtc_state(crtc->base.state));
+		readout_plane_state(crtc);
 
 		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
 			      crtc->base.base.id,