Re: [Intel-gfx] [PATCH v2 6/9] drm/i915: Fix g4x/vlv/chv CxSR vs. format/tiling/rotation changes

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

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
>