On Fri, 2020-07-03 at 14:38 +0300, Ville Syrjälä wrote:
> On Thu, Jul 02, 2020 at 11:24:04PM +0000, Souza, Jose wrote:
> > On Thu, 2020-07-02 at 18:37 +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > >
> > > 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@xxxxxxxxx>
> >
> > > Cc: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > > Fixes: 691f7ba58d52 ("drm/i915/display/fbc: Make fences a nice-to-have for GEN9+")
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > ---
> > > 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.
>
> Not really. Well, certainly not on pre-skl since they don't do 90/270
> rotation. And probably not even on skl+ since any wrong assumption
Checking rotation_is_valid() GEN5 up to GEN8 allows 90/270 rotation.
> about how the cfb stride is calculated by the hw would just cause it
> to scribble over stolen memory we didn't allocate. So unless we've
> started to use stolen more extensively in recent times such problems
> would probably go unnoticed.
>
> > > + 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
