[igt-dev] [PATCH i-g-t] tests/kms_cursor_crc: prevent the entire test from being aborted when a subtest fails
Juha-Pekka Heikkila
juhapekka.heikkila at gmail.com
Mon Oct 10 18:52:31 UTC 2022
On 10.10.2022 17.26, aemad wrote:
> 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.
At that moment when addfb gives failure it doesn't yet relate to cursor
testing, it is like any other framebuffer. As other kms tests work for
you there maybe limitation on vkms not supporting so small framebuffer
or it could be on libigt there's something missing for handling vkms
framebuffers.
I'm looking at kernel sources and I suspect you'd see in dmesg message
saying "GEM object size (%zu) smaller than minimum size (%u) for plane
%d\n" when that addfb failed? If it is that message then probably into
libigt for vkms there would be needed some minimum size alignment with
framebuffers.
>
>
>> 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