Re: [PATCH i-g-t v4 4/5] kms_vblank: Switch from using crtc0 statically to explicitly setting mode.

[Robert Foss <robert.foss@collabora.com>]

On 2016-05-09 11:38 AM, Marius Vlad wrote:
 > On Tue, Apr 26, 2016 at 11:54:21AM -0400, robert.foss@collabora.com 
wrote:
 >> From: Robert Foss <robert.foss@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@collabora.com>
 >> ---
 >>   tests/kms_vblank.c | 162 
++++++++++++++++++++++++++++++++++++++++++++---------
 >>   1 file changed, 137 insertions(+), 25 deletions(-)
 >>
 >> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
 >> index 40ab6fd..6c32c57 100644
 >> --- a/tests/kms_vblank.c
 >> +++ b/tests/kms_vblank.c
 >> @@ -44,6 +44,14 @@
 >>
 >>   IGT_TEST_DESCRIPTION("Test speed of WaitVblank.");
 >>
 >> +typedef struct {
 >> +	igt_display_t display;
 >> +	struct igt_fb primary_fb;
 >> +	igt_output_t *output;
 >> +	enum pipe pipe;
 >> +	uint8_t mode_busy:1;
 >> +} data_t;
 >> +
 >>   static double elapsed(const struct timespec *start,
 >>   		      const struct timespec *end,
 >>   		      int loop)
 >> @@ -51,38 +59,119 @@ 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 bool prepare_crtc(data_t *data, int fd, igt_output_t *output)
 >>   {
 >> -	union drm_wait_vblank vbl;
 >> +	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;
 >> +	}
 >>
 >> -	memset(&vbl, 0, sizeof(vbl));
 >> -	vbl.request.type = DRM_VBLANK_RELATIVE;
 >> -	return drmIoctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl) == 0;
 >> +	/* create and set the primary plane fb */
 >> +	mode = igt_output_get_mode(output);
 >> +	igt_create_color_fb(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(fd, data->pipe);
 >> +
 >> +	return true;
 >>   }
 >>
 >> -static void accuracy(int fd)
 >> +static void cleanup_crtc(data_t *data, int fd, igt_output_t *output)
 >> +{
 >> +	igt_display_t *display = &data->display;
 >> +	igt_plane_t *primary;
 >> +
 >> +	igt_remove_fb(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 run_test(data_t *data, int fd, void (*testfunc)(data_t 
*, int))
 >> +{
 >> +	igt_display_t *display = &data->display;
 >> +	igt_output_t *output;
 >> +	enum pipe p;
 >> +	unsigned int valid_tests = 0;
 >> +
 >> +	for_each_connected_output(display, output) {
 >> +		for_each_pipe(display, p) {
 >> +			data->pipe = p;
 >> +
 >> +			if (!prepare_crtc(data, fd, 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, fd);
 >> +
 >> +			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, fd, output);
 >> +		}
 >> +	}
 >> +
 >> +	igt_require_f(valid_tests, "no valid crtc/connector combinations 
found\n");
 >> +}
 >> +
 >> +static void accuracy(data_t *data, int fd)
 >>   {
 >>   	union drm_wait_vblank vbl;
 >>   	unsigned long target;
 >> +	uint32_t pipe_id_flag;
 >>   	int n;
 >>
 >>   	memset(&vbl, 0, sizeof(vbl));
 >> +	pipe_id_flag = pipe_id_to_vbl_flag(data->pipe);
 >>
 >>   	vbl.request.type = DRM_VBLANK_RELATIVE;
 >> +	vbl.request.type |= pipe_id_flag;
 >>   	vbl.request.sequence = 1;
 >>   	do_ioctl(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 |= pipe_id_flag;
 >>   		vbl.request.sequence = 1;
 >>   		do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
 >>
 >>   		vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
 >> +		vbl.request.type |= pipe_id_flag;
 >>   		vbl.request.sequence = target;
 >>   		do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
 >>   	}
 >>   	vbl.request.type = DRM_VBLANK_RELATIVE;
 >> +	vbl.request.type |= pipe_id_flag;
 >>   	vbl.request.sequence = 0;
 >>   	do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
 >>   	igt_assert_eq(vbl.reply.sequence, target);
 >> @@ -94,22 +183,26 @@ static void accuracy(int fd)
 >>   	}
 >>   }
 >
 > The accuracy test fails always on pipe B, with Failed assertion:
 > ev.sequence == target. Not sure if this a bug in kernel but it seems to
 > happen on a couple of machines I've tested this (and previously we
 > weren't enabling all pipes)

Thanks for putting this series through some more testing.
I've only done testing against VC4 on RPi2, and have not observed this 
behavior.

Just by looking at the test-code I can't see a reason for that assert 
ever to fail in a legitimate way.

How do you want to proceed?
 >
 >>
 >> -static void vblank_query(int fd, bool busy)
 >> +static void vblank_query(data_t *data, int fd)
 >>   {
 >>   	union drm_wait_vblank vbl;
 >>   	struct timespec start, end;
 >>   	unsigned long sq, count = 0;
 >>   	struct drm_event_vblank buf;
 >> +	uint32_t pipe_id_flag;
 >>
 >>   	memset(&vbl, 0, sizeof(vbl));
 >> +	pipe_id_flag = pipe_id_to_vbl_flag(data->pipe);
 >>
 >> -	if (busy) {
 >> +	if (data->mode_busy) {
 >>   		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
 >> +		vbl.request.type |= pipe_id_flag;
 >>   		vbl.request.sequence = 72;
 >>   		do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
 >>   	}
 >>
 >>   	vbl.request.type = DRM_VBLANK_RELATIVE;
 >> +	vbl.request.type |= pipe_id_flag;
 >>   	vbl.request.sequence = 0;
 >>   	do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
 >>
 >> @@ -118,6 +211,7 @@ static void vblank_query(int fd, bool busy)
 >>   	clock_gettime(CLOCK_MONOTONIC, &start);
 >>   	do {
 >>   		vbl.request.type = DRM_VBLANK_RELATIVE;
 >> +		vbl.request.type |= pipe_id_flag;
 >>   		vbl.request.sequence = 0;
 >>   		do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
 >>   		count++;
 >> @@ -125,28 +219,32 @@ static void vblank_query(int fd, bool busy)
 >>   	clock_gettime(CLOCK_MONOTONIC, &end);
 >>
 >>   	igt_info("Time to query current counter (%s):		%7.3fµs\n",
 >> -		 busy ? "busy" : "idle", elapsed(&start, &end, count));
 >> +		 data->mode_busy ? "busy" : "idle", elapsed(&start, &end, count));
 >>
 >> -	if (busy)
 >> +	if (data->mode_busy)
 >>   		igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
 >>   }
 >>
 >> -static void vblank_wait(int fd, bool busy)
 >> +static void vblank_wait(data_t *data, int fd)
 >>   {
 >>   	union drm_wait_vblank vbl;
 >>   	struct timespec start, end;
 >>   	unsigned long sq, count = 0;
 >>   	struct drm_event_vblank buf;
 >> +	uint32_t pipe_id_flag;
 >>
 >>   	memset(&vbl, 0, sizeof(vbl));
 >> +	pipe_id_flag = pipe_id_to_vbl_flag(data->pipe);
 >>
 >> -	if (busy) {
 >> +	if (data->mode_busy) {
 >>   		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
 >> +		vbl.request.type |= pipe_id_flag;
 >>   		vbl.request.sequence = 72;
 >>   		do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
 >>   	}
 >>
 >>   	vbl.request.type = DRM_VBLANK_RELATIVE;
 >> +	vbl.request.type |= pipe_id_flag;
 >>   	vbl.request.sequence = 0;
 >>   	do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
 >>
 >> @@ -155,6 +253,7 @@ static void vblank_wait(int fd, bool busy)
 >>   	clock_gettime(CLOCK_MONOTONIC, &start);
 >>   	do {
 >>   		vbl.request.type = DRM_VBLANK_RELATIVE;
 >> +		vbl.request.type |= pipe_id_flag;
 >>   		vbl.request.sequence = 1;
 >>   		do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
 >>   		count++;
 >> @@ -163,36 +262,49 @@ static void vblank_wait(int fd, bool busy)
 >>
 >>   	igt_info("Time to wait for %ld/%d vblanks (%s):		%7.3fµs\n",
 >>   		 count, (int)(vbl.reply.sequence - sq),
 >> -		 busy ? "busy" : "idle",
 >> +		 data->mode_busy ? "busy" : "idle",
 >>   		 elapsed(&start, &end, count));
 >>
 >> -	if (busy)
 >> +	if (data->mode_busy)
 >>   		igt_assert_eq(read(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));
 >> +		kmstest_set_vt_graphics_mode();
 >> +		igt_display_init(&data.display, fd);
 >>   	}
 >>
 >> -	igt_subtest("accuracy")
 >> -		accuracy(fd);
 >> +	igt_subtest("accuracy") {
 >> +		data.mode_busy = 0;
 >> +		run_test(&data, fd, accuracy);
 >> +	}
 >>
 >> -	igt_subtest("query-idle")
 >> -		vblank_query(fd, false);
 >> +	igt_subtest("query-idle") {
 >> +		data.mode_busy = 0;
 >> +		run_test(&data, fd, vblank_query);
 >> +	}
 >>
 >> -	igt_subtest("query-busy")
 >> -		vblank_query(fd, true);
 >> +	igt_subtest("query-busy") {
 >> +		data.mode_busy = 1;
 >> +		run_test(&data, fd, vblank_query);
 >> +	}
 >>
 >> -	igt_subtest("wait-idle")
 >> -		vblank_wait(fd, false);
 >> +	igt_subtest("wait-idle") {
 >> +		data.mode_busy = 0;
 >> +		run_test(&data, fd, vblank_wait);
 >> +	}
 >>
 >> -	igt_subtest("wait-busy")
 >> -		vblank_wait(fd, true);
 >> +	igt_subtest("wait-busy") {
 >> +		data.mode_busy = 1;
 >> +		run_test(&data, fd, vblank_wait);
 >> +	}
 >>   }
 >> +
 > white-space.
 >> --
 >> 2.5.0
 >>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx