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

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

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