[igt-dev] [PATCH i-g-t] tests/kms_cursor_crc: prevent the entire test from being aborted when a subtest fails

aemad aemad at igalia.com
Tue Nov 1 11:25:46 UTC 2022


On 2022-10-10 13:16, Juha-Pekka Heikkila wrote:
> Hi Alaa,
> 
> what's that invalid argument you're getting on addfb? I never tried
> this with vkms, normally just on i915. With i915 I cannot reproduce
> that abort you're seeing. Other kms tests run on your setup?
> 

Hi Juha-Pekka,

> Anyway, that revert you're suggesting would remove support for msm
> driver on this test. Let's first summon those who made these changes
> to comment. (cc'ing Modem and Swati)

Could you please explain why this patch will remove MSM driver support?


BR,
Alaa


> 
> /Juha-Pekka
> 
> On 9.10.2022 18.18, Alaa Emad wrote:
>> Any test failure prevents the other tests to be run, aborting the entire test loop. This behavior was introduced by commit
>> 9494d53d ("tests/kms_cursor_crc: Convert tests to dynamic").
>> For instance, when running the cursor-offscreen-32x10 subtest using VKMS,
>> the execution is aborted instead of SKIP/FAIL, as in the following output:
>>
>> $ ./tests/kms_cursor_crc --run-subtest cursor-offscreen-32x10
>> (kms_cursor_crc:7447) igt_fb-CRITICAL: Test assertion failure function igt_create_fb_with_bo_size, file ../lib/igt_fb.c:2078:
>> (kms_cursor_crc:7447) igt_fb-CRITICAL: Failed assertion: (__kms_addfb(fb->fd, fb->gem_handle, fb->width, fb->height, fb->drm_format, fb->modifier, fb->strides, fb->offsets, fb->num_planes, flags, &fb->fb_id)) == 0
>> (kms_cursor_crc:7447) igt_fb-CRITICAL: Last errno: 22, Invalid argument
>> Stack trace:
>> kms_cursor_crc: ../lib/igt_core.c:1667: igt_fail: Assertion `_igt_dynamic_tests_executed < 0 || dynamic_failed_one' failed.
>> Received signal SIGABRT.
>>
>> The test was aborted after convertig tests to dynamic with cursor size 32*10 that because of the assertion in the `igt_create_fb_with_bo_size` when creating cursor in `create_cursor_fb`.
>>
>> This happened because Within a igt_subtest_with_dynamic block, explicit failure (e.g. igt_assert) is not allowed,
>> only dynamic subsubtests themselves will produce test results.
>>
>> The previous approach was running each test for all cursor sizes then move to the next test  so fix this issue
>> by running all tests for one cursor size one by one, and check if cursor size is
>> supported or not before running the test.
>>
>> Fixes: 9494d53d ("tests/kms_cursor_crc: Convert tests to dynamic")
>>
>> Signed-off-by: Alaa Emad <aemad at igalia.com>
>> ---
>>   tests/kms_cursor_crc.c | 136 +++++++++++++++++++++--------------------
>>   1 file changed, 69 insertions(+), 67 deletions(-)
>>
>> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
>> index 8d3426dd..9b620389 100644
>> --- a/tests/kms_cursor_crc.c
>> +++ b/tests/kms_cursor_crc.c
>> @@ -72,7 +72,6 @@ typedef struct {
>>   	cairo_surface_t *surface;
>>   	uint32_t devid;
>>   	double alpha;
>> -	int vblank_wait_count; /* because of msm */
>>   } data_t;
>>     #define TEST_DPMS (1<<0)
>> @@ -125,12 +124,6 @@ static void cursor_disable(data_t *data)
>>   {
>>   	igt_plane_set_fb(data->cursor, NULL);
>>   	igt_plane_set_position(data->cursor, 0, 0);
>> -	igt_display_commit(&data->display);
>> -
>> -	/* do this wait here so it will not need to be added everywhere */
>> -	igt_wait_for_vblank_count(data->drm_fd,
>> -				  data->display.pipes[data->pipe].crtc_offset,
>> -				  data->vblank_wait_count);
>>   }
>>     static bool chv_cursor_broken(data_t *data, int x)
>> @@ -209,9 +202,8 @@ static void do_single_test(data_t *data, int x, int y, bool hw_test,
>>   		igt_display_commit(display);
>>     		/* Extra vblank wait is because nonblocking cursor ioctl */
>> -		igt_wait_for_vblank_count(data->drm_fd,
>> -					  display->pipes[data->pipe].crtc_offset,
>> -					  data->vblank_wait_count);
>> +		igt_wait_for_vblank(data->drm_fd,
>> +				display->pipes[data->pipe].crtc_offset);
>>     		igt_pipe_crc_get_current(data->drm_fd, pipe_crc, hwcrc);
>>   @@ -249,7 +241,11 @@ static void do_single_test(data_t *data, int x, int y, bool hw_test,
>>     		restore_image(data, swbufidx, &((cursorarea){x, y, data->curw, data->curh}));
>>   		igt_plane_set_fb(data->primary, &data->primary_fb[swbufidx]);
>> +
>>   		igt_display_commit(display);
>> +		igt_wait_for_vblank(data->drm_fd,
>> +				display->pipes[data->pipe].crtc_offset);
>> +
>>   		igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &crc);
>>   		igt_assert_crc_equal(&crc, hwcrc);
>>   	}
>> @@ -267,7 +263,9 @@ static void do_fail_test(data_t *data, int x, int y, int expect)
>>   	igt_plane_set_position(data->cursor, x, y);
>>   	ret = igt_display_try_commit2(display, COMMIT_LEGACY);
>>   +	igt_plane_set_position(data->cursor, 0, 0);
>>   	cursor_disable(data);
>> +	igt_display_commit(display);
>>     	igt_assert_eq(ret, expect);
>>   }
>> @@ -689,10 +687,28 @@ static void test_rapid_movement(data_t *data)
>>   	igt_assert_lt(usec, 0.9 * 400 * 1000000 / data->refresh);
>>   }
>>   -static void run_size_tests(data_t *data, void (*testfunc)(data_t *),
>> -			   int w, int h)
>> +static void run_size_tests(data_t *data, int w, int h)
>>   {
>>   	enum pipe pipe;
>> +	int i;
>> +
>> +	struct
>> +	{
>> +		const char *name;
>> +		void (*testfunc)(data_t *);
>> +		const char *desc;
>> +	} size_tests[] = {
>> +		{"cursor-onscreen", test_crc_onscreen,
>> +		 "Check if a given-size cursor is well-positioned inside the screen."},
>> +		{"cursor-offscreen", test_crc_offscreen,
>> +		 "Check if a given-size cursor is well-positioned outside the screen."},
>> +		{"cursor-sliding", test_crc_sliding,
>> +		 "Check the smooth and pixel-by-pixel given-size cursor movements on horizontal, vertical and diagonal."},
>> +		{"cursor-random", test_crc_random,
>> +		 "Check random placement of a cursor with given size."},
>> +		{"cursor-rapid-movement", test_rapid_movement,
>> +		 "Check the rapid update of given-size cursor movements."},
>> +	};
>>     	if (w == 0 && h == 0) {
>>   		w = data->cursor_max_w;
>> @@ -709,45 +725,38 @@ static void run_size_tests(data_t *data, void (*testfunc)(data_t *),
>>   		}
>>   	}
>>   -	create_cursor_fb(data, w, h);
>> -	if (require_cursor_size(data, w, h)) {
>> -		igt_info("Cursor size %dx%d not supported by driver\n", w, h);
>> -
>> -		igt_remove_fb(data->drm_fd, &data->fb);
>> -		return;
>> -	}
>> +	igt_fixture
>> +		create_cursor_fb(data, w, h);
>>   -	for_each_pipe(&data->display, pipe) {
>> -		data->pipe = pipe;
>> -		igt_dynamic_f("pipe-%s-%s",
>> -			      kmstest_pipe_name(pipe), igt_output_name(data->output))
>> -			run_test(data, testfunc, w, h);
>> +	for (i = 0; i < ARRAY_SIZE(size_tests); i++)
>> +	{
>> +		igt_describe(size_tests[i].desc);
>> +		igt_subtest_with_dynamic_f("%s-%dx%d", size_tests[i].name, w, h)
>> +		{
>> +			for_each_pipe(&data->display, pipe)
>> +			{
>> +				data->pipe = pipe;
>> +				if (require_cursor_size(data, w, h))
>> +				{
>> +					igt_info("Cursor size %dx%d not supported by driver\n", w, h);
>> +					continue;
>> +				}
>> +
>> +				igt_dynamic_f("pipe-%s-%s",
>> +							  kmstest_pipe_name(pipe), igt_output_name(data->output))
>> +					run_test(data, size_tests[i].testfunc, w, h);
>> +			}
>> +		}
>>   	}
>>   -	igt_remove_fb(data->drm_fd, &data->fb);
>> +	igt_fixture
>> +		igt_remove_fb(data->drm_fd, &data->fb);
>>   }
>>     static void run_tests_on_pipe(data_t *data)
>>   {
>>   	enum pipe pipe;
>>   	int cursor_size;
>> -	int i;
>> -	struct {
>> -		const char *name;
>> -		void (*testfunc)(data_t *);
>> -		const char *desc;
>> -	} size_tests[] = {
>> -		{ "cursor-onscreen", test_crc_onscreen,
>> -			"Check if a given-size cursor is well-positioned inside the screen." },
>> -		{ "cursor-offscreen", test_crc_offscreen,
>> -			"Check if a given-size cursor is well-positioned outside the screen." },
>> -		{ "cursor-sliding", test_crc_sliding,
>> -			"Check the smooth and pixel-by-pixel given-size cursor movements on horizontal, vertical and diagonal." },
>> -		{ "cursor-random", test_crc_random,
>> -			"Check random placement of a cursor with given size." },
>> -		{ "cursor-rapid-movement", test_rapid_movement,
>> -			"Check the rapid update of given-size cursor movements." },
>> -	};
>>     	igt_fixture {
>>   		data->output = igt_get_single_output_for_pipe(&data->display, pipe);
>> @@ -848,30 +857,26 @@ static void run_tests_on_pipe(data_t *data)
>>   	igt_fixture
>>   		igt_remove_fb(data->drm_fd, &data->fb);
>>   -	for (i = 0; i < ARRAY_SIZE(size_tests); i++) {
>> -		igt_describe(size_tests[i].desc);
>> -		igt_subtest_group {
>> -			for (cursor_size = 32; cursor_size <= 512; cursor_size *= 2) {
>> -				int w = cursor_size;
>> -				int h = cursor_size;
>> -
>> -				igt_subtest_with_dynamic_f("%s-%dx%d", size_tests[i].name, w, h)
>> -					run_size_tests(data, size_tests[i].testfunc, w, h);
>> -
>> -				/*
>> -				 * Test non-square cursors a bit on the platforms
>> -				 * that support such things. And make it a bit more
>> -				 * interesting by using a non-pot height.
>> -				 */
>> -				h /= 3;
>> -				igt_subtest_with_dynamic_f("%s-%dx%d", size_tests[i].name, w, h)
>> -					run_size_tests(data, size_tests[i].testfunc, w, h);
>> -			}
>> +	for (cursor_size = 32; cursor_size <= 512; cursor_size *= 2)
>> +	{
>> +		int w = cursor_size;
>> +		int h = cursor_size;
>>   -			igt_subtest_with_dynamic_f("%s-max-size", size_tests[i].name)
>> -				run_size_tests(data, size_tests[i].testfunc, 0, 0);
>> -		}
>> +		igt_subtest_group
>> +			run_size_tests(data, w, h);
>> +
>> +		/*
>> +		 * Test non-square cursors a bit on the platforms
>> +		 * that support such things. And make it a bit more
>> +		 * interesting by using a non-pot height.
>> +		 */
>> +		h /= 3;
>> +
>> +		igt_subtest_group
>> +			run_size_tests(data, w, h);
>>   	}
>> +
>> +	run_size_tests(data, 0, 0);
>>   }
>>     static data_t data;
>> @@ -896,8 +901,6 @@ igt_main
>>   		kmstest_set_vt_graphics_mode();
>>     		igt_require_pipe_crc(data.drm_fd);
>> -
>> -		data.vblank_wait_count = is_msm_device(data.drm_fd) ? 2 : 1;
>>   	}
>>     	data.cursor_max_w = cursor_width;
>> @@ -913,6 +916,5 @@ igt_main
>>   		}
>>     		igt_display_fini(&data.display);
>> -		close(data.drm_fd);
>>   	}
>>   }


More information about the igt-dev mailing list