Re: [igt-dev] [PATCH i-g-t 2/4] tests/kms_frontbuffer_tracking: Improve tiling test coverage

["Souza, Jose" <jose.souza@intel.com>]

On Thu, 2020-02-06 at 14:00 +0200, Ville Syrjälä wrote:
> On Wed, Feb 05, 2020 at 06:09:42PM -0800, José Roberto de Souza
> wrote:
> > Lets add some more tiling tests, if tiling is supported by FBC draw
> > some rectangles and compare the CRC against benchmark if not
> > supported run the test to guarantee that FBC is disabled.
> > 
> > This is a preparation for when kernel will allow FBC to be enabled
> > without fences, so we can better test linear and Y tiled
> > frontbuffers.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  tests/kms_frontbuffer_tracking.c | 50 +++++++++++++++++++++++++---
> > ----
> >  1 file changed, 39 insertions(+), 11 deletions(-)
> > 
> > diff --git a/tests/kms_frontbuffer_tracking.c
> > b/tests/kms_frontbuffer_tracking.c
> > index 63b5d12d..d5a64a4d 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -2159,6 +2159,23 @@ static void format_draw_subtest(const struct
> > test_mode *t)
> >  		badformat_subtest(t);
> >  }
> >  
> > +static bool tiling_is_valid(int feature_flags, enum tile_type
> > tile)
> > +{
> > +	if (!(feature_flags & FEATURE_FBC))
> > +		return true;
> > +
> > +	switch (tile) {
> > +	case TILE_LINEAR:
> > +		return false;
> > +	case TILE_X:
> > +	case TILE_Y:
> > +		return true;
> > +	default:
> > +		igt_assert(false);
> > +		return false;
> > +	}
> > +}
> > +
> >  /*
> >   * slow_draw - sleep a little bit between drawing operations
> >   *
> > @@ -2954,22 +2971,22 @@ static void stridechange_subtest(const
> > struct test_mode *t)
> >  }
> >  
> >  /**
> > - * tilingchange - alternate between tiled and untiled in multiple
> > ways
> > + * tiling_disable_fbc_subtest - Check that tiling causes FBC to be
> > disabled
> >   *
> >   * METHOD
> > - *   This test alternates between tiled and untiled frontbuffers
> > of the same
> > - *   size and format through multiple different APIs: the page
> > flip IOCTL,
> > - *   normal modesets and the plane APIs.
> > + *   This test alternates between a FBC supported and non-
> > supported tiled
> > + *   frontbuffers of the same size and format through multiple
> > different
> > + *   APIs: the page flip IOCTL, normal modesets and the plane
> > APIs.
> >   *
> >   * EXPECTED RESULTS
> > - *   FBC gets properly disabled for the untiled FB and reenabled
> > for the
> > - *   tiled FB.
> > + *   FBC gets properly disabled for the non-supported tiling and
> > reenabled for
> > + *   the supported tiling.
> >   *
> >   * FAILURES
> >   *   Bad Kernels may somehow leave FBC enabled, which can cause
> > FIFO underruns
> >   *   that lead to CRC assertion failures.
> >   */
> > -static void tilingchange_subtest(const struct test_mode *t)
> > +static void tiling_disable_fbc_subtest(const struct test_mode *t)
> >  {
> >  	struct igt_fb new_fb, *old_fb;
> >  	struct modeset_params *params = pick_params(t);
> > @@ -2980,13 +2997,13 @@ static void tilingchange_subtest(const
> > struct test_mode *t)
> >  	old_fb = params->primary.fb;
> >  
> >  	create_fb(t->format, params->primary.fb->width, params-
> > >primary.fb->height,
> > -		  LOCAL_DRM_FORMAT_MOD_NONE, t->plane, &new_fb);
> > +		  t->tile, t->plane, &new_fb);
> >  	fill_fb(&new_fb, COLOR_PRIM_BG);
> >  
> >  	for (flip_type = 0; flip_type < FLIP_COUNT; flip_type++) {
> >  		igt_debug("Flip type: %d\n", flip_type);
> >  
> > -		/* Set a buffer with no tiling. */
> > +		/* Set a buffer with new tiling. */
> >  		params->primary.fb = &new_fb;
> >  		page_flip_for_params(params, flip_type);
> >  		do_assertions(ASSERT_FBC_DISABLED);
> > @@ -3486,8 +3503,19 @@ igt_main_args("", long_options, help_str,
> > opt_handler, NULL)
> >  			igt_subtest_f("%s-stridechange",
> > feature_str(t.feature))
> >  				stridechange_subtest(&t);
> >  
> > -			igt_subtest_f("%s-tilingchange",
> > feature_str(t.feature))
> > -				tilingchange_subtest(&t);
> > +			for (t.tile = TILE_LINEAR; t.tile < TILE_COUNT;
> > +			     t.tile++) {
> 
> Ah, I guess the enum is for this loop.
> 
> s/tile/tiling/ whould be more customary language in general.
> Although, maybe people would then confuse it with the uapi
> definitions? Shrug.

Renaming to tiling.

> 
> > +				igt_subtest_f("%s-tiling-%s",
> > +					      feature_str(t.feature),
> > +					      tile_str(t.tile)) {
> > +
> > +					if (tiling_is_valid(t.feature,
> > t.tile))
> > +						draw_subtest(&t);
> > +					else
> > +						tiling_disable_fbc_subt
> > est(&t);
> > +				}
> > +			}
> > +			t.tile = opt.tiling;
> >  		}
> >  
> >  		if ((t.feature & FEATURE_PSR) || (t.feature &
> > FEATURE_DRRS))
> > -- 
> > 2.25.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev