[PATCH i-g-t] tests/kms_plane_multiple: Add dual display subtest
B, Jeevan
jeevan.b at intel.com
Wed Mar 5 09:31:03 UTC 2025
> -----Original Message-----
> From: B S, Karthik <karthik.b.s at intel.com>
> Sent: Wednesday, March 5, 2025 2:59 PM
> To: B, Jeevan <jeevan.b at intel.com>; igt-dev at lists.freedesktop.org
> Subject: Re: [PATCH i-g-t] tests/kms_plane_multiple: Add dual display subtest
>
> Hi,
>
> On 2/24/2025 11:54 AM, B, Jeevan wrote:
> >> -----Original Message-----
> >> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of
> >> Karthik B S
> >> Sent: Wednesday, February 19, 2025 11:42 AM
> >> To: igt-dev at lists.freedesktop.org
> >> Cc: B S, Karthik <karthik.b.s at intel.com>
> >> Subject: [PATCH i-g-t] tests/kms_plane_multiple: Add dual display
> >> subtest
> >>
> >> Add 2x subtest to verify MPO use case simulataneously on two display
> >> configurations.
> >>
> >> Signed-off-by: Karthik B S <karthik.b.s at intel.com>
> >> ---
> >> tests/kms_plane_multiple.c | 216 ++++++++++++++++++++++++++++---
> ------
> >> 1 file changed, 167 insertions(+), 49 deletions(-)
> >>
> >> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> >> index
> >> b7922c357..993162640 100644
> >> --- a/tests/kms_plane_multiple.c
> >> +++ b/tests/kms_plane_multiple.c
> >> @@ -58,6 +58,10 @@
> >> * @x: x-tiling
> >> * @y: y-tiling
> >> * @yf: yf-tiling
> >> + *
> >> + * SUBTEST: 2x
> >> + * Description: Check that the kernel handles atomic updates of multiple
> planes
> >> + * simultaneously committed on 2 displays.
> >> */
> >>
> >> IGT_TEST_DESCRIPTION("Test atomic mode setting with multiple
> >> planes."); @@
> >> -76,10 +80,10 @@ typedef struct { typedef struct {
> >> int drm_fd;
> >> igt_display_t display;
> >> - igt_crc_t ref_crc;
> >> - igt_pipe_crc_t *pipe_crc;
> >> - igt_plane_t **plane;
> >> - struct igt_fb *fb;
> >> + igt_crc_t ref_crc1, ref_crc2;
> >> + igt_pipe_crc_t *pipe_crc1, *pipe_crc2;
> >> + igt_plane_t **plane1, **plane2;
> >> + struct igt_fb *fb1, *fb2;
> >> } data_t;
> >>
> >> /* Command line parameters. */
> >> @@ -98,14 +102,14 @@ struct {
> >> */
> >> static void test_init(data_t *data, enum pipe pipe, int n_planes) {
> >> - data->pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe,
> >> + data->pipe_crc1 = igt_pipe_crc_new(data->drm_fd, pipe,
> >> IGT_PIPE_CRC_SOURCE_AUTO);
> >>
> >> - data->plane = calloc(n_planes, sizeof(*data->plane));
> >> - igt_assert_f(data->plane != NULL, "Failed to allocate memory for
> >> planes\n");
> >> + data->plane1 = calloc(n_planes, sizeof(*data->plane1));
> >> + igt_assert_f(data->plane1 != NULL, "Failed to allocate memory for
> >> +planes\n");
> >>
> >> - data->fb = calloc(n_planes, sizeof(struct igt_fb));
> >> - igt_assert_f(data->fb != NULL, "Failed to allocate memory for FBs\n");
> >> + data->fb1 = calloc(n_planes, sizeof(struct igt_fb));
> >> + igt_assert_f(data->fb1 != NULL, "Failed to allocate memory for
> >> +FBs\n");
> >> }
> >>
> >> static void test_fini(data_t *data, igt_output_t *output, int
> >> n_planes) @@ -
> >> 113,21 +117,21 @@ static void test_fini(data_t *data, igt_output_t
> >> *output, int
> >> n_planes)
> >> /* reset the constraint on the pipe */
> >> igt_output_set_pipe(output, PIPE_ANY);
> >>
> >> - igt_pipe_crc_free(data->pipe_crc);
> >> - data->pipe_crc = NULL;
> >> + igt_pipe_crc_free(data->pipe_crc1);
> >> + data->pipe_crc1 = NULL;
> >>
> >> - free(data->plane);
> >> - data->plane = NULL;
> >> + free(data->plane1);
> >> + data->plane1 = NULL;
> >>
> >> - free(data->fb);
> >> - data->fb = NULL;
> >> + free(data->fb1);
> >> + data->fb1 = NULL;
> >>
> >> igt_display_reset(&data->display);
> >> }
> >>
> >> static void
> >> -get_reference_crc(data_t *data, igt_output_t *output, enum pipe pipe,
> >> - color_t *color, uint64_t modifier)
> >> +get_reference_crc(data_t *data, igt_output_t *output, enum pipe
> >> +pipe,
> >> igt_pipe_crc_t *pipe_crc,
> >> + color_t *color, igt_plane_t **plane, uint64_t modifier,
> >> +igt_crc_t *ref_crc)
> >> {
> >> drmModeModeInfo *mode;
> >> igt_plane_t *primary;
> >> @@ -137,7 +141,7 @@ get_reference_crc(data_t *data, igt_output_t
> >> *output, enum pipe pipe,
> >> igt_output_set_pipe(output, pipe);
> >>
> >> primary = igt_output_get_plane_type(output,
> >> DRM_PLANE_TYPE_PRIMARY);
> >> - data->plane[primary->index] = primary;
> >> + plane[primary->index] = primary;
> >>
> >> mode = igt_output_get_mode(output);
> >>
> >> @@ -145,21 +149,21 @@ get_reference_crc(data_t *data, igt_output_t
> >> *output, enum pipe pipe,
> >> DRM_FORMAT_XRGB8888,
> >> modifier,
> >> color->red, color->green, color->blue,
> >> - &data->fb[primary->index]);
> >> + &data->fb1[primary->index]);
> >>
> >> - igt_plane_set_fb(data->plane[primary->index], &data->fb[primary-
> >>> index]);
> >> + igt_plane_set_fb(plane[primary->index],
> >> +&data->fb1[primary->index]);
> >>
> >> ret = igt_display_try_commit2(&data->display, COMMIT_ATOMIC);
> >> igt_skip_on(ret != 0);
> >>
> >> - igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc);
> >> + igt_pipe_crc_collect_crc(pipe_crc, ref_crc);
> >> }
> >>
> >> static void
> >> create_fb_for_mode_position(data_t *data, igt_output_t *output,
> >> drmModeModeInfo *mode,
> >> color_t *color, int *rect_x, int *rect_y,
> >> int *rect_w, int *rect_h, uint64_t modifier,
> >> - int max_planes)
> >> + int max_planes, igt_fb_t *fb)
> >> {
> >> unsigned int fb_id;
> >> cairo_t *cr;
> >> @@ -171,16 +175,16 @@ create_fb_for_mode_position(data_t *data,
> >> igt_output_t *output, drmModeModeInfo
> >> mode->hdisplay, mode->vdisplay,
> >> DRM_FORMAT_XRGB8888,
> >> modifier,
> >> - &data->fb[primary->index]);
> >> + &fb[primary->index]);
> >> igt_assert(fb_id);
> >>
> >> - cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[primary->index]);
> >> + cr = igt_get_cairo_ctx(data->drm_fd, &fb[primary->index]);
> >> igt_paint_color(cr, rect_x[0], rect_y[0],
> >> mode->hdisplay, mode->vdisplay,
> >> color->red, color->green, color->blue);
> >>
> >> for (int i = 0; i < max_planes; i++) {
> >> - if (data->plane[i]->type == DRM_PLANE_TYPE_PRIMARY)
> >> + if (data->plane1[i]->type == DRM_PLANE_TYPE_PRIMARY)
> >> continue;
> >> igt_paint_color(cr, rect_x[i], rect_y[i],
> >> rect_w[i], rect_h[i], 0.0, 0.0, 0.0); @@ -191,8
> >> +195,8 @@ create_fb_for_mode_position(data_t *data, igt_output_t
> >> +*output,
> >> drmModeModeInfo
> >>
> >>
> >> static void
> >> -prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
> >> - uint64_t modifier, int max_planes, igt_output_t *output)
> >> +prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
> >> +igt_plane_t
> >> **plane,
> >> + uint64_t modifier, int max_planes, igt_output_t *output,
> >> +igt_fb_t *fb)
> >> {
> >> drmModeModeInfo *mode;
> >> igt_pipe_t *pipe;
> >> @@ -252,15 +256,14 @@ prepare_planes(data_t *data, enum pipe
> pipe_id,
> >> color_t *color,
> >> * Here is made assumption primary plane will have
> >> * index zero.
> >> */
> >> - igt_plane_t *plane = igt_output_get_plane(output, suffle[i]);
> >> uint32_t plane_format;
> >> uint64_t plane_modifier;
> >>
> >> - data->plane[i] = plane;
> >> + plane[i] = igt_output_get_plane(output, suffle[i]);
> >>
> >> - if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> >> + if (plane[i]->type == DRM_PLANE_TYPE_PRIMARY)
> >> continue;
> >> - else if (plane->type == DRM_PLANE_TYPE_CURSOR)
> >> + else if (plane[i]->type == DRM_PLANE_TYPE_CURSOR)
> >> size[i] = SIZE_CURSOR;
> >> else
> >> size[i] = SIZE_PLANE;
> >> @@ -268,10 +271,10 @@ prepare_planes(data_t *data, enum pipe
> pipe_id,
> >> color_t *color,
> >> x[i] = rand() % (mode->hdisplay - size[i]);
> >> y[i] = rand() % (mode->vdisplay - size[i]);
> >>
> >> - plane_format = data->plane[i]->type ==
> >> DRM_PLANE_TYPE_CURSOR ? DRM_FORMAT_ARGB8888 :
> >> DRM_FORMAT_XRGB8888;
> >> - plane_modifier = data->plane[i]->type ==
> >> DRM_PLANE_TYPE_CURSOR ? DRM_FORMAT_MOD_LINEAR : modifier;
> >> + plane_format = plane[i]->type == DRM_PLANE_TYPE_CURSOR
> ?
> >> DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888;
> >> + plane_modifier = plane[i]->type ==
> DRM_PLANE_TYPE_CURSOR
> >> ?
> >> +DRM_FORMAT_MOD_LINEAR : modifier;
> >>
> > line length exceeds 100 for the above 2 please fix if possible.
>
> Thank you for the review.
>
> This is a change I just made in the existing line of code and even by fixing this I
> see a lot of other lines still being too long. IMHO, all those changes can be
> made in a separate patch. Would that be fine?
Yes its fine.
>
> >
> >> - igt_skip_on(!igt_plane_has_format_mod(plane,
> plane_format,
> >> + igt_skip_on(!igt_plane_has_format_mod(plane[i],
> plane_format,
> >> plane_modifier));
> >>
> >> igt_create_color_fb(data->drm_fd,
> >> @@ -279,17 +282,17 @@ prepare_planes(data_t *data, enum pipe
> pipe_id,
> >> color_t *color,
> >> plane_format,
> >> plane_modifier,
> >> color->red, color->green, color->blue,
> >> - &data->fb[i]);
> >> + &fb[i]);
> >>
> >> - igt_plane_set_position(data->plane[i], x[i], y[i]);
> >> - igt_plane_set_fb(data->plane[i], &data->fb[i]);
> >> + igt_plane_set_position(plane[i], x[i], y[i]);
> >> + igt_plane_set_fb(plane[i], &fb[i]);
> >> }
> >>
> >> /* primary plane */
> >> - data->plane[primary->index] = primary;
> >> + plane[primary->index] = primary;
> >> create_fb_for_mode_position(data, output, mode, color, x, y,
> >> - size, size, modifier, max_planes);
> >> - igt_plane_set_fb(data->plane[primary->index], &data->fb[primary-
> >>> index]);
> >> + size, size, modifier, max_planes,
> &fb[primary-
> >>> index]);
> >> + igt_plane_set_fb(plane[primary->index], &fb[primary->index]);
> >> free((void*)x);
> >> free((void*)y);
> >> free((void*)size);
> >> @@ -335,12 +338,13 @@ test_plane_position_with_output(data_t *data,
> enum
> >> pipe pipe,
> >>
> >> test_init(data, pipe, n_planes);
> >>
> >> - get_reference_crc(data, output, pipe, &blue, modifier);
> >> + get_reference_crc(data, output, pipe, data->pipe_crc1, &blue,
> >> + data->plane1, modifier, &data->ref_crc1);
> >>
> >> /* Find out how many planes are allowed simultaneously */
> >> do {
> >> c++;
> >> - prepare_planes(data, pipe, &blue, modifier, c, output);
> >> + prepare_planes(data, pipe, &blue, data->plane1, modifier, c,
> >> output,
> >> +data->fb1);
> >> err = igt_display_try_commit2(&data->display,
> >> COMMIT_ATOMIC);
> >>
> >> for_each_plane_on_pipe(&data->display, pipe, plane) @@ -
> >> 350,7 +354,7 @@ test_plane_position_with_output(data_t *data, enum
> pipe
> >> pipe,
> >> igt_display_commit2(&data->display, COMMIT_ATOMIC);
> >>
> >> for (int x = 0; x < c; x++)
> >> - igt_remove_fb(data->drm_fd, &data->fb[x]);
> >> + igt_remove_fb(data->drm_fd, &data->fb1[x]);
> >> } while (!err && c < n_planes);
> >>
> >> if (err)
> >> @@ -364,14 +368,14 @@ test_plane_position_with_output(data_t *data,
> enum
> >> pipe pipe,
> >> while (i < iterations || loop_forever) {
> >>
> >> /* randomize planes and set up the holes */
> >> - prepare_planes(data, pipe, &blue, modifier, c, output);
> >> + prepare_planes(data, pipe, &blue, data->plane1, modifier, c,
> >> output,
> >> +data->fb1);
> >>
> >> igt_display_commit2(&data->display, COMMIT_ATOMIC);
> >> - igt_pipe_crc_start(data->pipe_crc);
> >> + igt_pipe_crc_start(data->pipe_crc1);
> >>
> >> - igt_pipe_crc_get_current(data->display.drm_fd, data-
> >pipe_crc,
> >> &crc);
> >> - igt_assert_crc_equal(&data->ref_crc, &crc);
> >> - igt_pipe_crc_stop(data->pipe_crc);
> >> + igt_pipe_crc_get_current(data->display.drm_fd, data-
> >>> pipe_crc1, &crc);
> >> + igt_assert_crc_equal(&data->ref_crc1, &crc);
> >> + igt_pipe_crc_stop(data->pipe_crc1);
> >>
> >> for_each_plane_on_pipe(&data->display, pipe, plane)
> >> igt_plane_set_fb(plane, NULL);
> >> @@ -380,7 +384,7 @@ test_plane_position_with_output(data_t *data,
> enum
> >> pipe pipe,
> >> igt_display_commit2(&data->display, COMMIT_ATOMIC);
> >>
> >> for (int x = 0; x < c; x++)
> >> - igt_remove_fb(data->drm_fd, &data->fb[x]);
> >> + igt_remove_fb(data->drm_fd, &data->fb1[x]);
> >>
> >> i++;
> >> }
> >> @@ -403,6 +407,108 @@ test_plane_position(data_t *data, enum pipe
> pipe,
> >> igt_output_t *output, uint64_t
> >> n_planes, modifier);
> >> }
> >>
> >> +static void test_init_2_display(data_t *data, enum pipe pipe1, enum
> >> +pipe pipe2) {
> >> + data->pipe_crc1 = igt_pipe_crc_new(data->drm_fd, pipe1,
> >> + IGT_PIPE_CRC_SOURCE_AUTO);
> >> + data->pipe_crc2 = igt_pipe_crc_new(data->drm_fd, pipe2,
> >> + IGT_PIPE_CRC_SOURCE_AUTO);
> >> +
> >> + data->plane1 = calloc(2, sizeof(*data->plane1));
> >> + igt_assert_f(data->plane1 != NULL, "Failed to allocate memory for
> >> +planes\n");
> >> +
> >> + data->plane2 = calloc(2, sizeof(*data->plane2));
> >> + igt_assert_f(data->plane2 != NULL, "Failed to allocate memory for
> >> +planes\n");
> >> +
> >> + data->fb1 = calloc(2, sizeof(struct igt_fb));
> >> + igt_assert_f(data->fb1 != NULL, "Failed to allocate memory for
> >> +FBs\n");
> >> +
> >> + data->fb2 = calloc(2, sizeof(struct igt_fb));
> >> + igt_assert_f(data->fb2 != NULL, "Failed to allocate memory for
> >> +FBs\n"); }
> >> +
> >> +static void test_fini_2_display(data_t *data) {
> >> + igt_pipe_crc_stop(data->pipe_crc1);
> >> + igt_pipe_crc_stop(data->pipe_crc2);
> >> +
> >> + igt_display_reset(&data->display);
> >> +}
> >> +
> >> +static void test_plane_position_2_display(data_t *data, enum pipe pipe1,
> enum
> >> pipe pipe2,
> >> + igt_output_t *output1, igt_output_t
> >> *output2) {
> >> + color_t blue = { 0.0f, 0.0f, 1.0f };
> >> + igt_crc_t crc1, crc2;
> >> +
> >> + test_init_2_display(data, pipe1, pipe2);
> >> + get_reference_crc(data, output1, pipe1, data->pipe_crc1, &blue,
> >> + data->plane1, DRM_FORMAT_MOD_LINEAR, &data-
> >>> ref_crc1);
> >> + get_reference_crc(data, output2, pipe2, data->pipe_crc2, &blue,
> >> + data->plane2, DRM_FORMAT_MOD_LINEAR, &data-
> >>> ref_crc2);
> >> +
> >> + prepare_planes(data, pipe1, &blue, data->plane1,
> >> DRM_FORMAT_MOD_LINEAR, 2, output1, data->fb1);
> >> + prepare_planes(data, pipe2, &blue, data->plane2,
> >> +DRM_FORMAT_MOD_LINEAR, 2, output2, data->fb2);
> > Line exceeds here also.
> Will update this.
> > Can we target for all formats/modifiers like single display ??
>
> Sure, will update this.
>
> Thanks,
> Karthik.B.S
> >
> >> +
> >> + igt_display_commit2(&data->display, COMMIT_ATOMIC);
> >> + igt_pipe_crc_start(data->pipe_crc1);
> >> + igt_pipe_crc_start(data->pipe_crc2);
> >> +
> >> + igt_pipe_crc_get_current(data->display.drm_fd, data->pipe_crc1,
> &crc1);
> >> + igt_pipe_crc_get_current(data->display.drm_fd, data->pipe_crc2,
> >> +&crc2);
> >> +
> >> + igt_assert_crc_equal(&data->ref_crc1, &crc1);
> >> + igt_assert_crc_equal(&data->ref_crc2, &crc2); }
> >> +
> >> +#define for_each_connected_output_local(display, output) \
> >> + for (int j__ = 0; assert(igt_can_fail()), j__ < (display)->n_outputs;
> j__++)
> >> \
> >> + for_each_if((((output) = &(display)->outputs[j__]), \
> >> + igt_output_is_connected((output))))
> >> +
> >> +#define for_each_valid_output_on_pipe_local(display, pipe, output) \
> >> + for_each_connected_output_local((display), (output)) \
> >> + for_each_if(igt_pipe_connector_valid((pipe), (output)))
> >> +
> >> +static void run_2_display_test(data_t *data) {
> >> + enum pipe pipe1, pipe2;
> >> + igt_output_t *output1, *output2;
> >> + igt_display_t *display = &data->display;
> >> +
> >> + igt_display_reset(display);
> >> +
> >> + for_each_pipe(display, pipe1) {
> >> + for_each_valid_output_on_pipe(display, pipe1, output1) {
> >> + for_each_pipe(display, pipe2) {
> >> + if (pipe1 == pipe2)
> >> + continue;
> >> +
> >> + for_each_valid_output_on_pipe_local(display,
> >> pipe2, output2) {
> >> + if (output1 == output2)
> >> + continue;
> >> +
> >> + igt_display_reset(display);
> >> +
> >> + igt_output_set_pipe(output1, pipe1);
> >> + igt_output_set_pipe(output2, pipe2);
> >> +
> >> + if
> >> (!intel_pipe_output_combo_valid(display))
> >> + continue;
> >> +
> >> + igt_dynamic_f("pipe-%s-%s-pipe-%s-
> >> %s",
> >> +
> >> kmstest_pipe_name(pipe1), output1->name,
> >> +
> >> kmstest_pipe_name(pipe2), output2->name)
> >> +
> >> test_plane_position_2_display(data, pipe1, pipe2,
> >> +
> >> output1, output2);
> >> +
> >> + test_fini_2_display(data);
> >> + }
> >> + }
> >> + }
> >> + }
> >> +}
> >> +
> >> static void run_test(data_t *data, uint64_t modifier) {
> >> enum pipe pipe;
> >> @@ -506,6 +612,18 @@ igt_main_args("", long_options, help_str,
> opt_handler,
> >> NULL)
> >> run_test(&data, subtests[i].modifier);
> >> }
> >>
> >> + igt_subtest_with_dynamic("2x") {
> >> + int valid_outputs = 0;
> >> + igt_output_t *output;
> >> +
> >> + for_each_connected_output(&data.display, output)
> >> + valid_outputs++;
> >> +
> >> + igt_require(valid_outputs > 1);
> >> +
> >> + run_2_display_test(&data);
> >> + }
> >> +
> >> igt_fixture {
> >> igt_display_fini(&data.display);
> >> drm_close_driver(data.drm_fd);
> >> --
> >> 2.43.0
More information about the igt-dev
mailing list