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 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
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