Re: [PATCH 3/5] drm: add timeline support for syncobj export/import

zhoucm1 <zhoucm1@xxxxxxx> · Fri, 2 Nov 2018 17:42:04 +0800

On 2018年11月02日 16:46, Christian König wrote:
Am 02.11.18 um 09:25 schrieb Chunming Zhou:
user space can specify timeline point fence to export/import.

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>

From the coding it looks good to me, but I can't judge if that really 
makes sense to export/import individual points.

I would have rather expected that always the whole timeline is 
exported/imported. Otherwise the whole thing with waiting for sync 
points to appear doesn't really make sense to me.
We can Cpu wait on individual points, why can not we export/import them?
I confirmed with Daniel before, he said the extension need both them, 
export/import individual points and export/import the whole timeline 
semaphore.
More discussion is 
https://gitlab.khronos.org/vulkan/vulkan/merge_requests/2696

Thanks,
David Zhou

Christian.

---
  drivers/gpu/drm/drm_internal.h |  4 ++
  drivers/gpu/drm/drm_ioctl.c    |  4 ++
  drivers/gpu/drm/drm_syncobj.c  | 76 ++++++++++++++++++++++++++++++----
  include/uapi/drm/drm.h         | 11 +++++
  4 files changed, 88 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_internal.h 
b/drivers/gpu/drm/drm_internal.h
index 9c4826411a3c..5ad6cbdb68ab 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -181,6 +181,10 @@ int drm_syncobj_handle_to_fd_ioctl(struct 
drm_device *dev, void *data,
                     struct drm_file *file_private);
  int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
                     struct drm_file *file_private);
+int drm_syncobj_handle_to_fd_ioctl2(struct drm_device *dev, void *data,
+                    struct drm_file *file_private);
+int drm_syncobj_fd_to_handle_ioctl2(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,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 7578ef6dc1d1..364d26e949cf 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -673,6 +673,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
                DRM_UNLOCKED|DRM_RENDER_ALLOW),
      DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE, 
drm_syncobj_fd_to_handle_ioctl,
                DRM_UNLOCKED|DRM_RENDER_ALLOW),
+    DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD2, 
drm_syncobj_handle_to_fd_ioctl2,
+              DRM_UNLOCKED|DRM_RENDER_ALLOW),
+    DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE2, 
drm_syncobj_fd_to_handle_ioctl2,
+              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,
diff --git a/drivers/gpu/drm/drm_syncobj.c 
b/drivers/gpu/drm/drm_syncobj.c
index 9ad58d0d21cd..dffc42ba2f91 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -677,7 +677,7 @@ static int drm_syncobj_fd_to_handle(struct 
drm_file *file_private,
  }
    static int drm_syncobj_import_sync_file_fence(struct drm_file 
*file_private,
-                          int fd, int handle)
+                          int fd, int handle, uint64_t point)
  {
      struct dma_fence *fence = sync_file_get_fence(fd);
      struct drm_syncobj *syncobj;
@@ -691,14 +691,14 @@ static int 
drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
          return -ENOENT;
      }
  -    drm_syncobj_replace_fence(syncobj, 0, fence);
+    drm_syncobj_replace_fence(syncobj, point, fence);
      dma_fence_put(fence);
      drm_syncobj_put(syncobj);
      return 0;
  }
    static int drm_syncobj_export_sync_file(struct drm_file 
*file_private,
-                    int handle, int *p_fd)
+                    int handle, uint64_t point, int *p_fd)
  {
      int ret;
      struct dma_fence *fence;
@@ -708,7 +708,7 @@ static int drm_syncobj_export_sync_file(struct 
drm_file *file_private,
      if (fd < 0)
          return fd;
  -    ret = drm_syncobj_find_fence(file_private, handle, 0, 0, &fence);
+    ret = drm_syncobj_find_fence(file_private, handle, point, 0, 
&fence);
      if (ret)
          goto err_put_fd;
  @@ -817,9 +817,14 @@ drm_syncobj_handle_to_fd_ioctl(struct 
drm_device *dev, void *data,
          args->flags != 
DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
          return -EINVAL;
  -    if (args->flags & 
DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
+    if (args->flags & 
DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE) {
+        struct drm_syncobj *syncobj = drm_syncobj_find(file_private,
+                                   args->handle);
+        if (!syncobj || syncobj->type != DRM_SYNCOBJ_TYPE_BINARY)
+            return -EINVAL;
          return drm_syncobj_export_sync_file(file_private, 
args->handle,
-                            &args->fd);
+                            0, &args->fd);
+    }
        return drm_syncobj_handle_to_fd(file_private, args->handle,
                      &args->fd);
@@ -841,15 +846,72 @@ drm_syncobj_fd_to_handle_ioctl(struct 
drm_device *dev, void *data,
          args->flags != 
DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE)
          return -EINVAL;
  +    if (args->flags & 
DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE) {
+        struct drm_syncobj *syncobj = drm_syncobj_find(file_private,
+                                   args->handle);
+        if (!syncobj || syncobj->type != DRM_SYNCOBJ_TYPE_BINARY)
+            return -EINVAL;
+        return drm_syncobj_import_sync_file_fence(file_private,
+                              args->fd,
+                              args->handle,
+                              0);
+    }
+
+    return drm_syncobj_fd_to_handle(file_private, args->fd,
+                    &args->handle);
+}
+
+int
+drm_syncobj_handle_to_fd_ioctl2(struct drm_device *dev, void *data,
+                   struct drm_file *file_private)
+{
+    struct drm_syncobj_handle2 *args = data;
+
+    if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+        return -ENODEV;
+
+    if (args->pad)
+        return -EINVAL;
+
+    if (args->flags != 0 &&
+        args->flags != DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
+        return -EINVAL;
+
+    if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
+        return drm_syncobj_export_sync_file(file_private, args->handle,
+                            args->point, &args->fd);
+
+    return drm_syncobj_handle_to_fd(file_private, args->handle,
+                    &args->fd);
+}
+
+int
+drm_syncobj_fd_to_handle_ioctl2(struct drm_device *dev, void *data,
+                   struct drm_file *file_private)
+{
+    struct drm_syncobj_handle2 *args = data;
+
+    if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+        return -ENODEV;
+
+    if (args->pad)
+        return -EINVAL;
+
+    if (args->flags != 0 &&
+        args->flags != DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE)
+        return -EINVAL;
+
      if (args->flags & DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE)
          return drm_syncobj_import_sync_file_fence(file_private,
                                args->fd,
-                              args->handle);
+                              args->handle,
+                              args->point);
        return drm_syncobj_fd_to_handle(file_private, args->fd,
                      &args->handle);
  }
  +
  struct syncobj_wait_entry {
      struct task_struct *task;
      struct dma_fence *fence;
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 23c4979d8a1c..da5df2a42fc3 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -735,6 +735,15 @@ struct drm_syncobj_handle {
      __s32 fd;
      __u32 pad;
  };
+struct drm_syncobj_handle2 {
+    __u32 handle;
+    __u32 flags;
+    __u64 point;
+
+    __s32 fd;
+    __u32 pad;
+};
+
    #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0)
  #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1)
@@ -938,6 +947,8 @@ extern "C" {
    #define DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT    DRM_IOWR(0xCA, struct 
drm_syncobj_timeline_wait)
  #define DRM_IOCTL_SYNCOBJ_QUERY        DRM_IOWR(0xCB, struct 
drm_syncobj_timeline_query)
+#define DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD2    DRM_IOWR(0xCC, struct 
drm_syncobj_handle2)
+#define DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE2    DRM_IOWR(0xCD, struct 
drm_syncobj_handle2)
    /**
   * Device specific ioctls should only be in their respective headers


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







