[RFC PATCH 7/7] drm/prime: Support explicit fence on export
On Mon, Sep 29, 2014 at 09:46:48AM +0200, Daniel Vetter wrote: > On Fri, Sep 26, 2014 at 01:00:12PM +0300, Lauri Peltonen wrote: > > Allow user space to provide an explicit sync fence fd when exporting > > a dma-buf from gem handle. The fence will be stored as the explicit > > fence to the reservation object. > > > > Signed-off-by: Lauri Peltonen > > All existing userspace treats dma_bufs as long-lived objects. Well, all > the userspace that expects implicit syncing, afaik Android shovels lots of > dma-bufs around all the time (since ion is using them as it's native > buffer handles). > > So adding an exclusive fence once at export time isn't going to be > terribly useful. > > So I think a better approach would be to add this as ioctls to the dma-buf > fd itself. Then you can also add a "give me the fence(s) for this dma_buf" > ioctl, which is useful for interop the other way round (i.e. implicit -> > explicit). Yes, I like this. I thought that one could support long-lived dma-bufs by doing a "re-export" when a new fence needs to be attached, but your model is indeed much nicer! On Sat, Sep 27, 2014 at 08:49:39AM +0200, Maarten Lankhorst wrote: > This is never true. A default resv gets allocated, see dma_buf's create > function. Ah, ok. I'll keep that in mind when writing new versions. :-) Thanks, Lauri
[RFC PATCH 7/7] drm/prime: Support explicit fence on export
On Fri, Sep 26, 2014 at 01:00:12PM +0300, Lauri Peltonen wrote:
> Allow user space to provide an explicit sync fence fd when exporting
> a dma-buf from gem handle. The fence will be stored as the explicit
> fence to the reservation object.
>
> Signed-off-by: Lauri Peltonen
All existing userspace treats dma_bufs as long-lived objects. Well, all
the userspace that expects implicit syncing, afaik Android shovels lots of
dma-bufs around all the time (since ion is using them as it's native
buffer handles).
So adding an exclusive fence once at export time isn't going to be
terribly useful.
So I think a better approach would be to add this as ioctls to the dma-buf
fd itself. Then you can also add a "give me the fence(s) for this dma_buf"
ioctl, which is useful for interop the other way round (i.e. implicit ->
explicit).
-Daniel
> ---
> drivers/gpu/drm/drm_prime.c | 41 +
> include/uapi/drm/drm.h | 9 -
> 2 files changed, 41 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 2807a77..c69df2e 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -28,8 +28,10 @@
>
> #include
> #include
> +#include
> #include
> #include "drm_internal.h"
> +#include "../../staging/android/sync.h"
>
> /*
> * DMA-BUF/GEM Object references and lifetime overview:
> @@ -329,7 +331,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops
> = {
> * drm_gem_prime_export - helper library implemention of the export callback
> * @dev: drm_device to export from
> * @obj: GEM object to export
> - * @flags: flags like DRM_CLOEXEC
> + * @flags: flags like DRM_CLOEXEC or DRM_SYNC_FD
> *
> * This is the implementation of the gem_prime_export functions for GEM
> drivers
> * using the PRIME helpers.
> @@ -385,7 +387,7 @@ static struct dma_buf *export_and_register_object(struct
> drm_device *dev,
> * @dev: dev to export the buffer from
> * @file_priv: drm file-private structure
> * @handle: buffer handle to export
> - * @flags: flags like DRM_CLOEXEC
> + * @flags: flags like DRM_CLOEXEC or DRM_SYNC_FD
> * @prime_fd: pointer to storage for the fd id of the create dma-buf
> *
> * This is the PRIME export function which must be used mandatorily by GEM
> @@ -401,6 +403,24 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
> struct drm_gem_object *obj;
> int ret = 0;
> struct dma_buf *dmabuf;
> + struct fence *fence = NULL;
> +
> + if (flags & DRM_SYNC_FD) {
> +#ifdef CONFIG_SYNC
> + struct sync_fence *sf = sync_fence_fdget(*prime_fd);
> + if (!sf)
> + return -ENOENT;
> + if (sf->num_fences != 1) {
> + sync_fence_put(sf);
> + return -EINVAL;
> + }
> + fence = fence_get(sf->cbs[0].sync_pt);
> + sync_fence_put(sf);
> + flags &= ~DRM_SYNC_FD;
> +#else
> + return -ENODEV;
> +#endif
> + }
>
> mutex_lock(_priv->prime.lock);
> obj = drm_gem_object_lookup(dev, file_priv, handle);
> @@ -453,6 +473,14 @@ out_have_obj:
> goto fail_put_dmabuf;
>
> out_have_handle:
> + if (fence) {
> + if (!dmabuf->resv) {
> + ret = -ENODEV;
> + goto fail_put_dmabuf;
> + }
> + reservation_object_add_excl_fence(dmabuf->resv, fence);
> + }
> +
> ret = dma_buf_fd(dmabuf, flags);
> /*
>* We must _not_ remove the buffer from the handle cache since the newly
> @@ -475,6 +503,7 @@ out:
> drm_gem_object_unreference_unlocked(obj);
> out_unlock:
> mutex_unlock(_priv->prime.lock);
> + fence_put(fence);
>
> return ret;
> }
> @@ -624,7 +653,6 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev,
> void *data,
>struct drm_file *file_priv)
> {
> struct drm_prime_handle *args = data;
> - uint32_t flags;
>
> if (!drm_core_check_feature(dev, DRIVER_PRIME))
> return -EINVAL;
> @@ -633,14 +661,11 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device
> *dev, void *data,
> return -ENOSYS;
>
> /* check flags are valid */
> - if (args->flags & ~DRM_CLOEXEC)
> + if (args->flags & ~(DRM_CLOEXEC | DRM_SYNC_FD))
> return -EINVAL;
>
> - /* we only want to pass DRM_CLOEXEC which is == O_CLOEXEC */
> - flags = args->flags & DRM_CLOEXEC;
> -
> return dev->driver->prime_handle_to_fd(dev, file_priv,
> - args->handle, flags, >fd);
> + args->handle, args->flags, >fd);
> }
>
> int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index b0b8556..a11b893 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@
[RFC PATCH 7/7] drm/prime: Support explicit fence on export
On 26-09-14 12:00, Lauri Peltonen wrote:
> Allow user space to provide an explicit sync fence fd when exporting
> a dma-buf from gem handle. The fence will be stored as the explicit
> fence to the reservation object.
>
> Signed-off-by: Lauri Peltonen
> ---
> drivers/gpu/drm/drm_prime.c | 41 +
> include/uapi/drm/drm.h | 9 -
> 2 files changed, 41 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 2807a77..c69df2e 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -28,8 +28,10 @@
>
> #include
> #include
> +#include
> #include
> #include "drm_internal.h"
> +#include "../../staging/android/sync.h"
>
> /*
> * DMA-BUF/GEM Object references and lifetime overview:
> @@ -329,7 +331,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops
> = {
> * drm_gem_prime_export - helper library implemention of the export callback
> * @dev: drm_device to export from
> * @obj: GEM object to export
> - * @flags: flags like DRM_CLOEXEC
> + * @flags: flags like DRM_CLOEXEC or DRM_SYNC_FD
> *
> * This is the implementation of the gem_prime_export functions for GEM
> drivers
> * using the PRIME helpers.
> @@ -385,7 +387,7 @@ static struct dma_buf *export_and_register_object(struct
> drm_device *dev,
> * @dev: dev to export the buffer from
> * @file_priv: drm file-private structure
> * @handle: buffer handle to export
> - * @flags: flags like DRM_CLOEXEC
> + * @flags: flags like DRM_CLOEXEC or DRM_SYNC_FD
> * @prime_fd: pointer to storage for the fd id of the create dma-buf
> *
> * This is the PRIME export function which must be used mandatorily by GEM
> @@ -401,6 +403,24 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
> struct drm_gem_object *obj;
> int ret = 0;
> struct dma_buf *dmabuf;
> + struct fence *fence = NULL;
> +
> + if (flags & DRM_SYNC_FD) {
> +#ifdef CONFIG_SYNC
> + struct sync_fence *sf = sync_fence_fdget(*prime_fd);
> + if (!sf)
> + return -ENOENT;
> + if (sf->num_fences != 1) {
> + sync_fence_put(sf);
> + return -EINVAL;
> + }
> + fence = fence_get(sf->cbs[0].sync_pt);
> + sync_fence_put(sf);
> + flags &= ~DRM_SYNC_FD;
> +#else
> + return -ENODEV;
> +#endif
> + }
>
> mutex_lock(_priv->prime.lock);
> obj = drm_gem_object_lookup(dev, file_priv, handle);
> @@ -453,6 +473,14 @@ out_have_obj:
> goto fail_put_dmabuf;
>
> out_have_handle:
> + if (fence) {
> + if (!dmabuf->resv) {
> + ret = -ENODEV;
> + goto fail_put_dmabuf;
> + }
This is never true. A default resv gets allocated, see dma_buf's create
function.
~Maarten
~Maarten
[RFC PATCH 7/7] drm/prime: Support explicit fence on export
Allow user space to provide an explicit sync fence fd when exporting
a dma-buf from gem handle. The fence will be stored as the explicit
fence to the reservation object.
Signed-off-by: Lauri Peltonen
---
drivers/gpu/drm/drm_prime.c | 41 +
include/uapi/drm/drm.h | 9 -
2 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 2807a77..c69df2e 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -28,8 +28,10 @@
#include
#include
+#include
#include
#include "drm_internal.h"
+#include "../../staging/android/sync.h"
/*
* DMA-BUF/GEM Object references and lifetime overview:
@@ -329,7 +331,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =
{
* drm_gem_prime_export - helper library implemention of the export callback
* @dev: drm_device to export from
* @obj: GEM object to export
- * @flags: flags like DRM_CLOEXEC
+ * @flags: flags like DRM_CLOEXEC or DRM_SYNC_FD
*
* This is the implementation of the gem_prime_export functions for GEM drivers
* using the PRIME helpers.
@@ -385,7 +387,7 @@ static struct dma_buf *export_and_register_object(struct
drm_device *dev,
* @dev: dev to export the buffer from
* @file_priv: drm file-private structure
* @handle: buffer handle to export
- * @flags: flags like DRM_CLOEXEC
+ * @flags: flags like DRM_CLOEXEC or DRM_SYNC_FD
* @prime_fd: pointer to storage for the fd id of the create dma-buf
*
* This is the PRIME export function which must be used mandatorily by GEM
@@ -401,6 +403,24 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
struct drm_gem_object *obj;
int ret = 0;
struct dma_buf *dmabuf;
+ struct fence *fence = NULL;
+
+ if (flags & DRM_SYNC_FD) {
+#ifdef CONFIG_SYNC
+ struct sync_fence *sf = sync_fence_fdget(*prime_fd);
+ if (!sf)
+ return -ENOENT;
+ if (sf->num_fences != 1) {
+ sync_fence_put(sf);
+ return -EINVAL;
+ }
+ fence = fence_get(sf->cbs[0].sync_pt);
+ sync_fence_put(sf);
+ flags &= ~DRM_SYNC_FD;
+#else
+ return -ENODEV;
+#endif
+ }
mutex_lock(_priv->prime.lock);
obj = drm_gem_object_lookup(dev, file_priv, handle);
@@ -453,6 +473,14 @@ out_have_obj:
goto fail_put_dmabuf;
out_have_handle:
+ if (fence) {
+ if (!dmabuf->resv) {
+ ret = -ENODEV;
+ goto fail_put_dmabuf;
+ }
+ reservation_object_add_excl_fence(dmabuf->resv, fence);
+ }
+
ret = dma_buf_fd(dmabuf, flags);
/*
* We must _not_ remove the buffer from the handle cache since the newly
@@ -475,6 +503,7 @@ out:
drm_gem_object_unreference_unlocked(obj);
out_unlock:
mutex_unlock(_priv->prime.lock);
+ fence_put(fence);
return ret;
}
@@ -624,7 +653,6 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev,
void *data,
struct drm_file *file_priv)
{
struct drm_prime_handle *args = data;
- uint32_t flags;
if (!drm_core_check_feature(dev, DRIVER_PRIME))
return -EINVAL;
@@ -633,14 +661,11 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev,
void *data,
return -ENOSYS;
/* check flags are valid */
- if (args->flags & ~DRM_CLOEXEC)
+ if (args->flags & ~(DRM_CLOEXEC | DRM_SYNC_FD))
return -EINVAL;
- /* we only want to pass DRM_CLOEXEC which is == O_CLOEXEC */
- flags = args->flags & DRM_CLOEXEC;
-
return dev->driver->prime_handle_to_fd(dev, file_priv,
- args->handle, flags, >fd);
+ args->handle, args->flags, >fd);
}
int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index b0b8556..a11b893 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -661,13 +661,20 @@ struct drm_set_client_cap {
};
#define DRM_CLOEXEC O_CLOEXEC
+#define DRM_SYNC_FD O_DSYNC
struct drm_prime_handle {
__u32 handle;
/** Flags.. only applicable for handle->fd */
__u32 flags;
- /** Returned dmabuf file descriptor */
+ /**
+* DRM_IOCTL_PRIME_FD_TO_HANDLE:
+* in: dma-buf fd
+* DRM_IOCTL_PRIME_HANDLE_TO_FD:
+* in: sync fence fd if DRM_SYNC_FD flag is passed
+* out: dma-buf fd
+*/
__s32 fd;
};
--
1.8.1.5
