[PATCH i-g-t] tests/kms_plane_multiple: Add dual display subtest

Karthik B S karthik.b.s at intel.com
Wed Mar 5 09:29:21 UTC 2025


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?

>
>> -		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