[Intel-gfx] [PATCH i-g-t 6/7] kms_vblank: Switch from using crtc0 statically to explicitly setting mode.

Daniel Vetter daniel at ffwll.ch
Thu Apr 21 09:18:40 UTC 2016


On Wed, Apr 20, 2016 at 10:59:48AM -0400, robert.foss at collabora.com wrote:
> From: Robert Foss <robert.foss at collabora.com>
> 
> Previously crtc0 was statically used for VBLANK tests, but
> that assumption is not valid for the VC4 platform.
> Instead we're now explicitly setting the mode.
> 
> Also add support for testing all connected connectors during
> the same test.
> 
> Signed-off-by: Robert Foss <robert.foss at collabora.com>
> ---
>  tests/kms_vblank.c | 186 ++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 147 insertions(+), 39 deletions(-)
> 
> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> index 40ab6fd..1a026d2 100644
> --- a/tests/kms_vblank.c
> +++ b/tests/kms_vblank.c
> @@ -44,6 +44,20 @@
>  
>  IGT_TEST_DESCRIPTION("Test speed of WaitVblank.");
>  
> +typedef struct {
> +	int drm_fd;
> +	igt_display_t display;
> +	struct igt_fb primary_fb;
> +	struct igt_fb fb;
> +	igt_output_t *output;
> +	enum pipe pipe;
> +	int left, right, top, bottom;
> +	int screenw, screenh;
> +	int refresh;
> +	uint32_t devid;
> +	unsigned flags;
> +} data_t;
> +
>  static double elapsed(const struct timespec *start,
>  		      const struct timespec *end,
>  		      int loop)
> @@ -51,75 +65,165 @@ static double elapsed(const struct timespec *start,
>  	return (1e6*(end->tv_sec - start->tv_sec) + (end->tv_nsec - start->tv_nsec)/1000)/loop;
>  }
>  
> -static bool crtc0_active(int fd)
> +static uint32_t crtc_id_to_flag(uint32_t crtc_id)
>  {
> -	union drm_wait_vblank vbl;
> +	if (crtc_id == 0)
> +		return 0;
> +	else if (crtc_id == 1)
> +		return 1 | _DRM_VBLANK_SECONDARY;
> +	else
> +		return crtc_id << 1;
> +}
>  
> -	memset(&vbl, 0, sizeof(vbl));
> -	vbl.request.type = DRM_VBLANK_RELATIVE;
> -	return drmIoctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl) == 0;
> +static bool prepare_crtc(data_t *data, igt_output_t *output)
> +{
> +	drmModeModeInfo *mode;
> +	igt_display_t *display = &data->display;
> +	igt_plane_t *primary;
> +
> +	/* select the pipe we want to use */
> +	igt_output_set_pipe(output, data->pipe);
> +	igt_display_commit(display);
> +
> +	if (!output->valid) {
> +		igt_output_set_pipe(output, PIPE_ANY);
> +		igt_display_commit(display);
> +		return false;
> +	}
> +
> +	/* create and set the primary plane fb */
> +	mode = igt_output_get_mode(output);
> +	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> +			    DRM_FORMAT_XRGB8888,
> +			    LOCAL_DRM_FORMAT_MOD_NONE,
> +			    0.0, 0.0, 0.0,
> +			    &data->primary_fb);
> +
> +	primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> +	igt_plane_set_fb(primary, &data->primary_fb);
> +
> +	igt_display_commit(display);
> +
> +	igt_wait_for_vblank(data->drm_fd, data->pipe);
> +
> +	return true;
> +}
> +
> +static void cleanup_crtc(data_t *data, igt_output_t *output)
> +{
> +	igt_display_t *display = &data->display;
> +	igt_plane_t *primary;
> +
> +	igt_remove_fb(data->drm_fd, &data->primary_fb);
> +
> +	primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +
> +	igt_output_set_pipe(output, PIPE_ANY);
> +	igt_display_commit(display);
>  }
>  
> -static void accuracy(int fd)
> +static void run_test(data_t *data, void (*testfunc)(data_t *, bool), bool boolean)
> +{
> +	igt_display_t *display = &data->display;
> +	igt_output_t *output;
> +	enum pipe p;
> +	int valid_tests = 0;
> +
> +	for_each_connected_output(display, output) {
> +		data->output = output;
> +		for_each_pipe(display, p) {
> +			data->pipe = p;
> +
> +			if (!prepare_crtc(data, output))
> +				continue;
> +
> +			valid_tests++;
> +
> +			igt_info("Beginning %s on pipe %s, connector %s\n",
> +				 igt_subtest_name(),
> +				 kmstest_pipe_name(data->pipe),
> +				 igt_output_name(output));
> +
> +			testfunc(data, boolean);
> +
> +			igt_info("\n%s on pipe %s, connector %s: PASSED\n\n",
> +				 igt_subtest_name(),
> +				 kmstest_pipe_name(data->pipe),
> +				 igt_output_name(output));
> +
> +			/* cleanup what prepare_crtc() has done */
> +			cleanup_crtc(data, output);
> +		}
> +	}
> +
> +	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
> +}

Annoying that we have to have so much boilerplate, but no one yet figured
out how to make this prettier. Oh well.

Ack on patches 1-6, but I didn't do a detailed review. Would be good to
get that from Tomeu or Daniel Stone.
-Daniel

> +
> +static void accuracy(data_t *data, bool busy)
>  {
>  	union drm_wait_vblank vbl;
>  	unsigned long target;
> +	uint32_t crtc_id_flag;
>  	int n;
>  
>  	memset(&vbl, 0, sizeof(vbl));
> +	crtc_id_flag = crtc_id_to_flag(data->pipe);
>  
> -	vbl.request.type = DRM_VBLANK_RELATIVE;
> +	vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
>  	vbl.request.sequence = 1;
> -	do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> +	do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>  
>  	target = vbl.reply.sequence + 60;
>  	for (n = 0; n < 60; n++) {
> -		vbl.request.type = DRM_VBLANK_RELATIVE;
> +		vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
>  		vbl.request.sequence = 1;
> -		do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> +		do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>  
> -		vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
> +		vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT | crtc_id_flag;
>  		vbl.request.sequence = target;
> -		do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> +		do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>  	}
> -	vbl.request.type = DRM_VBLANK_RELATIVE;
> +	vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
>  	vbl.request.sequence = 0;
> -	do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> +	do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>  	igt_assert_eq(vbl.reply.sequence, target);
>  
>  	for (n = 0; n < 60; n++) {
>  		struct drm_event_vblank ev;
> -		igt_assert_eq(read(fd, &ev, sizeof(ev)), sizeof(ev));
> +		igt_assert_eq(read(data->drm_fd, &ev, sizeof(ev)), sizeof(ev));
>  		igt_assert_eq(ev.sequence, target);
>  	}
>  }
>  
> -static void vblank_query(int fd, bool busy)
> +static void vblank_query(data_t *data, bool busy)
>  {
>  	union drm_wait_vblank vbl;
>  	struct timespec start, end;
>  	unsigned long sq, count = 0;
>  	struct drm_event_vblank buf;
> +	uint32_t crtc_id_flag;
>  
>  	memset(&vbl, 0, sizeof(vbl));
> +	crtc_id_flag = crtc_id_to_flag(data->pipe);
>  
>  	if (busy) {
> -		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
> +		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT | crtc_id_flag;
>  		vbl.request.sequence = 72;
> -		do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> +		do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>  	}
>  
> -	vbl.request.type = DRM_VBLANK_RELATIVE;
> +	vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
>  	vbl.request.sequence = 0;
> -	do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> +	do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>  
>  	sq = vbl.reply.sequence;
>  
>  	clock_gettime(CLOCK_MONOTONIC, &start);
>  	do {
> -		vbl.request.type = DRM_VBLANK_RELATIVE;
> +		vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
>  		vbl.request.sequence = 0;
> -		do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> +		do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>  		count++;
>  	} while ((vbl.reply.sequence - sq) <= 60);
>  	clock_gettime(CLOCK_MONOTONIC, &end);
> @@ -128,35 +232,37 @@ static void vblank_query(int fd, bool busy)
>  		 busy ? "busy" : "idle", elapsed(&start, &end, count));
>  
>  	if (busy)
> -		igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
> +		igt_assert_eq(read(data->drm_fd, &buf, sizeof(buf)), sizeof(buf));
>  }
>  
> -static void vblank_wait(int fd, bool busy)
> +static void vblank_wait(data_t *data, bool busy)
>  {
>  	union drm_wait_vblank vbl;
>  	struct timespec start, end;
>  	unsigned long sq, count = 0;
>  	struct drm_event_vblank buf;
> +	uint32_t crtc_id_flag;
>  
>  	memset(&vbl, 0, sizeof(vbl));
> +	crtc_id_flag = crtc_id_to_flag(data->pipe);
>  
>  	if (busy) {
> -		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
> +		vbl.request.type |= DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT | crtc_id_flag;
>  		vbl.request.sequence = 72;
> -		do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> +		do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>  	}
>  
> -	vbl.request.type = DRM_VBLANK_RELATIVE;
> +	vbl.request.type |= DRM_VBLANK_RELATIVE | crtc_id_flag;
>  	vbl.request.sequence = 0;
> -	do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> +	do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>  
>  	sq = vbl.reply.sequence;
>  
>  	clock_gettime(CLOCK_MONOTONIC, &start);
>  	do {
> -		vbl.request.type = DRM_VBLANK_RELATIVE;
> +		vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
>  		vbl.request.sequence = 1;
> -		do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> +		do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>  		count++;
>  	} while ((vbl.reply.sequence - sq) <= 60);
>  	clock_gettime(CLOCK_MONOTONIC, &end);
> @@ -167,32 +273,34 @@ static void vblank_wait(int fd, bool busy)
>  		 elapsed(&start, &end, count));
>  
>  	if (busy)
> -		igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
> +		igt_assert_eq(read(data->drm_fd, &buf, sizeof(buf)), sizeof(buf));
>  }
>  
>  igt_main
>  {
> -	int fd;
> +	data_t data;
>  
>  	igt_skip_on_simulation();
>  
>  	igt_fixture {
> -		fd = drm_open_driver(DRIVER_ANY);
> -		igt_require(crtc0_active(fd));
> +		data.drm_fd = drm_open_driver(DRIVER_ANY);
> +		kmstest_set_vt_graphics_mode();
> +		igt_display_init(&data.display, data.drm_fd);
>  	}
>  
>  	igt_subtest("accuracy")
> -		accuracy(fd);
> +		run_test(&data, accuracy, false);
>  
>  	igt_subtest("query-idle")
> -		vblank_query(fd, false);
> +		run_test(&data, vblank_query, false);
>  
>  	igt_subtest("query-busy")
> -		vblank_query(fd, true);
> +		run_test(&data, vblank_query, true);
>  
>  	igt_subtest("wait-idle")
> -		vblank_wait(fd, false);
> +		run_test(&data, vblank_wait, false);
>  
>  	igt_subtest("wait-busy")
> -		vblank_wait(fd, true);
> +		run_test(&data, vblank_wait, true);
>  }
> +
> -- 
> 2.5.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the Intel-gfx mailing list