On Thu, 2020-07-02 at 18:37 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrj...@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.so...@intel.com>

> Cc: José Roberto de Souza <jose.so...@intel.com>
> Fixes: 691f7ba58d52 ("drm/i915/display/fbc: Make fences a nice-to-have for 
> GEN9+")
> Signed-off-by: Ville Syrjälä <ville.syrj...@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";
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to