| Message ID | 20250122093006.405711-2-vinod.govindapillai@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/xe3: FBC Dirty rect feature support | expand |
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@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
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@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 >
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; + intel_fbc_activate(fbc); }
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@intel.com> --- drivers/gpu/drm/i915/display/intel_fbc.c | 11 +++++++++++ 1 file changed, 11 insertions(+)