On 25/03/2019 08:32, Chunming Zhou wrote:
v2: individually allocate chain array, since chain node is free independently.
v3: all existing points must be already signaled before cpu perform signal 
operation,
     so add check condition for that.
v4: remove v3 change and add checking to prevent out-of-order
v5: unify binary and timeline

Signed-off-by: Chunming Zhou <david1.z...@amd.com>
Cc: Tobias Hector <tobias.hec...@amd.com>
Cc: Jason Ekstrand <ja...@jlekstrand.net>
Cc: Dave Airlie <airl...@redhat.com>
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwer...@intel.com>
---
  drivers/gpu/drm/drm_internal.h |  2 +
  drivers/gpu/drm/drm_ioctl.c    |  2 +
  drivers/gpu/drm/drm_syncobj.c  | 73 ++++++++++++++++++++++++++++++++++
  include/uapi/drm/drm.h         |  1 +
  4 files changed, 78 insertions(+)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index dd11ae5f1eef..d9a483a5fce0 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -190,6 +190,8 @@ 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,
                             struct drm_file *file_private);
+int drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data,
+                                     struct drm_file *file_private);
  int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
                            struct drm_file *file_private);
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 92b3b7b2fd81..d337f161909c 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -696,6 +696,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_SYNCOBJ_TIMELINE_SIGNAL, 
drm_syncobj_timeline_signal_ioctl,
+                     DRM_UNLOCKED|DRM_RENDER_ALLOW),
        DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl,
                      DRM_UNLOCKED|DRM_RENDER_ALLOW),
        DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, 
DRM_UNLOCKED),
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index ee2d66e047e7..099596190845 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1183,6 +1183,79 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void 
*data,
        return ret;
  }
+int
+drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data,
+                                 struct drm_file *file_private)
+{
+       struct drm_syncobj_timeline_array *args = data;
+       struct drm_syncobj **syncobjs;
+       struct dma_fence_chain **chains;
+       uint64_t *points;
+       uint32_t i, j;
+       int ret;
+
+       if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+               return -EOPNOTSUPP;
+
+       if (args->pad != 0)
+               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;
+
+       points = kmalloc_array(args->count_handles, sizeof(*points),
+                              GFP_KERNEL);
+       if (!points) {
+               ret = -ENOMEM;
+               goto out;
+       }
+       if (!u64_to_user_ptr(args->points)) {
+               memset(points, 0, args->count_handles * sizeof(uint64_t));
+       } else if (copy_from_user(points, u64_to_user_ptr(args->points),
+                                 sizeof(uint64_t) * args->count_handles)) {
+               ret = -EFAULT;
+               goto err_points;
+       }
+
+       chains = kmalloc_array(args->count_handles, sizeof(void *), GFP_KERNEL);
+       if (!chains) {
+               ret = -ENOMEM;
+               goto err_points;
+       }
+       for (i = 0; i < args->count_handles; i++) {
+               chains[i] = kzalloc(sizeof(struct dma_fence_chain), GFP_KERNEL);
+               if (!chains[i]) {
+                       for (j = 0; j < i; j++)
+                               kfree(chains[j]);
+                       ret = -ENOMEM;
+                       goto err_chains;
+               }
+       }
+
+       for (i = 0; i < args->count_handles; i++) {
+               struct dma_fence *fence = dma_fence_get_stub();
+
+               drm_syncobj_add_point(syncobjs[i], chains[i],
+                                     fence, points[i]);
+               dma_fence_put(fence);
+       }


I think I found an issue where the implementation doesn't match what the spec requires on host signaling.

Consider the following :


Timeline semaphore has a value of 4.

A device submits a commands to signal point 5.

The host signals point 6.


The value of the timeline will remain 4 until the device commands complete, at which point it will change to 6.

But the specification seems to say that the timeline value should be 6 once the host signaling completes.


The only option that I see working would be to add point 6 in the timeline and wait on that point to complete.


Christian, David, what do you think?


Thanks,


-Lionel


+err_chains:
+       kfree(chains);
+err_points:
+       kfree(points);
+out:
+       drm_syncobj_array_free(syncobjs, args->count_handles);
+
+       return ret;
+}
+
  int drm_syncobj_query_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 e8d0d6b51875..236b01a1fabf 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -943,6 +943,7 @@ 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_array)
  #define DRM_IOCTL_SYNCOBJ_TRANSFER    DRM_IOWR(0xCC, struct 
drm_syncobj_transfer)
+#define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL      DRM_IOWR(0xCD, struct 
drm_syncobj_timeline_array)
/**
   * Device specific ioctls should only be in their respective headers


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

Reply via email to