[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 11:16:11 UTC 2022
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?
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)
/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