[PATCH 0/3] drm: Add new vblank ioctls [v4]

[PATCH 0/3] drm: Add new vblank ioctls [v4]

[Keith Packard]

This series removes portions of the first patch which switched to
ktime_t as Arnd Bergmann posted a cleaner patch which did only
that. Otherwise, it's the same.

-keith

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/3] drm: Widen vblank count to 64-bits [v3]

[Keith Packard]

This modifies the datatypes used by the vblank code to provide 64 bits
of vblank count.

The driver interfaces have been left using 32 bits of vblank count;
all of the code necessary to widen that value for the user API was
already included to handle devices returning fewer than 32-bits.

This will provide the necessary datatypes for the Vulkan API.

v2:

 * Re-write wait_vblank ioctl to ABSOLUTE sequence

    When an application uses the WAIT_VBLANK ioctl with RELATIVE
    or NEXTONMISS bits set, the target vblank interval is updated
    within the kernel. We need to write that target back to the
    ioctl buffer and update the flags bits so that if the wait is
    interrupted by a signal, when it is re-started, it will target
    precisely the same vblank count as before.

 * Leave driver API with 32-bit vblank count

v3:

 * Rebase on top of Arnd Bergmann's patch which had
   the switch to ktime_t parts.

Suggested-by:  Michel Dänzer <michel@daenzer.net>
Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_vblank.c | 104 +++++++++++++++++++++++++++++--------------
 include/drm/drm_vblank.h     |  10 +++--
 2 files changed, 78 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 810a93fc558b..47460589d4cd 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -251,7 +251,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
 	}
 
 	DRM_DEBUG_VBL("updating vblank count on crtc %u:"
-		      " current=%u, diff=%u, hw=%u hw_last=%u\n",
+		      " current=%llu, diff=%u, hw=%u hw_last=%u\n",
 		      pipe, vblank->count, diff, cur_vblank, vblank->last);
 
 	if (diff == 0) {
@@ -740,17 +740,31 @@ drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe,
  * Returns:
  * The software vblank counter.
  */
-u32 drm_crtc_vblank_count(struct drm_crtc *crtc)
+u64 drm_crtc_vblank_count(struct drm_crtc *crtc)
 {
 	return drm_vblank_count(crtc->dev, drm_crtc_index(crtc));
 }
 EXPORT_SYMBOL(drm_crtc_vblank_count);
 
-static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
+/**
+ * drm_vblank_count_and_time - retrieve "cooked" vblank counter value and the
+ *     system timestamp corresponding to that vblank counter value.
+ * @dev: DRM device
+ * @pipe: index of CRTC whose counter to retrieve
+ * @vblanktime: Pointer to ktime_t to receive the vblank timestamp.
+ *
+ * Fetches the "cooked" vblank count value that represents the number of
+ * vblank events since the system was booted, including lost events due to
+ * modesetting activity. Returns corresponding system timestamp of the time
+ * of the vblank interval that corresponds to the current vblank counter value.
+ *
+ * This is the legacy version of drm_crtc_vblank_count_and_time().
+ */
+static u64 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
 				     ktime_t *vblanktime)
 {
 	struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
-	u32 vblank_count;
+	u64 vblank_count;
 	unsigned int seq;
 
 	if (WARN_ON(pipe >= dev->num_crtcs)) {
@@ -778,7 +792,7 @@ static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
  * modesetting activity. Returns corresponding system timestamp of the time
  * of the vblank interval that corresponds to the current vblank counter value.
  */
-u32 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
+u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
 				   ktime_t *vblanktime)
 {
 	return drm_vblank_count_and_time(crtc->dev, drm_crtc_index(crtc),
@@ -788,7 +802,7 @@ EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
 
 static void send_vblank_event(struct drm_device *dev,
 		struct drm_pending_vblank_event *e,
-		unsigned long seq, ktime_t now)
+		u64 seq, ktime_t now)
 {
 	struct timespec64 tv = ktime_to_timespec64(now);
 
@@ -854,7 +868,7 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
 	assert_spin_locked(&dev->event_lock);
 
 	e->pipe = pipe;
-	e->event.sequence = drm_vblank_count(dev, pipe);
+	e->sequence = drm_vblank_count(dev, pipe);
 	e->event.crtc_id = crtc->base.id;
 	list_add_tail(&e->base.link, &dev->vblank_event_list);
 }
@@ -875,7 +889,8 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
 				struct drm_pending_vblank_event *e)
 {
 	struct drm_device *dev = crtc->dev;
-	unsigned int seq, pipe = drm_crtc_index(crtc);
+	u64 seq;
+	unsigned int pipe = drm_crtc_index(crtc);
 	ktime_t now;
 
 	if (dev->num_crtcs > 0) {
@@ -1088,7 +1103,7 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
 
 	ktime_t now;
 	unsigned long irqflags;
-	unsigned int seq;
+	u64 seq;
 
 	if (WARN_ON(pipe >= dev->num_crtcs))
 		return;
@@ -1123,8 +1138,8 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
 		if (e->pipe != pipe)
 			continue;
 		DRM_DEBUG("Sending premature vblank event on disable: "
-			  "wanted %u, current %u\n",
-			  e->event.sequence, seq);
+			  "wanted %llu, current %llu\n",
+			  e->sequence, seq);
 		list_del(&e->base.link);
 		drm_vblank_put(dev, pipe);
 		send_vblank_event(dev, e, seq, now);
@@ -1296,12 +1311,13 @@ int drm_legacy_modeset_ctl_ioctl(struct drm_device *dev, void *data,
 	return 0;
 }
 
-static inline bool vblank_passed(u32 seq, u32 ref)
+static inline bool vblank_passed(u64 seq, u64 ref)
 {
 	return (seq - ref) <= (1 << 23);
 }
 
 static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
+				  u64 req_seq,
 				  union drm_wait_vblank *vblwait,
 				  struct drm_file *file_priv)
 {
@@ -1309,7 +1325,7 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
 	struct drm_pending_vblank_event *e;
 	ktime_t now;
 	unsigned long flags;
-	unsigned int seq;
+	u64 seq;
 	int ret;
 
 	e = kzalloc(sizeof(*e), GFP_KERNEL);
@@ -1344,21 +1360,20 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
 
 	seq = drm_vblank_count_and_time(dev, pipe, &now);
 
-	DRM_DEBUG("event on vblank count %u, current %u, crtc %u\n",
-		  vblwait->request.sequence, seq, pipe);
+	DRM_DEBUG("event on vblank count %llu, current %llu, crtc %u\n",
+		  req_seq, seq, pipe);
 
-	trace_drm_vblank_event_queued(file_priv, pipe,
-				      vblwait->request.sequence);
+	trace_drm_vblank_event_queued(file_priv, pipe, req_seq);
 
-	e->event.sequence = vblwait->request.sequence;
-	if (vblank_passed(seq, vblwait->request.sequence)) {
+	e->sequence = req_seq;
+	if (vblank_passed(seq, req_seq)) {
 		drm_vblank_put(dev, pipe);
 		send_vblank_event(dev, e, seq, now);
 		vblwait->reply.sequence = seq;
 	} else {
 		/* drm_handle_vblank_events will call drm_vblank_put */
 		list_add_tail(&e->base.link, &dev->vblank_event_list);
-		vblwait->reply.sequence = vblwait->request.sequence;
+		vblwait->reply.sequence = req_seq;
 	}
 
 	spin_unlock_irqrestore(&dev->event_lock, flags);
@@ -1384,6 +1399,22 @@ static bool drm_wait_vblank_is_query(union drm_wait_vblank *vblwait)
 					  _DRM_VBLANK_NEXTONMISS));
 }
 
+/*
+ * Widen a 32-bit param to 64-bits.
+ *
+ * \param narrow 32-bit value (missing upper 32 bits)
+ * \param near 64-bit value that should be 'close' to near
+ *
+ * This function returns a 64-bit value using the lower 32-bits from
+ * 'narrow' and constructing the upper 32-bits so that the result is
+ * as close as possible to 'near'.
+ */
+
+static u64 widen_32_to_64(u32 narrow, u64 near)
+{
+	return near + (s32) (narrow - near);
+}
+
 static void drm_wait_vblank_reply(struct drm_device *dev, unsigned int pipe,
 				  struct drm_wait_vblank_reply *reply)
 {
@@ -1407,7 +1438,8 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
 	struct drm_vblank_crtc *vblank;
 	union drm_wait_vblank *vblwait = data;
 	int ret;
-	unsigned int flags, seq, pipe, high_pipe;
+	u64 req_seq, seq;
+	unsigned int flags, pipe, high_pipe;
 
 	if (!dev->irq_enabled)
 		return -EINVAL;
@@ -1455,9 +1487,12 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
 
 	switch (vblwait->request.type & _DRM_VBLANK_TYPES_MASK) {
 	case _DRM_VBLANK_RELATIVE:
-		vblwait->request.sequence += seq;
+		req_seq = seq + vblwait->request.sequence;
+		vblwait->request.sequence = req_seq;
 		vblwait->request.type &= ~_DRM_VBLANK_RELATIVE;
+		break;
 	case _DRM_VBLANK_ABSOLUTE:
+		req_seq = widen_32_to_64(vblwait->request.sequence, seq);
 		break;
 	default:
 		ret = -EINVAL;
@@ -1465,22 +1500,25 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
 	}
 
 	if ((flags & _DRM_VBLANK_NEXTONMISS) &&
-	    vblank_passed(seq, vblwait->request.sequence))
-		vblwait->request.sequence = seq + 1;
+	    vblank_passed(seq, req_seq)) {
+		req_seq = seq + 1;
+		vblwait->request.type &= ~_DRM_VBLANK_NEXTONMISS;
+		vblwait->request.sequence = req_seq;
+	}
 
 	if (flags & _DRM_VBLANK_EVENT) {
 		/* must hold on to the vblank ref until the event fires
 		 * drm_vblank_put will be called asynchronously
 		 */
-		return drm_queue_vblank_event(dev, pipe, vblwait, file_priv);
+		return drm_queue_vblank_event(dev, pipe, req_seq, vblwait, file_priv);
 	}
 
-	if (vblwait->request.sequence != seq) {
-		DRM_DEBUG("waiting on vblank count %u, crtc %u\n",
-			  vblwait->request.sequence, pipe);
+	if (req_seq != seq) {
+		DRM_DEBUG("waiting on vblank count %llu, crtc %u\n",
+			  req_seq, pipe);
 		DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
 			    vblank_passed(drm_vblank_count(dev, pipe),
-					  vblwait->request.sequence) ||
+					  req_seq) ||
 			    !READ_ONCE(vblank->enabled));
 	}
 
@@ -1502,7 +1540,7 @@ static void drm_handle_vblank_events(struct drm_device *dev, unsigned int pipe)
 {
 	struct drm_pending_vblank_event *e, *t;
 	ktime_t now;
-	unsigned int seq;
+	u64 seq;
 
 	assert_spin_locked(&dev->event_lock);
 
@@ -1511,11 +1549,11 @@ static void drm_handle_vblank_events(struct drm_device *dev, unsigned int pipe)
 	list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) {
 		if (e->pipe != pipe)
 			continue;
-		if (!vblank_passed(seq, e->event.sequence))
+		if (!vblank_passed(seq, e->sequence))
 			continue;
 
-		DRM_DEBUG("vblank event on %u, current %u\n",
-			  e->event.sequence, seq);
+		DRM_DEBUG("vblank event on %llu, current %llu\n",
+			  e->sequence, seq);
 
 		list_del(&e->base.link);
 		drm_vblank_put(dev, pipe);
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index 6a58e2e91a0f..cce53130510f 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -47,6 +47,10 @@ struct drm_pending_vblank_event {
 	 * @pipe: drm_crtc_index() of the &drm_crtc this event is for.
 	 */
 	unsigned int pipe;
+	/**
+	 * @sequence: frame event should be triggered at
+	 */
+	u64 sequence;
 	/**
 	 * @event: Actual event which will be sent to userspace.
 	 */
@@ -88,7 +92,7 @@ struct drm_vblank_crtc {
 	/**
 	 * @count: Current software vblank counter.
 	 */
-	u32 count;
+	u64 count;
 	/**
 	 * @time: Vblank timestamp corresponding to @count.
 	 */
@@ -152,8 +156,8 @@ struct drm_vblank_crtc {
 };
 
 int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs);
-u32 drm_crtc_vblank_count(struct drm_crtc *crtc);
-u32 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
+u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
+u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
 				   ktime_t *vblanktime);
 void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
 			       struct drm_pending_vblank_event *e);
-- 
2.15.0.rc0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm: Reorganize drm_pending_event to support future event types [v2]

[Keith Packard]

Place drm_event_vblank in a new union that includes that and a bare
drm_event structure. This will allow new members of that union to be
added in the future without changing code related to the existing vbl
event type.

Assignments to the crtc_id field are now done when the event is
allocated, rather than when delievered. This way, delivery doesn't
need to have the crtc ID available.

v2:
 * Remove 'dev' argument from create_vblank_event

	It wasn't being used anyways, and if we need it in the future,
	we can always get it from crtc->dev.

 * Check for MODESETTING before looking for crtc in queue_vblank_event

	UMS drivers will oops if we try to get a crtc, so make sure
	we're modesetting before we try to find a crtc_id to fill into
	the event.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_atomic.c         |  7 +++---
 drivers/gpu/drm/drm_plane.c          |  2 +-
 drivers/gpu/drm/drm_vblank.c         | 41 +++++++++++++++++++++---------------
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  4 ++--
 include/drm/drm_vblank.h             |  8 ++++++-
 6 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 268969fecee7..1e2021bed265 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1815,7 +1815,7 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
  */
 
 static struct drm_pending_vblank_event *create_vblank_event(
-		struct drm_device *dev, uint64_t user_data)
+		struct drm_crtc *crtc, uint64_t user_data)
 {
 	struct drm_pending_vblank_event *e = NULL;
 
@@ -1825,7 +1825,8 @@ static struct drm_pending_vblank_event *create_vblank_event(
 
 	e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
 	e->event.base.length = sizeof(e->event);
-	e->event.user_data = user_data;
+	e->event.vbl.crtc_id = crtc->base.id;
+	e->event.vbl.user_data = user_data;
 
 	return e;
 }
@@ -2079,7 +2080,7 @@ static int prepare_crtc_signaling(struct drm_device *dev,
 		if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) {
 			struct drm_pending_vblank_event *e;
 
-			e = create_vblank_event(dev, arg->user_data);
+			e = create_vblank_event(crtc, arg->user_data);
 			if (!e)
 				return -ENOMEM;
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 6af02c7b5da3..574fd1475214 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -1039,7 +1039,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 		}
 		e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
 		e->event.base.length = sizeof(e->event);
-		e->event.user_data = page_flip->user_data;
+		e->event.vbl.user_data = page_flip->user_data;
 		ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
 		if (ret) {
 			kfree(e);
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 47460589d4cd..d250606cc771 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -804,20 +804,23 @@ static void send_vblank_event(struct drm_device *dev,
 		struct drm_pending_vblank_event *e,
 		u64 seq, ktime_t now)
 {
-	struct timespec64 tv = ktime_to_timespec64(now);
-
-	e->event.sequence = seq;
-	/*
-	 * e->event is a user space structure, with hardcoded unsigned
-	 * 32-bit seconds/microseconds. This is safe as we always use
-	 * monotonic timestamps since linux-4.15
-	 */
-	e->event.tv_sec = tv.tv_sec;
-	e->event.tv_usec = tv.tv_nsec / 1000;
-
-	trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe,
-					 e->event.sequence);
+	struct timespec64 tv;
 
+	switch (e->event.base.type) {
+	case DRM_EVENT_VBLANK:
+	case DRM_EVENT_FLIP_COMPLETE:
+		tv = ktime_to_timespec64(now);
+		e->event.vbl.sequence = seq;
+		/*
+		 * e->event is a user space structure, with hardcoded unsigned
+		 * 32-bit seconds/microseconds. This is safe as we always use
+		 * monotonic timestamps since linux-4.15
+		 */
+		e->event.vbl.tv_sec = tv.tv_sec;
+		e->event.vbl.tv_usec = tv.tv_nsec / 1000;
+		break;
+	}
+	trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq);
 	drm_send_event_locked(dev, &e->base);
 }
 
@@ -869,7 +872,6 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
 
 	e->pipe = pipe;
 	e->sequence = drm_vblank_count(dev, pipe);
-	e->event.crtc_id = crtc->base.id;
 	list_add_tail(&e->base.link, &dev->vblank_event_list);
 }
 EXPORT_SYMBOL(drm_crtc_arm_vblank_event);
@@ -901,7 +903,6 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
 		now = ktime_get();
 	}
 	e->pipe = pipe;
-	e->event.crtc_id = crtc->base.id;
 	send_vblank_event(dev, e, seq, now);
 }
 EXPORT_SYMBOL(drm_crtc_send_vblank_event);
@@ -1336,8 +1337,14 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
 
 	e->pipe = pipe;
 	e->event.base.type = DRM_EVENT_VBLANK;
-	e->event.base.length = sizeof(e->event);
-	e->event.user_data = vblwait->request.signal;
+	e->event.base.length = sizeof(e->event.vbl);
+	e->event.vbl.user_data = vblwait->request.signal;
+	e->event.vbl.crtc_id = 0;
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
+		if (crtc)
+			e->event.vbl.crtc_id = crtc->base.id;
+	}
 
 	spin_lock_irqsave(&dev->event_lock, flags);
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index d1552d3e0652..bc5f6026573d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -360,8 +360,8 @@ static int vmw_sou_crtc_page_flip(struct drm_crtc *crtc,
 
 		ret = vmw_event_fence_action_queue(file_priv, fence,
 						   &event->base,
-						   &event->event.tv_sec,
-						   &event->event.tv_usec,
+						   &event->event.vbl.tv_sec,
+						   &event->event.vbl.tv_usec,
 						   true);
 	}
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index ca3afae2db1f..90b5437fd787 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -549,8 +549,8 @@ static int vmw_stdu_crtc_page_flip(struct drm_crtc *crtc,
 
 		ret = vmw_event_fence_action_queue(file_priv, fence,
 						   &event->base,
-						   &event->event.tv_sec,
-						   &event->event.tv_usec,
+						   &event->event.vbl.tv_sec,
+						   &event->event.vbl.tv_usec,
 						   true);
 		vmw_fence_obj_unreference(&fence);
 	} else {
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index cce53130510f..bf8e07035a0a 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -54,7 +54,10 @@ struct drm_pending_vblank_event {
 	/**
 	 * @event: Actual event which will be sent to userspace.
 	 */
-	struct drm_event_vblank event;
+	union {
+		struct drm_event base;
+		struct drm_event_vblank vbl;
+	} event;
 };
 
 /**
@@ -163,6 +166,9 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
 			       struct drm_pending_vblank_event *e);
 void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
 			      struct drm_pending_vblank_event *e);
+void drm_vblank_set_event(struct drm_pending_vblank_event *e,
+			  u64 *seq,
+			  ktime_t *now);
 bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
 bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
 int drm_crtc_vblank_get(struct drm_crtc *crtc);
-- 
2.15.0.rc0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/3] drm: Add CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls [v3]

[Keith Packard]

These provide crtc-id based functions instead of pipe-number, while
also offering higher resolution time (ns) and wider frame count (64)
as required by the Vulkan API.

v2:

 * Check for DRIVER_MODESET in new crtc-based vblank ioctls

	Failing to check this will oops the driver.

 * Ensure vblank interupt is running in crtc_get_sequence ioctl

	The sequence and timing values are not correct while the
	interrupt is off, so make sure it's running before asking for
	them.

 * Short-circuit get_sequence if the counter is enabled and accurate

	Steal the idea from the code in wait_vblank to avoid the
	expense of drm_vblank_get/put

 * Return active state of crtc in crtc_get_sequence ioctl

	Might be useful for applications that aren't in charge of
	modesetting?

 * Use drm_crtc_vblank_get/put in new crtc-based vblank sequence ioctls

	Daniel Vetter prefers these over the old drm_vblank_put/get
	APIs.

 * Return s64 ns instead of u64 in new sequence event

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

v3:

 * Removed FIRST_PIXEL_OUT_FLAG
 * Document that the timestamp in the query and event are
   that of the first pixel leaving the display engine for
   the display (using the same wording as the Vulkan spec).

Suggested-by: Michel Dänzer <michel@daenzer.net>
Acked-by: Dave Airlie <airlied@redhat.com>

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_internal.h |   6 ++
 drivers/gpu/drm/drm_ioctl.c    |   2 +
 drivers/gpu/drm/drm_vblank.c   | 168 +++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_vblank.h       |   1 +
 include/uapi/drm/drm.h         |  36 +++++++++
 5 files changed, 213 insertions(+)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index edd921adcf33..c9d5a6cd4d41 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -70,6 +70,12 @@ int drm_legacy_modeset_ctl_ioctl(struct drm_device *dev, void *data,
 int drm_legacy_irq_control(struct drm_device *dev, void *data,
 			   struct drm_file *file_priv);
 
+int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data,
+				struct drm_file *filp);
+
+int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
+				  struct drm_file *filp);
+
 /* drm_auth.c */
 int drm_getmagic(struct drm_device *dev, void *data,
 		 struct drm_file *file_priv);
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index a78f03155466..9c435a4c0c82 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -663,6 +663,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 		      DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
 		      DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED),
 };
 
 #define DRM_CORE_IOCTL_COUNT	ARRAY_SIZE( drm_ioctls )
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index d250606cc771..deb080f62a29 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -819,6 +819,11 @@ static void send_vblank_event(struct drm_device *dev,
 		e->event.vbl.tv_sec = tv.tv_sec;
 		e->event.vbl.tv_usec = tv.tv_nsec / 1000;
 		break;
+	case DRM_EVENT_CRTC_SEQUENCE:
+		if (seq)
+			e->event.seq.sequence = seq;
+		e->event.seq.time_ns = ktime_to_ns(now);
+		break;
 	}
 	trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq);
 	drm_send_event_locked(dev, &e->base);
@@ -1650,3 +1655,166 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc)
 	return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc));
 }
 EXPORT_SYMBOL(drm_crtc_handle_vblank);
+
+/*
+ * Get crtc VBLANK count.
+ *
+ * \param dev DRM device
+ * \param data user arguement, pointing to a drm_crtc_get_sequence structure.
+ * \param file_priv drm file private for the user's open file descriptor
+ */
+
+int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data,
+				struct drm_file *file_priv)
+{
+	struct drm_crtc *crtc;
+	struct drm_vblank_crtc *vblank;
+	int pipe;
+	struct drm_crtc_get_sequence *get_seq = data;
+	ktime_t now;
+	bool vblank_enabled;
+	int ret;
+
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return -EINVAL;
+
+	if (!dev->irq_enabled)
+		return -EINVAL;
+
+	crtc = drm_crtc_find(dev, file_priv, get_seq->crtc_id);
+	if (!crtc)
+		return -ENOENT;
+
+	pipe = drm_crtc_index(crtc);
+
+	vblank = &dev->vblank[pipe];
+	vblank_enabled = dev->vblank_disable_immediate && READ_ONCE(vblank->enabled);
+
+	if (!vblank_enabled) {
+		ret = drm_crtc_vblank_get(crtc);
+		if (ret) {
+			DRM_DEBUG("crtc %d failed to acquire vblank counter, %d\n", pipe, ret);
+			return ret;
+		}
+	}
+	drm_modeset_lock(&crtc->mutex, NULL);
+	if (crtc->state)
+		get_seq->active = crtc->state->enable;
+	else
+		get_seq->active = crtc->enabled;
+	drm_modeset_unlock(&crtc->mutex);
+	get_seq->sequence = drm_vblank_count_and_time(dev, pipe, &now);
+	get_seq->sequence_ns = ktime_to_ns(now);
+	if (!vblank_enabled)
+		drm_crtc_vblank_put(crtc);
+	return 0;
+}
+
+/*
+ * Queue a event for VBLANK sequence
+ *
+ * \param dev DRM device
+ * \param data user arguement, pointing to a drm_crtc_queue_sequence structure.
+ * \param file_priv drm file private for the user's open file descriptor
+ */
+
+int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
+				  struct drm_file *file_priv)
+{
+	struct drm_crtc *crtc;
+	struct drm_vblank_crtc *vblank;
+	int pipe;
+	struct drm_crtc_queue_sequence *queue_seq = data;
+	ktime_t now;
+	struct drm_pending_vblank_event *e;
+	u32 flags;
+	u64 seq;
+	u64 req_seq;
+	int ret;
+	unsigned long spin_flags;
+
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return -EINVAL;
+
+	if (!dev->irq_enabled)
+		return -EINVAL;
+
+	crtc = drm_crtc_find(dev, file_priv, queue_seq->crtc_id);
+	if (!crtc)
+		return -ENOENT;
+
+	flags = queue_seq->flags;
+	/* Check valid flag bits */
+	if (flags & ~(DRM_CRTC_SEQUENCE_RELATIVE|
+		      DRM_CRTC_SEQUENCE_NEXT_ON_MISS))
+		return -EINVAL;
+
+	pipe = drm_crtc_index(crtc);
+
+	vblank = &dev->vblank[pipe];
+
+	e = kzalloc(sizeof(*e), GFP_KERNEL);
+	if (e == NULL)
+		return -ENOMEM;
+
+	ret = drm_crtc_vblank_get(crtc);
+	if (ret) {
+		DRM_DEBUG("crtc %d failed to acquire vblank counter, %d\n", pipe, ret);
+		goto err_free;
+	}
+
+	seq = drm_vblank_count_and_time(dev, pipe, &now);
+	req_seq = queue_seq->sequence;
+
+	if (flags & DRM_CRTC_SEQUENCE_RELATIVE)
+		req_seq += seq;
+
+	if ((flags & DRM_CRTC_SEQUENCE_NEXT_ON_MISS) && vblank_passed(seq, req_seq))
+		req_seq = seq + 1;
+
+	e->pipe = pipe;
+	e->event.base.type = DRM_EVENT_CRTC_SEQUENCE;
+	e->event.base.length = sizeof(e->event.seq);
+	e->event.seq.user_data = queue_seq->user_data;
+
+	spin_lock_irqsave(&dev->event_lock, spin_flags);
+
+	/*
+	 * drm_crtc_vblank_off() might have been called after we called
+	 * drm_crtc_vblank_get(). drm_crtc_vblank_off() holds event_lock around the
+	 * vblank disable, so no need for further locking.  The reference from
+	 * drm_crtc_vblank_get() protects against vblank disable from another source.
+	 */
+	if (!READ_ONCE(vblank->enabled)) {
+		ret = -EINVAL;
+		goto err_unlock;
+	}
+
+	ret = drm_event_reserve_init_locked(dev, file_priv, &e->base,
+					    &e->event.base);
+
+	if (ret)
+		goto err_unlock;
+
+	e->sequence = req_seq;
+
+	if (vblank_passed(seq, req_seq)) {
+		drm_crtc_vblank_put(crtc);
+		send_vblank_event(dev, e, seq, now);
+		queue_seq->sequence = seq;
+	} else {
+		/* drm_handle_vblank_events will call drm_vblank_put */
+		list_add_tail(&e->base.link, &dev->vblank_event_list);
+		queue_seq->sequence = req_seq;
+	}
+
+	spin_unlock_irqrestore(&dev->event_lock, spin_flags);
+	return 0;
+
+err_unlock:
+	spin_unlock_irqrestore(&dev->event_lock, spin_flags);
+	drm_crtc_vblank_put(crtc);
+err_free:
+	kfree(e);
+	return ret;
+}
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index bf8e07035a0a..848b463a0af5 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -57,6 +57,7 @@ struct drm_pending_vblank_event {
 	union {
 		struct drm_event base;
 		struct drm_event_vblank vbl;
+		struct drm_event_crtc_sequence seq;
 	} event;
 };
 
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 97677cd6964d..a106f6a7a0f9 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -737,6 +737,28 @@ struct drm_syncobj_array {
 	__u32 pad;
 };
 
+/* Query current scanout sequence number */
+struct drm_crtc_get_sequence {
+	__u32 crtc_id;		/* requested crtc_id */
+	__u32 active;		/* return: crtc output is active */
+	__u64 sequence;		/* return: most recent vblank sequence */
+	__s64 sequence_ns;	/* return: most recent time of first pixel out */
+};
+
+/* Queue event to be delivered at specified sequence. Time stamp marks
+ * when the first pixel of the refresh cycle left the display engine
+ * for the display
+ */
+#define DRM_CRTC_SEQUENCE_RELATIVE		0x00000001	/* sequence is relative to current */
+#define DRM_CRTC_SEQUENCE_NEXT_ON_MISS		0x00000002	/* Use next sequence if we've missed */
+
+struct drm_crtc_queue_sequence {
+	__u32 crtc_id;
+	__u32 flags;
+	__u64 sequence;		/* on input, target sequence. on output, actual sequence */
+	__u64 user_data;	/* user data passed to event */
+};
+
 #if defined(__cplusplus)
 }
 #endif
@@ -819,6 +841,9 @@ extern "C" {
 
 #define DRM_IOCTL_WAIT_VBLANK		DRM_IOWR(0x3a, union drm_wait_vblank)
 
+#define DRM_IOCTL_CRTC_GET_SEQUENCE	DRM_IOWR(0x3b, struct drm_crtc_get_sequence)
+#define DRM_IOCTL_CRTC_QUEUE_SEQUENCE	DRM_IOWR(0x3c, struct drm_crtc_queue_sequence)
+
 #define DRM_IOCTL_UPDATE_DRAW		DRM_IOW(0x3f, struct drm_update_draw)
 
 #define DRM_IOCTL_MODE_GETRESOURCES	DRM_IOWR(0xA0, struct drm_mode_card_res)
@@ -893,6 +918,7 @@ struct drm_event {
 
 #define DRM_EVENT_VBLANK 0x01
 #define DRM_EVENT_FLIP_COMPLETE 0x02
+#define DRM_EVENT_CRTC_SEQUENCE	0x03
 
 struct drm_event_vblank {
 	struct drm_event base;
@@ -903,6 +929,16 @@ struct drm_event_vblank {
 	__u32 crtc_id; /* 0 on older kernels that do not support this */
 };
 
+/* Event delivered at sequence. Time stamp marks when the first pixel
+ * of the refresh cycle left the display engine for the display
+ */
+struct drm_event_crtc_sequence {
+	struct drm_event	base;
+	__u64			user_data;
+	__s64			time_ns;
+	__u64			sequence;
+};
+
 /* typedef area */
 #ifndef __KERNEL__
 typedef struct drm_clip_rect drm_clip_rect_t;
-- 
2.15.0.rc0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 0/3] drm: add new vblank ioctls [v3]

[Keith Packard]

This version removes the FIRST_PIXEL_OUT flag as the existing code
already conforms to that requirement. It has also been rebased to
drm-next.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm: Reorganize drm_pending_event to support future event types [v2]

[Keith Packard]

Place drm_event_vblank in a new union that includes that and a bare
drm_event structure. This will allow new members of that union to be
added in the future without changing code related to the existing vbl
event type.

Assignments to the crtc_id field are now done when the event is
allocated, rather than when delievered. This way, delivery doesn't
need to have the crtc ID available.

v2:
 * Remove 'dev' argument from create_vblank_event

	It wasn't being used anyways, and if we need it in the future,
	we can always get it from crtc->dev.

 * Check for MODESETTING before looking for crtc in queue_vblank_event

	UMS drivers will oops if we try to get a crtc, so make sure
	we're modesetting before we try to find a crtc_id to fill into
	the event.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_atomic.c         |  7 ++++---
 drivers/gpu/drm/drm_plane.c          |  2 +-
 drivers/gpu/drm/drm_vblank.c         | 30 ++++++++++++++++++------------
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  4 ++--
 include/drm/drm_vblank.h             |  8 +++++++-
 6 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 366c56fe5f58..08ff1b023f7b 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1815,7 +1815,7 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
  */
 
 static struct drm_pending_vblank_event *create_vblank_event(
-		struct drm_device *dev, uint64_t user_data)
+		struct drm_crtc *crtc, uint64_t user_data)
 {
 	struct drm_pending_vblank_event *e = NULL;
 
@@ -1825,7 +1825,8 @@ static struct drm_pending_vblank_event *create_vblank_event(
 
 	e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
 	e->event.base.length = sizeof(e->event);
-	e->event.user_data = user_data;
+	e->event.vbl.crtc_id = crtc->base.id;
+	e->event.vbl.user_data = user_data;
 
 	return e;
 }
@@ -2079,7 +2080,7 @@ static int prepare_crtc_signaling(struct drm_device *dev,
 		if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) {
 			struct drm_pending_vblank_event *e;
 
-			e = create_vblank_event(dev, arg->user_data);
+			e = create_vblank_event(crtc, arg->user_data);
 			if (!e)
 				return -ENOMEM;
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 72cba9805edc..192071209baa 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -1039,7 +1039,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 		}
 		e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
 		e->event.base.length = sizeof(e->event);
-		e->event.user_data = page_flip->user_data;
+		e->event.vbl.user_data = page_flip->user_data;
 		ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
 		if (ret) {
 			kfree(e);
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 8c3f2fdd821a..7d2674e0362a 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -823,14 +823,16 @@ static void send_vblank_event(struct drm_device *dev,
 {
 	struct timeval tv;
 
-	tv = ktime_to_timeval(now);
-	e->event.sequence = seq;
-	e->event.tv_sec = tv.tv_sec;
-	e->event.tv_usec = tv.tv_usec;
-
-	trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe,
-					 e->event.sequence);
-
+	switch (e->event.base.type) {
+	case DRM_EVENT_VBLANK:
+	case DRM_EVENT_FLIP_COMPLETE:
+		tv = ktime_to_timeval(now);
+		e->event.vbl.sequence = seq;
+		e->event.vbl.tv_sec = tv.tv_sec;
+		e->event.vbl.tv_usec = tv.tv_usec;
+		break;
+	}
+	trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq);
 	drm_send_event_locked(dev, &e->base);
 }
 
@@ -882,7 +884,6 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
 
 	e->pipe = pipe;
 	e->sequence = drm_vblank_count(dev, pipe);
-	e->event.crtc_id = crtc->base.id;
 	list_add_tail(&e->base.link, &dev->vblank_event_list);
 }
 EXPORT_SYMBOL(drm_crtc_arm_vblank_event);
@@ -913,7 +914,6 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
 		now = get_drm_timestamp();
 	}
 	e->pipe = pipe;
-	e->event.crtc_id = crtc->base.id;
 	send_vblank_event(dev, e, seq, now);
 }
 EXPORT_SYMBOL(drm_crtc_send_vblank_event);
@@ -1347,8 +1347,14 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
 
 	e->pipe = pipe;
 	e->event.base.type = DRM_EVENT_VBLANK;
-	e->event.base.length = sizeof(e->event);
-	e->event.user_data = vblwait->request.signal;
+	e->event.base.length = sizeof(e->event.vbl);
+	e->event.vbl.user_data = vblwait->request.signal;
+	e->event.vbl.crtc_id = 0;
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
+		if (crtc)
+			e->event.vbl.crtc_id = crtc->base.id;
+	}
 
 	spin_lock_irqsave(&dev->event_lock, flags);
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index d1552d3e0652..bc5f6026573d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -360,8 +360,8 @@ static int vmw_sou_crtc_page_flip(struct drm_crtc *crtc,
 
 		ret = vmw_event_fence_action_queue(file_priv, fence,
 						   &event->base,
-						   &event->event.tv_sec,
-						   &event->event.tv_usec,
+						   &event->event.vbl.tv_sec,
+						   &event->event.vbl.tv_usec,
 						   true);
 	}
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index ca3afae2db1f..90b5437fd787 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -549,8 +549,8 @@ static int vmw_stdu_crtc_page_flip(struct drm_crtc *crtc,
 
 		ret = vmw_event_fence_action_queue(file_priv, fence,
 						   &event->base,
-						   &event->event.tv_sec,
-						   &event->event.tv_usec,
+						   &event->event.vbl.tv_sec,
+						   &event->event.vbl.tv_usec,
 						   true);
 		vmw_fence_obj_unreference(&fence);
 	} else {
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index cce53130510f..bf8e07035a0a 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -54,7 +54,10 @@ struct drm_pending_vblank_event {
 	/**
 	 * @event: Actual event which will be sent to userspace.
 	 */
-	struct drm_event_vblank event;
+	union {
+		struct drm_event base;
+		struct drm_event_vblank vbl;
+	} event;
 };
 
 /**
@@ -163,6 +166,9 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
 			       struct drm_pending_vblank_event *e);
 void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
 			      struct drm_pending_vblank_event *e);
+void drm_vblank_set_event(struct drm_pending_vblank_event *e,
+			  u64 *seq,
+			  ktime_t *now);
 bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
 bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
 int drm_crtc_vblank_get(struct drm_crtc *crtc);
-- 
2.13.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm: Reorganize drm_pending_event to support future event types [v2]

[Sean Paul]

On Tue, Oct 10, 2017 at 05:45:13PM -0700, Keith Packard wrote:
> Place drm_event_vblank in a new union that includes that and a bare
> drm_event structure. This will allow new members of that union to be
> added in the future without changing code related to the existing vbl
> event type.
> 
> Assignments to the crtc_id field are now done when the event is
> allocated, rather than when delievered. This way, delivery doesn't
> need to have the crtc ID available.
> 
> v2:
>  * Remove 'dev' argument from create_vblank_event
> 
> 	It wasn't being used anyways, and if we need it in the future,
> 	we can always get it from crtc->dev.
> 
>  * Check for MODESETTING before looking for crtc in queue_vblank_event
> 
> 	UMS drivers will oops if we try to get a crtc, so make sure
> 	we're modesetting before we try to find a crtc_id to fill into
> 	the event.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>

Reviewed-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/drm_atomic.c         |  7 ++++---
>  drivers/gpu/drm/drm_plane.c          |  2 +-
>  drivers/gpu/drm/drm_vblank.c         | 30 ++++++++++++++++++------------
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  4 ++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  4 ++--
>  include/drm/drm_vblank.h             |  8 +++++++-
>  6 files changed, 34 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 366c56fe5f58..08ff1b023f7b 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1815,7 +1815,7 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
>   */
>  
>  static struct drm_pending_vblank_event *create_vblank_event(
> -		struct drm_device *dev, uint64_t user_data)
> +		struct drm_crtc *crtc, uint64_t user_data)
>  {
>  	struct drm_pending_vblank_event *e = NULL;
>  
> @@ -1825,7 +1825,8 @@ static struct drm_pending_vblank_event *create_vblank_event(
>  
>  	e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
>  	e->event.base.length = sizeof(e->event);
> -	e->event.user_data = user_data;
> +	e->event.vbl.crtc_id = crtc->base.id;
> +	e->event.vbl.user_data = user_data;
>  
>  	return e;
>  }
> @@ -2079,7 +2080,7 @@ static int prepare_crtc_signaling(struct drm_device *dev,
>  		if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) {
>  			struct drm_pending_vblank_event *e;
>  
> -			e = create_vblank_event(dev, arg->user_data);
> +			e = create_vblank_event(crtc, arg->user_data);
>  			if (!e)
>  				return -ENOMEM;
>  
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 72cba9805edc..192071209baa 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -1039,7 +1039,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>  		}
>  		e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
>  		e->event.base.length = sizeof(e->event);
> -		e->event.user_data = page_flip->user_data;
> +		e->event.vbl.user_data = page_flip->user_data;
>  		ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
>  		if (ret) {
>  			kfree(e);
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 8c3f2fdd821a..7d2674e0362a 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -823,14 +823,16 @@ static void send_vblank_event(struct drm_device *dev,
>  {
>  	struct timeval tv;
>  
> -	tv = ktime_to_timeval(now);
> -	e->event.sequence = seq;
> -	e->event.tv_sec = tv.tv_sec;
> -	e->event.tv_usec = tv.tv_usec;
> -
> -	trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe,
> -					 e->event.sequence);
> -
> +	switch (e->event.base.type) {
> +	case DRM_EVENT_VBLANK:
> +	case DRM_EVENT_FLIP_COMPLETE:
> +		tv = ktime_to_timeval(now);
> +		e->event.vbl.sequence = seq;
> +		e->event.vbl.tv_sec = tv.tv_sec;
> +		e->event.vbl.tv_usec = tv.tv_usec;
> +		break;
> +	}
> +	trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq);
>  	drm_send_event_locked(dev, &e->base);
>  }
>  
> @@ -882,7 +884,6 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
>  
>  	e->pipe = pipe;
>  	e->sequence = drm_vblank_count(dev, pipe);
> -	e->event.crtc_id = crtc->base.id;
>  	list_add_tail(&e->base.link, &dev->vblank_event_list);
>  }
>  EXPORT_SYMBOL(drm_crtc_arm_vblank_event);
> @@ -913,7 +914,6 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
>  		now = get_drm_timestamp();
>  	}
>  	e->pipe = pipe;
> -	e->event.crtc_id = crtc->base.id;
>  	send_vblank_event(dev, e, seq, now);
>  }
>  EXPORT_SYMBOL(drm_crtc_send_vblank_event);
> @@ -1347,8 +1347,14 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
>  
>  	e->pipe = pipe;
>  	e->event.base.type = DRM_EVENT_VBLANK;
> -	e->event.base.length = sizeof(e->event);
> -	e->event.user_data = vblwait->request.signal;
> +	e->event.base.length = sizeof(e->event.vbl);
> +	e->event.vbl.user_data = vblwait->request.signal;
> +	e->event.vbl.crtc_id = 0;
> +	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> +		struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
> +		if (crtc)
> +			e->event.vbl.crtc_id = crtc->base.id;
> +	}
>  
>  	spin_lock_irqsave(&dev->event_lock, flags);
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> index d1552d3e0652..bc5f6026573d 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> @@ -360,8 +360,8 @@ static int vmw_sou_crtc_page_flip(struct drm_crtc *crtc,
>  
>  		ret = vmw_event_fence_action_queue(file_priv, fence,
>  						   &event->base,
> -						   &event->event.tv_sec,
> -						   &event->event.tv_usec,
> +						   &event->event.vbl.tv_sec,
> +						   &event->event.vbl.tv_usec,
>  						   true);
>  	}
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index ca3afae2db1f..90b5437fd787 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -549,8 +549,8 @@ static int vmw_stdu_crtc_page_flip(struct drm_crtc *crtc,
>  
>  		ret = vmw_event_fence_action_queue(file_priv, fence,
>  						   &event->base,
> -						   &event->event.tv_sec,
> -						   &event->event.tv_usec,
> +						   &event->event.vbl.tv_sec,
> +						   &event->event.vbl.tv_usec,
>  						   true);
>  		vmw_fence_obj_unreference(&fence);
>  	} else {
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index cce53130510f..bf8e07035a0a 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -54,7 +54,10 @@ struct drm_pending_vblank_event {
>  	/**
>  	 * @event: Actual event which will be sent to userspace.
>  	 */
> -	struct drm_event_vblank event;
> +	union {
> +		struct drm_event base;
> +		struct drm_event_vblank vbl;
> +	} event;
>  };
>  
>  /**
> @@ -163,6 +166,9 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
>  			       struct drm_pending_vblank_event *e);
>  void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
>  			      struct drm_pending_vblank_event *e);
> +void drm_vblank_set_event(struct drm_pending_vblank_event *e,
> +			  u64 *seq,
> +			  ktime_t *now);
>  bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
>  bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
>  int drm_crtc_vblank_get(struct drm_crtc *crtc);
> -- 
> 2.13.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS

[PATCH 0/3] drm: Add CRTC-id based ioctls for vblank query/event

[Keith Packard]

This patch series provides a new interface which fixes three issues
with the current VBLANK_WAIT ioctl:

 1) CRTC indices to select a target.
 2) 32-bits of count resolution.
 3) Microsecond time resolution.

The first makes it quite difficult to use this interface from a leased
DRM device; without the ability to see all of the crtcs for a DRM
device, it's not possible to compute the right index into the array of
them for this interface.

2) and 3) prevent the API from matching the requirements for Vulkan,
which asks for 64-bits of counter and nano-second time resolution.

I've split this series into three pieces, the first two adjust the
internal APIs without exposing new functionality, the third adds the
new IOCTLs.

 [PATCH 1/3] drm: Widen vblank count to 64 bits. Change vblank time

This changes all DRM-level internal interfaces to 64-bit vblank
counters and nano-second time resolution. Changing the driver
interface to 64-bits seems like the right plan, but as no drivers
currently support anything wider than 32-bits, it may be that we don't
want to bother at this point.

 [PATCH 2/3] drm: Reorganize drm_pending_event to support future event

This sticks the vblank event in a union inside of the pending event
structure so that I can add another parallel event in the next
patch. Importantly, this required that I move the assignment of the
event crtc_id field from event deliver to event allocation. That
"shouldn't" matter, but it should also be looked at with some care.

 [PATCH 3/3] drm: Add CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls

With the internal APIs ready, this patch is pretty simple.

-keith
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 0/3] drm: Add CRTC-id based ioctls for vblank query/event

[Keith Packard]

Here's an updated series for the proposed new IOCTLs. Major changes
since last time:

 * Leave driver API with 32-bit vblank counts
 * Use ktime_t instead of struct timespec.
 * Check for MODESETTING before using modesetting APIs
 * Ensure vblank is running in new get_sequence ioctl

There are other minor changes noted in each patch.

Thanks to helpful review from:

	Daniel Vetter <daniel@ffwll.ch>
	Michel Dänzer <michel@daenzer.net>
	Ville Syrjälä <ville.syrjala@linux.intel.com>

-keith

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm: Reorganize drm_pending_event to support future event types [v2]

[Keith Packard]

Place drm_event_vblank in a new union that includes that and a bare
drm_event structure. This will allow new members of that union to be
added in the future without changing code related to the existing vbl
event type.

Assignments to the crtc_id field are now done when the event is
allocated, rather than when delievered. This way, delivery doesn't
need to have the crtc ID available.

v2:
 * Remove 'dev' argument from create_vblank_event

	It wasn't being used anyways, and if we need it in the future,
	we can always get it from crtc->dev.

 * Check for MODESETTING before looking for crtc in queue_vblank_event

	UMS drivers will oops if we try to get a crtc, so make sure
	we're modesetting before we try to find a crtc_id to fill into
	the event.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_atomic.c         |  7 ++++---
 drivers/gpu/drm/drm_plane.c          |  2 +-
 drivers/gpu/drm/drm_vblank.c         | 30 ++++++++++++++++++------------
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  4 ++--
 include/drm/drm_vblank.h             |  8 +++++++-
 6 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index c0f336d23f9c..272b83ea9369 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1839,7 +1839,7 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
  */
 
 static struct drm_pending_vblank_event *create_vblank_event(
-		struct drm_device *dev, uint64_t user_data)
+		struct drm_crtc *crtc, uint64_t user_data)
 {
 	struct drm_pending_vblank_event *e = NULL;
 
@@ -1849,7 +1849,8 @@ static struct drm_pending_vblank_event *create_vblank_event(
 
 	e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
 	e->event.base.length = sizeof(e->event);
-	e->event.user_data = user_data;
+	e->event.vbl.crtc_id = crtc->base.id;
+	e->event.vbl.user_data = user_data;
 
 	return e;
 }
@@ -2052,7 +2053,7 @@ static int prepare_crtc_signaling(struct drm_device *dev,
 		if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) {
 			struct drm_pending_vblank_event *e;
 
-			e = create_vblank_event(dev, arg->user_data);
+			e = create_vblank_event(crtc, arg->user_data);
 			if (!e)
 				return -ENOMEM;
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 5dc8c4350602..fe9f31285bc2 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -918,7 +918,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 		}
 		e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
 		e->event.base.length = sizeof(e->event);
-		e->event.user_data = page_flip->user_data;
+		e->event.vbl.user_data = page_flip->user_data;
 		ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
 		if (ret) {
 			kfree(e);
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 346601ad698d..7e7119a5ada3 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -804,14 +804,16 @@ static void send_vblank_event(struct drm_device *dev,
 {
 	struct timeval tv;
 
-	tv = ktime_to_timeval(now);
-	e->event.sequence = seq;
-	e->event.tv_sec = tv.tv_sec;
-	e->event.tv_usec = tv.tv_usec;
-
-	trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe,
-					 e->event.sequence);
-
+	switch (e->event.base.type) {
+	case DRM_EVENT_VBLANK:
+	case DRM_EVENT_FLIP_COMPLETE:
+		tv = ktime_to_timeval(now);
+		e->event.vbl.sequence = seq;
+		e->event.vbl.tv_sec = tv.tv_sec;
+		e->event.vbl.tv_usec = tv.tv_usec;
+		break;
+	}
+	trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq);
 	drm_send_event_locked(dev, &e->base);
 }
 
@@ -863,7 +865,6 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
 
 	e->pipe = pipe;
 	e->sequence = drm_vblank_count(dev, pipe);
-	e->event.crtc_id = crtc->base.id;
 	list_add_tail(&e->base.link, &dev->vblank_event_list);
 }
 EXPORT_SYMBOL(drm_crtc_arm_vblank_event);
@@ -894,7 +895,6 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
 		now = get_drm_timestamp();
 	}
 	e->pipe = pipe;
-	e->event.crtc_id = crtc->base.id;
 	send_vblank_event(dev, e, seq, now);
 }
 EXPORT_SYMBOL(drm_crtc_send_vblank_event);
@@ -1354,8 +1354,14 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
 
 	e->pipe = pipe;
 	e->event.base.type = DRM_EVENT_VBLANK;
-	e->event.base.length = sizeof(e->event);
-	e->event.user_data = vblwait->request.signal;
+	e->event.base.length = sizeof(e->event.vbl);
+	e->event.vbl.user_data = vblwait->request.signal;
+	e->event.vbl.crtc_id = 0;
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
+		if (crtc)
+			e->event.vbl.crtc_id = crtc->base.id;
+	}
 
 	spin_lock_irqsave(&dev->event_lock, flags);
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index 8d7dc9def7c2..c13b97338310 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -358,8 +358,8 @@ static int vmw_sou_crtc_page_flip(struct drm_crtc *crtc,
 
 		ret = vmw_event_fence_action_queue(file_priv, fence,
 						   &event->base,
-						   &event->event.tv_sec,
-						   &event->event.tv_usec,
+						   &event->event.vbl.tv_sec,
+						   &event->event.vbl.tv_usec,
 						   true);
 	}
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index bad31bdf09b6..4e329588ce9c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -544,8 +544,8 @@ static int vmw_stdu_crtc_page_flip(struct drm_crtc *crtc,
 
 		ret = vmw_event_fence_action_queue(file_priv, fence,
 						   &event->base,
-						   &event->event.tv_sec,
-						   &event->event.tv_usec,
+						   &event->event.vbl.tv_sec,
+						   &event->event.vbl.tv_usec,
 						   true);
 		vmw_fence_obj_unreference(&fence);
 	} else {
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index e809ab244919..3013c55aec1d 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -54,7 +54,10 @@ struct drm_pending_vblank_event {
 	/**
 	 * @event: Actual event which will be sent to userspace.
 	 */
-	struct drm_event_vblank event;
+	union {
+		struct drm_event base;
+		struct drm_event_vblank vbl;
+	} event;
 };
 
 /**
@@ -163,6 +166,9 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
 			       struct drm_pending_vblank_event *e);
 void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
 			      struct drm_pending_vblank_event *e);
+void drm_vblank_set_event(struct drm_pending_vblank_event *e,
+			  u64 *seq,
+			  ktime_t *now);
 bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
 bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
 int drm_crtc_vblank_get(struct drm_crtc *crtc);
-- 
2.13.3

Re: [PATCH 2/3] drm: Reorganize drm_pending_event to support future event types [v2]

[Daniel Vetter]

On Mon, Jul 31, 2017 at 10:03:05PM -0700, Keith Packard wrote:
> Place drm_event_vblank in a new union that includes that and a bare
> drm_event structure. This will allow new members of that union to be
> added in the future without changing code related to the existing vbl
> event type.
> 
> Assignments to the crtc_id field are now done when the event is
> allocated, rather than when delievered. This way, delivery doesn't
> need to have the crtc ID available.
> 
> v2:
>  * Remove 'dev' argument from create_vblank_event
> 
> 	It wasn't being used anyways, and if we need it in the future,
> 	we can always get it from crtc->dev.
> 
>  * Check for MODESETTING before looking for crtc in queue_vblank_event
> 
> 	UMS drivers will oops if we try to get a crtc, so make sure
> 	we're modesetting before we try to find a crtc_id to fill into
> 	the event.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  drivers/gpu/drm/drm_atomic.c         |  7 ++++---
>  drivers/gpu/drm/drm_plane.c          |  2 +-
>  drivers/gpu/drm/drm_vblank.c         | 30 ++++++++++++++++++------------
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  4 ++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  4 ++--
>  include/drm/drm_vblank.h             |  8 +++++++-
>  6 files changed, 34 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index c0f336d23f9c..272b83ea9369 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1839,7 +1839,7 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
>   */
>  
>  static struct drm_pending_vblank_event *create_vblank_event(
> -		struct drm_device *dev, uint64_t user_data)
> +		struct drm_crtc *crtc, uint64_t user_data)
>  {
>  	struct drm_pending_vblank_event *e = NULL;
>  
> @@ -1849,7 +1849,8 @@ static struct drm_pending_vblank_event *create_vblank_event(
>  
>  	e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
>  	e->event.base.length = sizeof(e->event);
> -	e->event.user_data = user_data;
> +	e->event.vbl.crtc_id = crtc->base.id;
> +	e->event.vbl.user_data = user_data;
>  
>  	return e;
>  }
> @@ -2052,7 +2053,7 @@ static int prepare_crtc_signaling(struct drm_device *dev,
>  		if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) {
>  			struct drm_pending_vblank_event *e;
>  
> -			e = create_vblank_event(dev, arg->user_data);
> +			e = create_vblank_event(crtc, arg->user_data);
>  			if (!e)
>  				return -ENOMEM;
>  
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 5dc8c4350602..fe9f31285bc2 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -918,7 +918,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>  		}
>  		e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
>  		e->event.base.length = sizeof(e->event);
> -		e->event.user_data = page_flip->user_data;

You missed assigning crtc_id here. With that fixes:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Might also be good to check igt coverage for the various corner-cases
here.

> +		e->event.vbl.user_data = page_flip->user_data;
>  		ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
>  		if (ret) {
>  			kfree(e);
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 346601ad698d..7e7119a5ada3 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -804,14 +804,16 @@ static void send_vblank_event(struct drm_device *dev,
>  {
>  	struct timeval tv;
>  
> -	tv = ktime_to_timeval(now);
> -	e->event.sequence = seq;
> -	e->event.tv_sec = tv.tv_sec;
> -	e->event.tv_usec = tv.tv_usec;
> -
> -	trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe,
> -					 e->event.sequence);
> -
> +	switch (e->event.base.type) {
> +	case DRM_EVENT_VBLANK:
> +	case DRM_EVENT_FLIP_COMPLETE:
> +		tv = ktime_to_timeval(now);
> +		e->event.vbl.sequence = seq;
> +		e->event.vbl.tv_sec = tv.tv_sec;
> +		e->event.vbl.tv_usec = tv.tv_usec;
> +		break;
> +	}
> +	trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq);
>  	drm_send_event_locked(dev, &e->base);
>  }
>  
> @@ -863,7 +865,6 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
>  
>  	e->pipe = pipe;
>  	e->sequence = drm_vblank_count(dev, pipe);
> -	e->event.crtc_id = crtc->base.id;
>  	list_add_tail(&e->base.link, &dev->vblank_event_list);
>  }
>  EXPORT_SYMBOL(drm_crtc_arm_vblank_event);
> @@ -894,7 +895,6 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
>  		now = get_drm_timestamp();
>  	}
>  	e->pipe = pipe;
> -	e->event.crtc_id = crtc->base.id;
>  	send_vblank_event(dev, e, seq, now);
>  }
>  EXPORT_SYMBOL(drm_crtc_send_vblank_event);
> @@ -1354,8 +1354,14 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
>  
>  	e->pipe = pipe;
>  	e->event.base.type = DRM_EVENT_VBLANK;
> -	e->event.base.length = sizeof(e->event);
> -	e->event.user_data = vblwait->request.signal;
> +	e->event.base.length = sizeof(e->event.vbl);
> +	e->event.vbl.user_data = vblwait->request.signal;
> +	e->event.vbl.crtc_id = 0;
> +	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> +		struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
> +		if (crtc)
> +			e->event.vbl.crtc_id = crtc->base.id;
> +	}
>  
>  	spin_lock_irqsave(&dev->event_lock, flags);
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> index 8d7dc9def7c2..c13b97338310 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> @@ -358,8 +358,8 @@ static int vmw_sou_crtc_page_flip(struct drm_crtc *crtc,
>  
>  		ret = vmw_event_fence_action_queue(file_priv, fence,
>  						   &event->base,
> -						   &event->event.tv_sec,
> -						   &event->event.tv_usec,
> +						   &event->event.vbl.tv_sec,
> +						   &event->event.vbl.tv_usec,
>  						   true);
>  	}
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index bad31bdf09b6..4e329588ce9c 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -544,8 +544,8 @@ static int vmw_stdu_crtc_page_flip(struct drm_crtc *crtc,
>  
>  		ret = vmw_event_fence_action_queue(file_priv, fence,
>  						   &event->base,
> -						   &event->event.tv_sec,
> -						   &event->event.tv_usec,
> +						   &event->event.vbl.tv_sec,
> +						   &event->event.vbl.tv_usec,
>  						   true);
>  		vmw_fence_obj_unreference(&fence);
>  	} else {
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index e809ab244919..3013c55aec1d 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -54,7 +54,10 @@ struct drm_pending_vblank_event {
>  	/**
>  	 * @event: Actual event which will be sent to userspace.
>  	 */
> -	struct drm_event_vblank event;
> +	union {
> +		struct drm_event base;
> +		struct drm_event_vblank vbl;
> +	} event;
>  };
>  
>  /**
> @@ -163,6 +166,9 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
>  			       struct drm_pending_vblank_event *e);
>  void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
>  			      struct drm_pending_vblank_event *e);
> +void drm_vblank_set_event(struct drm_pending_vblank_event *e,
> +			  u64 *seq,
> +			  ktime_t *now);
>  bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
>  bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
>  int drm_crtc_vblank_get(struct drm_crtc *crtc);
> -- 
> 2.13.3
> 

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