[PATCH i-g-t 1/3] lib/igt_kms: Drop all stale events on first commit.

[PATCH i-g-t 1/3] lib/igt_kms: Drop all stale events on first commit.

[Maarten Lankhorst]

I've been trying to make kms_cursor_legacy work when subtests fail.
Other subtests will start failing too because of expired events or
stale pipe crc. The latter can be resolved in the test, but the former
could affect other tests

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 lib/igt_kms.c | 39 ++++++++++++++++++++++++++++++++++++++-
 lib/igt_kms.h |  1 +
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 223dbe4ca565..9e14f071ea57 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -2943,7 +2943,10 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
 			output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
 	}
 
-	display->first_commit = false;
+	if (display->first_commit) {
+		igt_display_drop_events(display);
+		display->first_commit = false;
+	}
 }
 
 /*
@@ -3024,6 +3027,10 @@ int igt_display_try_commit_atomic(igt_display_t *display, uint32_t flags, void *
 	if (ret || (flags & DRM_MODE_ATOMIC_TEST_ONLY))
 		return ret;
 
+	if (display->first_commit)
+		igt_fail_on_f(flags & (DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK),
+			      "First commit has to drop all stale events\n");
+
 	display_commit_changed(display, COMMIT_ATOMIC);
 
 	igt_debug_wait_for_keypress("modeset");
@@ -3121,6 +3128,36 @@ int igt_display_commit(igt_display_t *display)
 	return igt_display_commit2(display, COMMIT_LEGACY);
 }
 
+/**
+ * igt_display_drop_events:
+ * @display: DRM device handle
+ *
+ * Nonblockingly reads all current events and drops them, for highest
+ * reliablility, call igt_display_commit2() first to flush all outstanding
+ * events.
+ *
+ * This will be called on the first commit after igt_display_reset() too,
+ * to make sure any stale events are flushed.
+ */
+void igt_display_drop_events(igt_display_t *display)
+{
+	/* Clear all events from drm fd. */
+	struct pollfd pfd = {
+		.fd = display->drm_fd,
+		.events = POLLIN
+	};
+
+	while (poll(&pfd, 1, 0) > 0) {
+		struct drm_event ev;
+		char buf[128];
+
+		read(display->drm_fd, &ev, sizeof(ev));
+		igt_info("Dropping event type %u length %u\n", ev.type, ev.length);
+		igt_assert(ev.length <= sizeof(buf));
+		read(display->drm_fd, buf, ev.length);
+	}
+}
+
 const char *igt_output_name(igt_output_t *output)
 {
 	return output->name;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 2a480bf39956..342881a69177 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -360,6 +360,7 @@ int  igt_display_commit(igt_display_t *display);
 int  igt_display_try_commit_atomic(igt_display_t *display, uint32_t flags, void *user_data);
 void igt_display_commit_atomic(igt_display_t *display, uint32_t flags, void *user_data);
 int  igt_display_try_commit2(igt_display_t *display, enum igt_commit_style s);
+void igt_display_drop_events(igt_display_t *display);
 int  igt_display_get_n_pipes(igt_display_t *display);
 void igt_display_require_output(igt_display_t *display);
 void igt_display_require_output_on_pipe(igt_display_t *display, enum pipe pipe);
-- 
2.15.1

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

[PATCH i-g-t 2/3] tests/kms_cursor_legacy: Perform lazy cleanup between tests

[Maarten Lankhorst]

Instead of assuming each subtest cleans up after itself, assume it
fails and doesn't. Now that igt_kms can clean up stale events, we
can just force each subtest to only clean up its framebuffers,
which isn't harmful if it failed.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 tests/kms_cursor_legacy.c | 88 +++++++----------------------------------------
 1 file changed, 12 insertions(+), 76 deletions(-)

diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
index 5720dbef90d3..94d91e9c921a 100644
--- a/tests/kms_cursor_legacy.c
+++ b/tests/kms_cursor_legacy.c
@@ -45,6 +45,8 @@
 
 IGT_TEST_DESCRIPTION("Stress legacy cursor ioctl");
 
+igt_pipe_crc_t *pipe_crc;
+
 static void stress(igt_display_t *display,
 		   enum pipe pipe, int num_children, unsigned mode,
 		   int timeout)
@@ -203,22 +205,6 @@ static void populate_cursor_args(igt_display_t *display, enum pipe pipe,
 	arg[1] = *arg;
 }
 
-static void do_cleanup_display(igt_display_t *display)
-{
-	enum pipe pipe;
-	igt_output_t *output;
-	igt_plane_t *plane;
-
-	for_each_pipe(display, pipe)
-		for_each_plane_on_pipe(display, pipe, plane)
-			igt_plane_set_fb(plane, NULL);
-
-	for_each_connected_output(display, output)
-		igt_output_set_pipe(output, PIPE_NONE);
-
-	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
-}
-
 static enum pipe find_connected_pipe(igt_display_t *display, bool second)
 {
 	enum pipe pipe, first = PIPE_NONE;
@@ -226,6 +212,14 @@ static enum pipe find_connected_pipe(igt_display_t *display, bool second)
 	igt_output_t *first_output = NULL;
 	bool found = false;
 
+	if (!second) {
+		igt_pipe_crc_free(pipe_crc);
+		pipe_crc = NULL;
+
+		/* Clear display, events will be eaten by commit.. */
+		igt_display_reset(display);
+	}
+
 	for_each_pipe_with_valid_output(display, pipe, output) {
 		if (first == pipe || output == first_output)
 			continue;
@@ -451,8 +445,6 @@ static void flip(igt_display_t *display,
 
 	munmap(results, 4096);
 
-	do_cleanup_display(display);
-
 	igt_remove_fb(display->drm_fd, &fb_info);
 	if (flip_pipe != cursor_pipe)
 		igt_remove_fb(display->drm_fd, &fb_info2);
@@ -608,7 +600,6 @@ static void basic_flip_cursor(igt_display_t *display,
 	if (miss1 || miss2)
 		igt_info("Failed to evade %i vblanks and missed %i page flips\n", miss1, miss2);
 
-	do_cleanup_display(display);
 	igt_remove_fb(display->drm_fd, &fb_info);
 	igt_remove_fb(display->drm_fd, &cursor_fb);
 
@@ -758,7 +749,6 @@ static void flip_vs_cursor(igt_display_t *display, enum flip_test mode, int nloo
 		sched_setaffinity(0, sizeof(oldmask), &oldmask);
 	}
 
-	do_cleanup_display(display);
 	igt_remove_fb(display->drm_fd, &fb_info);
 	igt_remove_fb(display->drm_fd, &cursor_fb);
 
@@ -768,34 +758,12 @@ static void flip_vs_cursor(igt_display_t *display, enum flip_test mode, int nloo
 		igt_remove_fb(display->drm_fd, &cursor_fb2);
 }
 
-static bool skip_on_unsupported_nonblocking_modeset(igt_display_t *display)
-{
-	enum pipe pipe;
-	int ret;
-
-	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
-
-	ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET | DRM_MODE_ATOMIC_NONBLOCK, NULL);
-
-	if (ret == -EINVAL)
-		return true;
-
-	igt_assert_eq(ret, 0);
-
-	/* Force the next state to update all crtc's, to synchronize with the nonblocking modeset. */
-	for_each_pipe(display, pipe)
-		igt_pipe_refresh(display, pipe, false);
-
-	return false;
-}
-
 static void nonblocking_modeset_vs_cursor(igt_display_t *display, int loops)
 {
 	struct igt_fb fb_info, cursor_fb;
 	igt_output_t *output;
 	enum pipe pipe = find_connected_pipe(display, false);
 	struct drm_mode_cursor arg[2];
-	bool skip_test;
 	igt_plane_t *cursor = NULL, *plane;
 
 	igt_require(display->is_atomic);
@@ -815,13 +783,9 @@ static void nonblocking_modeset_vs_cursor(igt_display_t *display, int loops)
 
 	igt_skip_on(!cursor);
 
-	if ((skip_test = skip_on_unsupported_nonblocking_modeset(display)))
-		goto cleanup;
-
 	/*
-	 * Start disabled, because skip_on_unsupported_nonblocking_modeset
-	 * will have enabled this pipe. No way around it, since the first
-	 * atomic commit may be unreliable with amount of events sent.
+	 * Start disabled. No way around it, since the first atomic
+	 * commit may be unreliable with amount of events sent.
 	 */
 	igt_output_set_pipe(output, PIPE_NONE);
 	igt_display_commit2(display, COMMIT_ATOMIC);
@@ -873,13 +837,8 @@ static void nonblocking_modeset_vs_cursor(igt_display_t *display, int loops)
 		igt_reset_timeout();
 	}
 
-cleanup:
-	do_cleanup_display(display);
 	igt_remove_fb(display->drm_fd, &fb_info);
 	igt_remove_fb(display->drm_fd, &cursor_fb);
-
-	if (skip_test)
-		igt_skip("Nonblocking modeset is not supported by this kernel\n");
 }
 
 static void two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool modeset, bool atomic)
@@ -891,7 +850,6 @@ static void two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool
 	enum pipe pipe = find_connected_pipe(display, false);
 	enum pipe pipe2 = find_connected_pipe(display, true);
 	igt_output_t *output, *output2;
-	bool skip_test = false;
 
 	igt_fail_on(modeset && !atomic);
 
@@ -912,9 +870,6 @@ static void two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool
 
 	arg2[1].x = arg2[1].y = 192;
 
-	if (modeset && (skip_test = skip_on_unsupported_nonblocking_modeset(display)))
-		goto cleanup;
-
 	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
 
 	vblank_start = get_vblank(display->drm_fd, pipe, DRM_VBLANK_NEXTONMISS);
@@ -977,14 +932,9 @@ static void two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool
 		}
 	}
 
-cleanup:
-	do_cleanup_display(display);
 	igt_remove_fb(display->drm_fd, &fb_info);
 	igt_remove_fb(display->drm_fd, &fb2_info);
 	igt_remove_fb(display->drm_fd, &cursor_fb);
-
-	if (skip_test)
-		igt_skip("Nonblocking modeset is not supported by this kernel\n");
 }
 
 static void cursor_vs_flip(igt_display_t *display, enum flip_test mode, int nloops)
@@ -1066,7 +1016,6 @@ static void cursor_vs_flip(igt_display_t *display, enum flip_test mode, int nloo
 			     vrefresh*target, vrefresh*target / 2);
 	}
 
-	do_cleanup_display(display);
 	igt_remove_fb(display->drm_fd, &fb_info);
 	igt_remove_fb(display->drm_fd, &cursor_fb);
 	munmap((void *)shared, 4096);
@@ -1173,7 +1122,6 @@ static void two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool
 				    vrefresh[child]*target[child], vrefresh[child]*target[child] / 2);
 	}
 
-	do_cleanup_display(display);
 	igt_remove_fb(display->drm_fd, &fb_info[0]);
 	igt_remove_fb(display->drm_fd, &fb_info[1]);
 	igt_remove_fb(display->drm_fd, &cursor_fb);
@@ -1187,7 +1135,6 @@ static void flip_vs_cursor_crc(igt_display_t *display, bool atomic)
 	struct igt_fb fb_info, cursor_fb;
 	unsigned vblank_start;
 	enum pipe pipe = find_connected_pipe(display, false);
-	igt_pipe_crc_t *pipe_crc;
 	igt_crc_t crcs[3];
 
 	if (atomic)
@@ -1232,10 +1179,8 @@ static void flip_vs_cursor_crc(igt_display_t *display, bool atomic)
 		igt_assert_crc_equal(&crcs[i], &crcs[2]);
 	}
 
-	do_cleanup_display(display);
 	igt_remove_fb(display->drm_fd, &fb_info);
 	igt_remove_fb(display->drm_fd, &cursor_fb);
-	igt_pipe_crc_free(pipe_crc);
 }
 
 static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
@@ -1245,7 +1190,6 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
 	struct igt_fb fb_info[2], cursor_fb;
 	unsigned vblank_start;
 	enum pipe pipe = find_connected_pipe(display, false);
-	igt_pipe_crc_t *pipe_crc;
 	igt_pipe_t *pipe_connected = &display->pipes[pipe];
 	igt_plane_t *plane_primary = igt_pipe_get_plane_type(pipe_connected, DRM_PLANE_TYPE_PRIMARY);
 	igt_crc_t crcs[2];
@@ -1333,17 +1277,9 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
 		free(received_crcs);
 	}
 
-	do_cleanup_display(display);
 	igt_remove_fb(display->drm_fd, &fb_info[1]);
 	igt_remove_fb(display->drm_fd, &fb_info[0]);
 	igt_remove_fb(display->drm_fd, &cursor_fb);
-
-	/*
-	 * igt_pipe_crc_stop() may force a modeset for workarounds, call
-	 * it after do_cleanup_display since we disable the display anyway.
-	 */
-	igt_pipe_crc_stop(pipe_crc);
-	igt_pipe_crc_free(pipe_crc);
 }
 
 igt_main
-- 
2.15.1

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

[PATCH i-g-t 3/3] tests/kms_cursor_legacy: Rework the 2x-*-vs-cursor-* tests.

[Maarten Lankhorst]

Using the fancy new DRM_CAP_CRTC_IN_VBLANK_EVENT cap I can finally
make this test the work I originally intended to.

For the !modeset case that means performing a pageflip on both crtc's,
then requeueing as soon as the event is delivered and then check the
vblank counter against the original value, it should be advanced by 1.

The modeset case is slightly more complicated, ideally it's handled
the same, but if we can't perform a modeset and pageflip at the same
time, fall back to queueing both in a single commit, in which case
we can say nothing about the vblank counter.

There is a small race between flip_done and hw_done, so make
flip_nonblocking retry for a second when encountering -EBUSY.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101634
---
 tests/kms_cursor_legacy.c | 228 +++++++++++++++++++++++++++++++---------------
 1 file changed, 156 insertions(+), 72 deletions(-)

diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
index 94d91e9c921a..5011e78e5c2f 100644
--- a/tests/kms_cursor_legacy.c
+++ b/tests/kms_cursor_legacy.c
@@ -243,19 +243,27 @@ static enum pipe find_connected_pipe(igt_display_t *display, bool second)
 	return pipe;
 }
 
-static void flip_nonblocking(igt_display_t *display, enum pipe pipe_id, bool atomic, struct igt_fb *fb)
+static void flip_nonblocking(igt_display_t *display, enum pipe pipe_id, bool atomic, struct igt_fb *fb, void *data)
 {
 	igt_pipe_t *pipe = &display->pipes[pipe_id];
 	igt_plane_t *primary = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
+	int ret;
 
+	igt_set_timeout(1, "Scheduling page flip\n");
 	if (!atomic) {
 		/* Schedule a nonblocking flip for the next vblank */
-		do_or_die(drmModePageFlip(display->drm_fd, pipe->crtc_id, fb->fb_id,
-					DRM_MODE_PAGE_FLIP_EVENT, fb));
+		do {
+			ret = drmModePageFlip(display->drm_fd, pipe->crtc_id, fb->fb_id,
+					      DRM_MODE_PAGE_FLIP_EVENT, data);
+		} while (ret == -EBUSY);
 	} else {
 		igt_plane_set_fb(primary, fb);
-		igt_display_commit_atomic(display, DRM_MODE_ATOMIC_NONBLOCK | DRM_MODE_PAGE_FLIP_EVENT, fb);
+		do {
+			ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_NONBLOCK | DRM_MODE_PAGE_FLIP_EVENT, data);
+		} while (ret == -EBUSY);
 	}
+	igt_assert(!ret);
+	igt_reset_timeout();
 }
 
 enum flip_test {
@@ -424,7 +432,7 @@ static void flip(igt_display_t *display,
 
 			switch (mode) {
 			default:
-				flip_nonblocking(display, flip_pipe, mode >= flip_test_atomic, &fb_info);
+				flip_nonblocking(display, flip_pipe, mode >= flip_test_atomic, &fb_info, NULL);
 				break;
 			case flip_test_atomic_transitions:
 			case flip_test_atomic_transitions_varying_size:
@@ -533,7 +541,7 @@ static void basic_flip_cursor(igt_display_t *display,
 		case FLIP_BEFORE_CURSOR:
 			switch (mode) {
 			default:
-				flip_nonblocking(display, pipe, mode >= flip_test_atomic, &fb_info);
+				flip_nonblocking(display, pipe, mode >= flip_test_atomic, &fb_info, NULL);
 				break;
 			case flip_test_atomic_transitions:
 			case flip_test_atomic_transitions_varying_size:
@@ -555,7 +563,7 @@ static void basic_flip_cursor(igt_display_t *display,
 
 			switch (mode) {
 			default:
-				flip_nonblocking(display, pipe, mode >= flip_test_atomic, &fb_info);
+				flip_nonblocking(display, pipe, mode >= flip_test_atomic, &fb_info, NULL);
 				break;
 			case flip_test_atomic_transitions:
 			case flip_test_atomic_transitions_varying_size:
@@ -712,7 +720,7 @@ static void flip_vs_cursor(igt_display_t *display, enum flip_test mode, int nloo
 		vblank_start = get_vblank(display->drm_fd, pipe, DRM_VBLANK_NEXTONMISS);
 		switch (mode) {
 		default:
-			flip_nonblocking(display, pipe, mode >= flip_test_atomic, &fb_info);
+			flip_nonblocking(display, pipe, mode >= flip_test_atomic, &fb_info, NULL);
 			break;
 		case flip_test_atomic_transitions:
 		case flip_test_atomic_transitions_varying_size:
@@ -843,13 +851,26 @@ static void nonblocking_modeset_vs_cursor(igt_display_t *display, int loops)
 
 static void two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool modeset, bool atomic)
 {
-	struct drm_mode_cursor arg[2], arg2[2];
-	struct drm_event_vblank vbl;
+	struct drm_mode_cursor arg1[2], arg2[2];
 	struct igt_fb fb_info, fb2_info, cursor_fb;
-	unsigned vblank_start;
 	enum pipe pipe = find_connected_pipe(display, false);
 	enum pipe pipe2 = find_connected_pipe(display, true);
 	igt_output_t *output, *output2;
+	bool vblank_matches, enabled = false;
+	volatile unsigned long *shared;
+	unsigned flags = 0, vblank_start;
+	struct drm_event_vblank vbl;
+	int ret;
+
+	if (modeset) {
+		uint64_t val;
+
+		igt_fail_on(!atomic);
+		igt_require(drmGetCap(display->drm_fd, DRM_CAP_CRTC_IN_VBLANK_EVENT, &val) == 0);
+	}
+
+	shared = mmap(NULL, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
+	igt_assert(shared != MAP_FAILED);
 
 	igt_fail_on(modeset && !atomic);
 
@@ -861,77 +882,140 @@ static void two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool
 
 	igt_create_color_fb(display->drm_fd, 64, 64, DRM_FORMAT_ARGB8888, 0, 1., 1., 1., &cursor_fb);
 	set_cursor_on_pipe(display, pipe, &cursor_fb);
-	populate_cursor_args(display, pipe, arg, &cursor_fb);
+	populate_cursor_args(display, pipe, arg1, &cursor_fb);
 
-	arg[1].x = arg[1].y = 192;
+	arg1[1].x = arg1[1].y = 192;
 
 	set_cursor_on_pipe(display, pipe2, &cursor_fb);
 	populate_cursor_args(display, pipe2, arg2, &cursor_fb);
 
 	arg2[1].x = arg2[1].y = 192;
 
+
 	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
 
-	vblank_start = get_vblank(display->drm_fd, pipe, DRM_VBLANK_NEXTONMISS);
-	igt_assert_eq(get_vblank(display->drm_fd, pipe, 0), vblank_start);
-	do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR, &arg[0]);
-	do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR, &arg2[0]);
-	igt_assert_eq(get_vblank(display->drm_fd, pipe, 0), vblank_start);
+	igt_fork(child, 2) {
+		struct drm_mode_cursor *arg = child ? arg2 : arg1;
 
-	while (nloops--) {
-		/* Start with a synchronous query to align with the vblank */
+		while (!shared[0])
+			do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR,
+				 &arg[!shared[1]]);
+	}
+
+	if (modeset) {
+		igt_plane_t *plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+
+		flags = DRM_MODE_ATOMIC_ALLOW_MODESET |
+			DRM_MODE_ATOMIC_NONBLOCK | DRM_MODE_PAGE_FLIP_EVENT;
+
+		/* Disable pipe2 */
+		igt_output_set_pipe(output2, PIPE_NONE);
+		igt_display_commit_atomic(display, flags, NULL);
+		enabled = false;
+
+		/*
+		 * Try a page flip on crtc 1, if we succeed pump page flips and
+		 * modesets interleaved, else do a single atomic commit with both.
+		 */
 		vblank_start = get_vblank(display->drm_fd, pipe, DRM_VBLANK_NEXTONMISS);
-		do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR, &arg[nloops & 1]);
+		igt_plane_set_fb(plane, &fb_info);
+		ret = igt_display_try_commit_atomic(display, flags, (void*)(ptrdiff_t)vblank_start);
+		igt_assert(!ret || ret == -EBUSY);
+
+		if (ret == -EBUSY) {
+			/* Force completion on both pipes, and generate event. */
+			igt_display_commit_atomic(display, flags, NULL);
+
+			while (nloops--) {
+				shared[1] = nloops & 1;
+
+				igt_set_timeout(35, "Stuck modeset");
+				igt_assert_eq(read(display->drm_fd, &vbl, sizeof(vbl)), sizeof(vbl));
+				igt_assert_eq(read(display->drm_fd, &vbl, sizeof(vbl)), sizeof(vbl));
+				igt_reset_timeout();
+
+				if (!nloops)
+					break;
+
+				/* Commit page flip and modeset simultaneously. */
+				igt_plane_set_fb(plane, &fb_info);
+				igt_output_set_pipe(output2, enabled ? PIPE_NONE : pipe2);
+				enabled = !enabled;
+
+				igt_set_timeout(5, "Scheduling modeset\n");
+				do {
+					ret = igt_display_try_commit_atomic(display, flags, NULL);
+				} while (ret == -EBUSY);
+				igt_assert(!ret);
+				igt_reset_timeout();
+			}
 
-		if (!modeset)
-			flip_nonblocking(display, pipe, false, &fb_info);
-		else {
-			/*
-			 * There are 2 design issues that prevent us from doing
-			 * the test we would like here:
-			 *
-			 * - drm_event_vblank doesn't set crtc_id, so if we
-			 *   use events we don't know which pipe fired the event,
-			 *   no way to distinguish.
-			 * - Doing a modeset may add unrelated pipes, and fail with
-			 *   -EBUSY if a page flip is queued on one of those.
-			 *
-			 * Work around it by not setting an event, but doing a synchronous
-			 * commit to wait for completion, and queue the page flip and modeset
-			 * in the same commit.
-			 */
-
-			igt_plane_set_fb(igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY), &fb_info);
-			igt_output_set_pipe(output2, (nloops & 1) ? PIPE_NONE : pipe2);
-			igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET | DRM_MODE_ATOMIC_NONBLOCK, NULL);
+			goto done;
 		}
+	} else {
+		vblank_start = get_vblank(display->drm_fd, pipe, DRM_VBLANK_NEXTONMISS);
+		flip_nonblocking(display, pipe, atomic, &fb_info, (void*)(ptrdiff_t)vblank_start);
 
-		igt_assert_eq(get_vblank(display->drm_fd, pipe, 0), vblank_start);
-
-		do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR, &arg[nloops & 1]);
-		if (!modeset) {
-			do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR, &arg2[nloops & 1]);
-			do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR, &arg[nloops & 1]);
-			do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR, &arg2[nloops & 1]);
+		vblank_start = get_vblank(display->drm_fd, pipe2, DRM_VBLANK_NEXTONMISS);
+		flip_nonblocking(display, pipe2, atomic, &fb2_info, (void*)(ptrdiff_t)vblank_start);
+	}
 
-			igt_assert_eq(get_vblank(display->drm_fd, pipe, 0), vblank_start);
+	vblank_matches = false;
+	while (nloops) {
+		shared[1] = nloops & 1;
 
+		if (!modeset || nloops > 1)
 			igt_set_timeout(1, "Stuck page flip");
-			igt_ignore_warn(read(display->drm_fd, &vbl, sizeof(vbl)));
-			igt_assert_eq(get_vblank(display->drm_fd, pipe, 0), vblank_start + 1);
-			igt_reset_timeout();
-		} else {
-			do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR, &arg2[nloops & 1]);
+		else
+			igt_set_timeout(35, "Stuck modeset");
+		igt_assert_eq(read(display->drm_fd, &vbl, sizeof(vbl)), sizeof(vbl));
+		igt_reset_timeout();
+
+		vblank_start = vbl.user_data;
+		if (!modeset)
+			igt_assert_eq(vbl.sequence, vblank_start + 1);
+
+		if (vblank_start && vbl.sequence == vblank_start + 1)
+			vblank_matches = true;
+
+		/* Do not requeue on the last 2 events. */
+		if (nloops <= 2) {
+			nloops--;
+			continue;
 		}
 
-		if (modeset) {
-			/* wait for pending modeset and page flip to complete, to prevent -EBUSY */
-			igt_pipe_refresh(display, pipe, false);
-			igt_pipe_refresh(display, pipe2, false);
-			igt_display_commit2(display, COMMIT_ATOMIC);
+		if (vbl.crtc_id == display->pipes[pipe].crtc_id) {
+			vblank_start = get_vblank(display->drm_fd, pipe, DRM_VBLANK_NEXTONMISS);
+			flip_nonblocking(display, pipe, atomic, &fb_info, (void*)(ptrdiff_t)vblank_start);
+		} else {
+			igt_assert(vbl.crtc_id == display->pipes[pipe2].crtc_id);
+
+			nloops--;
+
+			if (!modeset) {
+				vblank_start = get_vblank(display->drm_fd, pipe2, DRM_VBLANK_NEXTONMISS);
+				flip_nonblocking(display, pipe2, atomic, &fb2_info, (void*)(ptrdiff_t)vblank_start);
+			} else {
+				igt_output_set_pipe(output2, enabled ? PIPE_NONE : pipe2);
+
+				igt_set_timeout(1, "Scheduling modeset\n");
+				do {
+					ret = igt_display_try_commit_atomic(display, flags, NULL);
+				} while (ret == -EBUSY);
+				igt_assert(!ret);
+				igt_reset_timeout();
+
+				enabled = !enabled;
+			}
 		}
 	}
 
+	igt_assert_f(vblank_matches, "During modeset at least 1 page flip needs to match!\n");
+
+done:
+	shared[0] = 1;
+	igt_waitchildren();
+
 	igt_remove_fb(display->drm_fd, &fb_info);
 	igt_remove_fb(display->drm_fd, &fb2_info);
 	igt_remove_fb(display->drm_fd, &cursor_fb);
@@ -982,7 +1066,7 @@ static void cursor_vs_flip(igt_display_t *display, enum flip_test mode, int nloo
 
 		switch (mode) {
 		default:
-			flip_nonblocking(display, pipe, mode >= flip_test_atomic, &fb_info);
+			flip_nonblocking(display, pipe, mode >= flip_test_atomic, &fb_info, NULL);
 			break;
 		case flip_test_atomic_transitions:
 		case flip_test_atomic_transitions_varying_size:
@@ -993,7 +1077,7 @@ static void cursor_vs_flip(igt_display_t *display, enum flip_test mode, int nloo
 		igt_assert_eq(read(display->drm_fd, &vbl, sizeof(vbl)), sizeof(vbl));
 		vblank_start = vblank_last = vbl.sequence;
 		for (int n = 0; n < vrefresh / 2; n++) {
-			flip_nonblocking(display, pipe, mode >= flip_test_atomic, &fb_info);
+			flip_nonblocking(display, pipe, mode >= flip_test_atomic, &fb_info, NULL);
 
 			igt_assert_eq(read(display->drm_fd, &vbl, sizeof(vbl)), sizeof(vbl));
 			if (vbl.sequence != vblank_last + 1) {
@@ -1083,14 +1167,14 @@ static void two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool
 			shared[child] = count;
 		}
 
-		flip_nonblocking(display, pipe[0], atomic, &fb_info[0]);
-		flip_nonblocking(display, pipe[1], atomic, &fb_info[1]);
+		flip_nonblocking(display, pipe[0], atomic, &fb_info[0], (void *)0UL);
+		flip_nonblocking(display, pipe[1], atomic, &fb_info[1], (void *)1UL);
 
 		for (int n = 0; n < vrefresh[0] / 2 + vrefresh[1] / 2; n++) {
-			int child;
+			unsigned long child;
 
 			igt_assert_eq(read(display->drm_fd, &vbl, sizeof(vbl)), sizeof(vbl));
-			child = vbl.user_data == (unsigned long)&fb_info[1];
+			child = vbl.user_data;
 
 			if (!done[child]++)
 				vblank_start[child] = vbl.sequence;
@@ -1101,7 +1185,7 @@ static void two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool
 			vblank_last[child] = vbl.sequence;
 
 			if (done[child] < vrefresh[child] / 2) {
-				flip_nonblocking(display, pipe[child], atomic, &fb_info[child]);
+				flip_nonblocking(display, pipe[child], atomic, &fb_info[child], (void *)child);
 			} else {
 				igt_assert_lte(vbl.sequence, vblank_start[child] + 5 * vrefresh[child] / 8);
 
@@ -1163,7 +1247,7 @@ static void flip_vs_cursor_crc(igt_display_t *display, bool atomic)
 	for (int i = 1; i >= 0; i--) {
 		vblank_start = get_vblank(display->drm_fd, pipe, DRM_VBLANK_NEXTONMISS);
 
-		flip_nonblocking(display, pipe, atomic, &fb_info);
+		flip_nonblocking(display, pipe, atomic, &fb_info, NULL);
 		do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR, &arg[i]);
 
 		igt_assert_eq(get_vblank(display->drm_fd, pipe, 0), vblank_start);
@@ -1247,7 +1331,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
 
 		vblank_start = get_vblank(display->drm_fd, pipe, DRM_VBLANK_NEXTONMISS);
 
-		flip_nonblocking(display, pipe, atomic, &fb_info[1]);
+		flip_nonblocking(display, pipe, atomic, &fb_info[1], NULL);
 		do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR, &arg[i]);
 
 		igt_assert_eq(get_vblank(display->drm_fd, pipe, 0), vblank_start);
@@ -1347,7 +1431,7 @@ igt_main
 		two_screens_flip_vs_cursor(&display, 8, false, false);
 
 	igt_subtest("2x-flip-vs-cursor-atomic")
-		two_screens_flip_vs_cursor(&display, 4, false, true);
+		two_screens_flip_vs_cursor(&display, 8, false, true);
 
 	igt_subtest("2x-cursor-vs-flip-legacy")
 		two_screens_cursor_vs_flip(&display, 8, false);
@@ -1362,13 +1446,13 @@ igt_main
 		two_screens_cursor_vs_flip(&display, 50, false);
 
 	igt_subtest("2x-nonblocking-modeset-vs-cursor-atomic")
-		two_screens_flip_vs_cursor(&display, 8, true, true);
+		two_screens_flip_vs_cursor(&display, 4, true, true);
 
 	igt_subtest("2x-cursor-vs-flip-atomic")
 		two_screens_cursor_vs_flip(&display, 8, true);
 
 	igt_subtest("2x-long-nonblocking-modeset-vs-cursor-atomic")
-		two_screens_flip_vs_cursor(&display, 150, true, true);
+		two_screens_flip_vs_cursor(&display, 15, true, true);
 
 	igt_subtest("2x-long-cursor-vs-flip-atomic")
 		two_screens_cursor_vs_flip(&display, 50, true);
-- 
2.15.1

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

Re: [PATCH i-g-t 1/3] lib/igt_kms: Drop all stale events on first commit.

[Chris Wilson]

Quoting Maarten Lankhorst (2017-12-07 13:40:25)
> I've been trying to make kms_cursor_legacy work when subtests fail.
> Other subtests will start failing too because of expired events or
> stale pipe crc. The latter can be resolved in the test, but the former
> could affect other tests
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  lib/igt_kms.c | 39 ++++++++++++++++++++++++++++++++++++++-
>  lib/igt_kms.h |  1 +
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 223dbe4ca565..9e14f071ea57 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -2943,7 +2943,10 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
>                         output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
>         }
>  
> -       display->first_commit = false;
> +       if (display->first_commit) {
> +               igt_display_drop_events(display);
> +               display->first_commit = false;
> +       }

So I spent quite a bit of time debating whether there is merit in "do
something; set-first-mode" that this would then clobber. After some
thought, no that doesn't seem like a wise test construction. I would
however suggest that we igt_debug() if we drop any events here.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 1/3] lib/igt_kms: Drop all stale events on first commit.

[Maarten Lankhorst]

Op 07-12-17 om 16:03 schreef Chris Wilson:
> Quoting Maarten Lankhorst (2017-12-07 13:40:25)
>> I've been trying to make kms_cursor_legacy work when subtests fail.
>> Other subtests will start failing too because of expired events or
>> stale pipe crc. The latter can be resolved in the test, but the former
>> could affect other tests
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  lib/igt_kms.c | 39 ++++++++++++++++++++++++++++++++++++++-
>>  lib/igt_kms.h |  1 +
>>  2 files changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 223dbe4ca565..9e14f071ea57 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -2943,7 +2943,10 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
>>                         output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
>>         }
>>  
>> -       display->first_commit = false;
>> +       if (display->first_commit) {
>> +               igt_display_drop_events(display);
>> +               display->first_commit = false;
>> +       }
> So I spent quite a bit of time debating whether there is merit in "do
> something; set-first-mode" that this would then clobber. After some
> thought, no that doesn't seem like a wise test construction. I would
> however suggest that we igt_debug() if we drop any events here.
> -Chris

Exactly. I'm using igt_info since it's supposed to be uncommon, is it ok if I upgrade it to a igt_warn() since nothing in CI will trigger it?

~Maarten

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

Re: [PATCH i-g-t 1/3] lib/igt_kms: Drop all stale events on first commit.

[Chris Wilson]

Quoting Maarten Lankhorst (2017-12-07 15:42:54)
> Op 07-12-17 om 16:03 schreef Chris Wilson:
> > Quoting Maarten Lankhorst (2017-12-07 13:40:25)
> >> I've been trying to make kms_cursor_legacy work when subtests fail.
> >> Other subtests will start failing too because of expired events or
> >> stale pipe crc. The latter can be resolved in the test, but the former
> >> could affect other tests
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >>  lib/igt_kms.c | 39 ++++++++++++++++++++++++++++++++++++++-
> >>  lib/igt_kms.h |  1 +
> >>  2 files changed, 39 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> >> index 223dbe4ca565..9e14f071ea57 100644
> >> --- a/lib/igt_kms.c
> >> +++ b/lib/igt_kms.c
> >> @@ -2943,7 +2943,10 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
> >>                         output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
> >>         }
> >>  
> >> -       display->first_commit = false;
> >> +       if (display->first_commit) {
> >> +               igt_display_drop_events(display);
> >> +               display->first_commit = false;
> >> +       }
> > So I spent quite a bit of time debating whether there is merit in "do
> > something; set-first-mode" that this would then clobber. After some
> > thought, no that doesn't seem like a wise test construction. I would
> > however suggest that we igt_debug() if we drop any events here.
> > -Chris
> 
> Exactly. I'm using igt_info since it's supposed to be uncommon, is it ok if I upgrade it to a igt_warn() since nothing in CI will trigger it?

Oh, I wouldn't have put it inside drop_events itself, since that is just
doing its job -- whether or not it is relevant depends on the caller. So
I would suggest reducing it to igt_debug, or just reporting the number
dropped and making the judgement in the caller.

No, I don't this should be an igt_warn, as then CI blames the following
test for environmental errors left over from previous runs.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 1/3] lib/igt_kms: Drop all stale events on first commit.

[Maarten Lankhorst]

Op 07-12-17 om 16:50 schreef Chris Wilson:
> Quoting Maarten Lankhorst (2017-12-07 15:42:54)
>> Op 07-12-17 om 16:03 schreef Chris Wilson:
>>> Quoting Maarten Lankhorst (2017-12-07 13:40:25)
>>>> I've been trying to make kms_cursor_legacy work when subtests fail.
>>>> Other subtests will start failing too because of expired events or
>>>> stale pipe crc. The latter can be resolved in the test, but the former
>>>> could affect other tests
>>>>
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> ---
>>>>  lib/igt_kms.c | 39 ++++++++++++++++++++++++++++++++++++++-
>>>>  lib/igt_kms.h |  1 +
>>>>  2 files changed, 39 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>>> index 223dbe4ca565..9e14f071ea57 100644
>>>> --- a/lib/igt_kms.c
>>>> +++ b/lib/igt_kms.c
>>>> @@ -2943,7 +2943,10 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
>>>>                         output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
>>>>         }
>>>>  
>>>> -       display->first_commit = false;
>>>> +       if (display->first_commit) {
>>>> +               igt_display_drop_events(display);
>>>> +               display->first_commit = false;
>>>> +       }
>>> So I spent quite a bit of time debating whether there is merit in "do
>>> something; set-first-mode" that this would then clobber. After some
>>> thought, no that doesn't seem like a wise test construction. I would
>>> however suggest that we igt_debug() if we drop any events here.
>>> -Chris
>> Exactly. I'm using igt_info since it's supposed to be uncommon, is it ok if I upgrade it to a igt_warn() since nothing in CI will trigger it?
> Oh, I wouldn't have put it inside drop_events itself, since that is just
> doing its job -- whether or not it is relevant depends on the caller. So
> I would suggest reducing it to igt_debug, or just reporting the number
> dropped and making the judgement in the caller.
>
> No, I don't this should be an igt_warn, as then CI blames the following
> test for environmental errors left over from previous runs.
> -Chris

It's not even possible, CI runs each test separately. When you open a
new copy of the drm fd you don't get events for the previous fd.

Only when subtests fail you should get a dropped event, because of this
it will never happen in CI and it's safe to change to a warn.

~Maarten

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

Re: [PATCH i-g-t 1/3] lib/igt_kms: Drop all stale events on first commit.

[Chris Wilson]

Quoting Maarten Lankhorst (2017-12-07 15:57:09)
> Op 07-12-17 om 16:50 schreef Chris Wilson:
> > Quoting Maarten Lankhorst (2017-12-07 15:42:54)
> >> Op 07-12-17 om 16:03 schreef Chris Wilson:
> >>> Quoting Maarten Lankhorst (2017-12-07 13:40:25)
> >>>> I've been trying to make kms_cursor_legacy work when subtests fail.
> >>>> Other subtests will start failing too because of expired events or
> >>>> stale pipe crc. The latter can be resolved in the test, but the former
> >>>> could affect other tests
> >>>>
> >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>> ---
> >>>>  lib/igt_kms.c | 39 ++++++++++++++++++++++++++++++++++++++-
> >>>>  lib/igt_kms.h |  1 +
> >>>>  2 files changed, 39 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> >>>> index 223dbe4ca565..9e14f071ea57 100644
> >>>> --- a/lib/igt_kms.c
> >>>> +++ b/lib/igt_kms.c
> >>>> @@ -2943,7 +2943,10 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
> >>>>                         output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
> >>>>         }
> >>>>  
> >>>> -       display->first_commit = false;
> >>>> +       if (display->first_commit) {
> >>>> +               igt_display_drop_events(display);
> >>>> +               display->first_commit = false;
> >>>> +       }
> >>> So I spent quite a bit of time debating whether there is merit in "do
> >>> something; set-first-mode" that this would then clobber. After some
> >>> thought, no that doesn't seem like a wise test construction. I would
> >>> however suggest that we igt_debug() if we drop any events here.
> >>> -Chris
> >> Exactly. I'm using igt_info since it's supposed to be uncommon, is it ok if I upgrade it to a igt_warn() since nothing in CI will trigger it?
> > Oh, I wouldn't have put it inside drop_events itself, since that is just
> > doing its job -- whether or not it is relevant depends on the caller. So
> > I would suggest reducing it to igt_debug, or just reporting the number
> > dropped and making the judgement in the caller.
> >
> > No, I don't this should be an igt_warn, as then CI blames the following
> > test for environmental errors left over from previous runs.
> > -Chris
> 
> It's not even possible, CI runs each test separately. When you open a
> new copy of the drm fd you don't get events for the previous fd.
> 
> Only when subtests fail you should get a dropped event, because of this
> it will never happen in CI and it's safe to change to a warn.

But others just run all the test as a whole, so the next subtest will
generate warns because of earlier subtests. There's also the concept of
a fork-server so that even different igt may end up in the same process,
continuing on from the same fd. (Depending on how fast we want to strive
for; certainly for fuzzing we want to retain as much state as safely can
be.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/3] lib/igt_kms: Drop all stale events on first commit.

[Patchwork]

== Series Details ==

Series: series starting with [1/3] lib/igt_kms: Drop all stale events on first commit.
URL   : https://patchwork.freedesktop.org/series/35032/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
2fc64acf8a4465d5eab3d6cfec9b3c1b5df30d73 igt/perf_pmu: Tweak wait_for_rc6, yet again

with latest DRM-Tip kernel build CI_DRM_3476
fb6aa7cd0d6b drm-tip: 2017y-12m-07d-17h-43m-29s UTC integration manifest

No testlist changes.

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-fail -> DMESG-WARN (fi-elk-e7500) fdo#103989 +6
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> FAIL       (fi-gdg-551) fdo#102618
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> INCOMPLETE (fi-hsw-4770) fdo#103375

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:438s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:385s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:531s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:285s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:514s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:508s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:492s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:482s
fi-elk-e7500     total:224  pass:169  dwarn:9   dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:177  dwarn:1   dfail:0   fail:2   skip:108 time:282s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:531s
fi-hsw-4770      total:244  pass:220  dwarn:0   dfail:0   fail:0   skip:23 
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:261s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:397s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:478s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:448s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:493s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:519s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:475s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:519s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:602s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:456s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:535s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:551s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:522s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:493s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:449s
fi-snb-2520m     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:544s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:425s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:602s
fi-cnl-y         total:251  pass:225  dwarn:0   dfail:0   fail:0   skip:25 
fi-glk-dsi       total:288  pass:257  dwarn:0   dfail:0   fail:1   skip:30  time:497s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_615/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for series starting with [1/3] lib/igt_kms: Drop all stale events on first commit.

[Patchwork]

== Series Details ==

Series: series starting with [1/3] lib/igt_kms: Drop all stale events on first commit.
URL   : https://patchwork.freedesktop.org/series/35032/
State : failure

== Summary ==

Test drv_suspend:
        Subgroup sysfs-reader-hibernate:
                fail       -> SKIP       (shard-snb) fdo#103375
Test kms_atomic_transition:
        Subgroup 1x-modeset-transitions-nonblocking:
                pass       -> FAIL       (shard-snb)
                pass       -> FAIL       (shard-hsw)
        Subgroup 1x-modeset-transitions-nonblocking-fencing:
                pass       -> FAIL       (shard-snb)
                pass       -> FAIL       (shard-hsw)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b:
                incomplete -> PASS       (shard-hsw)
Test kms_flip:
        Subgroup flip-vs-panning:
                incomplete -> PASS       (shard-hsw)
Test perf:
        Subgroup blocking:
                pass       -> FAIL       (shard-hsw) fdo#102252

fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-hsw        total:2679 pass:1533 dwarn:1   dfail:0   fail:13  skip:1132 time:9506s
shard-snb        total:2679 pass:1305 dwarn:1   dfail:0   fail:14  skip:1359 time:8096s
Blacklisted hosts:
shard-apl        total:2679 pass:1674 dwarn:1   dfail:0   fail:26  skip:977 time:13733s
shard-kbl        total:2603 pass:1749 dwarn:1   dfail:0   fail:24  skip:829 time:10010s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_615/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 1/3] lib/igt_kms: Drop all stale events on first commit.

[Chris Wilson]

Quoting Chris Wilson (2017-12-07 16:12:34)
> Quoting Maarten Lankhorst (2017-12-07 15:57:09)
> > Op 07-12-17 om 16:50 schreef Chris Wilson:
> > > Quoting Maarten Lankhorst (2017-12-07 15:42:54)
> > >> Op 07-12-17 om 16:03 schreef Chris Wilson:
> > >>> Quoting Maarten Lankhorst (2017-12-07 13:40:25)
> > >>>> I've been trying to make kms_cursor_legacy work when subtests fail.
> > >>>> Other subtests will start failing too because of expired events or
> > >>>> stale pipe crc. The latter can be resolved in the test, but the former
> > >>>> could affect other tests
> > >>>>
> > >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > >>>> ---
> > >>>>  lib/igt_kms.c | 39 ++++++++++++++++++++++++++++++++++++++-
> > >>>>  lib/igt_kms.h |  1 +
> > >>>>  2 files changed, 39 insertions(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > >>>> index 223dbe4ca565..9e14f071ea57 100644
> > >>>> --- a/lib/igt_kms.c
> > >>>> +++ b/lib/igt_kms.c
> > >>>> @@ -2943,7 +2943,10 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
> > >>>>                         output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
> > >>>>         }
> > >>>>  
> > >>>> -       display->first_commit = false;
> > >>>> +       if (display->first_commit) {
> > >>>> +               igt_display_drop_events(display);
> > >>>> +               display->first_commit = false;
> > >>>> +       }
> > >>> So I spent quite a bit of time debating whether there is merit in "do
> > >>> something; set-first-mode" that this would then clobber. After some
> > >>> thought, no that doesn't seem like a wise test construction. I would
> > >>> however suggest that we igt_debug() if we drop any events here.
> > >>> -Chris
> > >> Exactly. I'm using igt_info since it's supposed to be uncommon, is it ok if I upgrade it to a igt_warn() since nothing in CI will trigger it?
> > > Oh, I wouldn't have put it inside drop_events itself, since that is just
> > > doing its job -- whether or not it is relevant depends on the caller. So
> > > I would suggest reducing it to igt_debug, or just reporting the number
> > > dropped and making the judgement in the caller.
> > >
> > > No, I don't this should be an igt_warn, as then CI blames the following
> > > test for environmental errors left over from previous runs.
> > > -Chris
> > 
> > It's not even possible, CI runs each test separately. When you open a
> > new copy of the drm fd you don't get events for the previous fd.
> > 
> > Only when subtests fail you should get a dropped event, because of this
> > it will never happen in CI and it's safe to change to a warn.
> 
> But others just run all the test as a whole, so the next subtest will
> generate warns because of earlier subtests. There's also the concept of
> a fork-server so that even different igt may end up in the same process,
> continuing on from the same fd. (Depending on how fast we want to strive
> for; certainly for fuzzing we want to retain as much state as safely can
> be.)

So I don't really mind what happens with dropping the stale events. If
you made tests cleanup after themselves, then it would appropriate for a
warn. If other tests encounter events as they start up, that to me
suggests debug since it is noise for this particular test. I would still
just have a debug here, and a warn/assert higher up for dropped events
where appropriate.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 3/3] tests/kms_cursor_legacy: Rework the 2x-*-vs-cursor-* tests.

[Mika Kahola]

On Thu, 2017-12-07 at 14:40 +0100, Maarten Lankhorst wrote:
> Using the fancy new DRM_CAP_CRTC_IN_VBLANK_EVENT cap I can finally
> make this test the work I originally intended to.
> 
> For the !modeset case that means performing a pageflip on both
> crtc's,
> then requeueing as soon as the event is delivered and then check the
> vblank counter against the original value, it should be advanced by
> 1.
> 
> The modeset case is slightly more complicated, ideally it's handled
> the same, but if we can't perform a modeset and pageflip at the same
> time, fall back to queueing both in a single commit, in which case
> we can say nothing about the vblank counter.
> 
> There is a small race between flip_done and hw_done, so make
> flip_nonblocking retry for a second when encountering -EBUSY.
> 
Reviewed-by: Mika Kahola <mika.kahola@intel.com>

> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101634
> ---
>  tests/kms_cursor_legacy.c | 228 +++++++++++++++++++++++++++++++-----
> ----------
>  1 file changed, 156 insertions(+), 72 deletions(-)
> 
> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
> index 94d91e9c921a..5011e78e5c2f 100644
> --- a/tests/kms_cursor_legacy.c
> +++ b/tests/kms_cursor_legacy.c
> @@ -243,19 +243,27 @@ static enum pipe
> find_connected_pipe(igt_display_t *display, bool second)
>  	return pipe;
>  }
>  
> -static void flip_nonblocking(igt_display_t *display, enum pipe
> pipe_id, bool atomic, struct igt_fb *fb)
> +static void flip_nonblocking(igt_display_t *display, enum pipe
> pipe_id, bool atomic, struct igt_fb *fb, void *data)
>  {
>  	igt_pipe_t *pipe = &display->pipes[pipe_id];
>  	igt_plane_t *primary = igt_pipe_get_plane_type(pipe,
> DRM_PLANE_TYPE_PRIMARY);
> +	int ret;
>  
> +	igt_set_timeout(1, "Scheduling page flip\n");
>  	if (!atomic) {
>  		/* Schedule a nonblocking flip for the next vblank
> */
> -		do_or_die(drmModePageFlip(display->drm_fd, pipe-
> >crtc_id, fb->fb_id,
> -					DRM_MODE_PAGE_FLIP_EVENT,
> fb));
> +		do {
> +			ret = drmModePageFlip(display->drm_fd, pipe-
> >crtc_id, fb->fb_id,
> +					      DRM_MODE_PAGE_FLIP_EVE
> NT, data);
> +		} while (ret == -EBUSY);
>  	} else {
>  		igt_plane_set_fb(primary, fb);
> -		igt_display_commit_atomic(display,
> DRM_MODE_ATOMIC_NONBLOCK | DRM_MODE_PAGE_FLIP_EVENT, fb);
> +		do {
> +			ret = igt_display_try_commit_atomic(display,
> DRM_MODE_ATOMIC_NONBLOCK | DRM_MODE_PAGE_FLIP_EVENT, data);
> +		} while (ret == -EBUSY);
>  	}
> +	igt_assert(!ret);
> +	igt_reset_timeout();
>  }
>  
>  enum flip_test {
> @@ -424,7 +432,7 @@ static void flip(igt_display_t *display,
>  
>  			switch (mode) {
>  			default:
> -				flip_nonblocking(display, flip_pipe,
> mode >= flip_test_atomic, &fb_info);
> +				flip_nonblocking(display, flip_pipe,
> mode >= flip_test_atomic, &fb_info, NULL);
>  				break;
>  			case flip_test_atomic_transitions:
>  			case
> flip_test_atomic_transitions_varying_size:
> @@ -533,7 +541,7 @@ static void basic_flip_cursor(igt_display_t
> *display,
>  		case FLIP_BEFORE_CURSOR:
>  			switch (mode) {
>  			default:
> -				flip_nonblocking(display, pipe, mode
> >= flip_test_atomic, &fb_info);
> +				flip_nonblocking(display, pipe, mode
> >= flip_test_atomic, &fb_info, NULL);
>  				break;
>  			case flip_test_atomic_transitions:
>  			case
> flip_test_atomic_transitions_varying_size:
> @@ -555,7 +563,7 @@ static void basic_flip_cursor(igt_display_t
> *display,
>  
>  			switch (mode) {
>  			default:
> -				flip_nonblocking(display, pipe, mode
> >= flip_test_atomic, &fb_info);
> +				flip_nonblocking(display, pipe, mode
> >= flip_test_atomic, &fb_info, NULL);
>  				break;
>  			case flip_test_atomic_transitions:
>  			case
> flip_test_atomic_transitions_varying_size:
> @@ -712,7 +720,7 @@ static void flip_vs_cursor(igt_display_t
> *display, enum flip_test mode, int nloo
>  		vblank_start = get_vblank(display->drm_fd, pipe,
> DRM_VBLANK_NEXTONMISS);
>  		switch (mode) {
>  		default:
> -			flip_nonblocking(display, pipe, mode >=
> flip_test_atomic, &fb_info);
> +			flip_nonblocking(display, pipe, mode >=
> flip_test_atomic, &fb_info, NULL);
>  			break;
>  		case flip_test_atomic_transitions:
>  		case flip_test_atomic_transitions_varying_size:
> @@ -843,13 +851,26 @@ static void
> nonblocking_modeset_vs_cursor(igt_display_t *display, int loops)
>  
>  static void two_screens_flip_vs_cursor(igt_display_t *display, int
> nloops, bool modeset, bool atomic)
>  {
> -	struct drm_mode_cursor arg[2], arg2[2];
> -	struct drm_event_vblank vbl;
> +	struct drm_mode_cursor arg1[2], arg2[2];
>  	struct igt_fb fb_info, fb2_info, cursor_fb;
> -	unsigned vblank_start;
>  	enum pipe pipe = find_connected_pipe(display, false);
>  	enum pipe pipe2 = find_connected_pipe(display, true);
>  	igt_output_t *output, *output2;
> +	bool vblank_matches, enabled = false;
> +	volatile unsigned long *shared;
> +	unsigned flags = 0, vblank_start;
> +	struct drm_event_vblank vbl;
> +	int ret;
> +
> +	if (modeset) {
> +		uint64_t val;
> +
> +		igt_fail_on(!atomic);
> +		igt_require(drmGetCap(display->drm_fd,
> DRM_CAP_CRTC_IN_VBLANK_EVENT, &val) == 0);
> +	}
> +
> +	shared = mmap(NULL, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON,
> -1, 0);
> +	igt_assert(shared != MAP_FAILED);
>  
>  	igt_fail_on(modeset && !atomic);
>  
> @@ -861,77 +882,140 @@ static void
> two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool
>  
>  	igt_create_color_fb(display->drm_fd, 64, 64,
> DRM_FORMAT_ARGB8888, 0, 1., 1., 1., &cursor_fb);
>  	set_cursor_on_pipe(display, pipe, &cursor_fb);
> -	populate_cursor_args(display, pipe, arg, &cursor_fb);
> +	populate_cursor_args(display, pipe, arg1, &cursor_fb);
>  
> -	arg[1].x = arg[1].y = 192;
> +	arg1[1].x = arg1[1].y = 192;
>  
>  	set_cursor_on_pipe(display, pipe2, &cursor_fb);
>  	populate_cursor_args(display, pipe2, arg2, &cursor_fb);
>  
>  	arg2[1].x = arg2[1].y = 192;
>  
> +
>  	igt_display_commit2(display, display->is_atomic ?
> COMMIT_ATOMIC : COMMIT_LEGACY);
>  
> -	vblank_start = get_vblank(display->drm_fd, pipe,
> DRM_VBLANK_NEXTONMISS);
> -	igt_assert_eq(get_vblank(display->drm_fd, pipe, 0),
> vblank_start);
> -	do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR, &arg[0]);
> -	do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR, &arg2[0]);
> -	igt_assert_eq(get_vblank(display->drm_fd, pipe, 0),
> vblank_start);
> +	igt_fork(child, 2) {
> +		struct drm_mode_cursor *arg = child ? arg2 : arg1;
>  
> -	while (nloops--) {
> -		/* Start with a synchronous query to align with the
> vblank */
> +		while (!shared[0])
> +			do_ioctl(display->drm_fd,
> DRM_IOCTL_MODE_CURSOR,
> +				 &arg[!shared[1]]);
> +	}
> +
> +	if (modeset) {
> +		igt_plane_t *plane =
> igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +
> +		flags = DRM_MODE_ATOMIC_ALLOW_MODESET |
> +			DRM_MODE_ATOMIC_NONBLOCK |
> DRM_MODE_PAGE_FLIP_EVENT;
> +
> +		/* Disable pipe2 */
> +		igt_output_set_pipe(output2, PIPE_NONE);
> +		igt_display_commit_atomic(display, flags, NULL);
> +		enabled = false;
> +
> +		/*
> +		 * Try a page flip on crtc 1, if we succeed pump
> page flips and
> +		 * modesets interleaved, else do a single atomic
> commit with both.
> +		 */
>  		vblank_start = get_vblank(display->drm_fd, pipe,
> DRM_VBLANK_NEXTONMISS);
> -		do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR,
> &arg[nloops & 1]);
> +		igt_plane_set_fb(plane, &fb_info);
> +		ret = igt_display_try_commit_atomic(display, flags,
> (void*)(ptrdiff_t)vblank_start);
> +		igt_assert(!ret || ret == -EBUSY);
> +
> +		if (ret == -EBUSY) {
> +			/* Force completion on both pipes, and
> generate event. */
> +			igt_display_commit_atomic(display, flags,
> NULL);
> +
> +			while (nloops--) {
> +				shared[1] = nloops & 1;
> +
> +				igt_set_timeout(35, "Stuck
> modeset");
> +				igt_assert_eq(read(display->drm_fd,
> &vbl, sizeof(vbl)), sizeof(vbl));
> +				igt_assert_eq(read(display->drm_fd,
> &vbl, sizeof(vbl)), sizeof(vbl));
> +				igt_reset_timeout();
> +
> +				if (!nloops)
> +					break;
> +
> +				/* Commit page flip and modeset
> simultaneously. */
> +				igt_plane_set_fb(plane, &fb_info);
> +				igt_output_set_pipe(output2, enabled
> ? PIPE_NONE : pipe2);
> +				enabled = !enabled;
> +
> +				igt_set_timeout(5, "Scheduling
> modeset\n");
> +				do {
> +					ret =
> igt_display_try_commit_atomic(display, flags, NULL);
> +				} while (ret == -EBUSY);
> +				igt_assert(!ret);
> +				igt_reset_timeout();
> +			}
>  
> -		if (!modeset)
> -			flip_nonblocking(display, pipe, false,
> &fb_info);
> -		else {
> -			/*
> -			 * There are 2 design issues that prevent us
> from doing
> -			 * the test we would like here:
> -			 *
> -			 * - drm_event_vblank doesn't set crtc_id,
> so if we
> -			 *   use events we don't know which pipe
> fired the event,
> -			 *   no way to distinguish.
> -			 * - Doing a modeset may add unrelated
> pipes, and fail with
> -			 *   -EBUSY if a page flip is queued on one
> of those.
> -			 *
> -			 * Work around it by not setting an event,
> but doing a synchronous
> -			 * commit to wait for completion, and queue
> the page flip and modeset
> -			 * in the same commit.
> -			 */
> -
> -			igt_plane_set_fb(igt_output_get_plane_type(o
> utput, DRM_PLANE_TYPE_PRIMARY), &fb_info);
> -			igt_output_set_pipe(output2, (nloops & 1) ?
> PIPE_NONE : pipe2);
> -			igt_display_commit_atomic(display,
> DRM_MODE_ATOMIC_ALLOW_MODESET | DRM_MODE_ATOMIC_NONBLOCK, NULL);
> +			goto done;
>  		}
> +	} else {
> +		vblank_start = get_vblank(display->drm_fd, pipe,
> DRM_VBLANK_NEXTONMISS);
> +		flip_nonblocking(display, pipe, atomic, &fb_info,
> (void*)(ptrdiff_t)vblank_start);
>  
> -		igt_assert_eq(get_vblank(display->drm_fd, pipe, 0),
> vblank_start);
> -
> -		do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR,
> &arg[nloops & 1]);
> -		if (!modeset) {
> -			do_ioctl(display->drm_fd,
> DRM_IOCTL_MODE_CURSOR, &arg2[nloops & 1]);
> -			do_ioctl(display->drm_fd,
> DRM_IOCTL_MODE_CURSOR, &arg[nloops & 1]);
> -			do_ioctl(display->drm_fd,
> DRM_IOCTL_MODE_CURSOR, &arg2[nloops & 1]);
> +		vblank_start = get_vblank(display->drm_fd, pipe2,
> DRM_VBLANK_NEXTONMISS);
> +		flip_nonblocking(display, pipe2, atomic, &fb2_info,
> (void*)(ptrdiff_t)vblank_start);
> +	}
>  
> -			igt_assert_eq(get_vblank(display->drm_fd,
> pipe, 0), vblank_start);
> +	vblank_matches = false;
> +	while (nloops) {
> +		shared[1] = nloops & 1;
>  
> +		if (!modeset || nloops > 1)
>  			igt_set_timeout(1, "Stuck page flip");
> -			igt_ignore_warn(read(display->drm_fd, &vbl,
> sizeof(vbl)));
> -			igt_assert_eq(get_vblank(display->drm_fd,
> pipe, 0), vblank_start + 1);
> -			igt_reset_timeout();
> -		} else {
> -			do_ioctl(display->drm_fd,
> DRM_IOCTL_MODE_CURSOR, &arg2[nloops & 1]);
> +		else
> +			igt_set_timeout(35, "Stuck modeset");
> +		igt_assert_eq(read(display->drm_fd, &vbl,
> sizeof(vbl)), sizeof(vbl));
> +		igt_reset_timeout();
> +
> +		vblank_start = vbl.user_data;
> +		if (!modeset)
> +			igt_assert_eq(vbl.sequence, vblank_start +
> 1);
> +
> +		if (vblank_start && vbl.sequence == vblank_start +
> 1)
> +			vblank_matches = true;
> +
> +		/* Do not requeue on the last 2 events. */
> +		if (nloops <= 2) {
> +			nloops--;
> +			continue;
>  		}
>  
> -		if (modeset) {
> -			/* wait for pending modeset and page flip to
> complete, to prevent -EBUSY */
> -			igt_pipe_refresh(display, pipe, false);
> -			igt_pipe_refresh(display, pipe2, false);
> -			igt_display_commit2(display, COMMIT_ATOMIC);
> +		if (vbl.crtc_id == display->pipes[pipe].crtc_id) {
> +			vblank_start = get_vblank(display->drm_fd,
> pipe, DRM_VBLANK_NEXTONMISS);
> +			flip_nonblocking(display, pipe, atomic,
> &fb_info, (void*)(ptrdiff_t)vblank_start);
> +		} else {
> +			igt_assert(vbl.crtc_id == display-
> >pipes[pipe2].crtc_id);
> +
> +			nloops--;
> +
> +			if (!modeset) {
> +				vblank_start = get_vblank(display-
> >drm_fd, pipe2, DRM_VBLANK_NEXTONMISS);
> +				flip_nonblocking(display, pipe2,
> atomic, &fb2_info, (void*)(ptrdiff_t)vblank_start);
> +			} else {
> +				igt_output_set_pipe(output2, enabled
> ? PIPE_NONE : pipe2);
> +
> +				igt_set_timeout(1, "Scheduling
> modeset\n");
> +				do {
> +					ret =
> igt_display_try_commit_atomic(display, flags, NULL);
> +				} while (ret == -EBUSY);
> +				igt_assert(!ret);
> +				igt_reset_timeout();
> +
> +				enabled = !enabled;
> +			}
>  		}
>  	}
>  
> +	igt_assert_f(vblank_matches, "During modeset at least 1 page
> flip needs to match!\n");
> +
> +done:
> +	shared[0] = 1;
> +	igt_waitchildren();
> +
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	igt_remove_fb(display->drm_fd, &fb2_info);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
> @@ -982,7 +1066,7 @@ static void cursor_vs_flip(igt_display_t
> *display, enum flip_test mode, int nloo
>  
>  		switch (mode) {
>  		default:
> -			flip_nonblocking(display, pipe, mode >=
> flip_test_atomic, &fb_info);
> +			flip_nonblocking(display, pipe, mode >=
> flip_test_atomic, &fb_info, NULL);
>  			break;
>  		case flip_test_atomic_transitions:
>  		case flip_test_atomic_transitions_varying_size:
> @@ -993,7 +1077,7 @@ static void cursor_vs_flip(igt_display_t
> *display, enum flip_test mode, int nloo
>  		igt_assert_eq(read(display->drm_fd, &vbl,
> sizeof(vbl)), sizeof(vbl));
>  		vblank_start = vblank_last = vbl.sequence;
>  		for (int n = 0; n < vrefresh / 2; n++) {
> -			flip_nonblocking(display, pipe, mode >=
> flip_test_atomic, &fb_info);
> +			flip_nonblocking(display, pipe, mode >=
> flip_test_atomic, &fb_info, NULL);
>  
>  			igt_assert_eq(read(display->drm_fd, &vbl,
> sizeof(vbl)), sizeof(vbl));
>  			if (vbl.sequence != vblank_last + 1) {
> @@ -1083,14 +1167,14 @@ static void
> two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool
>  			shared[child] = count;
>  		}
>  
> -		flip_nonblocking(display, pipe[0], atomic,
> &fb_info[0]);
> -		flip_nonblocking(display, pipe[1], atomic,
> &fb_info[1]);
> +		flip_nonblocking(display, pipe[0], atomic,
> &fb_info[0], (void *)0UL);
> +		flip_nonblocking(display, pipe[1], atomic,
> &fb_info[1], (void *)1UL);
>  
>  		for (int n = 0; n < vrefresh[0] / 2 + vrefresh[1] /
> 2; n++) {
> -			int child;
> +			unsigned long child;
>  
>  			igt_assert_eq(read(display->drm_fd, &vbl,
> sizeof(vbl)), sizeof(vbl));
> -			child = vbl.user_data == (unsigned
> long)&fb_info[1];
> +			child = vbl.user_data;
>  
>  			if (!done[child]++)
>  				vblank_start[child] = vbl.sequence;
> @@ -1101,7 +1185,7 @@ static void
> two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool
>  			vblank_last[child] = vbl.sequence;
>  
>  			if (done[child] < vrefresh[child] / 2) {
> -				flip_nonblocking(display,
> pipe[child], atomic, &fb_info[child]);
> +				flip_nonblocking(display,
> pipe[child], atomic, &fb_info[child], (void *)child);
>  			} else {
>  				igt_assert_lte(vbl.sequence,
> vblank_start[child] + 5 * vrefresh[child] / 8);
>  
> @@ -1163,7 +1247,7 @@ static void flip_vs_cursor_crc(igt_display_t
> *display, bool atomic)
>  	for (int i = 1; i >= 0; i--) {
>  		vblank_start = get_vblank(display->drm_fd, pipe,
> DRM_VBLANK_NEXTONMISS);
>  
> -		flip_nonblocking(display, pipe, atomic, &fb_info);
> +		flip_nonblocking(display, pipe, atomic, &fb_info,
> NULL);
>  		do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR,
> &arg[i]);
>  
>  		igt_assert_eq(get_vblank(display->drm_fd, pipe, 0),
> vblank_start);
> @@ -1247,7 +1331,7 @@ static void
> flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
>  
>  		vblank_start = get_vblank(display->drm_fd, pipe,
> DRM_VBLANK_NEXTONMISS);
>  
> -		flip_nonblocking(display, pipe, atomic,
> &fb_info[1]);
> +		flip_nonblocking(display, pipe, atomic, &fb_info[1],
> NULL);
>  		do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR,
> &arg[i]);
>  
>  		igt_assert_eq(get_vblank(display->drm_fd, pipe, 0),
> vblank_start);
> @@ -1347,7 +1431,7 @@ igt_main
>  		two_screens_flip_vs_cursor(&display, 8, false,
> false);
>  
>  	igt_subtest("2x-flip-vs-cursor-atomic")
> -		two_screens_flip_vs_cursor(&display, 4, false,
> true);
> +		two_screens_flip_vs_cursor(&display, 8, false,
> true);
>  
>  	igt_subtest("2x-cursor-vs-flip-legacy")
>  		two_screens_cursor_vs_flip(&display, 8, false);
> @@ -1362,13 +1446,13 @@ igt_main
>  		two_screens_cursor_vs_flip(&display, 50, false);
>  
>  	igt_subtest("2x-nonblocking-modeset-vs-cursor-atomic")
> -		two_screens_flip_vs_cursor(&display, 8, true, true);
> +		two_screens_flip_vs_cursor(&display, 4, true, true);
>  
>  	igt_subtest("2x-cursor-vs-flip-atomic")
>  		two_screens_cursor_vs_flip(&display, 8, true);
>  
>  	igt_subtest("2x-long-nonblocking-modeset-vs-cursor-atomic")
> -		two_screens_flip_vs_cursor(&display, 150, true,
> true);
> +		two_screens_flip_vs_cursor(&display, 15, true,
> true);
>  
>  	igt_subtest("2x-long-cursor-vs-flip-atomic")
>  		two_screens_cursor_vs_flip(&display, 50, true);
-- 
Mika Kahola - Intel OTC

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

Re: [PATCH i-g-t 2/3] tests/kms_cursor_legacy: Perform lazy cleanup between tests

[Mika Kahola]

On Thu, 2017-12-07 at 14:40 +0100, Maarten Lankhorst wrote:
> Instead of assuming each subtest cleans up after itself, assume it
> fails and doesn't. Now that igt_kms can clean up stale events, we
> can just force each subtest to only clean up its framebuffers,
> which isn't harmful if it failed.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  tests/kms_cursor_legacy.c | 88 +++++++----------------------------
> ------------
>  1 file changed, 12 insertions(+), 76 deletions(-)
> 
> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
> index 5720dbef90d3..94d91e9c921a 100644
> --- a/tests/kms_cursor_legacy.c
> +++ b/tests/kms_cursor_legacy.c
> @@ -45,6 +45,8 @@
>  
>  IGT_TEST_DESCRIPTION("Stress legacy cursor ioctl");
>  
> +igt_pipe_crc_t *pipe_crc;
> +
>  static void stress(igt_display_t *display,
>  		   enum pipe pipe, int num_children, unsigned mode,
>  		   int timeout)
> @@ -203,22 +205,6 @@ static void populate_cursor_args(igt_display_t
> *display, enum pipe pipe,
>  	arg[1] = *arg;
>  }
>  
> -static void do_cleanup_display(igt_display_t *display)
> -{
> -	enum pipe pipe;
> -	igt_output_t *output;
> -	igt_plane_t *plane;
> -
> -	for_each_pipe(display, pipe)
> -		for_each_plane_on_pipe(display, pipe, plane)
> -			igt_plane_set_fb(plane, NULL);
> -
> -	for_each_connected_output(display, output)
> -		igt_output_set_pipe(output, PIPE_NONE);
> -
> -	igt_display_commit2(display, display->is_atomic ?
> COMMIT_ATOMIC : COMMIT_LEGACY);
> -}
> -
>  static enum pipe find_connected_pipe(igt_display_t *display, bool
> second)
>  {
>  	enum pipe pipe, first = PIPE_NONE;
> @@ -226,6 +212,14 @@ static enum pipe
> find_connected_pipe(igt_display_t *display, bool second)
>  	igt_output_t *first_output = NULL;
>  	bool found = false;
>  
> +	if (!second) {
> +		igt_pipe_crc_free(pipe_crc);
> +		pipe_crc = NULL;
> +
> +		/* Clear display, events will be eaten by commit..
> */
> +		igt_display_reset(display);
> +	}
> +
>  	for_each_pipe_with_valid_output(display, pipe, output) {
>  		if (first == pipe || output == first_output)
>  			continue;
> @@ -451,8 +445,6 @@ static void flip(igt_display_t *display,
>  
>  	munmap(results, 4096);
>  
> -	do_cleanup_display(display);
> -
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	if (flip_pipe != cursor_pipe)
>  		igt_remove_fb(display->drm_fd, &fb_info2);
> @@ -608,7 +600,6 @@ static void basic_flip_cursor(igt_display_t
> *display,
>  	if (miss1 || miss2)
>  		igt_info("Failed to evade %i vblanks and missed %i
> page flips\n", miss1, miss2);
>  
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
>  
> @@ -758,7 +749,6 @@ static void flip_vs_cursor(igt_display_t
> *display, enum flip_test mode, int nloo
>  		sched_setaffinity(0, sizeof(oldmask), &oldmask);
>  	}
>  
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
>  
> @@ -768,34 +758,12 @@ static void flip_vs_cursor(igt_display_t
> *display, enum flip_test mode, int nloo
>  		igt_remove_fb(display->drm_fd, &cursor_fb2);
>  }
>  
> -static bool skip_on_unsupported_nonblocking_modeset(igt_display_t
> *display)
> -{
> -	enum pipe pipe;
> -	int ret;
> -
> -	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY
> | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> -
> -	ret = igt_display_try_commit_atomic(display,
> DRM_MODE_ATOMIC_ALLOW_MODESET | DRM_MODE_ATOMIC_NONBLOCK, NULL);
> -
> -	if (ret == -EINVAL)
> -		return true;
> -
> -	igt_assert_eq(ret, 0);
> -
> -	/* Force the next state to update all crtc's, to synchronize
> with the nonblocking modeset. */
> -	for_each_pipe(display, pipe)
> -		igt_pipe_refresh(display, pipe, false);
> -
> -	return false;
> -}
Just wondering why we need to remove this check for nonblocking modeset
that is not supported?

> -
>  static void nonblocking_modeset_vs_cursor(igt_display_t *display,
> int loops)
>  {
>  	struct igt_fb fb_info, cursor_fb;
>  	igt_output_t *output;
>  	enum pipe pipe = find_connected_pipe(display, false);
>  	struct drm_mode_cursor arg[2];
> -	bool skip_test;
>  	igt_plane_t *cursor = NULL, *plane;
>  
>  	igt_require(display->is_atomic);
> @@ -815,13 +783,9 @@ static void
> nonblocking_modeset_vs_cursor(igt_display_t *display, int loops)
>  
>  	igt_skip_on(!cursor);
>  
> -	if ((skip_test =
> skip_on_unsupported_nonblocking_modeset(display)))
> -		goto cleanup;
> -
>  	/*
> -	 * Start disabled, because
> skip_on_unsupported_nonblocking_modeset
> -	 * will have enabled this pipe. No way around it, since the
> first
> -	 * atomic commit may be unreliable with amount of events
> sent.
> +	 * Start disabled. No way around it, since the first atomic
> +	 * commit may be unreliable with amount of events sent.
>  	 */
>  	igt_output_set_pipe(output, PIPE_NONE);
>  	igt_display_commit2(display, COMMIT_ATOMIC);
> @@ -873,13 +837,8 @@ static void
> nonblocking_modeset_vs_cursor(igt_display_t *display, int loops)
>  		igt_reset_timeout();
>  	}
>  
> -cleanup:
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
> -
> -	if (skip_test)
> -		igt_skip("Nonblocking modeset is not supported by
> this kernel\n");
>  }
>  
>  static void two_screens_flip_vs_cursor(igt_display_t *display, int
> nloops, bool modeset, bool atomic)
> @@ -891,7 +850,6 @@ static void
> two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool
>  	enum pipe pipe = find_connected_pipe(display, false);
>  	enum pipe pipe2 = find_connected_pipe(display, true);
>  	igt_output_t *output, *output2;
> -	bool skip_test = false;
>  
>  	igt_fail_on(modeset && !atomic);
>  
> @@ -912,9 +870,6 @@ static void
> two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool
>  
>  	arg2[1].x = arg2[1].y = 192;
>  
> -	if (modeset && (skip_test =
> skip_on_unsupported_nonblocking_modeset(display)))
> -		goto cleanup;
> -
>  	igt_display_commit2(display, display->is_atomic ?
> COMMIT_ATOMIC : COMMIT_LEGACY);
>  
>  	vblank_start = get_vblank(display->drm_fd, pipe,
> DRM_VBLANK_NEXTONMISS);
> @@ -977,14 +932,9 @@ static void
> two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool
>  		}
>  	}
>  
> -cleanup:
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	igt_remove_fb(display->drm_fd, &fb2_info);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
> -
> -	if (skip_test)
> -		igt_skip("Nonblocking modeset is not supported by
> this kernel\n");
>  }
>  
>  static void cursor_vs_flip(igt_display_t *display, enum flip_test
> mode, int nloops)
> @@ -1066,7 +1016,6 @@ static void cursor_vs_flip(igt_display_t
> *display, enum flip_test mode, int nloo
>  			     vrefresh*target, vrefresh*target / 2);
>  	}
>  
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
>  	munmap((void *)shared, 4096);
> @@ -1173,7 +1122,6 @@ static void
> two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool
>  				    vrefresh[child]*target[child],
> vrefresh[child]*target[child] / 2);
>  	}
>  
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info[0]);
>  	igt_remove_fb(display->drm_fd, &fb_info[1]);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
> @@ -1187,7 +1135,6 @@ static void flip_vs_cursor_crc(igt_display_t
> *display, bool atomic)
>  	struct igt_fb fb_info, cursor_fb;
>  	unsigned vblank_start;
>  	enum pipe pipe = find_connected_pipe(display, false);
> -	igt_pipe_crc_t *pipe_crc;
>  	igt_crc_t crcs[3];
>  
>  	if (atomic)
> @@ -1232,10 +1179,8 @@ static void flip_vs_cursor_crc(igt_display_t
> *display, bool atomic)
>  		igt_assert_crc_equal(&crcs[i], &crcs[2]);
>  	}
>  
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
> -	igt_pipe_crc_free(pipe_crc);
>  }
>  
>  static void flip_vs_cursor_busy_crc(igt_display_t *display, bool
> atomic)
> @@ -1245,7 +1190,6 @@ static void
> flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
>  	struct igt_fb fb_info[2], cursor_fb;
>  	unsigned vblank_start;
>  	enum pipe pipe = find_connected_pipe(display, false);
> -	igt_pipe_crc_t *pipe_crc;
>  	igt_pipe_t *pipe_connected = &display->pipes[pipe];
>  	igt_plane_t *plane_primary =
> igt_pipe_get_plane_type(pipe_connected, DRM_PLANE_TYPE_PRIMARY);
>  	igt_crc_t crcs[2];
> @@ -1333,17 +1277,9 @@ static void
> flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
>  		free(received_crcs);
>  	}
>  
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info[1]);
>  	igt_remove_fb(display->drm_fd, &fb_info[0]);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
> -
> -	/*
> -	 * igt_pipe_crc_stop() may force a modeset for workarounds,
> call
> -	 * it after do_cleanup_display since we disable the display
> anyway.
> -	 */
> -	igt_pipe_crc_stop(pipe_crc);
> -	igt_pipe_crc_free(pipe_crc);
>  }
>  
>  igt_main
-- 
Mika Kahola - Intel OTC

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

Re: [PATCH i-g-t 2/3] tests/kms_cursor_legacy: Perform lazy cleanup between tests

[Mika Kahola]

On Thu, 2017-12-07 at 14:40 +0100, Maarten Lankhorst wrote:
> Instead of assuming each subtest cleans up after itself, assume it
> fails and doesn't. Now that igt_kms can clean up stale events, we
> can just force each subtest to only clean up its framebuffers,
> which isn't harmful if it failed.
> 
Reviewed-by: Mika Kahola <mika.kahola@intel.com>

> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  tests/kms_cursor_legacy.c | 88 +++++++----------------------------
> ------------
>  1 file changed, 12 insertions(+), 76 deletions(-)
> 
> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
> index 5720dbef90d3..94d91e9c921a 100644
> --- a/tests/kms_cursor_legacy.c
> +++ b/tests/kms_cursor_legacy.c
> @@ -45,6 +45,8 @@
>  
>  IGT_TEST_DESCRIPTION("Stress legacy cursor ioctl");
>  
> +igt_pipe_crc_t *pipe_crc;
> +
>  static void stress(igt_display_t *display,
>  		   enum pipe pipe, int num_children, unsigned mode,
>  		   int timeout)
> @@ -203,22 +205,6 @@ static void populate_cursor_args(igt_display_t
> *display, enum pipe pipe,
>  	arg[1] = *arg;
>  }
>  
> -static void do_cleanup_display(igt_display_t *display)
> -{
> -	enum pipe pipe;
> -	igt_output_t *output;
> -	igt_plane_t *plane;
> -
> -	for_each_pipe(display, pipe)
> -		for_each_plane_on_pipe(display, pipe, plane)
> -			igt_plane_set_fb(plane, NULL);
> -
> -	for_each_connected_output(display, output)
> -		igt_output_set_pipe(output, PIPE_NONE);
> -
> -	igt_display_commit2(display, display->is_atomic ?
> COMMIT_ATOMIC : COMMIT_LEGACY);
> -}
> -
>  static enum pipe find_connected_pipe(igt_display_t *display, bool
> second)
>  {
>  	enum pipe pipe, first = PIPE_NONE;
> @@ -226,6 +212,14 @@ static enum pipe
> find_connected_pipe(igt_display_t *display, bool second)
>  	igt_output_t *first_output = NULL;
>  	bool found = false;
>  
> +	if (!second) {
> +		igt_pipe_crc_free(pipe_crc);
> +		pipe_crc = NULL;
> +
> +		/* Clear display, events will be eaten by commit..
> */
> +		igt_display_reset(display);
> +	}
> +
>  	for_each_pipe_with_valid_output(display, pipe, output) {
>  		if (first == pipe || output == first_output)
>  			continue;
> @@ -451,8 +445,6 @@ static void flip(igt_display_t *display,
>  
>  	munmap(results, 4096);
>  
> -	do_cleanup_display(display);
> -
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	if (flip_pipe != cursor_pipe)
>  		igt_remove_fb(display->drm_fd, &fb_info2);
> @@ -608,7 +600,6 @@ static void basic_flip_cursor(igt_display_t
> *display,
>  	if (miss1 || miss2)
>  		igt_info("Failed to evade %i vblanks and missed %i
> page flips\n", miss1, miss2);
>  
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
>  
> @@ -758,7 +749,6 @@ static void flip_vs_cursor(igt_display_t
> *display, enum flip_test mode, int nloo
>  		sched_setaffinity(0, sizeof(oldmask), &oldmask);
>  	}
>  
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
>  
> @@ -768,34 +758,12 @@ static void flip_vs_cursor(igt_display_t
> *display, enum flip_test mode, int nloo
>  		igt_remove_fb(display->drm_fd, &cursor_fb2);
>  }
>  
> -static bool skip_on_unsupported_nonblocking_modeset(igt_display_t
> *display)
> -{
> -	enum pipe pipe;
> -	int ret;
> -
> -	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY
> | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> -
> -	ret = igt_display_try_commit_atomic(display,
> DRM_MODE_ATOMIC_ALLOW_MODESET | DRM_MODE_ATOMIC_NONBLOCK, NULL);
> -
> -	if (ret == -EINVAL)
> -		return true;
> -
> -	igt_assert_eq(ret, 0);
> -
> -	/* Force the next state to update all crtc's, to synchronize
> with the nonblocking modeset. */
> -	for_each_pipe(display, pipe)
> -		igt_pipe_refresh(display, pipe, false);
> -
> -	return false;
> -}
> -
>  static void nonblocking_modeset_vs_cursor(igt_display_t *display,
> int loops)
>  {
>  	struct igt_fb fb_info, cursor_fb;
>  	igt_output_t *output;
>  	enum pipe pipe = find_connected_pipe(display, false);
>  	struct drm_mode_cursor arg[2];
> -	bool skip_test;
>  	igt_plane_t *cursor = NULL, *plane;
>  
>  	igt_require(display->is_atomic);
> @@ -815,13 +783,9 @@ static void
> nonblocking_modeset_vs_cursor(igt_display_t *display, int loops)
>  
>  	igt_skip_on(!cursor);
>  
> -	if ((skip_test =
> skip_on_unsupported_nonblocking_modeset(display)))
> -		goto cleanup;
> -
>  	/*
> -	 * Start disabled, because
> skip_on_unsupported_nonblocking_modeset
> -	 * will have enabled this pipe. No way around it, since the
> first
> -	 * atomic commit may be unreliable with amount of events
> sent.
> +	 * Start disabled. No way around it, since the first atomic
> +	 * commit may be unreliable with amount of events sent.
>  	 */
>  	igt_output_set_pipe(output, PIPE_NONE);
>  	igt_display_commit2(display, COMMIT_ATOMIC);
> @@ -873,13 +837,8 @@ static void
> nonblocking_modeset_vs_cursor(igt_display_t *display, int loops)
>  		igt_reset_timeout();
>  	}
>  
> -cleanup:
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
> -
> -	if (skip_test)
> -		igt_skip("Nonblocking modeset is not supported by
> this kernel\n");
>  }
>  
>  static void two_screens_flip_vs_cursor(igt_display_t *display, int
> nloops, bool modeset, bool atomic)
> @@ -891,7 +850,6 @@ static void
> two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool
>  	enum pipe pipe = find_connected_pipe(display, false);
>  	enum pipe pipe2 = find_connected_pipe(display, true);
>  	igt_output_t *output, *output2;
> -	bool skip_test = false;
>  
>  	igt_fail_on(modeset && !atomic);
>  
> @@ -912,9 +870,6 @@ static void
> two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool
>  
>  	arg2[1].x = arg2[1].y = 192;
>  
> -	if (modeset && (skip_test =
> skip_on_unsupported_nonblocking_modeset(display)))
> -		goto cleanup;
> -
>  	igt_display_commit2(display, display->is_atomic ?
> COMMIT_ATOMIC : COMMIT_LEGACY);
>  
>  	vblank_start = get_vblank(display->drm_fd, pipe,
> DRM_VBLANK_NEXTONMISS);
> @@ -977,14 +932,9 @@ static void
> two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool
>  		}
>  	}
>  
> -cleanup:
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	igt_remove_fb(display->drm_fd, &fb2_info);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
> -
> -	if (skip_test)
> -		igt_skip("Nonblocking modeset is not supported by
> this kernel\n");
>  }
>  
>  static void cursor_vs_flip(igt_display_t *display, enum flip_test
> mode, int nloops)
> @@ -1066,7 +1016,6 @@ static void cursor_vs_flip(igt_display_t
> *display, enum flip_test mode, int nloo
>  			     vrefresh*target, vrefresh*target / 2);
>  	}
>  
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
>  	munmap((void *)shared, 4096);
> @@ -1173,7 +1122,6 @@ static void
> two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool
>  				    vrefresh[child]*target[child],
> vrefresh[child]*target[child] / 2);
>  	}
>  
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info[0]);
>  	igt_remove_fb(display->drm_fd, &fb_info[1]);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
> @@ -1187,7 +1135,6 @@ static void flip_vs_cursor_crc(igt_display_t
> *display, bool atomic)
>  	struct igt_fb fb_info, cursor_fb;
>  	unsigned vblank_start;
>  	enum pipe pipe = find_connected_pipe(display, false);
> -	igt_pipe_crc_t *pipe_crc;
>  	igt_crc_t crcs[3];
>  
>  	if (atomic)
> @@ -1232,10 +1179,8 @@ static void flip_vs_cursor_crc(igt_display_t
> *display, bool atomic)
>  		igt_assert_crc_equal(&crcs[i], &crcs[2]);
>  	}
>  
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
> -	igt_pipe_crc_free(pipe_crc);
>  }
>  
>  static void flip_vs_cursor_busy_crc(igt_display_t *display, bool
> atomic)
> @@ -1245,7 +1190,6 @@ static void
> flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
>  	struct igt_fb fb_info[2], cursor_fb;
>  	unsigned vblank_start;
>  	enum pipe pipe = find_connected_pipe(display, false);
> -	igt_pipe_crc_t *pipe_crc;
>  	igt_pipe_t *pipe_connected = &display->pipes[pipe];
>  	igt_plane_t *plane_primary =
> igt_pipe_get_plane_type(pipe_connected, DRM_PLANE_TYPE_PRIMARY);
>  	igt_crc_t crcs[2];
> @@ -1333,17 +1277,9 @@ static void
> flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
>  		free(received_crcs);
>  	}
>  
> -	do_cleanup_display(display);
>  	igt_remove_fb(display->drm_fd, &fb_info[1]);
>  	igt_remove_fb(display->drm_fd, &fb_info[0]);
>  	igt_remove_fb(display->drm_fd, &cursor_fb);
> -
> -	/*
> -	 * igt_pipe_crc_stop() may force a modeset for workarounds,
> call
> -	 * it after do_cleanup_display since we disable the display
> anyway.
> -	 */
> -	igt_pipe_crc_stop(pipe_crc);
> -	igt_pipe_crc_free(pipe_crc);
>  }
>  
>  igt_main
-- 
Mika Kahola - Intel OTC

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

Re: [PATCH i-g-t 3/3] tests/kms_cursor_legacy: Rework the 2x-*-vs-cursor-* tests.

[Maarten Lankhorst]

Op 22-12-17 om 09:55 schreef Mika Kahola:
> On Thu, 2017-12-07 at 14:40 +0100, Maarten Lankhorst wrote:
>> Using the fancy new DRM_CAP_CRTC_IN_VBLANK_EVENT cap I can finally
>> make this test the work I originally intended to.
>>
>> For the !modeset case that means performing a pageflip on both
>> crtc's,
>> then requeueing as soon as the event is delivered and then check the
>> vblank counter against the original value, it should be advanced by
>> 1.
>>
>> The modeset case is slightly more complicated, ideally it's handled
>> the same, but if we can't perform a modeset and pageflip at the same
>> time, fall back to queueing both in a single commit, in which case
>> we can say nothing about the vblank counter.
>>
>> There is a small race between flip_done and hw_done, so make
>> flip_nonblocking retry for a second when encountering -EBUSY.
>>
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>
Thanks, pushed.

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