[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
Mon Oct 10 14:26:19 UTC 2022
Hi Juha,
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
I am not sure what is the invalid argument, but the failure of this test
with a cursor size 32x10 is that VKMS does not support this size yet so
the test unable to create a cursor FB.
> this with vkms, normally just on i915. With i915 I cannot reproduce
> that abort you're seeing. Other kms tests run on your setup?
I think the abortion related to VKMS driver only, but I didn't try to
run it on i915.
yes, I ran other kms test but with VKMS driver.
>
> 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)
I see, so I can check that after Modem and Swati reply.
>
> /Juha-Pekka
BR,
Alaa
>
> 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