Re: [igt-dev] [PATCH i-g-t] tests/kms_big_fb: Add retry mechanism for async flip subtests

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

On Mon, Oct 04, 2021 at 12:19:01PM +0300, Ville Syrjälä wrote:
> On Mon, Oct 04, 2021 at 02:26:29PM +0530, Karthik B S wrote:
> > Async flip subtests fail sporadically with CRC failure on CI.
> > This is expected as these tests are not run on highest priority by the
> > scheduler, but this creates noise on CI. Add retry mechanism to rerun
> > the test once if failure is seen.
> > 
> > Signed-off-by: Karthik B S <karthik.b.s@intel.com>
> > ---
> >  tests/i915/kms_big_fb.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/tests/i915/kms_big_fb.c b/tests/i915/kms_big_fb.c
> > index 308227c9..8c09f59e 100644
> > --- a/tests/i915/kms_big_fb.c
> > +++ b/tests/i915/kms_big_fb.c
> > @@ -481,6 +481,7 @@ max_hw_stride_async_flip_test(data_t *data)
> >  		       h = data->output->config.default_mode.vdisplay;
> >  	igt_plane_t *primary;
> >  	igt_crc_t compare_crc, async_crc;
> > +	bool retried = false;
> >  
> >  	igt_require(data->display.is_atomic);
> >  	igt_output_set_pipe(data->output, data->pipe);
> > @@ -513,6 +514,7 @@ max_hw_stride_async_flip_test(data_t *data)
> >  					  INTEL_PIPE_CRC_SOURCE_AUTO);
> >  	igt_pipe_crc_start(data->pipe_crc);
> >  
> > +retry:
> >  	igt_set_timeout(5, "Async pageflipping loop got stuck!\n");
> >  	for (int i = 0; i < 2; i++) {
> >  		igt_plane_set_fb(primary, &data->big_fb);
> > @@ -548,6 +550,13 @@ max_hw_stride_async_flip_test(data_t *data)
> >  		igt_assert_f(kmstest_get_vblank(data->drm_fd, data->pipe, 0) -
> >  			     startframe == 1, "lost frames\n");
> >  
> > +		/* Test is not running at real time priority, so allow one failure*/
> > +		if (!(igt_check_crc_equal(&compare_crc, &async_crc)^(i^1)) && !retried) {
> > +			retried = true;
> > +			igt_reset_timeout();
> > +			goto retry;
> > +		}
> > +
> 
> This test seems to entirely fit kms_big_fb in general. I don't
                     ^
		     not

is what I meant to write. Somewhat important small word.

> think kms_big_fb should be testing any timing sensitive stuff.
> 
> So I think we should change this to a form that follows the rest
> of kms_big_fb to validate that each page flip just presents the
> correct data on screen. The timing sensitive stuff is best left
> for kms_async_flip.
> 
> So this should maybe be something like: flip to a correctly sized
> temp fb with the wrong contents and change the plane src coordinates,
> and then async flip back to the correct fb and validate the
> correct data is now on screen.
> 
> Oh and the async flip test should not be limited to the
> max-hw-stride part. We should test that async flips do the
> right thing with gtt remapping as well.
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel