Re: [PATCH i-g-t v2 20/33] tests/kms_plane_multiple: Add support for dynamic number of planes

[Mika Kahola <mika.kahola@intel.com>]

On Tue, 2017-01-24 at 18:33 -0500, Robert Foss wrote:
> Add changes reflecting the new support for dynamic number of planes
> per pipe.
> 
> Signed-off-by: Robert Foss <robert.foss@collabora.com>
> ---
>  tests/kms_plane_multiple.c | 209 ++++++++++++++++++++++++++++-------
> ----------
>  1 file changed, 131 insertions(+), 78 deletions(-)
> 
> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> index 2b43f0c5..e10bbea3 100644
> --- a/tests/kms_plane_multiple.c
> +++ b/tests/kms_plane_multiple.c
> @@ -47,8 +47,8 @@ typedef struct {
>  	int drm_fd;
>  	igt_display_t display;
>  	igt_pipe_crc_t *pipe_crc;
> -	igt_plane_t *plane[IGT_MAX_PLANES];
> -	struct igt_fb fb[IGT_MAX_PLANES];
> +	igt_plane_t **plane;
> +	struct igt_fb *fb;
>  } data_t;
>  
>  typedef struct {
> @@ -70,20 +70,35 @@ struct {
>  /*
>   * Common code across all tests, acting on data_t
>   */
> -static void test_init(data_t *data, enum pipe pipe)
> +static void test_init(data_t *data, enum pipe pipe, int max_planes)
>  {
>  	data->pipe_crc = igt_pipe_crc_new(pipe,
> INTEL_PIPE_CRC_SOURCE_AUTO);
> +
> +	data->plane = malloc(max_planes * sizeof(data->plane));
> +	igt_assert_f(data->plane != NULL, "Failed to allocate memory
> for planes\n");
> +
> +	data->fb = malloc(max_planes * sizeof(struct igt_fb));
> +	igt_assert_f(data->fb != NULL, "Failed to allocate memory
> for FBs\n");
Running this test I received a following error on 'atomic-pipe-A-
tiling-none-planes'

Testing connector DP-1 using pipe A with 0 planes for 1 iteration with
seed 1485427602
*** Error in `./kms_plane_multiple': malloc(): memory corruption:
0x0000000002471ae0 ***

and with legacy version

Testing connector DP-1 using pipe A with 0 planes for 1 iteration with
seed 1485427971
*** Error in `./kms_plane_multiple': double free or corruption (out):
0x00000000018507f0 ***

>  }
>  
>  static void test_fini(data_t *data, igt_output_t *output, int
> max_planes)
>  {
> -	for (int i = IGT_PLANE_PRIMARY; i <= max_planes; i++)
> -		igt_plane_set_fb(data->plane[i], NULL);
> +	for (int i = 0; i <= max_planes; i++) {
> +		igt_plane_t *plane = data->plane[i];
> +		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> +			continue;
> +		igt_plane_set_fb(plane, NULL);
> +	}
>  
>  	/* reset the constraint on the pipe */
>  	igt_output_set_pipe(output, PIPE_ANY);
>  
>  	igt_pipe_crc_free(data->pipe_crc);
> +
> +	free(data->plane);
> +	data->plane = NULL;
> +	free(data->fb);
> +	data->fb = NULL;
>  }
>  
>  static void
> @@ -91,11 +106,13 @@ test_grab_crc(data_t *data, igt_output_t
> *output, enum pipe pipe, bool atomic,
>  	      color_t *color, uint64_t tiling, igt_crc_t *crc /* out
> */)
>  {
>  	drmModeModeInfo *mode;
> +	igt_plane_t *primary;
>  	int ret, n;
>  
>  	igt_output_set_pipe(output, pipe);
>  
> -	data->plane[IGT_PLANE_PRIMARY] =
> igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> +	primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> +	data->plane[primary->index] = primary;
>  
>  	mode = igt_output_get_mode(output);
>  
> @@ -103,9 +120,9 @@ test_grab_crc(data_t *data, igt_output_t *output,
> enum pipe pipe, bool atomic,
>  			    DRM_FORMAT_XRGB8888,
>  			    LOCAL_DRM_FORMAT_MOD_NONE,
>  			    color->red, color->green, color->blue,
> -			    &data->fb[IGT_PLANE_PRIMARY]);
> +			    &data->fb[primary->index]);
>  
> -	igt_plane_set_fb(data->plane[IGT_PLANE_PRIMARY], &data-
> >fb[IGT_PLANE_PRIMARY]);
> +	igt_plane_set_fb(data->plane[primary->index], &data-
> >fb[primary->index]);
>  
>  	ret = igt_display_try_commit2(&data->display,
>  				      atomic ? COMMIT_ATOMIC :
> COMMIT_LEGACY);
> @@ -128,29 +145,35 @@ test_grab_crc(data_t *data, igt_output_t
> *output, enum pipe pipe, bool atomic,
>   */
>  
>  static void
> -create_fb_for_mode_position(data_t *data, drmModeModeInfo *mode,
> +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
> tiling,
>  			    int max_planes)
>  {
>  	unsigned int fb_id;
>  	cairo_t *cr;
> +	igt_plane_t *primary;
> +
> +	primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
>  
>  	fb_id = igt_create_fb(data->drm_fd,
>  			      mode->hdisplay, mode->vdisplay,
>  			      DRM_FORMAT_XRGB8888,
>  			      tiling,
> -			      &data->fb[IGT_PLANE_PRIMARY]);
> +			      &data->fb[primary->index]);
>  	igt_assert(fb_id);
>  
> -	cr = igt_get_cairo_ctx(data->drm_fd, &data-
> >fb[IGT_PLANE_PRIMARY]);
> +	cr = igt_get_cairo_ctx(data->drm_fd, &data->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 = IGT_PLANE_2; i <= max_planes; i++)
> +	for (int i = 0; i <= max_planes; i++) {
> +		if (data->plane[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);
> +		}
>  
>  	igt_assert(cairo_status(cr) == 0);
>  	cairo_destroy(cr);
> @@ -158,37 +181,52 @@ create_fb_for_mode_position(data_t *data,
> drmModeModeInfo *mode,
>  
>  
>  static void
> -prepare_planes(data_t *data, enum pipe pipe, color_t *color,
> +prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
>  	       uint64_t tiling, int max_planes, igt_output_t
> *output)
>  {
>  	drmModeModeInfo *mode;
> -	int x[IGT_MAX_PLANES];
> -	int y[IGT_MAX_PLANES];
> -	int size[IGT_MAX_PLANES];
> +	igt_pipe_t *pipe;
> +	igt_plane_t *primary;
> +	int *x;
> +	int *y;
> +	int *size;
>  	int i;
>  
> -	igt_output_set_pipe(output, pipe);
> +	igt_output_set_pipe(output, pipe_id);
> +	primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> +	pipe = primary->pipe;
> +
> +	x = malloc(pipe->n_planes * sizeof(*x));
> +	igt_assert_f(x, "Failed to allocate %ld bytes for variable
> x\n", (long int) (pipe->n_planes * sizeof(*x)));
> +	y = malloc(pipe->n_planes * sizeof(*y));
> +	igt_assert_f(y, "Failed to allocate %ld bytes for variable
> y\n", (long int) (pipe->n_planes * sizeof(*y)));
> +	size = malloc(pipe->n_planes * sizeof(*size));
> +	igt_assert_f(size, "Failed to allocate %ld bytes for
> variable size\n", (long int) (pipe->n_planes * sizeof(*size)));
>  
>  	mode = igt_output_get_mode(output);
>  
>  	/* planes with random positions */
> -	x[IGT_PLANE_PRIMARY] = 0;
> -	y[IGT_PLANE_PRIMARY] = 0;
> -	for (i = IGT_PLANE_2; i <= max_planes; i++) {
> -		if (i == IGT_PLANE_CURSOR)
> +	x[primary->index] = 0;
> +	y[primary->index] = 0;
> +	for (i = 1; i <= max_planes; i++) {
> +		igt_plane_t *plane = igt_output_get_plane(output,
> i);
> +
> +		if (plane->type == DRM_PLANE_TYPE_CURSOR)
>  			size[i] = SIZE_CURSOR;
> +		else if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> +			continue;
>  		else
>  			size[i] = SIZE_PLANE;
>  
>  		x[i] = rand() % (mode->hdisplay - size[i]);
>  		y[i] = rand() % (mode->vdisplay - size[i]);
>  
> -		data->plane[i] = igt_output_get_plane(output, i);
> +		data->plane[i] = plane;
>  
>  		igt_create_color_fb(data->drm_fd,
>  				    size[i], size[i],
> -				    data->plane[i]->is_cursor ?
> DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888,
> -				    data->plane[i]->is_cursor ?
> LOCAL_DRM_FORMAT_MOD_NONE : tiling,
> +				    data->plane[i]->type ==
> DRM_PLANE_TYPE_CURSOR ? DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888,
> +				    data->plane[i]->type ==
> DRM_PLANE_TYPE_CURSOR ? LOCAL_DRM_FORMAT_MOD_NONE : tiling,
>  				    color->red, color->green, color-
> >blue,
>  				    &data->fb[i]);
>  
> @@ -197,10 +235,10 @@ prepare_planes(data_t *data, enum pipe pipe,
> color_t *color,
>  	}
>  
>  	/* primary plane */
> -	data->plane[IGT_PLANE_PRIMARY] =
> igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> -	create_fb_for_mode_position(data, mode, color, x, y,
> +	data->plane[primary->index] = primary;
> +	create_fb_for_mode_position(data, output, mode, color, x, y,
>  				    size, size, tiling, max_planes);
> -	igt_plane_set_fb(data->plane[IGT_PLANE_PRIMARY], &data-
> >fb[IGT_PLANE_PRIMARY]);
> +	igt_plane_set_fb(data->plane[primary->index], &data-
> >fb[primary->index]);
>  }
>  
>  static void
> @@ -232,7 +270,7 @@ test_atomic_plane_position_with_output(data_t
> *data, enum pipe pipe,
>  		 igt_output_name(output), kmstest_pipe_name(pipe),
> max_planes,
>  		 info, opt.seed);
>  
> -	test_init(data, pipe);
> +	test_init(data, pipe, max_planes);
>  
>  	test_grab_crc(data, output, pipe, true, &blue, tiling,
>  		      &test.reference_crc);
> @@ -298,7 +336,7 @@ test_legacy_plane_position_with_output(data_t
> *data, enum pipe pipe,
>  		 igt_output_name(output), kmstest_pipe_name(pipe),
> max_planes,
>  		 info, opt.seed);
>  
> -	test_init(data, pipe);
> +	test_init(data, pipe, max_planes);
>  
>  	test_grab_crc(data, output, pipe, false, &blue, tiling,
>  		      &test.reference_crc);
> @@ -367,54 +405,71 @@ test_plane_position(data_t *data, enum pipe
> pipe, bool atomic, int max_planes,
>  }
>  
>  static void
> -run_tests_for_pipe_plane(data_t *data, enum pipe pipe, int
> max_planes)
> +run_tests_for_pipe_plane(data_t *data, enum pipe pipe)
>  {
> -	igt_subtest_f("legacy-pipe-%s-tiling-none-planes-%d",
> -		      kmstest_pipe_name(pipe), max_planes)
> -		test_plane_position(data, pipe, false, max_planes,
> -				    LOCAL_DRM_FORMAT_MOD_NONE);
> -
> -	igt_subtest_f("atomic-pipe-%s-tiling-none-planes-%d",
> -		      kmstest_pipe_name(pipe), max_planes)
> -		test_plane_position(data, pipe, true, max_planes,
> -				    LOCAL_I915_FORMAT_MOD_X_TILED);
> -
> -	igt_subtest_f("legacy-pipe-%s-tiling-x-planes-%d",
> -		      kmstest_pipe_name(pipe), max_planes)
> -		test_plane_position(data, pipe, false, max_planes,
> -				    LOCAL_I915_FORMAT_MOD_X_TILED);
> -
> -	igt_subtest_f("atomic-pipe-%s-tiling-x-planes-%d",
> -		      kmstest_pipe_name(pipe), max_planes)
> -		test_plane_position(data, pipe, true, max_planes,
> -				    LOCAL_I915_FORMAT_MOD_X_TILED);
> -
> -	igt_subtest_f("legacy-pipe-%s-tiling-y-planes-%d",
> -		      kmstest_pipe_name(pipe), max_planes)
> -		test_plane_position(data, pipe, false, max_planes,
> -				    LOCAL_I915_FORMAT_MOD_Y_TILED);
> -
> -	igt_subtest_f("atomic-pipe-%s-tiling-y-planes-%d",
> -		      kmstest_pipe_name(pipe), max_planes)
> -		test_plane_position(data, pipe, true, max_planes,
> -				    LOCAL_I915_FORMAT_MOD_Y_TILED);
> -
> -	igt_subtest_f("legacy-pipe-%s-tiling-yf-planes-%d",
> -		      kmstest_pipe_name(pipe), max_planes)
> -		test_plane_position(data, pipe, false, max_planes,
> -				    LOCAL_I915_FORMAT_MOD_Yf_TILED);
> -
> -	igt_subtest_f("atomic-pipe-%s-tiling-yf-planes-%d",
> -		      kmstest_pipe_name(pipe), max_planes)
> -		test_plane_position(data, pipe, true, max_planes,
> -				    LOCAL_I915_FORMAT_MOD_Yf_TILED);
> -}
> +	igt_subtest_f("legacy-pipe-%s-tiling-none-planes",
> +		      kmstest_pipe_name(pipe)) {
> +		int n_planes = data->display.pipes[pipe].n_planes;
> +		for (int planes = 0; planes < n_planes; planes++)
> +			test_plane_position(data, pipe, false,
> planes,
> +				LOCAL_DRM_FORMAT_MOD_NONE);
> +	}
>  
> -static void
> -run_tests_for_pipe(data_t *data, enum pipe pipe)
> -{
> -	for (int planes = IGT_PLANE_PRIMARY; planes <
> IGT_MAX_PLANES; planes++)
> -		run_tests_for_pipe_plane(data, pipe, planes);
> +	igt_subtest_f("atomic-pipe-%s-tiling-none-planes",
I think we could drop the reference to -planes from the subtest name as
we run through all available planes anyway.

> +		      kmstest_pipe_name(pipe)) {
> +		int n_planes = data->display.pipes[pipe].n_planes;
> +		for (int planes = 0; planes < n_planes; planes++)
For this test, we could change this by start testing with at least one
overlay plane. Originally, we run this test without any overlay planes
but since we are here we could do this change.
 
> +			test_plane_position(data, pipe, true,
> planes,
> +				LOCAL_I915_FORMAT_MOD_X_TILED);
> +	}
> +
> +	igt_subtest_f("legacy-pipe-%s-tiling-x-planes",
> +		      kmstest_pipe_name(pipe)) {
> +		int n_planes = data->display.pipes[pipe].n_planes;
> +		for (int planes = 0; planes < n_planes; planes++)
> +			test_plane_position(data, pipe, false,
> planes,
> +				LOCAL_I915_FORMAT_MOD_X_TILED);
> +	}
> +
> +	igt_subtest_f("atomic-pipe-%s-tiling-x-planes",
> +		      kmstest_pipe_name(pipe)) {
> +		int n_planes = data->display.pipes[pipe].n_planes;
> +		for (int planes = 0; planes < n_planes; planes++)
> +			test_plane_position(data, pipe, true,
> planes,
> +				LOCAL_I915_FORMAT_MOD_X_TILED);
> +	}
> +
> +	igt_subtest_f("legacy-pipe-%s-tiling-y-planes",
> +		      kmstest_pipe_name(pipe)) {
> +		int n_planes = data->display.pipes[pipe].n_planes;
> +		for (int planes = 0; planes < n_planes; planes++)
> +			test_plane_position(data, pipe, false,
> planes,
> +				LOCAL_I915_FORMAT_MOD_Y_TILED);
> +	}
> +
> +	igt_subtest_f("atomic-pipe-%s-tiling-y-planes",
> +		      kmstest_pipe_name(pipe)) {
> +		int n_planes = data->display.pipes[pipe].n_planes;
> +		for (int planes = 0; planes < n_planes; planes++)
> +			test_plane_position(data, pipe, true,
> planes,
> +				LOCAL_I915_FORMAT_MOD_Y_TILED);
> +	}
> +
> +	igt_subtest_f("legacy-pipe-%s-tiling-yf-planes",
> +		      kmstest_pipe_name(pipe)) {
> +		int n_planes = data->display.pipes[pipe].n_planes;
> +		for (int planes = 0; planes < n_planes; planes++)
> +			test_plane_position(data, pipe, false,
> planes,
> +				LOCAL_I915_FORMAT_MOD_Yf_TILED);
> +	}
> +
> +	igt_subtest_f("atomic-pipe-%s-tiling-yf-planes",
> +		      kmstest_pipe_name(pipe)) {
> +		int n_planes = data->display.pipes[pipe].n_planes;
> +		for (int planes = 0; planes < n_planes; planes++)
> +			test_plane_position(data, pipe, true,
> planes,
> +				LOCAL_I915_FORMAT_MOD_Yf_TILED);
> +	}
>  }
>  
>  static data_t data;
> @@ -461,15 +516,13 @@ int main(int argc, char *argv[])
>  
>  	igt_fixture {
>  		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> -
>  		kmstest_set_vt_graphics_mode();
> -
>  		igt_require_pipe_crc();
>  		igt_display_init(&data.display, data.drm_fd);
>  	}
>  
>  	for (int pipe = 0; pipe < I915_MAX_PIPES; pipe++)
> -		run_tests_for_pipe(&data, pipe);
> +		run_tests_for_pipe_plane(&data, pipe);
>  
>  	igt_fixture {
>  		igt_display_fini(&data.display);
-- 
Mika Kahola - Intel OTC

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx