Re: [PATCH 1/5] drm: add support of syncobj timeline point wait v4

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 02.11.18 um 09:25 schrieb Chunming Zhou:
points array is one-to-one match with syncobjs array.
v2:
add seperate ioctl for timeline point wait, otherwise break uapi.
v3:
userspace can specify two kinds waits::
a. Wait for time point to be completed.
b. and wait for time point to become available
v4:
rebase

Signed-off-by: Chunming Zhou <david1.zhou@xxxxxxx>
Cc: Daniel Rakos <Daniel.Rakos@xxxxxxx>
Cc: Jason Ekstrand <jason@xxxxxxxxxxxxxx>
Cc: Bas Nieuwenhuizen <bas@xxxxxxxxxxxxxxxxxxx>
Cc: Dave Airlie <airlied@xxxxxxxxxx>
Cc: Christian König <christian.koenig@xxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/drm_internal.h |   2 +
  drivers/gpu/drm/drm_ioctl.c    |   2 +
  drivers/gpu/drm/drm_syncobj.c  | 115 ++++++++++++++++++++++++++++-----
  include/uapi/drm/drm.h         |  18 ++++++
  4 files changed, 121 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 0c4eb4a9ab31..566d44e3c782 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
  				   struct drm_file *file_private);
  int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
  			   struct drm_file *file_private);
+int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
+				    struct drm_file *file_private);
  int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
  			    struct drm_file *file_private);
  int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 94bd872d56c4..a9a17ed35cc4 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -675,6 +675,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  		      DRM_UNLOCKED|DRM_RENDER_ALLOW),
  	DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
  		      DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, drm_syncobj_timeline_wait_ioctl,
+		      DRM_UNLOCKED|DRM_RENDER_ALLOW),
  	DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl,
  		      DRM_UNLOCKED|DRM_RENDER_ALLOW),
  	DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 4dca5f7e8c4b..9dc54a345480 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -153,11 +153,12 @@ static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
  }
static void drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
+						  uint64_t point,
  						  struct dma_fence **fence,
  						  struct drm_syncobj_cb *cb,
  						  drm_syncobj_func_t func)
  {
-	u64 pt_value = 0;
+	u64 pt_value = point;
WARN_ON(*fence); @@ -854,6 +855,7 @@ struct syncobj_wait_entry {
  	struct dma_fence *fence;
  	struct dma_fence_cb fence_cb;
  	struct drm_syncobj_cb syncobj_cb;
+	u64    point;
  };
static void syncobj_wait_fence_func(struct dma_fence *fence,
@@ -871,12 +873,13 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj,
  	struct syncobj_wait_entry *wait =
  		container_of(cb, struct syncobj_wait_entry, syncobj_cb);
- drm_syncobj_search_fence(syncobj, 0, 0, &wait->fence);
+	drm_syncobj_search_fence(syncobj, wait->point, 0, &wait->fence);
wake_up_process(wait->task);
  }
static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
+						  void __user *user_points,
  						  uint32_t count,
  						  uint32_t flags,
  						  signed long timeout,
@@ -884,12 +887,37 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
  {
  	struct syncobj_wait_entry *entries;
  	struct dma_fence *fence;
+	uint64_t *points;
  	uint32_t signaled_count, i;
- entries = kcalloc(count, sizeof(*entries), GFP_KERNEL);
-	if (!entries)
+	points = kmalloc_array(count, sizeof(*points), GFP_KERNEL);
+	if (points == NULL)
  		return -ENOMEM;
+ if (!user_points) {
+		memset(points, 0, count * sizeof(uint64_t));
+	} else if (copy_from_user(points, user_points, sizeof(uint64_t) * count)) {
+		timeout = -EFAULT;
+		goto err_free_points;
+	}
+
+

Double empty line here.

+	if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE) {
+		struct dma_fence *fence;
+		for (i = 0; i < count; ++i) {
+			timeout = drm_syncobj_search_fence(syncobjs[i], points[i],
+							   DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+							   &fence);
+			if (timeout)
+				goto err_free_points;
+		}
+		goto err_free_points;

A comment would be nice here to note that we only wait for the fences to become available.

Alternatively it might be a good idea to make this an separate IOCTL.

Apart from those nit picks looks quite good to me,
Christian.

+	}
+	entries = kcalloc(count, sizeof(*entries), GFP_KERNEL);
+	if (!entries) {
+		timeout = -ENOMEM;
+		goto err_free_points;
+	}
  	/* Walk the list of sync objects and initialize entries.  We do
  	 * this up-front so that we can properly return -EINVAL if there is
  	 * a syncobj with a missing fence and then never have the chance of
@@ -898,7 +926,8 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
  	signaled_count = 0;
  	for (i = 0; i < count; ++i) {
  		entries[i].task = current;
-		drm_syncobj_search_fence(syncobjs[i], 0, 0,
+		entries[i].point = points[i];
+		drm_syncobj_search_fence(syncobjs[i], points[i], 0,
  					 &entries[i].fence);
  		if (!entries[i].fence) {
  			if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
@@ -934,6 +963,7 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
  				continue;
drm_syncobj_fence_get_or_add_callback(syncobjs[i],
+							      entries[i].point,
  							      &entries[i].fence,
  							      &entries[i].syncobj_cb,
  							      syncobj_wait_syncobj_func);
@@ -996,6 +1026,9 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
  	}
  	kfree(entries);
+err_free_points:
+	kfree(points);
+
  	return timeout;
  }
@@ -1034,19 +1067,33 @@ static signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec)
  static int drm_syncobj_array_wait(struct drm_device *dev,
  				  struct drm_file *file_private,
  				  struct drm_syncobj_wait *wait,
-				  struct drm_syncobj **syncobjs)
+				  struct drm_syncobj_timeline_wait *timeline_wait,
+				  struct drm_syncobj **syncobjs, bool timeline)
  {
-	signed long timeout = drm_timeout_abs_to_jiffies(wait->timeout_nsec);
+	signed long timeout = 0;
  	uint32_t first = ~0;
- timeout = drm_syncobj_array_wait_timeout(syncobjs,
-						 wait->count_handles,
-						 wait->flags,
-						 timeout, &first);
-	if (timeout < 0)
-		return timeout;
-
-	wait->first_signaled = first;
+	if (!timeline) {
+		timeout = drm_timeout_abs_to_jiffies(wait->timeout_nsec);
+		timeout = drm_syncobj_array_wait_timeout(syncobjs,
+							 NULL,
+							 wait->count_handles,
+							 wait->flags,
+							 timeout, &first);
+		if (timeout < 0)
+			return timeout;
+		wait->first_signaled = first;
+	} else {
+		timeout = drm_timeout_abs_to_jiffies(timeline_wait->timeout_nsec);
+		timeout = drm_syncobj_array_wait_timeout(syncobjs,
+							 u64_to_user_ptr(timeline_wait->points),
+							 timeline_wait->count_handles,
+							 timeline_wait->flags,
+							 timeout, &first);
+		if (timeout < 0)
+			return timeout;
+		timeline_wait->first_signaled = first;
+	}
  	return 0;
  }
@@ -1132,13 +1179,49 @@ drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
  		return ret;
ret = drm_syncobj_array_wait(dev, file_private,
-				     args, syncobjs);
+				     args, NULL, syncobjs, false);
drm_syncobj_array_free(syncobjs, args->count_handles); return ret;
  }
+int
+drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
+				struct drm_file *file_private)
+{
+	struct drm_syncobj_timeline_wait *args = data;
+	struct drm_syncobj **syncobjs;
+	int ret = 0;
+
+	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+		return -ENODEV;
+
+	if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
+			    DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
+			    DRM_SYNCOBJ_WAIT_FLAGS_WAIT_COMPLETED |
+			    DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE))
+		return -EINVAL;
+
+	if (args->count_handles == 0)
+		return -EINVAL;
+
+	ret = drm_syncobj_array_find(file_private,
+				     u64_to_user_ptr(args->handles),
+				     args->count_handles,
+				     &syncobjs);
+	if (ret < 0)
+		return ret;
+
+	ret = drm_syncobj_array_wait(dev, file_private,
+				     NULL, args, syncobjs, true);
+
+	drm_syncobj_array_free(syncobjs, args->count_handles);
+
+	return ret;
+}
+
+
  int
  drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
  			struct drm_file *file_private)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index cebdb2541eb7..c8bc1414753d 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -738,6 +738,10 @@ struct drm_syncobj_handle {
#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0)
  #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1)
+/* wait for time point to be completed */
+#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_COMPLETED (1 << 2)
+/* wait for time point to become available */
+#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE (1 << 3)
  struct drm_syncobj_wait {
  	__u64 handles;
  	/* absolute timeout */
@@ -748,6 +752,19 @@ struct drm_syncobj_wait {
  	__u32 pad;
  };
+struct drm_syncobj_timeline_wait {
+	__u64 handles;
+	/* wait on specific timeline point for every handles*/
+	__u64 points;
+	/* absolute timeout */
+	__s64 timeout_nsec;
+	__u32 count_handles;
+	__u32 flags;
+	__u32 first_signaled; /* only valid when not waiting all */
+	__u32 pad;
+};
+
+
  struct drm_syncobj_array {
  	__u64 handles;
  	__u32 count_handles;
@@ -910,6 +927,7 @@ extern "C" {
  #define DRM_IOCTL_MODE_GET_LEASE	DRM_IOWR(0xC8, struct drm_mode_get_lease)
  #define DRM_IOCTL_MODE_REVOKE_LEASE	DRM_IOWR(0xC9, struct drm_mode_revoke_lease)
+#define DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT DRM_IOWR(0xCA, struct drm_syncobj_timeline_wait)
  /**
   * Device specific ioctls should only be in their respective headers
   * The device specific ioctl range is from 0x40 to 0x9f.

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux