Re: [PATCH i-g-t] tests/kms_plane_lowres: Rework tests to work without fbcon.

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

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

On Fri, 2017-10-13 at 16:58 +0200, Maarten Lankhorst wrote:
> kmstest_get_crtc was skipping because at that point the crtc was not
> active yet, instead we should only use igt_assert_plane_visible
> directly. Unexport kmstest_get_crtc, since nothing here should need
> it.
> While at it fix a small leak in igt_assert_plane_visible, the only
> remaining user.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> Resend, messed up my git-send-email
> 
>  lib/igt_kms.c            |  5 ++--
>  lib/igt_kms.h            |  1 -
>  tests/kms_plane_lowres.c | 64 ++++++++++++++++++++----------------
> ------------
>  3 files changed, 30 insertions(+), 40 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index cb2bc2b8df98..1c50484a613c 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1416,7 +1416,7 @@ static void parse_crtc(char *info, struct
> kmstest_crtc *crtc)
>  	igt_assert_eq(ret, 2);
>  }
>  
> -void kmstest_get_crtc(int device, enum pipe pipe, struct
> kmstest_crtc *crtc)
> +static void kmstest_get_crtc(int device, enum pipe pipe, struct
> kmstest_crtc *crtc)
>  {
>  	char tmp[256];
>  	FILE *file;
> @@ -1460,7 +1460,7 @@ void kmstest_get_crtc(int device, enum pipe
> pipe, struct kmstest_crtc *crtc)
>  	fclose(file);
>  	close(fd);
>  
> -	igt_skip_on(ncrtc == 0);
> +	igt_assert(ncrtc == 1);
>  }
>  
>  void igt_assert_plane_visible(int fd, enum pipe pipe, bool
> visibility)
> @@ -1485,6 +1485,7 @@ void igt_assert_plane_visible(int fd, enum pipe
> pipe, bool visibility)
>  		}
>  	}
>  
> +	free(crtc.planes);
>  	igt_assert_eq(visible, visibility);
>  }
>  
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 200f35e63308..acc82913e0b7 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -221,7 +221,6 @@ uint32_t kmstest_dumb_create(int fd, int width,
> int height, int bpp,
>  void *kmstest_dumb_map_buffer(int fd, uint32_t handle, uint64_t
> size,
>  			      unsigned prot);
>  unsigned int kmstest_get_vblank(int fd, int pipe, unsigned int
> flags);
> -void kmstest_get_crtc(int fd, enum pipe pipe, struct kmstest_crtc
> *crtc);
>  void igt_assert_plane_visible(int fd, enum pipe pipe, bool
> visibility);
>  
>  /*
> diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
> index b16c8cd433b2..44e0ada92ead 100644
> --- a/tests/kms_plane_lowres.c
> +++ b/tests/kms_plane_lowres.c
> @@ -40,7 +40,6 @@ typedef struct {
>  	int drm_fd;
>  	igt_display_t display;
>  	igt_pipe_crc_t *pipe_crc;
> -	igt_plane_t **plane;
>  	struct igt_fb *fb;
>  } data_t;
>  
> @@ -113,30 +112,27 @@ static void
>  test_init(data_t *data, enum pipe pipe)
>  {
>  	data->pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe,
> INTEL_PIPE_CRC_SOURCE_AUTO);
> -	data->plane = calloc(data->display.pipes[pipe].n_planes,
> sizeof(data->plane));\
> -	igt_assert_f(data->plane, "Failed to allocate memory for %d
> planes\n",
> -	             data->display.pipes[pipe].n_planes);
>  	data->fb = calloc(data->display.pipes[pipe].n_planes,
> sizeof(struct igt_fb));
>  	igt_assert_f(data->fb, "Failed to allocate memory for %d
> FBs\n",
>  	             data->display.pipes[pipe].n_planes);
>  }
>  
>  static void
> -test_fini(data_t *data, igt_output_t *output)
> +test_fini(data_t *data, igt_output_t *output, enum pipe pipe)
>  {
> +	igt_plane_t *plane;
> +
>  	/* restore original mode */
>  	igt_output_override_mode(output, NULL);
>  
> -	for (int i = 0; i < 2; i++)
> -		igt_plane_set_fb(data->plane[i], NULL);
> +	for_each_plane_on_pipe(&data->display, pipe, plane)
> +		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;
>  }
> @@ -184,19 +180,13 @@ static drmModeModeInfo *
>  test_setup(data_t *data, enum pipe pipe, uint64_t modifier, int
> flags,
>  	   igt_output_t *output)
>  {
> -	struct kmstest_crtc crtc;
>  	drmModeModeInfo *mode;
>  	int size;
>  	int i, x, y;
> +	igt_plane_t *plane;
>  
>  	igt_output_set_pipe(output, pipe);
>  
> -	kmstest_get_crtc(data->drm_fd, pipe, &crtc);
> -	igt_skip_on(crtc.n_planes > data-
> >display.pipes[pipe].n_planes);
> -	igt_skip_on(crtc.n_planes == 0);
> -
> -	for (i = 0; i < crtc.n_planes; i++)
> -		data->plane[i] = igt_output_get_plane(output,
> crtc.planes[i].index);
>  
>  	mode = igt_output_get_mode(output);
>  
> @@ -206,13 +196,14 @@ test_setup(data_t *data, enum pipe pipe,
> uint64_t modifier, int flags,
>  			    0.0, 0.0, 1.0,
>  			    &data->fb[0]);
>  
> -	igt_plane_set_fb(data->plane[0], &data->fb[0]);
> -
>  	/* yellow sprite plane in lower left corner */
> -	for (i = 0; i < crtc.n_planes; i++) {
> -		if (data->plane[i]->type == DRM_PLANE_TYPE_PRIMARY)
> +	for_each_plane_on_pipe(&data->display, pipe, plane) {
> +		if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> +			igt_plane_set_fb(plane, &data->fb[0]);
>  			continue;
> -		if (data->plane[i]->type == DRM_PLANE_TYPE_CURSOR)
> +		}
> +
> +		if (plane->type == DRM_PLANE_TYPE_CURSOR)
>  			size = 64;
>  		else
>  			size = SIZE;
> @@ -222,13 +213,13 @@ test_setup(data_t *data, enum pipe pipe,
> uint64_t modifier, int flags,
>  
>  		igt_create_color_fb(data->drm_fd,
>  				    size, size,
> -				    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 : modifier,
> +				    plane->type ==
> DRM_PLANE_TYPE_CURSOR ? DRM_FORMAT_ARGB8888 : DRM_FORMAT_XRGB8888,
> +				    plane->type ==
> DRM_PLANE_TYPE_CURSOR ? LOCAL_DRM_FORMAT_MOD_NONE : modifier,
>  				    1.0, 1.0, 0.0,
>  				    &data->fb[i]);
>  
> -		igt_plane_set_position(data->plane[i], x, y);
> -		igt_plane_set_fb(data->plane[i], &data->fb[i]);
> +		igt_plane_set_position(plane, x, y);
> +		igt_plane_set_fb(plane, &data->fb[i]);
>  	}
>  
>  	return mode;
> @@ -289,35 +280,32 @@ test_plane_position_with_output(data_t *data,
> enum pipe pipe,
>  
>  	igt_pipe_crc_stop(data->pipe_crc);
>  
> -	test_fini(data, output);
> +	test_fini(data, output, pipe);
>  }
>  
>  static void
>  test_plane_position(data_t *data, enum pipe pipe, uint64_t modifier)
>  {
>  	igt_output_t *output;
> -	int connected_outs;
>  	const int gen = intel_gen(intel_get_drm_devid(data-
> >drm_fd));
>  
> -	igt_require(data->display.is_atomic);
> -	igt_skip_on(pipe >= data->display.n_pipes);
> -
>  	if (modifier == LOCAL_I915_FORMAT_MOD_Y_TILED ||
>  	    modifier == LOCAL_I915_FORMAT_MOD_Yf_TILED)
>  		igt_skip_on(gen < 9);
>  
> -	connected_outs = 0;
> -	for_each_valid_output_on_pipe(&data->display, pipe, output)
> {
> +	for_each_valid_output_on_pipe(&data->display, pipe, output)
>  		test_plane_position_with_output(data, pipe, output,
> modifier);
> -		connected_outs++;
> -	}
> -
> -	igt_skip_on(connected_outs == 0);
>  }
>  
>  static void
>  run_tests_for_pipe(data_t *data, enum pipe pipe)
>  {
> +	igt_fixture {
> +		igt_skip_on(pipe >= data->display.n_pipes);
> +
> +		igt_display_require_output_on_pipe(&data->display,
> pipe);
> +	}
> +
>  	igt_subtest_f("pipe-%s-tiling-none",
>  		      kmstest_pipe_name(pipe))
>  		test_plane_position(data, pipe,
> LOCAL_DRM_FORMAT_MOD_NONE);
> @@ -350,10 +338,12 @@ igt_main
>  
>  		igt_require_pipe_crc(data.drm_fd);
>  		igt_display_init(&data.display, data.drm_fd);
> +		igt_require(data.display.is_atomic);
>  	}
>  
>  	for_each_pipe_static(pipe)
> -		run_tests_for_pipe(&data, pipe);
> +		igt_subtest_group
> +			run_tests_for_pipe(&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