[RFC 7/7] drm: make the callers of drm_wait_vblank() allocate the memory

[Paulo Zanoni <przanoni@gmail.com>]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

This way, the Kernel users will be able to fully control the lifetime
of struct drm_vblank_wait_item, and they will also be able to wrap it
to store their own information. As a result, one less memory
allocation will happen, and the Kernel codepath will not set all those
drm_pending_vblank_event struct fields.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/drm_irq.c           | 92 ++++++++++++++++++++-----------------
 drivers/gpu/drm/i915/i915_debugfs.c | 11 ++---
 include/drm/drmP.h                  |  2 +-
 3 files changed, 57 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index dd091c3..5fa5431 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1402,36 +1402,26 @@ static void drm_vblank_event_work_func(struct work_struct *work)
 
 static int drm_queue_vblank_event(struct drm_device *dev, int pipe,
 				  union drm_wait_vblank *vblwait,
-				  struct drm_file *file_priv,
 				  bool callback_from_work,
-				  drm_vblank_callback_t callback)
+				  drm_vblank_callback_t callback,
+				  struct drm_vblank_wait_item *item)
 {
 	struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
-	struct drm_pending_vblank_event *e;
 	struct timeval now;
 	unsigned long flags;
 	unsigned int seq;
 	int ret;
 
-	e = kzalloc(sizeof *e, GFP_KERNEL);
-	if (e == NULL) {
-		ret = -ENOMEM;
+	if (WARN_ON(!item)) {
+		ret = -EINVAL;
 		goto err_put;
 	}
 
-	e->item.pipe = pipe;
-	e->base.pid = current->pid;
-	e->event.base.type = DRM_EVENT_VBLANK;
-	e->event.base.length = sizeof e->event;
-	e->event.user_data = vblwait->request.signal;
-	e->base.event = &e->event.base;
-	e->base.file_priv = file_priv;
-	e->base.destroy = (void (*) (struct drm_pending_event *)) kfree;
-	e->item.from_user_space = true;
-	e->item.callback = callback;
-	e->item.callback_from_work = callback_from_work;
+	item->pipe = pipe;
+	item->callback = callback;
+	item->callback_from_work = callback_from_work;
 	if (callback_from_work)
-		INIT_WORK(&e->item.callback_work, drm_vblank_event_work_func);
+		INIT_WORK(&item->callback_work, drm_vblank_event_work_func);
 
 	spin_lock_irqsave(&dev->event_lock, flags);
 
@@ -1447,15 +1437,6 @@ static int drm_queue_vblank_event(struct drm_device *dev, int pipe,
 		goto err_unlock;
 	}
 
-	if (file_priv) {
-		if (file_priv->event_space < sizeof e->event) {
-			ret = -EBUSY;
-			goto err_unlock;
-		}
-
-		file_priv->event_space -= sizeof e->event;
-	}
-
 	seq = drm_vblank_count_and_time(dev, pipe, &now);
 
 	if ((vblwait->request.type & _DRM_VBLANK_NEXTONMISS) &&
@@ -1470,14 +1451,14 @@ static int drm_queue_vblank_event(struct drm_device *dev, int pipe,
 	trace_drm_vblank_event_queued(current->pid, pipe,
 				      vblwait->request.sequence);
 
-	e->item.wanted_seq = vblwait->request.sequence;
+	item->wanted_seq = vblwait->request.sequence;
 	if ((seq - vblwait->request.sequence) <= (1 << 23)) {
 		drm_vblank_put(dev, pipe);
-		drm_wait_vblank_callback(dev, &e->item, seq, &now, false);
+		drm_wait_vblank_callback(dev, item, seq, &now, false);
 		vblwait->reply.sequence = seq;
 	} else {
 		/* drm_handle_vblank_events will call drm_vblank_put */
-		list_add_tail(&e->item.link, &dev->vblank_event_list);
+		list_add_tail(&item->link, &dev->vblank_event_list);
 		vblwait->reply.sequence = vblwait->request.sequence;
 	}
 
@@ -1487,7 +1468,6 @@ static int drm_queue_vblank_event(struct drm_device *dev, int pipe,
 
 err_unlock:
 	spin_unlock_irqrestore(&dev->event_lock, flags);
-	kfree(e);
 err_put:
 	drm_vblank_put(dev, pipe);
 	return ret;
@@ -1509,9 +1489,9 @@ err_put:
  */
 static int __drm_wait_vblank(struct drm_device *dev,
 			     union drm_wait_vblank *vblwait,
-			     struct drm_file *file_priv,
 			     bool callback_from_work,
-			     drm_vblank_callback_t callback)
+			     drm_vblank_callback_t callback,
+			     struct drm_vblank_wait_item *item)
 {
 	struct drm_vblank_crtc *vblank;
 	int ret;
@@ -1566,8 +1546,9 @@ static int __drm_wait_vblank(struct drm_device *dev,
 		/* must hold on to the vblank ref until the event fires
 		 * drm_vblank_put will be called asynchronously
 		 */
-		return drm_queue_vblank_event(dev, crtc, vblwait, file_priv,
-					      callback_from_work, callback);
+		return drm_queue_vblank_event(dev, crtc, vblwait,
+					      callback_from_work, callback,
+					      item);
 	}
 
 	if ((flags & _DRM_VBLANK_NEXTONMISS) &&
@@ -1606,15 +1587,44 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *file_priv)
 {
 	union drm_wait_vblank *vblwait = data;
+	struct drm_pending_vblank_event *e = NULL;
+	unsigned int flags = vblwait->request.type & _DRM_VBLANK_FLAGS_MASK;
+	int ret;
+
+	if (flags & _DRM_VBLANK_EVENT) {
+		e = kzalloc(sizeof *e, GFP_KERNEL);
+		if (e == NULL)
+			return -ENOMEM;
+
+		e->base.pid = current->pid;
+		e->event.base.type = DRM_EVENT_VBLANK;
+		e->event.base.length = sizeof e->event;
+		e->event.user_data = vblwait->request.signal;
+		e->base.event = &e->event.base;
+		e->base.file_priv = file_priv;
+		e->base.destroy = (void (*) (struct drm_pending_event *)) kfree;
+		e->item.from_user_space = true;
 
-	return __drm_wait_vblank(dev, vblwait, file_priv, false,
-				 send_vblank_event);
+		if (file_priv->event_space < sizeof e->event) {
+			kfree(e);
+			return -EBUSY;
+		}
+		file_priv->event_space -= sizeof e->event;
+	}
+
+	ret = __drm_wait_vblank(dev, vblwait, false, send_vblank_event,
+				&e->item);
+
+	if (ret && e)
+		kfree(e);
+
+	return ret;
 }
 
 int drm_wait_vblank_kernel(struct drm_crtc *crtc, int count, bool absolute,
 			   bool callback_from_work,
 			   drm_vblank_callback_t callback,
-			   unsigned long user_data)
+			   struct drm_vblank_wait_item *item)
 {
 	struct drm_device *dev = crtc->dev;
 	union drm_wait_vblank vblwait;
@@ -1627,10 +1637,10 @@ int drm_wait_vblank_kernel(struct drm_crtc *crtc, int count, bool absolute,
 
 	vblwait.request.type = type;
 	vblwait.request.sequence = count;
-	vblwait.request.signal = user_data;
+	vblwait.request.signal = 0;
 
-	return __drm_wait_vblank(dev, &vblwait, NULL, callback_from_work,
-				 callback);
+	return __drm_wait_vblank(dev, &vblwait, callback_from_work, callback,
+				 item);
 }
 EXPORT_SYMBOL(drm_wait_vblank_kernel);
 
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b5c3f81..d2f3ca9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2719,6 +2719,7 @@ struct vblank_data {
 	int eight;
 	const char *message;
 	bool can_sleep;
+	struct drm_vblank_wait_item item;
 };
 
 static void vblank_callback(struct drm_device *dev,
@@ -2726,16 +2727,14 @@ static void vblank_callback(struct drm_device *dev,
 			    unsigned long seq, struct timeval *now,
 			    bool premature)
 {
-	struct drm_pending_vblank_event *e =
-		container_of(item, struct drm_pending_vblank_event, item);
-	struct vblank_data *data = (struct vblank_data *)e->event.user_data;
+	struct vblank_data *data =
+		container_of(item, struct vblank_data, item);
 
 	WARN_ON(data->can_sleep != drm_can_sleep());
 	WARN_ON(data->eight != 8);
 	DRM_DEBUG_KMS("vblank callback, seq: %lu, premature: %s message:%s\n",
 		      seq, yesno(premature), data->message);
 
-	e->base.destroy(&e->base);
 	kfree(data);
 }
 
@@ -2783,7 +2782,7 @@ static int i915_vblank_queue_test_new(struct seq_file *m, void *unused)
 
 	DRM_DEBUG_KMS("scheduling 60 vblanks (with callback, can't sleep)\n");
 	ret = drm_wait_vblank_kernel(&crtc->base, 60, false, false,
-				     vblank_callback, (unsigned long) data1);
+				     vblank_callback, &data1->item);
 	if (ret) {
 		DRM_DEBUG_KMS("vblank schedule error: %d\n", ret);
 		kfree(data1);
@@ -2794,7 +2793,7 @@ static int i915_vblank_queue_test_new(struct seq_file *m, void *unused)
 
 	DRM_DEBUG_KMS("scheduling 60 vblanks (with callback, can sleep)\n");
 	ret = drm_wait_vblank_kernel(&crtc->base, 60, false, true,
-				     vblank_callback, (unsigned long) data2);
+				     vblank_callback, &data2->item);
 	if (ret) {
 		DRM_DEBUG_KMS("vblank schedule error: %d\n", ret);
 		kfree(data2);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 46724d9..55d73d0 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -926,7 +926,7 @@ extern int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
 extern int drm_wait_vblank_kernel(struct drm_crtc *crtc, int count,
 				  bool absolute, bool callback_from_work,
 				  drm_vblank_callback_t callback,
-				  unsigned long user_data);
+				  struct drm_vblank_wait_item *item);
 extern u32 drm_vblank_count(struct drm_device *dev, int crtc);
 extern u32 drm_vblank_count_and_time(struct drm_device *dev, int crtc,
 				     struct timeval *vblanktime);
-- 
2.1.1

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