diff mbox series

[v2,04/13] drm/i915: Pass the new crtc_state to ->disable_plane()

Message ID 20181114210729.16185-5-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Program SKL+ watermarks/ddb more carefully | expand

Commit Message

Ville Syrjala Nov. 14, 2018, 9:07 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We're going to need access to the new crtc state in ->disable_plane()
for SKL+ wm/ddb programming and pre-skl pipe gamma/csc control. Pass
the crtc state down.

We'll also try to make intel_crtc_disable_planes() do the right
thing as much as it's possible. The fact that we don't have a
separate crtc state for the disabled state when we're going to
re-enable the crtc later means we might end up poking at a few
extra planes in there. But that's harmless. I suppose one might
argue that we wouldn't have to care about proper ddb/wm/csc/gamma
if the pipe is going to permanently disable anyway, but the state
checker probably cares so we should try our best to make sure
everything is programmed correctly even in that case.

v2: Fix the commit message a bit (Matt)

Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  2 +-
 drivers/gpu/drm/i915/intel_display.c      | 39 ++++++++++++++---------
 drivers/gpu/drm/i915/intel_display.h      |  8 +++++
 drivers/gpu/drm/i915/intel_drv.h          |  2 +-
 drivers/gpu/drm/i915/intel_sprite.c       | 12 ++++---
 5 files changed, 42 insertions(+), 21 deletions(-)

Comments

Matt Roper Nov. 19, 2018, 11:14 p.m. UTC | #1 
On Wed, Nov 14, 2018 at 11:07:20PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We're going to need access to the new crtc state in ->disable_plane()
> for SKL+ wm/ddb programming and pre-skl pipe gamma/csc control. Pass
> the crtc state down.
> 
> We'll also try to make intel_crtc_disable_planes() do the right
> thing as much as it's possible. The fact that we don't have a
> separate crtc state for the disabled state when we're going to
> re-enable the crtc later means we might end up poking at a few
> extra planes in there. But that's harmless. I suppose one might
> argue that we wouldn't have to care about proper ddb/wm/csc/gamma
> if the pipe is going to permanently disable anyway, but the state
> checker probably cares so we should try our best to make sure
> everything is programmed correctly even in that case.
> 
> v2: Fix the commit message a bit (Matt)
> 
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_atomic_plane.c |  2 +-
>  drivers/gpu/drm/i915/intel_display.c      | 39 ++++++++++++++---------
>  drivers/gpu/drm/i915/intel_display.h      |  8 +++++
>  drivers/gpu/drm/i915/intel_drv.h          |  2 +-
>  drivers/gpu/drm/i915/intel_sprite.c       | 12 ++++---
>  5 files changed, 42 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 010269a12390..69fc7010190c 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -210,7 +210,7 @@ void intel_update_planes_on_crtc(struct intel_atomic_state *old_state,
>  		} else {
>  			trace_intel_disable_plane(&plane->base, crtc);
>  
> -			plane->disable_plane(plane, crtc);
> +			plane->disable_plane(plane, new_crtc_state);
>  		}
>  	}
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 065c8befc6f8..0caba7258fee 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2767,7 +2767,7 @@ static void intel_plane_disable_noatomic(struct intel_crtc *crtc,
>  		intel_pre_disable_primary_noatomic(&crtc->base);
>  
>  	trace_intel_disable_plane(&plane->base, crtc);
> -	plane->disable_plane(plane, crtc);
> +	plane->disable_plane(plane, crtc_state);
>  }
>  
>  static void
> @@ -3372,7 +3372,7 @@ static void i9xx_update_plane(struct intel_plane *plane,
>  }
>  
>  static void i9xx_disable_plane(struct intel_plane *plane,
> -			       struct intel_crtc *crtc)
> +			       const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  	enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
> @@ -5405,23 +5405,32 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
>  		intel_update_watermarks(crtc);
>  }
>  
> -static void intel_crtc_disable_planes(struct intel_crtc *crtc, unsigned plane_mask)
> +static void intel_crtc_disable_planes(struct intel_atomic_state *state,
> +				      struct intel_crtc *crtc)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	const struct intel_crtc_state *new_crtc_state =
> +		intel_atomic_get_new_crtc_state(state, crtc);
> +	unsigned int update_mask = new_crtc_state->update_planes;
> +	const struct intel_plane_state *old_plane_state;
>  	struct intel_plane *plane;
>  	unsigned fb_bits = 0;
> +	int i;
>  
>  	intel_crtc_dpms_overlay_disable(crtc);
>  
> -	for_each_intel_plane_on_crtc(dev, crtc, plane) {
> -		if (plane_mask & BIT(plane->id)) {
> -			plane->disable_plane(plane, crtc);
> +	for_each_old_intel_plane_in_state(state, plane, old_plane_state, i) {
> +		if (crtc->pipe != plane->pipe ||
> +		    !(update_mask & BIT(plane->id)))
> +			continue;
>  
> +		plane->disable_plane(plane, new_crtc_state);
> +
> +		if (old_plane_state->base.visible)
>  			fb_bits |= plane->frontbuffer_bit;
> -		}
>  	}
>  
> -	intel_frontbuffer_flip(to_i915(dev), fb_bits);
> +	intel_frontbuffer_flip(dev_priv, fb_bits);
>  }
>  
>  static void intel_encoders_pre_pll_enable(struct drm_crtc *crtc,
> @@ -9866,9 +9875,9 @@ static void i845_update_cursor(struct intel_plane *plane,
>  }
>  
>  static void i845_disable_cursor(struct intel_plane *plane,
> -				struct intel_crtc *crtc)
> +				const struct intel_crtc_state *crtc_state)
>  {
> -	i845_update_cursor(plane, NULL, NULL);
> +	i845_update_cursor(plane, crtc_state, NULL);
>  }
>  
>  static bool i845_cursor_get_hw_state(struct intel_plane *plane,
> @@ -10095,9 +10104,9 @@ static void i9xx_update_cursor(struct intel_plane *plane,
>  }
>  
>  static void i9xx_disable_cursor(struct intel_plane *plane,
> -				struct intel_crtc *crtc)
> +				const struct intel_crtc_state *crtc_state)
>  {
> -	i9xx_update_cursor(plane, NULL, NULL);
> +	i9xx_update_cursor(plane, crtc_state, NULL);
>  }
>  
>  static bool i9xx_cursor_get_hw_state(struct intel_plane *plane,
> @@ -12861,7 +12870,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>  		intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state);
>  
>  		if (old_crtc_state->active) {
> -			intel_crtc_disable_planes(intel_crtc, old_intel_crtc_state->active_planes);
> +			intel_crtc_disable_planes(intel_state, intel_crtc);
>  
>  			/*
>  			 * We need to disable pipe CRC before disabling the pipe,
> @@ -13711,7 +13720,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
>  					  to_intel_plane_state(plane->state));
>  	} else {
>  		trace_intel_disable_plane(plane, to_intel_crtc(crtc));
> -		intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc));
> +		intel_plane->disable_plane(intel_plane, crtc_state);
>  	}
>  
>  	intel_plane_unpin_fb(to_intel_plane_state(old_plane_state));
> diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> index 5d50decbcbb5..df9e6ebb27de 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -382,6 +382,14 @@ struct intel_link_m_n {
>  	for_each_power_well_rev(__dev_priv, __power_well)		        \
>  		for_each_if((__power_well)->desc->domains & (__domain_mask))
>  
> +#define for_each_old_intel_plane_in_state(__state, plane, old_plane_state, __i) \
> +	for ((__i) = 0; \
> +	     (__i) < (__state)->base.dev->mode_config.num_total_plane && \
> +		     ((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \
> +		      (old_plane_state) = to_intel_plane_state((__state)->base.planes[__i].old_state), 1); \
> +	     (__i)++) \
> +		for_each_if(plane)
> +
>  #define for_each_new_intel_plane_in_state(__state, plane, new_plane_state, __i) \
>  	for ((__i) = 0; \
>  	     (__i) < (__state)->base.dev->mode_config.num_total_plane && \
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b0a24a81780a..23b33970db17 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1015,7 +1015,7 @@ struct intel_plane {
>  			     const struct intel_crtc_state *crtc_state,
>  			     const struct intel_plane_state *plane_state);
>  	void (*disable_plane)(struct intel_plane *plane,
> -			      struct intel_crtc *crtc);
> +			      const struct intel_crtc_state *crtc_state);
>  	bool (*get_hw_state)(struct intel_plane *plane, enum pipe *pipe);
>  	int (*check_plane)(struct intel_crtc_state *crtc_state,
>  			   struct intel_plane_state *plane_state);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 6403ef2219d0..74d904a49bf9 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -594,7 +594,8 @@ icl_update_slave(struct intel_plane *plane,
>  }
>  
>  static void
> -skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
> +skl_disable_plane(struct intel_plane *plane,
> +		  const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  	enum plane_id plane_id = plane->id;
> @@ -857,7 +858,8 @@ vlv_update_plane(struct intel_plane *plane,
>  }
>  
>  static void
> -vlv_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
> +vlv_disable_plane(struct intel_plane *plane,
> +		  const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  	enum pipe pipe = plane->pipe;
> @@ -1024,7 +1026,8 @@ ivb_update_plane(struct intel_plane *plane,
>  }
>  
>  static void
> -ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
> +ivb_disable_plane(struct intel_plane *plane,
> +		  const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  	enum pipe pipe = plane->pipe;
> @@ -1190,7 +1193,8 @@ g4x_update_plane(struct intel_plane *plane,
>  }
>  
>  static void
> -g4x_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
> +g4x_disable_plane(struct intel_plane *plane,
> +		  const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  	enum pipe pipe = plane->pipe;
> -- 
> 2.18.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 010269a12390..69fc7010190c 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -210,7 +210,7 @@  void intel_update_planes_on_crtc(struct intel_atomic_state *old_state,
 		} else {
 			trace_intel_disable_plane(&plane->base, crtc);
 
-			plane->disable_plane(plane, crtc);
+			plane->disable_plane(plane, new_crtc_state);
 		}
 	}
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 065c8befc6f8..0caba7258fee 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2767,7 +2767,7 @@  static void intel_plane_disable_noatomic(struct intel_crtc *crtc,
 		intel_pre_disable_primary_noatomic(&crtc->base);
 
 	trace_intel_disable_plane(&plane->base, crtc);
-	plane->disable_plane(plane, crtc);
+	plane->disable_plane(plane, crtc_state);
 }
 
 static void
@@ -3372,7 +3372,7 @@  static void i9xx_update_plane(struct intel_plane *plane,
 }
 
 static void i9xx_disable_plane(struct intel_plane *plane,
-			       struct intel_crtc *crtc)
+			       const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
@@ -5405,23 +5405,32 @@  static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
 		intel_update_watermarks(crtc);
 }
 
-static void intel_crtc_disable_planes(struct intel_crtc *crtc, unsigned plane_mask)
+static void intel_crtc_disable_planes(struct intel_atomic_state *state,
+				      struct intel_crtc *crtc)
 {
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct intel_crtc_state *new_crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
+	unsigned int update_mask = new_crtc_state->update_planes;
+	const struct intel_plane_state *old_plane_state;
 	struct intel_plane *plane;
 	unsigned fb_bits = 0;
+	int i;
 
 	intel_crtc_dpms_overlay_disable(crtc);
 
-	for_each_intel_plane_on_crtc(dev, crtc, plane) {
-		if (plane_mask & BIT(plane->id)) {
-			plane->disable_plane(plane, crtc);
+	for_each_old_intel_plane_in_state(state, plane, old_plane_state, i) {
+		if (crtc->pipe != plane->pipe ||
+		    !(update_mask & BIT(plane->id)))
+			continue;
 
+		plane->disable_plane(plane, new_crtc_state);
+
+		if (old_plane_state->base.visible)
 			fb_bits |= plane->frontbuffer_bit;
-		}
 	}
 
-	intel_frontbuffer_flip(to_i915(dev), fb_bits);
+	intel_frontbuffer_flip(dev_priv, fb_bits);
 }
 
 static void intel_encoders_pre_pll_enable(struct drm_crtc *crtc,
@@ -9866,9 +9875,9 @@  static void i845_update_cursor(struct intel_plane *plane,
 }
 
 static void i845_disable_cursor(struct intel_plane *plane,
-				struct intel_crtc *crtc)
+				const struct intel_crtc_state *crtc_state)
 {
-	i845_update_cursor(plane, NULL, NULL);
+	i845_update_cursor(plane, crtc_state, NULL);
 }
 
 static bool i845_cursor_get_hw_state(struct intel_plane *plane,
@@ -10095,9 +10104,9 @@  static void i9xx_update_cursor(struct intel_plane *plane,
 }
 
 static void i9xx_disable_cursor(struct intel_plane *plane,
-				struct intel_crtc *crtc)
+				const struct intel_crtc_state *crtc_state)
 {
-	i9xx_update_cursor(plane, NULL, NULL);
+	i9xx_update_cursor(plane, crtc_state, NULL);
 }
 
 static bool i9xx_cursor_get_hw_state(struct intel_plane *plane,
@@ -12861,7 +12870,7 @@  static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 		intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state);
 
 		if (old_crtc_state->active) {
-			intel_crtc_disable_planes(intel_crtc, old_intel_crtc_state->active_planes);
+			intel_crtc_disable_planes(intel_state, intel_crtc);
 
 			/*
 			 * We need to disable pipe CRC before disabling the pipe,
@@ -13711,7 +13720,7 @@  intel_legacy_cursor_update(struct drm_plane *plane,
 					  to_intel_plane_state(plane->state));
 	} else {
 		trace_intel_disable_plane(plane, to_intel_crtc(crtc));
-		intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc));
+		intel_plane->disable_plane(intel_plane, crtc_state);
 	}
 
 	intel_plane_unpin_fb(to_intel_plane_state(old_plane_state));
diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index 5d50decbcbb5..df9e6ebb27de 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -382,6 +382,14 @@  struct intel_link_m_n {
 	for_each_power_well_rev(__dev_priv, __power_well)		        \
 		for_each_if((__power_well)->desc->domains & (__domain_mask))
 
+#define for_each_old_intel_plane_in_state(__state, plane, old_plane_state, __i) \
+	for ((__i) = 0; \
+	     (__i) < (__state)->base.dev->mode_config.num_total_plane && \
+		     ((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \
+		      (old_plane_state) = to_intel_plane_state((__state)->base.planes[__i].old_state), 1); \
+	     (__i)++) \
+		for_each_if(plane)
+
 #define for_each_new_intel_plane_in_state(__state, plane, new_plane_state, __i) \
 	for ((__i) = 0; \
 	     (__i) < (__state)->base.dev->mode_config.num_total_plane && \
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b0a24a81780a..23b33970db17 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1015,7 +1015,7 @@  struct intel_plane {
 			     const struct intel_crtc_state *crtc_state,
 			     const struct intel_plane_state *plane_state);
 	void (*disable_plane)(struct intel_plane *plane,
-			      struct intel_crtc *crtc);
+			      const struct intel_crtc_state *crtc_state);
 	bool (*get_hw_state)(struct intel_plane *plane, enum pipe *pipe);
 	int (*check_plane)(struct intel_crtc_state *crtc_state,
 			   struct intel_plane_state *plane_state);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 6403ef2219d0..74d904a49bf9 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -594,7 +594,8 @@  icl_update_slave(struct intel_plane *plane,
 }
 
 static void
-skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
+skl_disable_plane(struct intel_plane *plane,
+		  const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	enum plane_id plane_id = plane->id;
@@ -857,7 +858,8 @@  vlv_update_plane(struct intel_plane *plane,
 }
 
 static void
-vlv_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
+vlv_disable_plane(struct intel_plane *plane,
+		  const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	enum pipe pipe = plane->pipe;
@@ -1024,7 +1026,8 @@  ivb_update_plane(struct intel_plane *plane,
 }
 
 static void
-ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
+ivb_disable_plane(struct intel_plane *plane,
+		  const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	enum pipe pipe = plane->pipe;
@@ -1190,7 +1193,8 @@  g4x_update_plane(struct intel_plane *plane,
 }
 
 static void
-g4x_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
+g4x_disable_plane(struct intel_plane *plane,
+		  const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	enum pipe pipe = plane->pipe;