[PATCH v4 2/3] kms_async_flips: Refactor data options

[Melissa Wen]

On 03/27, André Almeida wrote:
> Setting the test data options as true and false for every test is error
> prone. Instead, reset all the data to false at the end of a test and
> just set the needed options to true before running a test.
> 
> Signed-off-by: André Almeida <andrealmeid at igalia.com>
> ---
>  tests/kms_async_flips.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
> index 2975925c2..5c7e1ae13 100644
> --- a/tests/kms_async_flips.c
> +++ b/tests/kms_async_flips.c
> @@ -230,6 +230,12 @@ static void test_init(data_t *data)
>  	data->plane = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY);
>  }
>  
> +static void test_finish(data_t *data)
> +{
> +	data->alternate_sync_async = false;
> +	data->atomic_path = false;
> +}
> +
>  static void test_init_fbs(data_t *data)
>  {
>  	int i;
> @@ -717,6 +723,8 @@ static void run_test(data_t *data, void (*test)(data_t *))
>  			test(data);
>  		}
>  	}
> +
> +	test_finish(data);

In case of subtest failure, this `test_finish(data)` here never runs. I
think the more straighforward solution is just calling
`test_finish(data)` as the first step of any igt_subtest_with_dynamic(),
changing the name as it will be in the beginning of the test execution.

Another alternative would be use igt_fixture, but I think it might need
a huge refactor to rearrange the test subgroups.

Also here, you can check this issue by running it in a kernel without
atomic async flip support.

Melissa

>  }
>  
>  static void run_test_with_modifiers(data_t *data, void (*test)(data_t *))
> @@ -750,6 +758,8 @@ static void run_test_with_modifiers(data_t *data, void (*test)(data_t *))
>  			}
>  		}
>  	}
> +
> +	test_finish(data);
>  }
>  
>  static data_t data;
> @@ -778,8 +788,6 @@ igt_main
>  
>  		igt_describe("Wait for page flip events in between successive asynchronous flips");
>  		igt_subtest_with_dynamic("async-flip-with-page-flip-events") {
> -			data.alternate_sync_async = false;
> -			data.atomic_path = false;
>  			if (is_intel_device(data.drm_fd))
>  				run_test_with_modifiers(&data, test_async_flip);
>  			else
> @@ -789,7 +797,6 @@ igt_main
>  		igt_describe("Wait for page flip events in between successive "
>  			     "asynchronous flips using atomic path");
>  		igt_subtest_with_dynamic("async-flip-with-page-flip-events-atomic") {
> -			data.alternate_sync_async = false;
>  			data.atomic_path = true;
>  			if (is_intel_device(data.drm_fd))
>  				run_test_with_modifiers(&data, test_async_flip);
> @@ -800,7 +807,6 @@ igt_main
>  		igt_describe("Alternate between sync and async flips");
>  		igt_subtest_with_dynamic("alternate-sync-async-flip") {
>  			data.alternate_sync_async = true;
> -			data.atomic_path = false;
>  			run_test(&data, test_async_flip);
>  		}
>  
> @@ -814,7 +820,6 @@ igt_main
>  		igt_describe("Verify that the async flip timestamp does not "
>  			     "coincide with either previous or next vblank");
>  		igt_subtest_with_dynamic("test-time-stamp") {
> -			data.atomic_path = false;
>  			run_test(&data, test_timestamp);
>  		}
>  
> @@ -837,7 +842,6 @@ igt_main
>  			      "PSR2 sel fetch causes cursor to be added to primary plane "
>  			      "pages flips and async flip is not supported in cursor\n");
>  
> -		data.atomic_path = false;
>  		run_test(&data, test_cursor);
>  	}
>  
> @@ -865,7 +869,6 @@ igt_main
>  		igt_require(igt_display_has_format_mod(&data.display, DRM_FORMAT_XRGB8888,
>  						       I915_FORMAT_MOD_Y_TILED));
>  
> -		data.atomic_path = false;
>  		run_test(&data, test_invalid);
>  	}
>  
> @@ -888,7 +891,6 @@ igt_main
>  		/* Devices without CRC can't run this test */
>  		igt_require_pipe_crc(data.drm_fd);
>  
> -		data.atomic_path = false;
>  		run_test(&data, test_crc);
>  	}
>  
> -- 
> 2.49.0
>