Re: [PATCH i-g-t 1/4] tests/intel/kms_frontbuffer_tracking: update the outdated fbc status checks

Ville Syrjälä
2026-03-26T22:45:18Z

On Thu, Mar 26, 2026 at 11:27:47AM +0200, Jani Nikula wrote:
> On Thu, 26 Mar 2026, Vinod Govindapillai <vinod.govindapillai@intel.com> wrote:
> > Replace the fbc status check for "mode too large for compression" wchich
> > is no longer being set by the driver with "plane size too big" and
> > "surface size too big" fbc status checks.
> >
> > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> > ---
> > tests/intel/kms_frontbuffer_tracking.c | 17 +++++++++++++----
> > 1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/intel/kms_frontbuffer_tracking.c b/tests/intel/kms_frontbuffer_tracking.c
> > index c16f63199..e764c9ce1 100644
> > --- a/tests/intel/kms_frontbuffer_tracking.c
> > +++ b/tests/intel/kms_frontbuffer_tracking.c
> > @@ -1664,15 +1664,23 @@ static bool fbc_stride_not_supported(void)
> > char buf[128];
> >
> > debugfs_read_crtc("i915_fbc_status", buf);
> > - return strstr(buf, "FBC disabled: framebuffer stride not supported\n");
> > + return strstr(buf, "FBC disabled: stride not supported\n");
> > }
> >
> > -static bool fbc_mode_too_large(void)
> > +static bool fbc_plane_size_too_big(void)
> > {
> > char buf[128];
> >
> > debugfs_read_crtc("i915_fbc_status", buf);
> > - return strstr(buf, "FBC disabled: mode too large for compression\n");
> > + return strstr(buf, "FBC disabled: plane size too big\n");
> > +}
> > +
> > +static bool fbc_surface_size_too_big(void)
> > +{
> > + char buf[128];
> > +
> > + debugfs_read_crtc("i915_fbc_status", buf);
> > + return strstr(buf, "FBC disabled: surface size too big\n");
> > }
> >
> > static bool fbc_psr_not_possible(void)
> > @@ -2356,7 +2364,8 @@ static void do_status_assertions(int flags)
> > if (flags & ASSERT_FBC_ENABLED) {
> > igt_require(!fbc_not_enough_stolen());
> > igt_require(!fbc_stride_not_supported());
> > - igt_require(!fbc_mode_too_large());
> > + igt_require(!fbc_plane_size_too_big());
> > + igt_require(!fbc_surface_size_too_big());
>
> I think the problem with the current implementation and this series is
> that it still keeps all those i915_fbc_status reads in place, instead of
> reading it *once* and then figuring it out.
>
> You could e.g. have a function that reads i915_fbc_status, parses it
> into, I don't know, an enum on the igt side, with some handling of
> unknown "FBC disabled: xxx" messages, and returns that.
>
> And then you get that status once, and check that, instead of reading
> the file N times.

The other big problem is that the buffer size used for this is too
small. A while ago I noticed that it was getting chopped off so I
have a patch in some branch that doubles it. But dunno if even that
is sufficient on the latest platforms with more and more FBC capable
planes. Someone should probably check that...

--
Ville Syrjälä
Intel