Re: [PATCH v3 1/4] drm/i915/display: avoid calling fbc activate if fbc is active

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jan 14, 2025 at 02:07:16PM +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.
> "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 followup patches. Nuke on flip in that case will
> remove all the benefits of fetching only the modified region.
> 
> 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@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index df05904bac8a..fd540ff5e57e 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -1561,7 +1561,8 @@ static void __intel_fbc_post_update(struct intel_fbc *fbc)
>  	fbc->flip_pending = false;
>  	fbc->busy_bits = 0;
>  
> -	intel_fbc_activate(fbc);
> +	if (!fbc->active)
> +		intel_fbc_activate(fbc);

We'll need to keep the actual activate part (eg. to update the fence).
But we should be able to elide the explicit nuke if FBC was already
active (that implies a flip nuke has occurred anyway, vs. if FBC was
previously disabled then it might have been disabled by a frontbuffer
invalidate and if it hasn't been disabled for a full frame then the
hardware won't automagically cause a nuke when we reactivate it).

>  }
>  
>  void intel_fbc_post_update(struct intel_atomic_state *state,
> -- 
> 2.43.0

-- 
Ville Syrjälä
Intel