[PATCH 2/5] drm/syncobj: add sync obj wait interface. (v3)

Dave Airlie airlied at gmail.com
Mon May 29 04:39:19 UTC 2017


On 25 May 2017 at 03:25, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Wed, May 24, 2017 at 12:06 AM, Dave Airlie <airlied at gmail.com> wrote:
>>
>> From: Dave Airlie <airlied at redhat.com>
>>
>> This interface will allow sync object to be used to back
>> Vulkan fences. This API is pretty much the vulkan fence waiting
>> API, and I've ported the code from amdgpu.
>>
>> v2: accept relative timeout, pass remaining time back
>> to userspace.
>> v3: return to absolute timeouts.
>>
>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>> ---
>>  drivers/gpu/drm/drm_internal.h |   2 +
>>  drivers/gpu/drm/drm_ioctl.c    |   2 +
>>  drivers/gpu/drm/drm_syncobj.c  | 164
>> +++++++++++++++++++++++++++++++++++++++++
>>  include/uapi/drm/drm.h         |  14 ++++
>>  4 files changed, 182 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_internal.h
>> b/drivers/gpu/drm/drm_internal.h
>> index 3fdef2c..53e3f6b 100644
>> --- a/drivers/gpu/drm/drm_internal.h
>> +++ b/drivers/gpu/drm/drm_internal.h
>> @@ -156,3 +156,5 @@ 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_wait_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 f1e5681..385ce74 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -657,6 +657,8 @@ 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_WAIT, drm_syncobj_wait_ioctl,
>> +                     DRM_UNLOCKED|DRM_RENDER_ALLOW),
>>  };
>>
>>  #define DRM_CORE_IOCTL_COUNT   ARRAY_SIZE( drm_ioctls )
>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>> index b611480..8b87594 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -1,5 +1,7 @@
>>  /*
>>   * Copyright 2017 Red Hat
>> + * Parts ported from amdgpu (fence wait code).
>> + * Copyright 2016 Advanced Micro Devices, Inc.
>>   *
>>   * Permission is hereby granted, free of charge, to any person obtaining
>> a
>>   * copy of this software and associated documentation files (the
>> "Software"),
>> @@ -31,6 +33,9 @@
>>   * that contain an optional fence. The fence can be updated with a new
>>   * fence, or be NULL.
>>   *
>> + * syncobj's can be waited upon, where it will wait for the underlying
>> + * fence.
>> + *
>>   * syncobj's can be export to fd's and back, these fd's are opaque and
>>   * have no other use case, except passing the syncobj between processes.
>>   *
>> @@ -375,3 +380,162 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device
>> *dev, void *data,
>>         return drm_syncobj_fd_to_handle(file_private, args->fd,
>>                                         &args->handle);
>>  }
>> +
>> +
>> +/**
>> + * drm_timeout_abs_to_jiffies - calculate jiffies timeout from absolute
>> value
>> + *
>> + * @timeout_ns: timeout in ns
>> + *
>> + * Calculate the timeout in jiffies from an absolute timeout in ns.
>> + */
>> +unsigned long drm_timeout_abs_to_jiffies(uint64_t timeout_ns)
>> +{
>> +       unsigned long timeout_jiffies;
>> +       ktime_t timeout;
>> +
>> +       /* clamp timeout if it's to large */
>> +       if (((int64_t)timeout_ns) < 0)
>> +               return MAX_SCHEDULE_TIMEOUT;
>> +
>> +       timeout = ktime_sub(ns_to_ktime(timeout_ns), ktime_get());
>> +       if (ktime_to_ns(timeout) < 0)
>> +               return 0;
>> +
>> +       timeout_jiffies = nsecs_to_jiffies(ktime_to_ns(timeout));
>> +       /*  clamp timeout to avoid unsigned-> signed overflow */
>> +       if (timeout_jiffies > MAX_SCHEDULE_TIMEOUT )
>> +               return MAX_SCHEDULE_TIMEOUT - 1;
>> +
>> +       return timeout_jiffies;
>> +}
>> +
>> +static int drm_syncobj_wait_all_fences(struct drm_device *dev,
>> +                                      struct drm_file *file_private,
>> +                                      struct drm_syncobj_wait *wait,
>> +                                      uint32_t *handles)
>> +{
>> +       uint32_t i;
>> +       int ret;
>> +       unsigned long timeout =
>> drm_timeout_abs_to_jiffies(wait->timeout_ns);
>> +
>> +       for (i = 0; i < wait->count_handles; i++) {
>> +               struct dma_fence *fence;
>> +
>> +               ret = drm_syncobj_fence_get(file_private, handles[i],
>> +                                           &fence);
>> +               if (ret)
>> +                       return ret;
>> +
>> +               if (!fence)
>> +                       continue;
>> +
>> +               ret = dma_fence_wait_timeout(fence, true, timeout);
>> +
>> +               dma_fence_put(fence);
>> +               if (ret < 0)
>> +                       return ret;
>> +               if (ret == 0)
>> +                       break;
>> +               timeout = ret;
>> +       }
>> +
>> +       wait->out_timeout_ns = jiffies_to_nsecs(ret);
>> +       wait->out_status = (ret > 0);
>> +       wait->first_signaled = 0;
>> +       return 0;
>> +}
>> +
>> +static int drm_syncobj_wait_any_fence(struct drm_device *dev,
>> +                                     struct drm_file *file_private,
>> +                                     struct drm_syncobj_wait *wait,
>> +                                     uint32_t *handles)
>> +{
>> +       unsigned long timeout =
>> drm_timeout_abs_to_jiffies(wait->timeout_ns);
>> +       struct dma_fence **array;
>> +       uint32_t i;
>> +       int ret;
>> +       uint32_t first = ~0;
>> +
>> +       /* Prepare the fence array */
>> +       array = kcalloc(wait->count_handles,
>> +                       sizeof(struct dma_fence *), GFP_KERNEL);
>> +
>> +       if (array == NULL)
>> +               return -ENOMEM;
>> +
>> +       for (i = 0; i < wait->count_handles; i++) {
>> +               struct dma_fence *fence;
>> +
>> +               ret = drm_syncobj_fence_get(file_private, handles[i],
>> +                                           &fence);
>> +               if (ret)
>> +                       goto err_free_fence_array;
>> +               else if (fence)
>> +                       array[i] = fence;
>> +               else { /* NULL, the fence has been already signaled */
>> +                       ret = 1;
>> +                       goto out;
>> +               }
>> +       }
>> +
>> +       ret = dma_fence_wait_any_timeout(array, wait->count_handles, true,
>> timeout,
>> +                                        &first);
>> +       if (ret < 0)
>> +               goto err_free_fence_array;
>> +out:
>> +       wait->out_timeout_ns = jiffies_to_nsecs(ret);
>> +       wait->out_status = (ret > 0);
>> +       wait->first_signaled = first;
>> +       /* set return value 0 to indicate success */
>> +       ret = 0;
>> +
>> +err_free_fence_array:
>> +       for (i = 0; i < wait->count_handles; i++)
>> +               dma_fence_put(array[i]);
>> +       kfree(array);
>> +
>> +       return ret;
>> +}
>> +
>> +int
>> +drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
>> +                      struct drm_file *file_private)
>> +{
>> +       struct drm_syncobj_wait *args = data;
>> +       uint32_t *handles;
>> +       int ret = 0;
>> +
>> +       if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>> +               return -ENODEV;
>> +
>> +       if (args->flags != 0 && args->flags !=
>> DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL)
>> +               return -EINVAL;
>> +
>> +       if (args->count_handles == 0)
>> +               return 0;
>> +
>> +       /* Get the handles from userspace */
>> +       handles = kmalloc_array(args->count_handles, sizeof(uint32_t),
>> +                               GFP_KERNEL);
>> +       if (handles == NULL)
>> +               return -ENOMEM;
>> +
>> +       if (copy_from_user(handles,
>> +                          (void __user *)(unsigned long)(args->handles),
>> +                          sizeof(uint32_t) * args->count_handles)) {
>> +               ret = -EFAULT;
>> +               goto err_free_handles;
>> +       }
>> +
>> +       if (args->flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL)
>> +               ret = drm_syncobj_wait_all_fences(dev, file_private,
>> +                                                 args, handles);
>> +       else
>> +               ret = drm_syncobj_wait_any_fence(dev, file_private,
>> +                                                args, handles);
>> +err_free_handles:
>> +       kfree(handles);
>> +
>> +       return ret;
>> +}
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index 96c5c78..d6e2f62 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -716,6 +716,19 @@ struct drm_syncobj_handle {
>>         __u32 pad;
>>  };
>>
>> +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0)
>> +struct drm_syncobj_wait {
>> +       __u64 handles;
>> +       /* absolute timeout */
>> +       __u64 timeout_ns;
>> +       /* remaining timeout */
>> +       __u64 out_timeout_ns;
>
>
> Any particular reason why we don't just make timeout_ns an in/out?  I don't
> really care and haven't given it much thought; mostly just curious.

Timeout's are absolute, so if you replace the in timeout with a new
value, and get -ERESTARTSYS, then you'd restart the ioctl with the
wrong value.

Dave.


More information about the amd-gfx mailing list