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.govindapil...@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.
> +
> intel_fbc_activate(fbc);
> }
>
> --
> 2.43.0
--
Ville Syrjälä
Intel
