[Intel-gfx] [PATCH 1/4] drm/i915/fbc: Use the correct plane stride

[Souza, Jose]

On Thu, 2020-07-02 at 18:37 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Consult the actual plane stride instead of the fb stride. The two
> will disagree when we remap the gtt. The plane stride is what the
> hw will be fed so that's what we should look at for the FBC
> retrictions/cfb allocation.
> 
> Since we no longer require a fence we are going to attempt using
> FBC with remapping, and so we should look at correct stride.
> 
> With 90/270 degree rotation the plane stride is stored in units
> of pixels, so we need to conver it to bytes for the purposes
> of calculating the cfb stride. Not entirely sure if this matches
> the hw behaviour though. Need to reverse engineer that at some
> point...
> 
> We also need to reorder the pixel format check vs. stride check
> to avoid triggering a spurious WARN(stride & 63) with cpp==1 and
> plane stride==32.
> 
> v2: Try to deal with rotated stride and related WARN
> 

Reviewed-by: José Roberto de Souza <jose.souza at intel.com>

> Cc: José Roberto de Souza <jose.souza at intel.com>
> Fixes: 691f7ba58d52 ("drm/i915/display/fbc: Make fences a nice-to-have for GEN9+")
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 69a0682ddb6a..d30c2a389294 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -695,9 +695,13 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
>  	cache->plane.pixel_blend_mode = plane_state->hw.pixel_blend_mode;
>  
>  	cache->fb.format = fb->format;
> -	cache->fb.stride = fb->pitches[0];
>  	cache->fb.modifier = fb->modifier;
>  
> +	/* FIXME is this correct? */
> +	cache->fb.stride = plane_state->color_plane[0].stride;
> +	if (drm_rotation_90_or_270(plane_state->hw.rotation))

If it was wrong our CI would caught this in BDW or SNB for example.

> +		cache->fb.stride *= fb->format->cpp[0];
> +
>  	/* FBC1 compression interval: arbitrary choice of 1 second */
>  	cache->interval = drm_mode_vrefresh(&crtc_state->hw.adjusted_mode);
>  
> @@ -797,6 +801,11 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
>  		return false;
>  	}
>  
> +	if (!pixel_format_is_valid(dev_priv, cache->fb.format->format)) {
> +		fbc->no_fbc_reason = "pixel format is invalid";
> +		return false;
> +	}
> +
>  	if (!rotation_is_valid(dev_priv, cache->fb.format->format,
>  			       cache->plane.rotation)) {
>  		fbc->no_fbc_reason = "rotation unsupported";
> @@ -813,11 +822,6 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
>  		return false;
>  	}
>  
> -	if (!pixel_format_is_valid(dev_priv, cache->fb.format->format)) {
> -		fbc->no_fbc_reason = "pixel format is invalid";
> -		return false;
> -	}
> -
>  	if (cache->plane.pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE &&
>  	    cache->fb.format->has_alpha) {
>  		fbc->no_fbc_reason = "per-pixel alpha blending is incompatible with FBC";