diff mbox series

[v2,6/9] drm/i915: Fix g4x/vlv/chv CxSR vs. format/tiling/rotation changes

Message ID 20220622155452.32587-7-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: g4x/vlv/chv CxSR/wm fixes/cleanups | expand

Commit Message

Ville Syrjala June 22, 2022, 3:54 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On g4x/vlv/chv the hardware seems incapable of changing the pixel
format, rotation, or YUV->RGB CSC matrix while in CxSR.

Additionally on VLV/CHV the sprites seem incapable of tiling
changes while in CxSR. On g4x CxSR is not even possible with
the sprite enabled. Curiously the primary plane seems perfectly
happy when changing tiling during CxSR.

Pimp up the code to account for these when determining whether
CxSR needs to be disabled. Since it looks like most of the plane
control register bits are affected let's just compare that.
But in the name of efficiency we'll make an exception for the
primary plane tiling changes (avoids some extra vblank waits).

v2: Just use the pre-computed plane control register values

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../gpu/drm/i915/display/intel_atomic_plane.c | 53 ++++++++++++++++---
 1 file changed, 45 insertions(+), 8 deletions(-)

Comments

Lisovskiy, Stanislav Oct. 7, 2022, 5:57 a.m. UTC | #1 
On Wed, Jun 22, 2022 at 06:54:49PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On g4x/vlv/chv the hardware seems incapable of changing the pixel
> format, rotation, or YUV->RGB CSC matrix while in CxSR.
> 
> Additionally on VLV/CHV the sprites seem incapable of tiling
> changes while in CxSR. On g4x CxSR is not even possible with
> the sprite enabled. Curiously the primary plane seems perfectly
> happy when changing tiling during CxSR.
> 
> Pimp up the code to account for these when determining whether
> CxSR needs to be disabled. Since it looks like most of the plane
> control register bits are affected let's just compare that.
> But in the name of efficiency we'll make an exception for the
> primary plane tiling changes (avoids some extra vblank waits).
> 
> v2: Just use the pre-computed plane control register values
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> ---
>  .../gpu/drm/i915/display/intel_atomic_plane.c | 53 ++++++++++++++++---
>  1 file changed, 45 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index efe8591619e3..e5ad6a437a97 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -426,6 +426,47 @@ static bool intel_plane_do_async_flip(struct intel_plane *plane,
>  	return DISPLAY_VER(i915) < 13 || old_crtc_state->uapi.async_flip;
>  }
>  
> +static bool i9xx_must_disable_cxsr(const struct intel_crtc_state *new_crtc_state,
> +				   const struct intel_plane_state *old_plane_state,
> +				   const struct intel_plane_state *new_plane_state)
> +{
> +	struct intel_plane *plane = to_intel_plane(new_plane_state->uapi.plane);
> +	bool old_visible = old_plane_state->uapi.visible;
> +	bool new_visible = new_plane_state->uapi.visible;
> +	u32 old_ctl = old_plane_state->ctl;
> +	u32 new_ctl = new_plane_state->ctl;
> +	bool modeset, turn_on, turn_off;
> +
> +	if (plane->id == PLANE_CURSOR)
> +		return false;
> +
> +	modeset = intel_crtc_needs_modeset(new_crtc_state);
> +	turn_off = old_visible && (!new_visible || modeset);
> +	turn_on = new_visible && (!old_visible || modeset);
> +
> +	/* Must disable CxSR around plane enable/disable */
> +	if (turn_on || turn_off)
> +		return true;
> +
> +	if (!old_visible || !new_visible)
> +		return false;
> +
> +	/*
> +	 * Most plane control register updates are blocked while in CxSR.
> +	 *
> +	 * Tiling mode is one exception where the primary plane can
> +	 * apparently handle it, whereas the sprites can not (the
> +	 * sprite issue being only relevant on VLV/CHV where CxSR
> +	 * is actually possible with a sprite enabled).
> +	 */
> +	if (plane->id == PLANE_PRIMARY) {
> +		old_ctl &= ~DISP_TILED;
> +		new_ctl &= ~DISP_TILED;
> +	}
> +
> +	return old_ctl != new_ctl;
> +}
> +
>  static int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_state,
>  					   struct intel_crtc_state *new_crtc_state,
>  					   const struct intel_plane_state *old_plane_state,
> @@ -483,17 +524,9 @@ static int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_cr
>  	if (turn_on) {
>  		if (DISPLAY_VER(dev_priv) < 5 && !IS_G4X(dev_priv))
>  			new_crtc_state->update_wm_pre = true;
> -
> -		/* must disable cxsr around plane enable/disable */
> -		if (plane->id != PLANE_CURSOR)
> -			new_crtc_state->disable_cxsr = true;
>  	} else if (turn_off) {
>  		if (DISPLAY_VER(dev_priv) < 5 && !IS_G4X(dev_priv))
>  			new_crtc_state->update_wm_post = true;
> -
> -		/* must disable cxsr around plane enable/disable */
> -		if (plane->id != PLANE_CURSOR)
> -			new_crtc_state->disable_cxsr = true;
>  	} else if (intel_wm_need_update(old_plane_state, new_plane_state)) {
>  		if (DISPLAY_VER(dev_priv) < 5 && !IS_G4X(dev_priv)) {
>  			/* FIXME bollocks */
> @@ -505,6 +538,10 @@ static int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_cr
>  	if (visible || was_visible)
>  		new_crtc_state->fb_bits |= plane->frontbuffer_bit;
>  
> +	if (HAS_GMCH(dev_priv) &&
> +	    i9xx_must_disable_cxsr(new_crtc_state, old_plane_state, new_plane_state))
> +		new_crtc_state->disable_cxsr = true;
> +
>  	/*
>  	 * ILK/SNB DVSACNTR/Sprite Enable
>  	 * IVB SPR_CTL/Sprite Enable
> -- 
> 2.35.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index efe8591619e3..e5ad6a437a97 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -426,6 +426,47 @@  static bool intel_plane_do_async_flip(struct intel_plane *plane,
 	return DISPLAY_VER(i915) < 13 || old_crtc_state->uapi.async_flip;
 }
 
+static bool i9xx_must_disable_cxsr(const struct intel_crtc_state *new_crtc_state,
+				   const struct intel_plane_state *old_plane_state,
+				   const struct intel_plane_state *new_plane_state)
+{
+	struct intel_plane *plane = to_intel_plane(new_plane_state->uapi.plane);
+	bool old_visible = old_plane_state->uapi.visible;
+	bool new_visible = new_plane_state->uapi.visible;
+	u32 old_ctl = old_plane_state->ctl;
+	u32 new_ctl = new_plane_state->ctl;
+	bool modeset, turn_on, turn_off;
+
+	if (plane->id == PLANE_CURSOR)
+		return false;
+
+	modeset = intel_crtc_needs_modeset(new_crtc_state);
+	turn_off = old_visible && (!new_visible || modeset);
+	turn_on = new_visible && (!old_visible || modeset);
+
+	/* Must disable CxSR around plane enable/disable */
+	if (turn_on || turn_off)
+		return true;
+
+	if (!old_visible || !new_visible)
+		return false;
+
+	/*
+	 * Most plane control register updates are blocked while in CxSR.
+	 *
+	 * Tiling mode is one exception where the primary plane can
+	 * apparently handle it, whereas the sprites can not (the
+	 * sprite issue being only relevant on VLV/CHV where CxSR
+	 * is actually possible with a sprite enabled).
+	 */
+	if (plane->id == PLANE_PRIMARY) {
+		old_ctl &= ~DISP_TILED;
+		new_ctl &= ~DISP_TILED;
+	}
+
+	return old_ctl != new_ctl;
+}
+
 static int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_state,
 					   struct intel_crtc_state *new_crtc_state,
 					   const struct intel_plane_state *old_plane_state,
@@ -483,17 +524,9 @@  static int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_cr
 	if (turn_on) {
 		if (DISPLAY_VER(dev_priv) < 5 && !IS_G4X(dev_priv))
 			new_crtc_state->update_wm_pre = true;
-
-		/* must disable cxsr around plane enable/disable */
-		if (plane->id != PLANE_CURSOR)
-			new_crtc_state->disable_cxsr = true;
 	} else if (turn_off) {
 		if (DISPLAY_VER(dev_priv) < 5 && !IS_G4X(dev_priv))
 			new_crtc_state->update_wm_post = true;
-
-		/* must disable cxsr around plane enable/disable */
-		if (plane->id != PLANE_CURSOR)
-			new_crtc_state->disable_cxsr = true;
 	} else if (intel_wm_need_update(old_plane_state, new_plane_state)) {
 		if (DISPLAY_VER(dev_priv) < 5 && !IS_G4X(dev_priv)) {
 			/* FIXME bollocks */
@@ -505,6 +538,10 @@  static int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_cr
 	if (visible || was_visible)
 		new_crtc_state->fb_bits |= plane->frontbuffer_bit;
 
+	if (HAS_GMCH(dev_priv) &&
+	    i9xx_must_disable_cxsr(new_crtc_state, old_plane_state, new_plane_state))
+		new_crtc_state->disable_cxsr = true;
+
 	/*
 	 * ILK/SNB DVSACNTR/Sprite Enable
 	 * IVB SPR_CTL/Sprite Enable