intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH i-g-t] tests: atomic: add test to verify page flip event emissions
@ 2016-04-21 16:01 Lionel Landwerlin
  2016-04-22 12:59 ` Daniel Vetter
  0 siblings, 1 reply; 6+ messages in thread
From: Lionel Landwerlin @ 2016-04-21 16:01 UTC (permalink / raw)
  To: intel-gfx

It seems we don't have a test verifying events with atomic commits
yet. Here is a first step.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 tests/kms_atomic.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 127 insertions(+), 10 deletions(-)

diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
index f27ee46..bc75108 100644
--- a/tests/kms_atomic.c
+++ b/tests/kms_atomic.c
@@ -252,10 +252,10 @@ static uint32_t blob_duplicate(int fd, uint32_t id_orig)
 	plane_check_current_state(plane_old, relax); \
 }
 
-#define plane_commit_atomic(plane, req, relax) { \
+#define plane_commit_atomic(plane, req, flags, relax) {	\
 	drmModeAtomicSetCursor(req, 0); \
 	plane_populate_req(plane, req); \
-	do_atomic_commit((plane)->state->desc->fd, req, 0); \
+	do_atomic_commit((plane)->state->desc->fd, req, flags); \
 	plane_check_current_state(plane, relax); \
 }
 
@@ -886,10 +886,10 @@ static void plane_overlay(struct kms_atomic_crtc_state *crtc,
 
 	/* Enable the overlay plane using the atomic API, and double-check
 	 * state is what we think it should be. */
-	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Disable the plane and check the state matches the old. */
-	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Re-enable the plane through the legacy plane API, and verify through
 	 * atomic. */
@@ -983,10 +983,10 @@ static void plane_cursor(struct kms_atomic_crtc_state *crtc,
 
 	/* Flip the cursor plane using the atomic API, and double-check
 	 * state is what we think it should be. */
-	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Restore the cursor plane and check the state matches the old. */
-	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Re-enable the plane through the legacy cursor API, and verify
 	 * through atomic. */
@@ -1010,7 +1010,7 @@ static void plane_cursor(struct kms_atomic_crtc_state *crtc,
 	plane_check_current_state(plane_old, ATOMIC_RELAX_NONE);
 
 	/* Finally, restore to the original state. */
-	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
 
 	drmModeAtomicFree(req);
 }
@@ -1043,7 +1043,7 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
 	                        ATOMIC_RELAX_NONE, EINVAL);
 
 	plane.fb_id = plane_old->fb_id;
-	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Pass a series of invalid object IDs for the CRTC ID. */
 	plane.crtc_id = plane.obj;
@@ -1063,7 +1063,7 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
 	                        ATOMIC_RELAX_NONE, EINVAL);
 
 	plane.crtc_id = plane_old->crtc_id;
-	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Create a framebuffer too small for the plane configuration. */
 	igt_require(format != 0);
@@ -1085,7 +1085,7 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
 	                        ATOMIC_RELAX_NONE, ENOSPC);
 
 	/* Restore the primary plane and check the state matches the old. */
-	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
 
 	drmModeAtomicFree(req);
 }
@@ -1280,6 +1280,112 @@ static void atomic_invalid_params(struct kms_atomic_crtc_state *crtc,
 	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
 }
 
+static void get_events(struct kms_atomic_crtc_state *crtc,
+		       int *page_flips,
+		       int *vblank_count,
+		       int msec)
+{
+	struct kms_atomic_desc *desc = crtc->state->desc;
+	struct timeval timeout = { .tv_sec = 0, .tv_usec = msec * 1000 };
+	fd_set fds;
+	char buffer[1024];
+	int idx, len, ret;
+
+	FD_ZERO(&fds);
+	FD_SET(desc->fd, &fds);
+
+	ret = select(1, &fds, NULL, NULL, &timeout);
+	if (ret < 1)
+		return;
+
+	len = read(desc->fd, buffer, sizeof(buffer));
+	if (len == 0)
+		return;
+
+	idx = 0;
+	while (idx < len) {
+		struct drm_event event;
+
+		igt_assert_lte(sizeof(event), len - idx);
+
+		memcpy(&event, &buffer[idx], sizeof(event));
+		switch (event.type) {
+		case DRM_EVENT_FLIP_COMPLETE:
+			(*page_flips)++;
+			break;
+		case DRM_EVENT_VBLANK:
+			(*vblank_count)++;
+			break;
+		default:
+			break;
+		}
+
+		idx += event.length;
+	}
+}
+
+static void plane_pageflip_events(struct kms_atomic_crtc_state *crtc,
+				  struct kms_atomic_plane_state *plane_old)
+{
+	struct drm_mode_modeinfo *mode = crtc->mode.data;
+	struct kms_atomic_plane_state plane = *plane_old;
+	uint32_t format = plane_get_igt_format(&plane);
+	drmModeAtomicReq *req = drmModeAtomicAlloc();
+	struct igt_fb fb[2];
+	int i, pageflip_count = 0, vblank_count = 0;
+	int flags = DRM_MODE_PAGE_FLIP_EVENT;
+
+	igt_require(format != 0);
+
+	for (i = 0; i < ARRAY_SIZE(fb); i++)
+		igt_create_pattern_fb(plane.state->desc->fd,
+				      plane.crtc_w, plane.crtc_h,
+				      format, I915_TILING_NONE, &fb[i]);
+
+	plane.src_x = 0;
+	plane.src_y = 0;
+	plane.src_w = mode->hdisplay << 16;
+	plane.src_h = mode->vdisplay << 16;
+	plane.crtc_x = 0;
+	plane.crtc_y = 0;
+	plane.crtc_w = mode->hdisplay;
+	plane.crtc_h = mode->vdisplay;
+	plane.crtc_id = crtc->obj;
+	plane.fb_id = fb[0].fb_id;
+
+	/* Flip the primary plane using the atomic API, and double-check
+	 * state is what we think it should be. */
+	crtc_commit_atomic(crtc, &plane, req, ATOMIC_RELAX_NONE);
+
+	get_events(crtc, &pageflip_count, &vblank_count, 0);
+	igt_assert_eq(1, pageflip_count);
+
+	drmModeAtomicFree(req);
+	req = drmModeAtomicAlloc();
+
+	/* Change the framebuffer on the plane, we should get one
+	 * pageflip event. */
+	plane.fb_id = fb[1].fb_id;
+	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
+	get_events(crtc, &pageflip_count, &vblank_count, 20);
+	igt_assert_eq(2, pageflip_count);
+
+	/* No change, page flip event count should remain the same. */
+	plane.fb_id = fb[1].fb_id;
+	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
+	get_events(crtc, &pageflip_count, &vblank_count, 20);
+	igt_assert_eq(2, pageflip_count);
+
+	/* Change back the plane's framebuffer to its original one, we
+	 * should get a page flip event. */
+	plane.fb_id = fb[0].fb_id;
+	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
+	get_events(crtc, &pageflip_count, &vblank_count, 20);
+	igt_assert_eq(3, pageflip_count);
+
+	drmModeAtomicFree(req);
+}
+
 igt_main
 {
 	struct kms_atomic_desc desc;
@@ -1373,6 +1479,17 @@ igt_main
 		atomic_state_free(scratch);
 	}
 
+	igt_subtest("atomic_pageflip_events") {
+		struct kms_atomic_state *scratch = atomic_state_dup(current);
+		struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
+		struct kms_atomic_plane_state *plane =
+			find_plane(scratch, PLANE_TYPE_PRIMARY, crtc);
+
+		igt_require(plane);
+		plane_pageflip_events(crtc, plane);
+		atomic_state_free(scratch);
+	}
+
 	atomic_state_free(current);
 
 	igt_fixture
-- 
2.8.0.rc3.226.g39d4020

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH i-g-t] tests: atomic: add test to verify page flip event emissions
@ 2016-04-21 17:43 Lionel Landwerlin
  2016-04-25  8:39 ` Maarten Lankhorst
  0 siblings, 1 reply; 6+ messages in thread
From: Lionel Landwerlin @ 2016-04-21 17:43 UTC (permalink / raw)
  To: intel-gfx

It seems we don't have a test verifying events with atomic commits
yet. Here is a first step.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 tests/kms_atomic.c | 149 +++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 133 insertions(+), 16 deletions(-)

diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
index f27ee46..707fe12 100644
--- a/tests/kms_atomic.c
+++ b/tests/kms_atomic.c
@@ -234,7 +234,7 @@ static uint32_t blob_duplicate(int fd, uint32_t id_orig)
 	igt_assert_eq(errno, err); \
 }
 
-#define crtc_commit_atomic(crtc, plane, req, relax) { \
+#define crtc_commit_atomic(crtc, plane, req, flags, relax) {	\
 	drmModeAtomicSetCursor(req, 0); \
 	crtc_populate_req(crtc, req); \
 	plane_populate_req(plane, req); \
@@ -252,10 +252,10 @@ static uint32_t blob_duplicate(int fd, uint32_t id_orig)
 	plane_check_current_state(plane_old, relax); \
 }
 
-#define plane_commit_atomic(plane, req, relax) { \
+#define plane_commit_atomic(plane, req, flags, relax) {	\
 	drmModeAtomicSetCursor(req, 0); \
 	plane_populate_req(plane, req); \
-	do_atomic_commit((plane)->state->desc->fd, req, 0); \
+	do_atomic_commit((plane)->state->desc->fd, req, flags); \
 	plane_check_current_state(plane, relax); \
 }
 
@@ -886,10 +886,10 @@ static void plane_overlay(struct kms_atomic_crtc_state *crtc,
 
 	/* Enable the overlay plane using the atomic API, and double-check
 	 * state is what we think it should be. */
-	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Disable the plane and check the state matches the old. */
-	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Re-enable the plane through the legacy plane API, and verify through
 	 * atomic. */
@@ -928,10 +928,10 @@ static void plane_primary(struct kms_atomic_crtc_state *crtc,
 
 	/* Flip the primary plane using the atomic API, and double-check
 	 * state is what we think it should be. */
-	crtc_commit_atomic(crtc, &plane, req, ATOMIC_RELAX_NONE);
+	crtc_commit_atomic(crtc, &plane, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Restore the primary plane and check the state matches the old. */
-	crtc_commit_atomic(crtc, plane_old, req, ATOMIC_RELAX_NONE);
+	crtc_commit_atomic(crtc, plane_old, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Re-enable the plane through the legacy CRTC/primary-plane API, and
 	 * verify through atomic. */
@@ -942,7 +942,7 @@ static void plane_primary(struct kms_atomic_crtc_state *crtc,
 	crtc_commit_legacy(crtc, plane_old, CRTC_RELAX_MODE);
 
 	/* Finally, restore to the original state. */
-	crtc_commit_atomic(crtc, plane_old, req, ATOMIC_RELAX_NONE);
+	crtc_commit_atomic(crtc, plane_old, req, 0, ATOMIC_RELAX_NONE);
 
 	drmModeAtomicFree(req);
 }
@@ -983,10 +983,10 @@ static void plane_cursor(struct kms_atomic_crtc_state *crtc,
 
 	/* Flip the cursor plane using the atomic API, and double-check
 	 * state is what we think it should be. */
-	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Restore the cursor plane and check the state matches the old. */
-	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Re-enable the plane through the legacy cursor API, and verify
 	 * through atomic. */
@@ -1010,7 +1010,7 @@ static void plane_cursor(struct kms_atomic_crtc_state *crtc,
 	plane_check_current_state(plane_old, ATOMIC_RELAX_NONE);
 
 	/* Finally, restore to the original state. */
-	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
 
 	drmModeAtomicFree(req);
 }
@@ -1043,7 +1043,7 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
 	                        ATOMIC_RELAX_NONE, EINVAL);
 
 	plane.fb_id = plane_old->fb_id;
-	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Pass a series of invalid object IDs for the CRTC ID. */
 	plane.crtc_id = plane.obj;
@@ -1063,7 +1063,7 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
 	                        ATOMIC_RELAX_NONE, EINVAL);
 
 	plane.crtc_id = plane_old->crtc_id;
-	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Create a framebuffer too small for the plane configuration. */
 	igt_require(format != 0);
@@ -1085,7 +1085,7 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
 	                        ATOMIC_RELAX_NONE, ENOSPC);
 
 	/* Restore the primary plane and check the state matches the old. */
-	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
+	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
 
 	drmModeAtomicFree(req);
 }
@@ -1117,7 +1117,7 @@ static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old,
 	                       ATOMIC_RELAX_NONE, EINVAL);
 
 	crtc.mode.id = crtc_old->mode.id;
-	crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE);
+	crtc_commit_atomic(&crtc, plane, req, 0, ATOMIC_RELAX_NONE);
 
 	/* Create a blob which is the wrong size to be a valid mode. */
 	do_or_die(drmModeCreatePropertyBlob(crtc.state->desc->fd,
@@ -1136,7 +1136,7 @@ static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old,
 	                       ATOMIC_RELAX_NONE, EINVAL);
 
 	/* Restore the CRTC and check the state matches the old. */
-	crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE);
+	crtc_commit_atomic(crtc_old, plane, req, 0, ATOMIC_RELAX_NONE);
 
 	drmModeAtomicFree(req);
 }
@@ -1280,6 +1280,112 @@ static void atomic_invalid_params(struct kms_atomic_crtc_state *crtc,
 	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
 }
 
+static void get_events(struct kms_atomic_crtc_state *crtc,
+		       int *page_flips,
+		       int *vblank_count,
+		       int msec)
+{
+	struct kms_atomic_desc *desc = crtc->state->desc;
+	struct timeval timeout = { .tv_sec = 0, .tv_usec = msec * 1000 };
+	fd_set fds;
+	char buffer[1024];
+	int idx, len, ret;
+
+	FD_ZERO(&fds);
+	FD_SET(desc->fd, &fds);
+
+	ret = select(1, &fds, NULL, NULL, &timeout);
+	if (ret < 1)
+		return;
+
+	len = read(desc->fd, buffer, sizeof(buffer));
+	if (len == 0)
+		return;
+
+	idx = 0;
+	while (idx < len) {
+		struct drm_event event;
+
+		igt_assert_lte(sizeof(event), len - idx);
+
+		memcpy(&event, &buffer[idx], sizeof(event));
+		switch (event.type) {
+		case DRM_EVENT_FLIP_COMPLETE:
+			(*page_flips)++;
+			break;
+		case DRM_EVENT_VBLANK:
+			(*vblank_count)++;
+			break;
+		default:
+			break;
+		}
+
+		idx += event.length;
+	}
+}
+
+static void plane_pageflip_events(struct kms_atomic_crtc_state *crtc,
+				  struct kms_atomic_plane_state *plane_old)
+{
+	struct drm_mode_modeinfo *mode = crtc->mode.data;
+	struct kms_atomic_plane_state plane = *plane_old;
+	uint32_t format = plane_get_igt_format(&plane);
+	drmModeAtomicReq *req = drmModeAtomicAlloc();
+	struct igt_fb fb[2];
+	int i, pageflip_count = 0, vblank_count = 0;
+	int flags = DRM_MODE_PAGE_FLIP_EVENT;
+
+	igt_require(format != 0);
+
+	for (i = 0; i < ARRAY_SIZE(fb); i++)
+		igt_create_pattern_fb(plane.state->desc->fd,
+				      plane.crtc_w, plane.crtc_h,
+				      format, I915_TILING_NONE, &fb[i]);
+
+	plane.src_x = 0;
+	plane.src_y = 0;
+	plane.src_w = mode->hdisplay << 16;
+	plane.src_h = mode->vdisplay << 16;
+	plane.crtc_x = 0;
+	plane.crtc_y = 0;
+	plane.crtc_w = mode->hdisplay;
+	plane.crtc_h = mode->vdisplay;
+	plane.crtc_id = crtc->obj;
+	plane.fb_id = fb[0].fb_id;
+
+	/* Flip the primary plane using the atomic API, and double-check
+	 * state is what we think it should be. */
+	crtc_commit_atomic(crtc, &plane, req, flags, ATOMIC_RELAX_NONE);
+
+	get_events(crtc, &pageflip_count, &vblank_count, 20);
+	igt_assert_eq(1, pageflip_count);
+
+	drmModeAtomicFree(req);
+	req = drmModeAtomicAlloc();
+
+	/* Change the framebuffer on the plane, we should get one
+	 * pageflip event. */
+	plane.fb_id = fb[1].fb_id;
+	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
+	get_events(crtc, &pageflip_count, &vblank_count, 20);
+	igt_assert_eq(2, pageflip_count);
+
+	/* No change, page flip event count should remain the same. */
+	plane.fb_id = fb[1].fb_id;
+	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
+	get_events(crtc, &pageflip_count, &vblank_count, 20);
+	igt_assert_eq(2, pageflip_count);
+
+	/* Change back the plane's framebuffer to its original one, we
+	 * should get a page flip event. */
+	plane.fb_id = fb[0].fb_id;
+	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
+	get_events(crtc, &pageflip_count, &vblank_count, 20);
+	igt_assert_eq(3, pageflip_count);
+
+	drmModeAtomicFree(req);
+}
+
 igt_main
 {
 	struct kms_atomic_desc desc;
@@ -1373,6 +1479,17 @@ igt_main
 		atomic_state_free(scratch);
 	}
 
+	igt_subtest("atomic_pageflip_events") {
+		struct kms_atomic_state *scratch = atomic_state_dup(current);
+		struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
+		struct kms_atomic_plane_state *plane =
+			find_plane(scratch, PLANE_TYPE_PRIMARY, crtc);
+
+		igt_require(plane);
+		plane_pageflip_events(crtc, plane);
+		atomic_state_free(scratch);
+	}
+
 	atomic_state_free(current);
 
 	igt_fixture
-- 
2.8.0.rc3.226.g39d4020

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH i-g-t] tests: atomic: add test to verify page flip event emissions
  2016-04-21 16:01 Lionel Landwerlin
@ 2016-04-22 12:59 ` Daniel Vetter
  2016-04-22 13:27   ` Lionel Landwerlin
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2016-04-22 12:59 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

On Thu, Apr 21, 2016 at 05:01:31PM +0100, Lionel Landwerlin wrote:
> It seems we don't have a test verifying events with atomic commits
> yet. Here is a first step.
> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

Feel like also making a testcase variant for the other cases of crtc's
ACTIVE property?
- off -> off: kernel should reject event generation
- off -> on: kernel should generate event, and the frame counter should be
  just a few frames less (at most) than what you get from the vblank ioctl
  right after the modeset completes
- on -> off: same as off -> on but in reverse

Would be awesome ...
-Daniel

> ---
>  tests/kms_atomic.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 127 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
> index f27ee46..bc75108 100644
> --- a/tests/kms_atomic.c
> +++ b/tests/kms_atomic.c
> @@ -252,10 +252,10 @@ static uint32_t blob_duplicate(int fd, uint32_t id_orig)
>  	plane_check_current_state(plane_old, relax); \
>  }
>  
> -#define plane_commit_atomic(plane, req, relax) { \
> +#define plane_commit_atomic(plane, req, flags, relax) {	\
>  	drmModeAtomicSetCursor(req, 0); \
>  	plane_populate_req(plane, req); \
> -	do_atomic_commit((plane)->state->desc->fd, req, 0); \
> +	do_atomic_commit((plane)->state->desc->fd, req, flags); \
>  	plane_check_current_state(plane, relax); \
>  }
>  
> @@ -886,10 +886,10 @@ static void plane_overlay(struct kms_atomic_crtc_state *crtc,
>  
>  	/* Enable the overlay plane using the atomic API, and double-check
>  	 * state is what we think it should be. */
> -	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
> +	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
>  
>  	/* Disable the plane and check the state matches the old. */
> -	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
> +	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
>  
>  	/* Re-enable the plane through the legacy plane API, and verify through
>  	 * atomic. */
> @@ -983,10 +983,10 @@ static void plane_cursor(struct kms_atomic_crtc_state *crtc,
>  
>  	/* Flip the cursor plane using the atomic API, and double-check
>  	 * state is what we think it should be. */
> -	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
> +	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
>  
>  	/* Restore the cursor plane and check the state matches the old. */
> -	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
> +	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
>  
>  	/* Re-enable the plane through the legacy cursor API, and verify
>  	 * through atomic. */
> @@ -1010,7 +1010,7 @@ static void plane_cursor(struct kms_atomic_crtc_state *crtc,
>  	plane_check_current_state(plane_old, ATOMIC_RELAX_NONE);
>  
>  	/* Finally, restore to the original state. */
> -	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
> +	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
>  
>  	drmModeAtomicFree(req);
>  }
> @@ -1043,7 +1043,7 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
>  	                        ATOMIC_RELAX_NONE, EINVAL);
>  
>  	plane.fb_id = plane_old->fb_id;
> -	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
> +	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
>  
>  	/* Pass a series of invalid object IDs for the CRTC ID. */
>  	plane.crtc_id = plane.obj;
> @@ -1063,7 +1063,7 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
>  	                        ATOMIC_RELAX_NONE, EINVAL);
>  
>  	plane.crtc_id = plane_old->crtc_id;
> -	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
> +	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
>  
>  	/* Create a framebuffer too small for the plane configuration. */
>  	igt_require(format != 0);
> @@ -1085,7 +1085,7 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
>  	                        ATOMIC_RELAX_NONE, ENOSPC);
>  
>  	/* Restore the primary plane and check the state matches the old. */
> -	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
> +	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
>  
>  	drmModeAtomicFree(req);
>  }
> @@ -1280,6 +1280,112 @@ static void atomic_invalid_params(struct kms_atomic_crtc_state *crtc,
>  	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
>  }
>  
> +static void get_events(struct kms_atomic_crtc_state *crtc,
> +		       int *page_flips,
> +		       int *vblank_count,
> +		       int msec)
> +{
> +	struct kms_atomic_desc *desc = crtc->state->desc;
> +	struct timeval timeout = { .tv_sec = 0, .tv_usec = msec * 1000 };
> +	fd_set fds;
> +	char buffer[1024];
> +	int idx, len, ret;
> +
> +	FD_ZERO(&fds);
> +	FD_SET(desc->fd, &fds);
> +
> +	ret = select(1, &fds, NULL, NULL, &timeout);
> +	if (ret < 1)
> +		return;
> +
> +	len = read(desc->fd, buffer, sizeof(buffer));
> +	if (len == 0)
> +		return;
> +
> +	idx = 0;
> +	while (idx < len) {
> +		struct drm_event event;
> +
> +		igt_assert_lte(sizeof(event), len - idx);
> +
> +		memcpy(&event, &buffer[idx], sizeof(event));
> +		switch (event.type) {
> +		case DRM_EVENT_FLIP_COMPLETE:
> +			(*page_flips)++;
> +			break;
> +		case DRM_EVENT_VBLANK:
> +			(*vblank_count)++;
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		idx += event.length;
> +	}
> +}
> +
> +static void plane_pageflip_events(struct kms_atomic_crtc_state *crtc,
> +				  struct kms_atomic_plane_state *plane_old)
> +{
> +	struct drm_mode_modeinfo *mode = crtc->mode.data;
> +	struct kms_atomic_plane_state plane = *plane_old;
> +	uint32_t format = plane_get_igt_format(&plane);
> +	drmModeAtomicReq *req = drmModeAtomicAlloc();
> +	struct igt_fb fb[2];
> +	int i, pageflip_count = 0, vblank_count = 0;
> +	int flags = DRM_MODE_PAGE_FLIP_EVENT;
> +
> +	igt_require(format != 0);
> +
> +	for (i = 0; i < ARRAY_SIZE(fb); i++)
> +		igt_create_pattern_fb(plane.state->desc->fd,
> +				      plane.crtc_w, plane.crtc_h,
> +				      format, I915_TILING_NONE, &fb[i]);
> +
> +	plane.src_x = 0;
> +	plane.src_y = 0;
> +	plane.src_w = mode->hdisplay << 16;
> +	plane.src_h = mode->vdisplay << 16;
> +	plane.crtc_x = 0;
> +	plane.crtc_y = 0;
> +	plane.crtc_w = mode->hdisplay;
> +	plane.crtc_h = mode->vdisplay;
> +	plane.crtc_id = crtc->obj;
> +	plane.fb_id = fb[0].fb_id;
> +
> +	/* Flip the primary plane using the atomic API, and double-check
> +	 * state is what we think it should be. */
> +	crtc_commit_atomic(crtc, &plane, req, ATOMIC_RELAX_NONE);
> +
> +	get_events(crtc, &pageflip_count, &vblank_count, 0);
> +	igt_assert_eq(1, pageflip_count);
> +
> +	drmModeAtomicFree(req);
> +	req = drmModeAtomicAlloc();
> +
> +	/* Change the framebuffer on the plane, we should get one
> +	 * pageflip event. */
> +	plane.fb_id = fb[1].fb_id;
> +	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
> +	get_events(crtc, &pageflip_count, &vblank_count, 20);
> +	igt_assert_eq(2, pageflip_count);
> +
> +	/* No change, page flip event count should remain the same. */
> +	plane.fb_id = fb[1].fb_id;
> +	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
> +	get_events(crtc, &pageflip_count, &vblank_count, 20);
> +	igt_assert_eq(2, pageflip_count);
> +
> +	/* Change back the plane's framebuffer to its original one, we
> +	 * should get a page flip event. */
> +	plane.fb_id = fb[0].fb_id;
> +	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
> +	get_events(crtc, &pageflip_count, &vblank_count, 20);
> +	igt_assert_eq(3, pageflip_count);
> +
> +	drmModeAtomicFree(req);
> +}
> +
>  igt_main
>  {
>  	struct kms_atomic_desc desc;
> @@ -1373,6 +1479,17 @@ igt_main
>  		atomic_state_free(scratch);
>  	}
>  
> +	igt_subtest("atomic_pageflip_events") {
> +		struct kms_atomic_state *scratch = atomic_state_dup(current);
> +		struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
> +		struct kms_atomic_plane_state *plane =
> +			find_plane(scratch, PLANE_TYPE_PRIMARY, crtc);
> +
> +		igt_require(plane);
> +		plane_pageflip_events(crtc, plane);
> +		atomic_state_free(scratch);
> +	}
> +
>  	atomic_state_free(current);
>  
>  	igt_fixture
> -- 
> 2.8.0.rc3.226.g39d4020
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH i-g-t] tests: atomic: add test to verify page flip event emissions
  2016-04-22 12:59 ` Daniel Vetter
@ 2016-04-22 13:27   ` Lionel Landwerlin
  2016-04-26 13:23     ` Daniel Vetter
  0 siblings, 1 reply; 6+ messages in thread
From: Lionel Landwerlin @ 2016-04-22 13:27 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On 22/04/16 13:59, Daniel Vetter wrote:
> On Thu, Apr 21, 2016 at 05:01:31PM +0100, Lionel Landwerlin wrote:
>> It seems we don't have a test verifying events with atomic commits
>> yet. Here is a first step.
>>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Feel like also making a testcase variant for the other cases of crtc's
> ACTIVE property?
> - off -> off: kernel should reject event generation
> - off -> on: kernel should generate event, and the frame counter should be
>    just a few frames less (at most) than what you get from the vblank ioctl
>    right after the modeset completes
> - on -> off: same as off -> on but in reverse
>
> Would be awesome ...
> -Daniel

Sure, I will add more test for those cases.

Though right now, I would like to know whether this test seems wrong or 
broken in any way.
As I mentioned on IRC, I'm not getting any event when doing atomic 
commits and this test fails.
I'm concerned that our driver doesn't behave properly with regards to 
events with atomic enabled.

Cheers,

-
Lionel

>
>> ---
>>   tests/kms_atomic.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 127 insertions(+), 10 deletions(-)
>>
>> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
>> index f27ee46..bc75108 100644
>> --- a/tests/kms_atomic.c
>> +++ b/tests/kms_atomic.c
>> @@ -252,10 +252,10 @@ static uint32_t blob_duplicate(int fd, uint32_t id_orig)
>>   	plane_check_current_state(plane_old, relax); \
>>   }
>>   
>> -#define plane_commit_atomic(plane, req, relax) { \
>> +#define plane_commit_atomic(plane, req, flags, relax) {	\
>>   	drmModeAtomicSetCursor(req, 0); \
>>   	plane_populate_req(plane, req); \
>> -	do_atomic_commit((plane)->state->desc->fd, req, 0); \
>> +	do_atomic_commit((plane)->state->desc->fd, req, flags); \
>>   	plane_check_current_state(plane, relax); \
>>   }
>>   
>> @@ -886,10 +886,10 @@ static void plane_overlay(struct kms_atomic_crtc_state *crtc,
>>   
>>   	/* Enable the overlay plane using the atomic API, and double-check
>>   	 * state is what we think it should be. */
>> -	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
>> +	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
>>   
>>   	/* Disable the plane and check the state matches the old. */
>> -	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
>> +	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
>>   
>>   	/* Re-enable the plane through the legacy plane API, and verify through
>>   	 * atomic. */
>> @@ -983,10 +983,10 @@ static void plane_cursor(struct kms_atomic_crtc_state *crtc,
>>   
>>   	/* Flip the cursor plane using the atomic API, and double-check
>>   	 * state is what we think it should be. */
>> -	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
>> +	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
>>   
>>   	/* Restore the cursor plane and check the state matches the old. */
>> -	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
>> +	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
>>   
>>   	/* Re-enable the plane through the legacy cursor API, and verify
>>   	 * through atomic. */
>> @@ -1010,7 +1010,7 @@ static void plane_cursor(struct kms_atomic_crtc_state *crtc,
>>   	plane_check_current_state(plane_old, ATOMIC_RELAX_NONE);
>>   
>>   	/* Finally, restore to the original state. */
>> -	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
>> +	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
>>   
>>   	drmModeAtomicFree(req);
>>   }
>> @@ -1043,7 +1043,7 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
>>   	                        ATOMIC_RELAX_NONE, EINVAL);
>>   
>>   	plane.fb_id = plane_old->fb_id;
>> -	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
>> +	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
>>   
>>   	/* Pass a series of invalid object IDs for the CRTC ID. */
>>   	plane.crtc_id = plane.obj;
>> @@ -1063,7 +1063,7 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
>>   	                        ATOMIC_RELAX_NONE, EINVAL);
>>   
>>   	plane.crtc_id = plane_old->crtc_id;
>> -	plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
>> +	plane_commit_atomic(&plane, req, 0, ATOMIC_RELAX_NONE);
>>   
>>   	/* Create a framebuffer too small for the plane configuration. */
>>   	igt_require(format != 0);
>> @@ -1085,7 +1085,7 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
>>   	                        ATOMIC_RELAX_NONE, ENOSPC);
>>   
>>   	/* Restore the primary plane and check the state matches the old. */
>> -	plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
>> +	plane_commit_atomic(plane_old, req, 0, ATOMIC_RELAX_NONE);
>>   
>>   	drmModeAtomicFree(req);
>>   }
>> @@ -1280,6 +1280,112 @@ static void atomic_invalid_params(struct kms_atomic_crtc_state *crtc,
>>   	do_ioctl_err(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
>>   }
>>   
>> +static void get_events(struct kms_atomic_crtc_state *crtc,
>> +		       int *page_flips,
>> +		       int *vblank_count,
>> +		       int msec)
>> +{
>> +	struct kms_atomic_desc *desc = crtc->state->desc;
>> +	struct timeval timeout = { .tv_sec = 0, .tv_usec = msec * 1000 };
>> +	fd_set fds;
>> +	char buffer[1024];
>> +	int idx, len, ret;
>> +
>> +	FD_ZERO(&fds);
>> +	FD_SET(desc->fd, &fds);
>> +
>> +	ret = select(1, &fds, NULL, NULL, &timeout);
>> +	if (ret < 1)
>> +		return;
>> +
>> +	len = read(desc->fd, buffer, sizeof(buffer));
>> +	if (len == 0)
>> +		return;
>> +
>> +	idx = 0;
>> +	while (idx < len) {
>> +		struct drm_event event;
>> +
>> +		igt_assert_lte(sizeof(event), len - idx);
>> +
>> +		memcpy(&event, &buffer[idx], sizeof(event));
>> +		switch (event.type) {
>> +		case DRM_EVENT_FLIP_COMPLETE:
>> +			(*page_flips)++;
>> +			break;
>> +		case DRM_EVENT_VBLANK:
>> +			(*vblank_count)++;
>> +			break;
>> +		default:
>> +			break;
>> +		}
>> +
>> +		idx += event.length;
>> +	}
>> +}
>> +
>> +static void plane_pageflip_events(struct kms_atomic_crtc_state *crtc,
>> +				  struct kms_atomic_plane_state *plane_old)
>> +{
>> +	struct drm_mode_modeinfo *mode = crtc->mode.data;
>> +	struct kms_atomic_plane_state plane = *plane_old;
>> +	uint32_t format = plane_get_igt_format(&plane);
>> +	drmModeAtomicReq *req = drmModeAtomicAlloc();
>> +	struct igt_fb fb[2];
>> +	int i, pageflip_count = 0, vblank_count = 0;
>> +	int flags = DRM_MODE_PAGE_FLIP_EVENT;
>> +
>> +	igt_require(format != 0);
>> +
>> +	for (i = 0; i < ARRAY_SIZE(fb); i++)
>> +		igt_create_pattern_fb(plane.state->desc->fd,
>> +				      plane.crtc_w, plane.crtc_h,
>> +				      format, I915_TILING_NONE, &fb[i]);
>> +
>> +	plane.src_x = 0;
>> +	plane.src_y = 0;
>> +	plane.src_w = mode->hdisplay << 16;
>> +	plane.src_h = mode->vdisplay << 16;
>> +	plane.crtc_x = 0;
>> +	plane.crtc_y = 0;
>> +	plane.crtc_w = mode->hdisplay;
>> +	plane.crtc_h = mode->vdisplay;
>> +	plane.crtc_id = crtc->obj;
>> +	plane.fb_id = fb[0].fb_id;
>> +
>> +	/* Flip the primary plane using the atomic API, and double-check
>> +	 * state is what we think it should be. */
>> +	crtc_commit_atomic(crtc, &plane, req, ATOMIC_RELAX_NONE);
>> +
>> +	get_events(crtc, &pageflip_count, &vblank_count, 0);
>> +	igt_assert_eq(1, pageflip_count);
>> +
>> +	drmModeAtomicFree(req);
>> +	req = drmModeAtomicAlloc();
>> +
>> +	/* Change the framebuffer on the plane, we should get one
>> +	 * pageflip event. */
>> +	plane.fb_id = fb[1].fb_id;
>> +	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
>> +	get_events(crtc, &pageflip_count, &vblank_count, 20);
>> +	igt_assert_eq(2, pageflip_count);
>> +
>> +	/* No change, page flip event count should remain the same. */
>> +	plane.fb_id = fb[1].fb_id;
>> +	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
>> +	get_events(crtc, &pageflip_count, &vblank_count, 20);
>> +	igt_assert_eq(2, pageflip_count);
>> +
>> +	/* Change back the plane's framebuffer to its original one, we
>> +	 * should get a page flip event. */
>> +	plane.fb_id = fb[0].fb_id;
>> +	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
>> +	get_events(crtc, &pageflip_count, &vblank_count, 20);
>> +	igt_assert_eq(3, pageflip_count);
>> +
>> +	drmModeAtomicFree(req);
>> +}
>> +
>>   igt_main
>>   {
>>   	struct kms_atomic_desc desc;
>> @@ -1373,6 +1479,17 @@ igt_main
>>   		atomic_state_free(scratch);
>>   	}
>>   
>> +	igt_subtest("atomic_pageflip_events") {
>> +		struct kms_atomic_state *scratch = atomic_state_dup(current);
>> +		struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
>> +		struct kms_atomic_plane_state *plane =
>> +			find_plane(scratch, PLANE_TYPE_PRIMARY, crtc);
>> +
>> +		igt_require(plane);
>> +		plane_pageflip_events(crtc, plane);
>> +		atomic_state_free(scratch);
>> +	}
>> +
>>   	atomic_state_free(current);
>>   
>>   	igt_fixture
>> -- 
>> 2.8.0.rc3.226.g39d4020
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH i-g-t] tests: atomic: add test to verify page flip event emissions
  2016-04-21 17:43 [PATCH i-g-t] tests: atomic: add test to verify page flip event emissions Lionel Landwerlin
@ 2016-04-25  8:39 ` Maarten Lankhorst
  0 siblings, 0 replies; 6+ messages in thread
From: Maarten Lankhorst @ 2016-04-25  8:39 UTC (permalink / raw)
  To: Lionel Landwerlin, intel-gfx

Op 21-04-16 om 19:43 schreef Lionel Landwerlin:
> It seems we don't have a test verifying events with atomic commits
> yet. Here is a first step.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>  tests/kms_atomic.c | 149 +++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 133 insertions(+), 16 deletions(-)
>
If you send a newer version, please add a changelog and bump the version in commit.
Else I don't know which is newer, or what's changed between the 2 versions. :-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH i-g-t] tests: atomic: add test to verify page flip event emissions
  2016-04-22 13:27   ` Lionel Landwerlin
@ 2016-04-26 13:23     ` Daniel Vetter
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2016-04-26 13:23 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

On Fri, Apr 22, 2016 at 02:27:28PM +0100, Lionel Landwerlin wrote:
> On 22/04/16 13:59, Daniel Vetter wrote:
> >On Thu, Apr 21, 2016 at 05:01:31PM +0100, Lionel Landwerlin wrote:
> >>It seems we don't have a test verifying events with atomic commits
> >>yet. Here is a first step.
> >>
> >>Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> >Feel like also making a testcase variant for the other cases of crtc's
> >ACTIVE property?
> >- off -> off: kernel should reject event generation
> >- off -> on: kernel should generate event, and the frame counter should be
> >   just a few frames less (at most) than what you get from the vblank ioctl
> >   right after the modeset completes
> >- on -> off: same as off -> on but in reverse
> >
> >Would be awesome ...
> >-Daniel
> 
> Sure, I will add more test for those cases.
> 
> Though right now, I would like to know whether this test seems wrong or
> broken in any way.
> As I mentioned on IRC, I'm not getting any event when doing atomic commits
> and this test fails.
> I'm concerned that our driver doesn't behave properly with regards to events
> with atomic enabled.

Oh I forgot that. lgtm (without checking details, just top-level
semantics), one comment below.


> >>+static void plane_pageflip_events(struct kms_atomic_crtc_state *crtc,
> >>+				  struct kms_atomic_plane_state *plane_old)
> >>+{
> >>+	struct drm_mode_modeinfo *mode = crtc->mode.data;
> >>+	struct kms_atomic_plane_state plane = *plane_old;
> >>+	uint32_t format = plane_get_igt_format(&plane);
> >>+	drmModeAtomicReq *req = drmModeAtomicAlloc();
> >>+	struct igt_fb fb[2];
> >>+	int i, pageflip_count = 0, vblank_count = 0;
> >>+	int flags = DRM_MODE_PAGE_FLIP_EVENT;
> >>+
> >>+	igt_require(format != 0);
> >>+
> >>+	for (i = 0; i < ARRAY_SIZE(fb); i++)
> >>+		igt_create_pattern_fb(plane.state->desc->fd,
> >>+				      plane.crtc_w, plane.crtc_h,
> >>+				      format, I915_TILING_NONE, &fb[i]);
> >>+
> >>+	plane.src_x = 0;
> >>+	plane.src_y = 0;
> >>+	plane.src_w = mode->hdisplay << 16;
> >>+	plane.src_h = mode->vdisplay << 16;
> >>+	plane.crtc_x = 0;
> >>+	plane.crtc_y = 0;
> >>+	plane.crtc_w = mode->hdisplay;
> >>+	plane.crtc_h = mode->vdisplay;
> >>+	plane.crtc_id = crtc->obj;
> >>+	plane.fb_id = fb[0].fb_id;
> >>+
> >>+	/* Flip the primary plane using the atomic API, and double-check
> >>+	 * state is what we think it should be. */
> >>+	crtc_commit_atomic(crtc, &plane, req, ATOMIC_RELAX_NONE);
> >>+
> >>+	get_events(crtc, &pageflip_count, &vblank_count, 0);
> >>+	igt_assert_eq(1, pageflip_count);
> >>+
> >>+	drmModeAtomicFree(req);
> >>+	req = drmModeAtomicAlloc();
> >>+
> >>+	/* Change the framebuffer on the plane, we should get one
> >>+	 * pageflip event. */
> >>+	plane.fb_id = fb[1].fb_id;
> >>+	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
> >>+	get_events(crtc, &pageflip_count, &vblank_count, 20);
> >>+	igt_assert_eq(2, pageflip_count);
> >>+
> >>+	/* No change, page flip event count should remain the same. */
> >>+	plane.fb_id = fb[1].fb_id;
> >>+	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
> >>+	get_events(crtc, &pageflip_count, &vblank_count, 20);
> >>+	igt_assert_eq(2, pageflip_count);

no-op atomic updates should still result in successful event generation.
But there's no guarantee whether we'll short-circuit the entire thing, or
whether we'll do a dummy flip and hence the vblank count will be
incremented by 1 compared to the last test.

Otherwise the assumption of the test that every atomic commit should
result in an even (if you ask for it) is correct.
-Daniel

> >>+	/* Change back the plane's framebuffer to its original one, we
> >>+	 * should get a page flip event. */
> >>+	plane.fb_id = fb[0].fb_id;
> >>+	plane_commit_atomic(&plane, req, flags, ATOMIC_RELAX_NONE);
> >>+	get_events(crtc, &pageflip_count, &vblank_count, 20);
> >>+	igt_assert_eq(3, pageflip_count);
> >>+
> >>+	drmModeAtomicFree(req);
> >>+}
> >>+
> >>  igt_main
> >>  {
> >>  	struct kms_atomic_desc desc;
> >>@@ -1373,6 +1479,17 @@ igt_main
> >>  		atomic_state_free(scratch);
> >>  	}
> >>+	igt_subtest("atomic_pageflip_events") {
> >>+		struct kms_atomic_state *scratch = atomic_state_dup(current);
> >>+		struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
> >>+		struct kms_atomic_plane_state *plane =
> >>+			find_plane(scratch, PLANE_TYPE_PRIMARY, crtc);
> >>+
> >>+		igt_require(plane);
> >>+		plane_pageflip_events(crtc, plane);
> >>+		atomic_state_free(scratch);
> >>+	}
> >>+
> >>  	atomic_state_free(current);
> >>  	igt_fixture
> >>-- 
> >>2.8.0.rc3.226.g39d4020
> >>
> >>_______________________________________________
> >>Intel-gfx mailing list
> >>Intel-gfx@lists.freedesktop.org
> >>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-04-26 13:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-21 17:43 [PATCH i-g-t] tests: atomic: add test to verify page flip event emissions Lionel Landwerlin
2016-04-25  8:39 ` Maarten Lankhorst
  -- strict thread matches above, loose matches on Subject: below --
2016-04-21 16:01 Lionel Landwerlin
2016-04-22 12:59 ` Daniel Vetter
2016-04-22 13:27   ` Lionel Landwerlin
2016-04-26 13:23     ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).