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

2017-05-12 Thread Daniel Vetter
On Fri, May 12, 2017 at 10:34:54AM +1000, Dave Airlie wrote:
> From: Dave Airlie 
> 
> 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.
> 
> Signed-off-by: Dave Airlie 

Acked-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_internal.h |   2 +
>  drivers/gpu/drm/drm_ioctl.c|   2 +
>  drivers/gpu/drm/drm_syncobj.c  | 139 
> -
>  include/uapi/drm/drm.h |  12 
>  4 files changed, 154 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 44ef903..a508ad9 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 6da7adc..b142466 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -653,6 +653,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 835e987..9a8c690 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,10 +33,13 @@
>   * 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.
>   *
> - * TODO: sync_file interactions, waiting
> + * TODO: sync_file interactions.
>   *
>   * Their primary use-case is to implement Vulkan fences and semaphores.
>   *
> @@ -383,3 +388,135 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, 
> void *data,
>   return drm_syncobj_fd_to_handle(file_private, args->fd,
>   &args->handle);
>  }
> +
> +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 = 0;
> + unsigned long timeout = nsecs_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;
> + }
> +
> + if (ret > 0)
> + wait->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 = nsecs_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)

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

2017-05-12 Thread Chris Wilson
On Fri, May 12, 2017 at 10:34:54AM +1000, Dave Airlie wrote:
> +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 = 0;
> + unsigned long timeout = nsecs_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);

Doesn't handle -EINTR yet with timeout. If having a drmIoctl() that
can't be tricked into turning a short waiting into an indefinite one is a
goal.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2017-05-12 Thread Christian König

Am 12.05.2017 um 10:49 schrieb Chris Wilson:

On Fri, May 12, 2017 at 10:34:54AM +1000, Dave Airlie wrote:

+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 = 0;
+   unsigned long timeout = nsecs_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);

Doesn't handle -EINTR yet with timeout. If having a drmIoctl() that
can't be tricked into turning a short waiting into an indefinite one is a
goal.


Yeah, Daniel summarized that once nicely by noting that timeouts should 
be absolute not relative.


Christian.


-Chris



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


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

2017-05-12 Thread Sean Paul
On Fri, May 12, 2017 at 10:34:54AM +1000, Dave Airlie wrote:
> From: Dave Airlie 
> 
> 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.
> 
> Signed-off-by: Dave Airlie 

Reviewed-by: Sean Paul 

> ---
>  drivers/gpu/drm/drm_internal.h |   2 +
>  drivers/gpu/drm/drm_ioctl.c|   2 +
>  drivers/gpu/drm/drm_syncobj.c  | 139 
> -
>  include/uapi/drm/drm.h |  12 
>  4 files changed, 154 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 44ef903..a508ad9 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 6da7adc..b142466 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -653,6 +653,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 835e987..9a8c690 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,10 +33,13 @@
>   * 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.
>   *
> - * TODO: sync_file interactions, waiting
> + * TODO: sync_file interactions.
>   *
>   * Their primary use-case is to implement Vulkan fences and semaphores.
>   *
> @@ -383,3 +388,135 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, 
> void *data,
>   return drm_syncobj_fd_to_handle(file_private, args->fd,
>   &args->handle);
>  }
> +
> +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 = 0;
> + unsigned long timeout = nsecs_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;
> + }
> +
> + if (ret > 0)
> + wait->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 = nsecs_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)