[PATCH v4 1/6] drm/i915/xe3: avoid calling fbc activate if fbc is active

[Govindapillai, Vinod]

On Wed, 2025-01-22 at 20:13 +0200, Ville Syrjälä wrote:
> On Wed, Jan 22, 2025 at 11:30:01AM +0200, Vinod Govindapillai wrote:
> > If FBC is already active, we don't need to call FBC activate
> > routine again during the post plane update. As this will
> > explicitly call the nuke and also rewrite the FBC ctl registers.
> > Xe doesn't support legacy fences. Hence fence programming also
> > not required as part of this fbc_haw_activate.
> > 
> > "intel_atomic_commit_tail-> intel_post_plane_update->
> > intel_fbc_post_update-> _intel_fbc_post_update" path will be
> > executed during the normal flip cases. FBC HW will nuke on sync
> > flip event and driver do not need to call the nuke explicitly.
> > 
> > This is much more relevant in case of dirty rectangle support
> > in FBC with the follow-up patches. Nuke on flip in that case will
> > remove all the benefits of fetching only the modified region.
> > Also any FBC related register updates with dirty rectangle
> > support enabled will trigger nuke by FBC HW.
> > 
> > The front buffer rendering sequence will call intel_fbc_flush()
> > and which will call intel_fbc_nuke() or intel_fbc_activate()
> > based on FBC status explicitly and won't get impacted by this
> > change.
> > 
> > Signed-off-by: Vinod Govindapillai <vinod.govindapillai at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_fbc.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index df05904bac8a..ab9649dd606c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -1556,11 +1556,22 @@ static void __intel_fbc_disable(struct intel_fbc *fbc)
> >  
> >  static void __intel_fbc_post_update(struct intel_fbc *fbc)
> >  {
> > +	struct intel_display *display = fbc->display;
> > +
> >  	lockdep_assert_held(&fbc->lock);
> >  
> >  	fbc->flip_pending = false;
> >  	fbc->busy_bits = 0;
> >  
> > +	/*
> > +	 * When dirty rectangle is enabled, any updates to FBC registers will
> > +	 * trigger nuke. So avoid calling intel_fbc_activate if fbc is already
> > +	 * active and for XE3 cases. Xe doesn't support legacy fences. So
> > +	 * no need to update the fences as well.
> > +	 */
> > +	if (DISPLAY_VER(display) >= 30 && fbc->active)
> > +		return;
> 
> Don't like platform checks in generic code. Either we should 
> skip the stride programming inside the hw specific function,
> or we pull the strie programming into a separate hook which
> we can then skip completely when FBC was already active.

What I understood from Hw team is that any FBC register touch will trigger nuke not just the stride.
So in this case we need to skip the entire hw_activate if the fbc is already active.

BR
Vinod

> 
> > +
> >  	intel_fbc_activate(fbc);
> >  }
> >  
> > -- 
> > 2.43.0
>