> -----Original Message-----
> From: Ville Syrjälä <[email protected]>
> Sent: Thursday, September 10, 2020 7:22 PM
> To: Shankar, Uma <[email protected]>
> Cc: Jani Nikula <[email protected]>; [email protected]
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Add a vblank wait for FBC
> activation within same frame
> 
> On Thu, Sep 10, 2020 at 01:42:23PM +0000, Shankar, Uma wrote:
> >
> >
> > > -----Original Message-----
> > > From: Jani Nikula <[email protected]>
> > > Sent: Thursday, September 10, 2020 7:04 PM
> > > To: Shankar, Uma <[email protected]>;
> > > [email protected]
> > > Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Add a vblank wait
> > > for FBC activation within same frame
> > >
> > > On Thu, 10 Sep 2020, Uma Shankar <[email protected]> wrote:
> > > > Add a vblank wait when fbc activation request comes for the same
> > > > frame on TGL. This helps fix underrun related to fbc.
> > > >
> > > > Signed-off-by: Uma Shankar <[email protected]>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_fbc.c | 13 +++++++++++--
> > > >  1 file changed, 11 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> > > > b/drivers/gpu/drm/i915/display/intel_fbc.c
> > > > index 135f5e8a4d70..3e1d715e4a4e 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > > > @@ -1055,6 +1055,7 @@ static void __intel_fbc_post_update(struct
> > > > intel_crtc *crtc)  {
> > > >         struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > >         struct intel_fbc *fbc = &dev_priv->fbc;
> > > > +       static u32 old_frame_cnt, new_frame_cnt;
> > >
> > > In the strongest terms, do not add static data. These would be
> > > shared across devices, which is certainly a bug.
> >
> > Oh ok, yeah will remove this. I guess adding them to intel_fbc should be ok
> right ?
> 
> I didn't think we found what's really going on here. So this still seems like
> random duct tape which just changs some timings.

Yeah Ville, this still is a WA to avoid multiple FBC activation within frame. 
Adding the frame counter checks
to avoid waits on all updates.

More analysis still ongoing and issue is raised with hw team as well for 
further analysis.
My idea was to get the health of this WA on CI and general feedback if this can 
be considered as a
fallback option.

Regards,
Uma Shankar

 > > >
> > > >         drm_WARN_ON(&dev_priv->drm, !mutex_is_locked(&fbc->lock));
> > > >
> > > > @@ -1075,10 +1076,18 @@ static void __intel_fbc_post_update(struct
> > > intel_crtc *crtc)
> > > >         if (!intel_fbc_can_activate(crtc))
> > > >                 return;
> > > >
> > > > -       if (!fbc->busy_bits)
> > > > +       old_frame_cnt = new_frame_cnt;
> > > > +       new_frame_cnt = intel_crtc_get_vblank_counter(crtc);
> > > > +       if (!fbc->busy_bits) {
> > > > +               if (IS_TIGERLAKE(dev_priv) &&
> > > > +                   old_frame_cnt == new_frame_cnt) {
> > > > +                       drm_dbg_kms(&dev_priv->drm, "Wait for vblank 
> > > > before
> > > Activating FBC");
> > > > +                       intel_wait_for_vblank_if_active(dev_priv, 
> > > > crtc->pipe);
> > > > +               }
> > > >                 intel_fbc_hw_activate(dev_priv);
> > > > -       else
> > > > +       } else {
> > > >                 intel_fbc_deactivate(dev_priv, "frontbuffer write");
> > > > +       }
> > > >  }
> > > >
> > > >  void intel_fbc_post_update(struct intel_atomic_state *state,
> > >
> > > --
> > > Jani Nikula, Intel Open Source Graphics Center
> > _______________________________________________
> > Intel-gfx mailing list
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to