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

Kamil Konieczny
2026-04-10T14:14:29Z

Hi Ville,
On 2026-03-27 at 00:45:06 +0200, Ville Syrjälä wrote:
> 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...
>

+cc few more KMS devs
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Juha-Pekka Heikkila <juha-pekka.heikkila@intel.com>
Cc: Karthik B S <karthik.b.s@intel.com>

Btw maybe it is a good candidate for a new work-item on igt GitLab?

Regards,
Kamil

> --
> Ville Syrjälä
> Intel