[igt-dev] [i-g-t] tests/kms_plane_cursor: Convert tests to dynamic
Karthik B S
karthik.b.s at intel.com
Wed Aug 17 05:36:07 UTC 2022
On 8/10/2022 5:50 PM, Bhanuprakash Modem wrote:
> Convert the existing subtests to dynamic subtests at pipe/output
> level. And create an array of structures to populate subtests to
> avoid code duplication.
>
> This patch will also do some clenup to have a single function for
> all subtests.
>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> ---
> tests/kms_plane_cursor.c | 159 ++++++++++++++-------------------------
> 1 file changed, 56 insertions(+), 103 deletions(-)
>
> diff --git a/tests/kms_plane_cursor.c b/tests/kms_plane_cursor.c
> index 73085cc8..247d6715 100644
> --- a/tests/kms_plane_cursor.c
> +++ b/tests/kms_plane_cursor.c
> @@ -30,6 +30,12 @@
> * - DRM index indicates z-ordering, higher index = higher z-order
> */
>
> +enum {
> + TEST_PRIMARY = 0,
> + TEST_OVERLAY = 1 << 0,
> + TEST_VIEWPORT = 1 << 1,
> +};
> +
> typedef struct {
> int x;
> int y;
> @@ -58,18 +64,16 @@ typedef struct data {
> } data_t;
>
> /* Common test setup. */
> -static void test_init(data_t *data, enum pipe pipe_id)
> +static void test_init(data_t *data, enum pipe pipe_id, igt_output_t *output)
> {
> igt_display_t *display = &data->display;
>
> data->pipe_id = pipe_id;
> data->pipe = &data->display.pipes[data->pipe_id];
> + data->output = output;
>
> igt_display_reset(display);
>
> - data->output = igt_get_single_output_for_pipe(&data->display, pipe_id);
> - igt_require(data->output);
> -
> data->mode = igt_output_get_mode(data->output);
>
> data->primary = igt_pipe_get_plane_type(data->pipe, DRM_PLANE_TYPE_PRIMARY);
> @@ -202,92 +206,25 @@ static void test_cursor_spots(data_t *data, igt_fb_t *pfb, igt_fb_t *ofb,
> }
> }
>
> -/*
> - * Tests atomic cursor positioning on a primary and overlay plane.
> - * Assumes the cursor can be placed on top of the overlay.
> - */
> -static void test_cursor_overlay(data_t *data, int size, enum pipe pipe_id)
> -{
> - igt_fb_t pfb, ofb, cfb;
> - int sw, sh;
> -
> - test_init(data, pipe_id);
> - igt_require(data->overlay);
> -
> - sw = data->mode->hdisplay;
> - sh = data->mode->vdisplay;
> -
> - igt_create_color_fb(data->drm_fd, sw, sh, DRM_FORMAT_XRGB8888, 0,
> - 1.0, 1.0, 1.0, &pfb);
> -
> - igt_create_color_fb(data->drm_fd, data->or.w, data->or.h,
> - DRM_FORMAT_XRGB8888, 0, 0.5, 0.5, 0.5, &ofb);
> -
> - igt_create_color_fb(data->drm_fd, size, size, DRM_FORMAT_ARGB8888, 0,
> - 1.0, 0.0, 1.0, &cfb);
> -
> - igt_plane_set_fb(data->primary, &pfb);
> - igt_display_commit2(&data->display, COMMIT_ATOMIC);
> -
> - test_cursor_spots(data, &pfb, &ofb, &cfb, &data->or, size);
> -
> - test_fini(data);
> -
> - igt_remove_fb(data->drm_fd, &cfb);
> - igt_remove_fb(data->drm_fd, &ofb);
> - igt_remove_fb(data->drm_fd, &pfb);
> -}
> -
> -/* Tests atomic cursor positioning on a primary plane. */
> -static void test_cursor_primary(data_t *data, int size, enum pipe pipe_id)
> -{
> - igt_fb_t pfb, cfb;
> - int sw, sh;
> -
> - test_init(data, pipe_id);
> -
> - sw = data->mode->hdisplay;
> - sh = data->mode->vdisplay;
> -
> - igt_create_color_fb(data->drm_fd, sw, sh, DRM_FORMAT_XRGB8888, 0,
> - 1.0, 1.0, 1.0, &pfb);
> -
> - igt_create_color_fb(data->drm_fd, size, size, DRM_FORMAT_ARGB8888, 0,
> - 1.0, 0.0, 1.0, &cfb);
> -
> - igt_plane_set_fb(data->primary, &pfb);
> - igt_display_commit2(&data->display, COMMIT_ATOMIC);
> -
> - test_cursor_spots(data, &pfb, NULL, &cfb, &data->or, size);
> -
> - test_fini(data);
> -
> - igt_remove_fb(data->drm_fd, &cfb);
> - igt_remove_fb(data->drm_fd, &pfb);
> -}
> -
> -/*
> - * Tests atomic cursor positioning on a primary and overlay plane.
> - * The overlay's buffer is larger than the viewport actually used
> - * for display.
> - */
> -static void test_cursor_viewport(data_t *data, int size, enum pipe pipe_id)
> +static void test_cursor(data_t *data, int size, unsigned int flags)
> {
> igt_fb_t pfb, ofb, cfb;
> int sw, sh;
> int pad = 128;
>
> - test_init(data, pipe_id);
> - igt_require(data->overlay);
> -
> sw = data->mode->hdisplay;
> sh = data->mode->vdisplay;
>
> igt_create_color_fb(data->drm_fd, sw, sh, DRM_FORMAT_XRGB8888, 0,
> 1.0, 1.0, 1.0, &pfb);
>
> - igt_create_color_fb(data->drm_fd, data->or.w + pad, data->or.h + pad,
> - DRM_FORMAT_XRGB8888, 0, 0.5, 0.5, 0.5, &ofb);
> + if (flags & TEST_OVERLAY) {
> + int width = (flags & TEST_VIEWPORT) ? data->or.w + pad : data->or.w;
> + int height = (flags & TEST_VIEWPORT) ? data->or.h + pad : data->or.h;
> +
> + igt_create_color_fb(data->drm_fd, width, height,
> + DRM_FORMAT_XRGB8888, 0, 0.5, 0.5, 0.5, &ofb);
> + }
>
> igt_create_color_fb(data->drm_fd, size, size, DRM_FORMAT_ARGB8888, 0,
> 1.0, 0.0, 1.0, &cfb);
> @@ -295,12 +232,11 @@ static void test_cursor_viewport(data_t *data, int size, enum pipe pipe_id)
> igt_plane_set_fb(data->primary, &pfb);
> igt_display_commit2(&data->display, COMMIT_ATOMIC);
>
> - test_cursor_spots(data, &pfb, &ofb, &cfb, &data->or, size);
> -
> - test_fini(data);
> + test_cursor_spots(data, &pfb, (flags & TEST_OVERLAY)? &ofb : NULL, &cfb, &data->or, size);
>
> igt_remove_fb(data->drm_fd, &cfb);
> - igt_remove_fb(data->drm_fd, &ofb);
> + if (flags & TEST_OVERLAY)
> + igt_remove_fb(data->drm_fd, &ofb);
Hi,
Could you please move the fb cleanup part also to test_fini as we've
multiple checks before this where test could fail and if test fails then
we will miss the fb cleanup.
> igt_remove_fb(data->drm_fd, &pfb);
> }
>
> @@ -309,7 +245,21 @@ igt_main
> static const int cursor_sizes[] = { 64, 128, 256 };
> data_t data = {};
> enum pipe pipe;
> - int i;
> + igt_output_t *output;
> + int i, j;
> + struct {
> + const char *name;
> + unsigned int flags;
> + const char *desc;
> + } tests[] = {
> + { "primary", TEST_PRIMARY,
> + "Tests atomic cursor positioning on primary plane" },
> + { "overlay", TEST_PRIMARY | TEST_OVERLAY,
> + "Tests atomic cursor positioning on primary plane and overlay plane" },
> + { "viewport", TEST_PRIMARY | TEST_OVERLAY | TEST_VIEWPORT,
> + "Tests atomic cursor positioning on primary plane and overlay plane "
> + "with buffer larger than viewport used for display" },
> + };
>
> igt_fixture {
> data.drm_fd = drm_open_driver_master(DRIVER_ANY);
> @@ -321,26 +271,29 @@ igt_main
> igt_display_require_output(&data.display);
> }
>
> - for_each_pipe_static(pipe)
> - for (i = 0; i < ARRAY_SIZE(cursor_sizes); ++i) {
> - int size = cursor_sizes[i];
> -
> - igt_describe("Tests atomic cursor positioning on primary plane and overlay plane");
> - igt_subtest_f("pipe-%s-overlay-size-%d",
> - kmstest_pipe_name(pipe), size)
> - test_cursor_overlay(&data, size, pipe);
> -
> - igt_describe("Tests atomic cursor positioning on primary plane");
> - igt_subtest_f("pipe-%s-primary-size-%d",
> - kmstest_pipe_name(pipe), size)
> - test_cursor_primary(&data, size, pipe);
> -
> - igt_describe("Tests atomic cursor positioning on primary plane and overlay plane"
> - "with buffer larger than viewport used for display");
> - igt_subtest_f("pipe-%s-viewport-size-%d",
> - kmstest_pipe_name(pipe), size)
> - test_cursor_viewport(&data, size, pipe);
> + for (i = 0; i < ARRAY_SIZE(tests); i++) {
> + igt_describe_f("%s", tests[i].desc);
> + igt_subtest_with_dynamic_f("%s", tests[i].name) {
> + for_each_pipe_with_single_output(&data.display, pipe, output) {
> + test_init(&data, pipe, output);
> +
> + if ((tests[i].flags & TEST_OVERLAY) && !data.overlay)
> + goto cleanup;
Is it possible to populate data.overlay outside test_init? So that we
can completely avoid initializing other things in test_init and also
inturn avoid calling test_fini?
Thanks,
Karthik.B.S
> +
> + for (j = 0; j < ARRAY_SIZE(cursor_sizes); j++) {
> + int size = cursor_sizes[j];
> +
> + igt_dynamic_f("pipe-%s-%s-size-%d",
> + kmstest_pipe_name(pipe),
> + igt_output_name(output),
> + size)
> + test_cursor(&data, size, tests[i].flags);
> + }
> +cleanup:
> + test_fini(&data);
> + }
> }
> + }
>
> igt_fixture {
> igt_display_fini(&data.display);
More information about the igt-dev
mailing list