drm syncobj - can we get some r-b/a-bs?
Subsystems (4)
Okay I'm not convinced this is going to get any better out of tree, I've polished what I can, and fixed up the last few comments from people,
I'd like to rebase on drm-misc probably at some point and send a pull request for it.
This mostly just addresses things around naming that Chris pointed out.
Dave.
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): 20170512003457.24936-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
PATCH 1/5 drm: introduce sync objects (v2)
From: Dave Airlie <[email protected]>
Sync objects are new toplevel drm object, that contain a pointer to a fence. This fence can be updated via command submission ioctls via drivers.
There is also a generic wait obj API modelled on the vulkan wait API (with code modelled on some amdgpu code).
These objects can be converted to an opaque fd that can be passes between processes.
v2: rename reference/unreference to put/get (Chris) fix leaked reference (David Zhou)
drop mutex in favour of cmpxchg (Chris) document drm_syncobj_fence_get
use ENOENT for syncobj lookup.
fixup
--- Documentation/gpu/drm-internals.rst | 3 + Documentation/gpu/drm-mm.rst | 6 + drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_fops.c | 8 + drivers/gpu/drm/drm_internal.h | 13 ++ drivers/gpu/drm/drm_ioctl.c | 12 ++ drivers/gpu/drm/drm_syncobj.c | 385 ++++++++++++++++++++++++++++++++++++ include/drm/drmP.h | 5 + include/drm/drm_drv.h | 1 + include/drm/drm_syncobj.h | 87 ++++++++ include/uapi/drm/drm.h | 25 +++ 11 files changed, 546 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_syncobj.c create mode 100644 include/drm/drm_syncobj.h
Documentation/gpu/drm-internals.rst +3 -0
@@ -98,6 +98,9 @@ DRIVER_ATOMIC implement appropriate obj->atomic_get_property() vfuncs for any modeset objects with driver specific properties. +DRIVER_SYNCOBJ + Driver support drm sync objects. + Major, Minor and Patchlevel ~~~~~~~~~~~~~~~~~~~~~~~~~~~
Documentation/gpu/drm-mm.rst +6 -0
@@ -483,3 +483,9 @@ DRM Cache Handling .. kernel-doc:: drivers/gpu/drm/drm_cache.c :export: + +DRM Sync Objects +=========================== + +.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c + :export:
drivers/gpu/drm/Makefile +1 -1
@@ -16,7 +16,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \ drm_framebuffer.o drm_connector.o drm_blend.o \ drm_encoder.o drm_mode_object.o drm_property.o \ drm_plane.o drm_color_mgmt.o drm_print.o \ - drm_dumb_buffers.o drm_mode_config.o + drm_dumb_buffers.o drm_mode_config.o drm_syncobj.o drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o drm-$(CONFIG_DRM_VM) += drm_vm.o
drivers/gpu/drm/drm_fops.c +8 -0
@@ -219,6 +219,9 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)if (drm_core_check_feature (dev,DRIVER_GEM ))drm_gem_open (dev, priv); +if (drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +drm_syncobj_open (priv); +if (drm_core_check_feature (dev,DRIVER_PRIME ))drm_prime_init_file_private (&priv->prime);
@@ -266,6 +269,8 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor) out_prime_destroy:if (drm_core_check_feature (dev,DRIVER_PRIME ))drm_prime_destroy_file_private (&priv->prime); +if (drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +drm_syncobj_release (priv);if (drm_core_check_feature (dev,DRIVER_GEM ))drm_gem_release (dev, priv);put_pid (priv->pid);
@@ -400,6 +405,9 @@ int drm_release(struct inode *inode, struct file *filp)drm_property_destroy_user_blobs (dev, file_priv); } +if (drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +drm_syncobj_release (file_priv); +if (drm_core_check_feature (dev,DRIVER_GEM ))drm_gem_release (dev, file_priv);
drivers/gpu/drm/drm_internal.h +13 -0
@@ -142,4 +142,17 @@ static inline int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc) {return 0 ; } +#endif + +/* drm_syncobj.c */ +void drm_syncobj_open (struct drm_file *file_private); +void drm_syncobj_release (struct drm_file *file_private); +int drm_syncobj_create_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private); +int drm_syncobj_destroy_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private); +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);
drivers/gpu/drm/drm_ioctl.c +12 -0
@@ -240,6 +240,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ req->value |= dev->driver->prime_fd_to_handle ?DRM_PRIME_CAP_IMPORT :0 ; req->value |= dev->driver->prime_handle_to_fd ?DRM_PRIME_CAP_EXPORT :0 ;return 0 ; +case DRM_CAP_SYNCOBJ : + req->value =drm_core_check_feature (dev,DRIVER_SYNCOBJ ); +return 0 ; }/* Other caps only work with KMS drivers */
@@ -641,6 +644,15 @@ static const struct drm_ioctl_desc drm_ioctls[] = {DRM_IOCTL_DEF (DRM_IOCTL_MODE_ATOMIC , drm_mode_atomic_ioctl,DRM_MASTER |DRM_CONTROL_ALLOW |DRM_UNLOCKED ),DRM_IOCTL_DEF (DRM_IOCTL_MODE_CREATEPROPBLOB , drm_mode_createblob_ioctl,DRM_CONTROL_ALLOW |DRM_UNLOCKED ),DRM_IOCTL_DEF (DRM_IOCTL_MODE_DESTROYPROPBLOB , drm_mode_destroyblob_ioctl,DRM_CONTROL_ALLOW |DRM_UNLOCKED ), + +DRM_IOCTL_DEF (DRM_IOCTL_SYNCOBJ_CREATE , drm_syncobj_create_ioctl, +DRM_UNLOCKED |DRM_RENDER_ALLOW ), +DRM_IOCTL_DEF (DRM_IOCTL_SYNCOBJ_DESTROY , drm_syncobj_destroy_ioctl, +DRM_UNLOCKED |DRM_RENDER_ALLOW ), +DRM_IOCTL_DEF (DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD , drm_syncobj_handle_to_fd_ioctl, +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), };#define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )
drivers/gpu/drm/drm_syncobj.c new file +385 -0
385 lines not on this page · raw message
include/drm/drmP.h +5 -0
@@ -405,6 +405,11 @@ struct drm_file {/** Lock for synchronization of access to object_idr. */ spinlock_t table_lock; +/** Mapping of sync object handles to object pointers. */ +struct idr syncobj_idr; +/** Lock for synchronization of access to syncobj_idr. */ +spinlock_t syncobj_table_lock; +struct file *filp;void *driver_priv;
include/drm/drm_drv.h +1 -0
@@ -53,6 +53,7 @@ struct drm_mode_create_dumb;#define DRIVER_RENDER 0x8000#define DRIVER_ATOMIC 0x10000#define DRIVER_KMS_LEGACY_CONTEXT 0x20000 +#define DRIVER_SYNCOBJ 0x40000/** * struct drm_driver - DRM driver structure
include/drm/drm_syncobj.h new file +87 -0
87 lines not on this page · raw message
include/uapi/drm/drm.h +25 -0
@@ -647,6 +647,7 @@ struct drm_gem_open {#define DRM_CAP_CURSOR_HEIGHT 0x9#define DRM_CAP_ADDFB2_MODIFIERS 0x10#define DRM_CAP_PAGE_FLIP_TARGET 0x11 +#define DRM_CAP_SYNCOBJ 0x13/** DRM_IOCTL_GET_CAP ioctl argument type */ struct drm_get_cap {
@@ -696,6 +697,25 @@ struct drm_prime_handle {__s32 fd; }; +struct drm_syncobj_create { +__u32 handle; +__u32 flags; +}; + +struct drm_syncobj_destroy { +__u32 handle; +__u32 pad; +}; + +struct drm_syncobj_handle { +__u32 handle; +/** Flags.. only applicable for handle->fd */ +__u32 flags; + +__s32 fd; +__u32 pad; +}; +#if defined(__cplusplus) }#endif
@@ -814,6 +834,11 @@ extern "C" {#define DRM_IOCTL_MODE_CREATEPROPBLOB DRM_IOWR(0xBD, struct drm_mode_create_blob)#define DRM_IOCTL_MODE_DESTROYPROPBLOB DRM_IOWR(0xBE, struct drm_mode_destroy_blob) +#define DRM_IOCTL_SYNCOBJ_CREATE DRM_IOWR(0xBF, struct drm_syncobj_create) +#define DRM_IOCTL_SYNCOBJ_DESTROY DRM_IOWR(0xC0, struct drm_syncobj_destroy) +#define DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD DRM_IOWR(0xC1, struct drm_syncobj_handle) +#define DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE DRM_IOWR(0xC2, struct drm_syncobj_handle) +/** * Device specific ioctls should only be in their respective headers * The device specific ioctl range is from 0x40 to 0x9f.
Signature · 6 lines
2.9.3 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): 20170512003457.24936-2-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Re: [PATCH 1/5] drm: introduce sync objects (v2)
Quoted 18 lines · Dave Airlie
From: Dave Airlie <[email protected]>
Sync objects are new toplevel drm object, that contain a pointer to a fence. This fence can be updated via command submission ioctls via drivers.
There is also a generic wait obj API modelled on the vulkan wait API (with code modelled on some amdgpu code).
These objects can be converted to an opaque fd that can be passes between processes.
v2: rename reference/unreference to put/get (Chris) fix leaked reference (David Zhou)
drop mutex in favour of cmpxchg (Chris) documentdrm_syncobj_fence_get
use ENOENT for syncobj lookup.
Quoted 3 lines
Signed-off-by Dave Airlie <[email protected]>fixup
Seems like a small git rebase -i artifact is leftover here.
Quoted 695 lines
--- Documentation/gpu/drm-internals.rst | 3 + Documentation/gpu/drm-mm.rst | 6 + drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_fops.c | 8 + drivers/gpu/drm/drm_internal.h | 13 ++ drivers/gpu/drm/drm_ioctl.c | 12 ++ drivers/gpu/drm/drm_syncobj.c | 385 ++++++++++++++++++++++++++++++++++++ include/drm/drmP.h | 5 + include/drm/drm_drv.h | 1 + include/drm/drm_syncobj.h | 87 ++++++++ include/uapi/drm/drm.h | 25 +++ 11 files changed, 546 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_syncobj.c create mode 100644 include/drm/drm_syncobj.hDocumentation/gpu/drm-internals.rst +3
-0@@ -98,6 +98,9 @@ DRIVER_ATOMIC implement appropriate obj->atomic_get_property() vfuncs for any modeset objects with driver specific properties. +DRIVER_SYNCOBJ + Driver support drm sync objects. + Major, Minor and Patchlevel ~~~~~~~~~~~~~~~~~~~~~~~~~~~Documentation/gpu/drm-mm.rst +6
-0@@ -483,3 +483,9 @@ DRM Cache Handling .. kernel-doc:: drivers/gpu/drm/drm_cache.c :export: + +DRM Sync Objects +=========================== + +.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c + :export:drivers/gpu/drm/Makefile +1
-1@@ -16,7 +16,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \ drm_framebuffer.o drm_connector.o drm_blend.o \ drm_encoder.o drm_mode_object.o drm_property.o \ drm_plane.o drm_color_mgmt.o drm_print.o \ - drm_dumb_buffers.o drm_mode_config.o + drm_dumb_buffers.o drm_mode_config.o drm_syncobj.o drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o drm-$(CONFIG_DRM_VM) += drm_vm.odrivers/gpu/drm/drm_fops.c +8
-0@@ -219,6 +219,9 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)if (drm_core_check_feature (dev,DRIVER_GEM ))drm_gem_open (dev, priv); +if (drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +drm_syncobj_open (priv); +if (drm_core_check_feature (dev,DRIVER_PRIME ))drm_prime_init_file_private (&priv->prime);@@ -266,6 +269,8 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor) out_prime_destroy:if (drm_core_check_feature (dev,DRIVER_PRIME ))drm_prime_destroy_file_private (&priv->prime); +if (drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +drm_syncobj_release (priv);if (drm_core_check_feature (dev,DRIVER_GEM ))drm_gem_release (dev, priv);put_pid (priv->pid);@@ -400,6 +405,9 @@ int drm_release(struct inode *inode, struct file *filp)drm_property_destroy_user_blobs (dev, file_priv); } +if (drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +drm_syncobj_release (file_priv); +if (drm_core_check_feature (dev,DRIVER_GEM ))drm_gem_release (dev, file_priv);drivers/gpu/drm/drm_internal.h +13
-0@@ -142,4 +142,17 @@ static inline int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc) {return 0 ; } +#endif + +/* drm_syncobj.c */ +void drm_syncobj_open (struct drm_file *file_private); +void drm_syncobj_release (struct drm_file *file_private); +int drm_syncobj_create_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private); +int drm_syncobj_destroy_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private); +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);drivers/gpu/drm/drm_ioctl.c +12
-0@@ -240,6 +240,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ req->value |= dev->driver->prime_fd_to_handle ?DRM_PRIME_CAP_IMPORT :0 ; req->value |= dev->driver->prime_handle_to_fd ?DRM_PRIME_CAP_EXPORT :0 ;return 0 ; +case DRM_CAP_SYNCOBJ : + req->value =drm_core_check_feature (dev,DRIVER_SYNCOBJ ); +return 0 ; }/* Other caps only work with KMS drivers */ @@ -641,6 +644,15 @@ static const struct drm_ioctl_desc drm_ioctls[] = {DRM_IOCTL_DEF (DRM_IOCTL_MODE_ATOMIC , drm_mode_atomic_ioctl,DRM_MASTER |DRM_CONTROL_ALLOW |DRM_UNLOCKED ),DRM_IOCTL_DEF (DRM_IOCTL_MODE_CREATEPROPBLOB , drm_mode_createblob_ioctl,DRM_CONTROL_ALLOW |DRM_UNLOCKED ),DRM_IOCTL_DEF (DRM_IOCTL_MODE_DESTROYPROPBLOB , drm_mode_destroyblob_ioctl,DRM_CONTROL_ALLOW |DRM_UNLOCKED ), + +DRM_IOCTL_DEF (DRM_IOCTL_SYNCOBJ_CREATE , drm_syncobj_create_ioctl, +DRM_UNLOCKED |DRM_RENDER_ALLOW ), +DRM_IOCTL_DEF (DRM_IOCTL_SYNCOBJ_DESTROY , drm_syncobj_destroy_ioctl, +DRM_UNLOCKED |DRM_RENDER_ALLOW ), +DRM_IOCTL_DEF (DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD , drm_syncobj_handle_to_fd_ioctl, +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), };#define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )drivers/gpu/drm/drm_syncobj.c new file +385
-0@@ -0,0 +1,385 @@ +/* +* Copyright 2017 Red Hat +* +* Permission is hereby granted, free of charge, to any person obtaining a +* copy of this software and associated documentation files (the "Software"), +* to deal in the Software without restriction, including without limitation +* the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following conditions: +* +* The above copyright notice and this permission notice (including the next +* paragraph) shall be included in all copies or substantial portions of the +* Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +* IN THE SOFTWARE. +* +* Authors: +* +*/ + +/** +* DOC: Overview +* +* DRM synchronisation objects (syncobj) are a persistent objects, +* that contain an optional fence. The fence can be updated with a new +* fence, or be NULL. +* +* 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 +* +* Their primary use-case is to implement Vulkan fences and semaphores. +* +* syncobj have a kref reference count, but also have an optional file. +* The file is only created once the syncobj is exported. +* The file takes a reference on the kref. +*/ + +#include <drm/drmP.h> +#include <linux/file.h> +#include <linux/fs.h> +#include <linux/anon_inodes.h> + +#include "drm_internal.h" +#include <drm/drm_syncobj.h> + +static struct drm_syncobj *drm_syncobj_find (struct drm_file *file_private, +u32 handle) +{ +struct drm_syncobj *syncobj; + +spin_lock (&file_private->syncobj_table_lock); + +/* Check if we currently have a reference on the object */ + syncobj =idr_find (&file_private->syncobj_idr, handle); +if (syncobj) +drm_syncobj_get (syncobj); + +spin_unlock (&file_private->syncobj_table_lock); + +return syncobj; +} + +/** +* drm_syncobj_replace_fence - lookup and replace fence in a sync object. +* @file_private - drm file private pointer. +* @handle - syncobj handle to lookup +* @fence - fence to install in sync file. +* Returns: +* 0 on success, or -ENOENT when the handle doesn't point at a valid sem file. +* +* This looks up a sync object and replaces the fence on it, freeing +* the old one. +*/ +int drm_syncobj_replace_fence (struct drm_file *file_private, +u32 handle, +struct dma_fence *fence) +{ +struct drm_syncobj *syncobj =drm_syncobj_find (file_private, handle); +struct dma_fence *old_fence =NULL ; + +if (!syncobj) +return -ENOENT ; + +if (fence) +dma_fence_get (fence); + old_fence =xchg (&syncobj->fence, fence); + +dma_fence_put (old_fence); +drm_syncobj_put (syncobj); + +return 0 ; +} +EXPORT_SYMBOL (drm_syncobj_replace_fence); + +/** +* drm_syncobj_fence_get - lookup sync object and reference fence +* @file_private - drm file private pointer. +* @handle - syncobj handle to lookup +* @fence - returned referenced fence +* Returns: +* 0 on success, or -ENOENT where syncobj is invalid, +* or -EINVAL when the syncobj has no fence. +* +* This returns a reference to underlying fence backing the +* syncobj if one exists. +*/ +int drm_syncobj_fence_get (struct drm_file *file_private, +u32 handle, +struct dma_fence **fence) +{ +struct drm_syncobj *syncobj =drm_syncobj_find (file_private, handle); +int ret =0 ; + +if (!syncobj) +return -ENOENT ; + + *fence =dma_fence_get (syncobj->fence); +if (!*fence) { + ret = -EINVAL ; + } +drm_syncobj_put (syncobj); +return ret; +} +EXPORT_SYMBOL (drm_syncobj_fence_get); + +void drm_syncobj_free (struct kref *kref) +{ +struct drm_syncobj *syncobj =container_of (kref, + struct drm_syncobj, + refcount); +dma_fence_put (syncobj->fence); +kfree (syncobj); +} + +static int drm_syncobj_create (struct drm_file *file_private, +u32 *handle) +{ +int ret; +struct drm_syncobj *syncobj; + + syncobj =kzalloc (sizeof (struct drm_syncobj ),GFP_KERNEL ); +if (!syncobj) +return -ENOMEM ; + +kref_init (&syncobj->refcount); + +idr_preload (GFP_KERNEL ); +spin_lock (&file_private->syncobj_table_lock); + ret =idr_alloc (&file_private->syncobj_idr, syncobj,1 ,0 ,GFP_NOWAIT ); +spin_unlock (&file_private->syncobj_table_lock); + +idr_preload_end (); + +if (ret <0 ) { +drm_syncobj_put (syncobj); +return ret; + } + + *handle = ret; +return 0 ; +} + +static int drm_syncobj_destroy (struct drm_file *file_private, +u32 handle) +{ +struct drm_syncobj *syncobj; + +spin_lock (&file_private->syncobj_table_lock); + syncobj =idr_remove (&file_private->syncobj_idr, handle); +spin_unlock (&file_private->syncobj_table_lock); + +if (!syncobj) +return -ENOENT ; + +drm_syncobj_put (syncobj); +return 0 ; +} + +static int drm_syncobj_file_release (struct inode *inode,struct file *file) +{ +struct drm_syncobj *syncobj = file->private_data; + +drm_syncobj_put (syncobj); +return 0 ; +} + +static const struct file_operations drm_syncobj_file_fops = { + .release = drm_syncobj_file_release, +}; + +static int drm_syncobj_alloc_file (struct drm_syncobj *syncobj) +{ +struct file *file =anon_inode_getfile ("syncobj_file" , + &drm_syncobj_file_fops, + syncobj,0 ); +if (IS_ERR (file)) +return PTR_ERR (file); + +drm_syncobj_get (syncobj); +if (cmpxchg (&syncobj->file,NULL , file)) { +/* lost the race */ +fput (file); + } + +return 0 ; +} + +static int drm_syncobj_handle_to_fd (struct drm_file *file_private, +u32 handle,int *p_fd) +{ +struct drm_syncobj *syncobj =drm_syncobj_find (file_private, handle); +int ret; +int fd; + +if (!syncobj) +return -EINVAL ; + + fd =get_unused_fd_flags (O_CLOEXEC ); +if (fd <0 ) { +drm_syncobj_put (syncobj); +return fd; + } + +if (!syncobj->file) { + ret =drm_syncobj_alloc_file (syncobj); +if (ret) + goto out_put_fd; + } +fd_install (fd, syncobj->file); +drm_syncobj_put (syncobj); + *p_fd = fd; +return 0 ; +out_put_fd: +put_unused_fd (fd); +drm_syncobj_put (syncobj); +return ret; +} + +static struct drm_syncobj *drm_syncobj_fdget (int fd) +{ +struct file *file =fget (fd); + +if (!file) +return NULL ; +if (file->f_op != &drm_syncobj_file_fops) + goto err; + +return file->private_data; +err: +fput (file); +return NULL ; +}; + +static int drm_syncobj_fd_to_handle (struct drm_file *file_private, +int fd,u32 *handle) +{ +struct drm_syncobj *syncobj =drm_syncobj_fdget (fd); +int ret; + +if (!syncobj) +return -EINVAL ; + +/* take a reference to put in the idr */ +drm_syncobj_get (syncobj); + +idr_preload (GFP_KERNEL ); +spin_lock (&file_private->syncobj_table_lock); + ret =idr_alloc (&file_private->syncobj_idr, syncobj,1 ,0 ,GFP_NOWAIT ); +spin_unlock (&file_private->syncobj_table_lock); +idr_preload_end (); + +if (ret <0 ) { +fput (syncobj->file); +return ret; + } + *handle = ret; +return 0 ; +} + +/** +* drm_syncobj_open - initalizes syncobj file-private structures at devnode open time +* @dev: drm_device which is being opened by userspace +* @file_private: drm file-private structure to set up +* +* Called at device open time, sets up the structure for handling refcounting +* of sync objects. +*/ +void +drm_syncobj_open (struct drm_file *file_private) +{ +idr_init (&file_private->syncobj_idr); +spin_lock_init (&file_private->syncobj_table_lock); +} + +static int +drm_syncobj_release_handle (int id,void *ptr,void *data) +{ +struct drm_syncobj *syncobj = ptr; + +drm_syncobj_put (syncobj); +return 0 ; +} + +/** +* drm_syncobj_release - release file-private sync object resources +* @dev: drm_device which is being closed by userspace +* @file_private: drm file-private structure to clean up +* +* Called at close time when the filp is going away. +* +* Releases any remaining references on objects by this filp. +*/ +void +drm_syncobj_release (struct drm_file *file_private) +{ +idr_for_each (&file_private->syncobj_idr, + &drm_syncobj_release_handle, file_private); +idr_destroy (&file_private->syncobj_idr); +} + +int +drm_syncobj_create_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private) +{ +struct drm_syncobj_create *args = data; + +if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +return -ENODEV ; + +/* no valid flags yet */ +if (args->flags) +return -EINVAL ; + +return drm_syncobj_create (file_private, + &args->handle); +} + +int +drm_syncobj_destroy_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private) +{ +struct drm_syncobj_destroy *args = data; + +if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +return -ENODEV ; + +/* make sure padding is empty */ +if (args->pad) +return -EINVAL ; +return drm_syncobj_destroy (file_private, args->handle); +} + +int +drm_syncobj_handle_to_fd_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private) +{ +struct drm_syncobj_handle *args = data; + +if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +return -ENODEV ; + +return drm_syncobj_handle_to_fd (file_private, args->handle, + &args->fd); +} + +int +drm_syncobj_fd_to_handle_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private) +{ +struct drm_syncobj_handle *args = data; + +if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +return -ENODEV ; + +return drm_syncobj_fd_to_handle (file_private, args->fd, + &args->handle); +}include/drm/drmP.h +5
-0@@ -405,6 +405,11 @@ struct drm_file {/** Lock for synchronization of access to object_idr. */ spinlock_t table_lock; +/** Mapping of sync object handles to object pointers. */ +struct idr syncobj_idr; +/** Lock for synchronization of access to syncobj_idr. */ +spinlock_t syncobj_table_lock; +struct file *filp;void *driver_priv;include/drm/drm_drv.h +1
-0@@ -53,6 +53,7 @@ struct drm_mode_create_dumb;#define DRIVER_RENDER 0x8000#define DRIVER_ATOMIC 0x10000#define DRIVER_KMS_LEGACY_CONTEXT 0x20000 +#define DRIVER_SYNCOBJ 0x40000/** * struct drm_driver - DRM driver structure include/drm/drm_syncobj.h new file +87
-0@@ -0,0 +1,87 @@ +/* +* Copyright © 2017 Red Hat +* +* Permission is hereby granted, free of charge, to any person obtaining a +* copy of this software and associated documentation files (the "Software"), +* to deal in the Software without restriction, including without limitation +* the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following conditions: +* +* The above copyright notice and this permission notice (including the next +* paragraph) shall be included in all copies or substantial portions of the +* Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +* IN THE SOFTWARE. +* +* Authors: +* +*/ +#ifndef __DRM_SYNCOBJ_H__ +#define __DRM_SYNCOBJ_H__ + +#include "linux/dma-fence.h" + +/** +* struct drm_syncobj - sync object. +* +* This structure defines a generic sync object which wraps a dma fence. +*/ +struct drm_syncobj { +/** +* @refcount: +* +* Reference count of this object. +*/ +struct kref refcount; +/** +* @fence: +* NULL or a pointer to the fence bound to this object. +*/ +struct dma_fence *fence; +/** +* @file +* a file backing for this syncobj. +*/ +struct file *file; +}; + +void drm_syncobj_free (struct kref *kref); + +/** +* drm_syncobj_get - acquire a syncobj reference +* @obj: sync object +* +* This acquires additional reference to @obj. It is illegal to call this +* without already holding a reference. No locks required. +*/ +static inline void +drm_syncobj_get (struct drm_syncobj *obj) +{ +kref_get (&obj->refcount); +} + +/** +* drm_syncobj_put - release a reference to a sync object. +* @obj: sync object. +*/ +static inline void +drm_syncobj_put (struct drm_syncobj *obj) +{ +kref_put (&obj->refcount, drm_syncobj_free); +} + +int drm_syncobj_fence_get (struct drm_file *file_private, +u32 handle, +struct dma_fence **fence); +int drm_syncobj_replace_fence (struct drm_file *file_private, +u32 handle, +struct dma_fence *fence); + +#endif include/uapi/drm/drm.h +20
-0@@ -647,6 +647,7 @@ struct drm_gem_open {#define DRM_CAP_CURSOR_HEIGHT 0x9#define DRM_CAP_ADDFB2_MODIFIERS 0x10#define DRM_CAP_PAGE_FLIP_TARGET 0x11 +#define DRM_CAP_SYNCOBJ 0x13/** DRM_IOCTL_GET_CAP ioctl argument type */ struct drm_get_cap {@@ -696,6 +697,25 @@ struct drm_prime_handle {__s32 fd; }; +struct drm_syncobj_create { +__u32 handle; +__u32 flags; +}; + +struct drm_syncobj_destroy { +__u32 handle; +__u32 pad; +}; + +struct drm_syncobj_handle { +__u32 handle; +/** Flags.. only applicable for handle->fd */ +__u32 flags; + +__s32 fd; +__u32 pad; +}; +#if defined(__cplusplus) }#endif @@ -814,6 +834,11 @@ extern "C" {#define DRM_IOCTL_MODE_CREATEPROPBLOB DRM_IOWR(0xBD, struct drm_mode_create_blob)
include/uapi/drm/drm.h extern "C" { · line 844
Quoted 10 lines
include/uapi/drm/drm.h +5
-0#define DRM_IOCTL_MODE_DESTROYPROPBLOB DRM_IOWR(0xBE, struct drm_mode_destroy_blob) +#define DRM_IOCTL_SYNCOBJ_CREATE DRM_IOWR(0xBF, struct drm_syncobj_create) +#define DRM_IOCTL_SYNCOBJ_DESTROY DRM_IOWR(0xC0, struct drm_syncobj_destroy) +#define DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD DRM_IOWR(0xC1, struct drm_syncobj_handle) +#define DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE DRM_IOWR(0xC2, struct drm_syncobj_handle) +/** * Device specific ioctls should only be in their respective headers * The device specific ioctl range is from 0x40 to 0x9f.
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Daniel Vetter
Acked-byRe: [PATCH 1/5] drm: introduce sync objects (v2)
In reply to a message not in the archiveQuoted 18 lines · Dave Airlie
From: Dave Airlie <[email protected]>
Sync objects are new toplevel drm object, that contain a pointer to a fence. This fence can be updated via command submission ioctls via drivers.
There is also a generic wait obj API modelled on the vulkan wait API (with code modelled on some amdgpu code).
These objects can be converted to an opaque fd that can be passes between processes.
v2: rename reference/unreference to put/get (Chris) fix leaked reference (David Zhou)
drop mutex in favour of cmpxchg (Chris) documentdrm_syncobj_fence_get
use ENOENT for syncobj lookup.
Quoted 3 lines
Signed-off-by Dave Airlie <[email protected]>fixup
Found a few ioctl and other nits, with those addressed
Quoted 35 lines
--- Documentation/gpu/drm-internals.rst | 3 + Documentation/gpu/drm-mm.rst | 6 + drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_fops.c | 8 + drivers/gpu/drm/drm_internal.h | 13 ++ drivers/gpu/drm/drm_ioctl.c | 12 ++ drivers/gpu/drm/drm_syncobj.c | 385 ++++++++++++++++++++++++++++++++++++ include/drm/drmP.h | 5 + include/drm/drm_drv.h | 1 + include/drm/drm_syncobj.h | 87 ++++++++ include/uapi/drm/drm.h | 25 +++ 11 files changed, 546 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_syncobj.c create mode 100644 include/drm/drm_syncobj.hDocumentation/gpu/drm-internals.rst +3
-0@@ -98,6 +98,9 @@ DRIVER_ATOMIC implement appropriate obj->atomic_get_property() vfuncs for any modeset objects with driver specific properties. +DRIVER_SYNCOBJ + Driver support drm sync objects. + Major, Minor and Patchlevel ~~~~~~~~~~~~~~~~~~~~~~~~~~~Documentation/gpu/drm-mm.rst +0
-0
Documentation/gpu/drm-mm.rst DRM Cache Handling · line 491
Quoted 9 lines
Documentation/gpu/drm-mm.rst +6
-0@@ -483,3 +483,9 @@ DRM Cache Handling .. kernel-doc:: drivers/gpu/drm/drm_cache.c :export: + +DRM Sync Objects +=========================== + +.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c + :export:
You're missing parts of your pretty kernel-doc!
.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c
:doc: Overview
.. kernel-doc:: include/drm/drm_syncobj.h
:export:
.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c
:export:
Quoted 131 lines
drivers/gpu/drm/Makefile +1
-1@@ -16,7 +16,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \ drm_framebuffer.o drm_connector.o drm_blend.o \ drm_encoder.o drm_mode_object.o drm_property.o \ drm_plane.o drm_color_mgmt.o drm_print.o \ - drm_dumb_buffers.o drm_mode_config.o + drm_dumb_buffers.o drm_mode_config.o drm_syncobj.o drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o drm-$(CONFIG_DRM_VM) += drm_vm.odrivers/gpu/drm/drm_fops.c +8
-0@@ -219,6 +219,9 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)if (drm_core_check_feature (dev,DRIVER_GEM ))drm_gem_open (dev, priv); +if (drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +drm_syncobj_open (priv); +if (drm_core_check_feature (dev,DRIVER_PRIME ))drm_prime_init_file_private (&priv->prime);@@ -266,6 +269,8 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor) out_prime_destroy:if (drm_core_check_feature (dev,DRIVER_PRIME ))drm_prime_destroy_file_private (&priv->prime); +if (drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +drm_syncobj_release (priv);if (drm_core_check_feature (dev,DRIVER_GEM ))drm_gem_release (dev, priv);put_pid (priv->pid);@@ -400,6 +405,9 @@ int drm_release(struct inode *inode, struct file *filp)drm_property_destroy_user_blobs (dev, file_priv); } +if (drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +drm_syncobj_release (file_priv); +if (drm_core_check_feature (dev,DRIVER_GEM ))drm_gem_release (dev, file_priv);drivers/gpu/drm/drm_internal.h +13
-0@@ -142,4 +142,17 @@ static inline int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc) {return 0 ; } +#endif + +/* drm_syncobj.c */ +void drm_syncobj_open (struct drm_file *file_private); +void drm_syncobj_release (struct drm_file *file_private); +int drm_syncobj_create_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private); +int drm_syncobj_destroy_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private); +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);drivers/gpu/drm/drm_ioctl.c +12
-0@@ -240,6 +240,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ req->value |= dev->driver->prime_fd_to_handle ?DRM_PRIME_CAP_IMPORT :0 ; req->value |= dev->driver->prime_handle_to_fd ?DRM_PRIME_CAP_EXPORT :0 ;return 0 ; +case DRM_CAP_SYNCOBJ : + req->value =drm_core_check_feature (dev,DRIVER_SYNCOBJ ); +return 0 ; }/* Other caps only work with KMS drivers */ @@ -641,6 +644,15 @@ static const struct drm_ioctl_desc drm_ioctls[] = {DRM_IOCTL_DEF (DRM_IOCTL_MODE_ATOMIC , drm_mode_atomic_ioctl,DRM_MASTER |DRM_CONTROL_ALLOW |DRM_UNLOCKED ),DRM_IOCTL_DEF (DRM_IOCTL_MODE_CREATEPROPBLOB , drm_mode_createblob_ioctl,DRM_CONTROL_ALLOW |DRM_UNLOCKED ),DRM_IOCTL_DEF (DRM_IOCTL_MODE_DESTROYPROPBLOB , drm_mode_destroyblob_ioctl,DRM_CONTROL_ALLOW |DRM_UNLOCKED ), + +DRM_IOCTL_DEF (DRM_IOCTL_SYNCOBJ_CREATE , drm_syncobj_create_ioctl, +DRM_UNLOCKED |DRM_RENDER_ALLOW ), +DRM_IOCTL_DEF (DRM_IOCTL_SYNCOBJ_DESTROY , drm_syncobj_destroy_ioctl, +DRM_UNLOCKED |DRM_RENDER_ALLOW ), +DRM_IOCTL_DEF (DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD , drm_syncobj_handle_to_fd_ioctl, +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), };#define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )drivers/gpu/drm/drm_syncobj.c new file +27
-0@@ -0,0 +1,385 @@ +/* +* Copyright 2017 Red Hat +* +* Permission is hereby granted, free of charge, to any person obtaining a +* copy of this software and associated documentation files (the "Software"), +* to deal in the Software without restriction, including without limitation +* the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following conditions: +* +* The above copyright notice and this permission notice (including the next +* paragraph) shall be included in all copies or substantial portions of the +* Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +* IN THE SOFTWARE. +* +* Authors: +* +*/ + +/**
drivers/gpu/drm/drm_syncobj.c line 37
Quoted 10 lines
drivers/gpu/drm/drm_syncobj.c new file +10
-0+* DOC: Overview +* +* DRM synchronisation objects (syncobj) are a persistent objects, +* that contain an optional fence. The fence can be updated with a new +* fence, or be NULL. +* +* 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
Since you address that with patches 2&3, drop that?
Quoted 321 lines
drivers/gpu/drm/drm_syncobj.c new file +321
-0+* +* Their primary use-case is to implement Vulkan fences and semaphores. +* +* syncobj have a kref reference count, but also have an optional file. +* The file is only created once the syncobj is exported. +* The file takes a reference on the kref. +*/ + +#include <drm/drmP.h> +#include <linux/file.h> +#include <linux/fs.h> +#include <linux/anon_inodes.h> + +#include "drm_internal.h" +#include <drm/drm_syncobj.h> + +static struct drm_syncobj *drm_syncobj_find (struct drm_file *file_private, +u32 handle) +{ +struct drm_syncobj *syncobj; + +spin_lock (&file_private->syncobj_table_lock); + +/* Check if we currently have a reference on the object */ + syncobj =idr_find (&file_private->syncobj_idr, handle); +if (syncobj) +drm_syncobj_get (syncobj); + +spin_unlock (&file_private->syncobj_table_lock); + +return syncobj; +} + +/** +* drm_syncobj_replace_fence - lookup and replace fence in a sync object. +* @file_private - drm file private pointer. +* @handle - syncobj handle to lookup +* @fence - fence to install in sync file. +* Returns: +* 0 on success, or -ENOENT when the handle doesn't point at a valid sem file. +* +* This looks up a sync object and replaces the fence on it, freeing +* the old one. +*/ +int drm_syncobj_replace_fence (struct drm_file *file_private, +u32 handle, +struct dma_fence *fence) +{ +struct drm_syncobj *syncobj =drm_syncobj_find (file_private, handle); +struct dma_fence *old_fence =NULL ; + +if (!syncobj) +return -ENOENT ; + +if (fence) +dma_fence_get (fence); + old_fence =xchg (&syncobj->fence, fence); + +dma_fence_put (old_fence); +drm_syncobj_put (syncobj); + +return 0 ; +} +EXPORT_SYMBOL (drm_syncobj_replace_fence); + +/** +* drm_syncobj_fence_get - lookup sync object and reference fence +* @file_private - drm file private pointer. +* @handle - syncobj handle to lookup +* @fence - returned referenced fence +* Returns: +* 0 on success, or -ENOENT where syncobj is invalid, +* or -EINVAL when the syncobj has no fence. +* +* This returns a reference to underlying fence backing the +* syncobj if one exists. +*/ +int drm_syncobj_fence_get (struct drm_file *file_private, +u32 handle, +struct dma_fence **fence) +{ +struct drm_syncobj *syncobj =drm_syncobj_find (file_private, handle); +int ret =0 ; + +if (!syncobj) +return -ENOENT ; + + *fence =dma_fence_get (syncobj->fence); +if (!*fence) { + ret = -EINVAL ; + } +drm_syncobj_put (syncobj); +return ret; +} +EXPORT_SYMBOL (drm_syncobj_fence_get); + +void drm_syncobj_free (struct kref *kref) +{ +struct drm_syncobj *syncobj =container_of (kref, + struct drm_syncobj, + refcount); +dma_fence_put (syncobj->fence); +kfree (syncobj); +} + +static int drm_syncobj_create (struct drm_file *file_private, +u32 *handle) +{ +int ret; +struct drm_syncobj *syncobj; + + syncobj =kzalloc (sizeof (struct drm_syncobj ),GFP_KERNEL ); +if (!syncobj) +return -ENOMEM ; + +kref_init (&syncobj->refcount); + +idr_preload (GFP_KERNEL ); +spin_lock (&file_private->syncobj_table_lock); + ret =idr_alloc (&file_private->syncobj_idr, syncobj,1 ,0 ,GFP_NOWAIT ); +spin_unlock (&file_private->syncobj_table_lock); + +idr_preload_end (); + +if (ret <0 ) { +drm_syncobj_put (syncobj); +return ret; + } + + *handle = ret; +return 0 ; +} + +static int drm_syncobj_destroy (struct drm_file *file_private, +u32 handle) +{ +struct drm_syncobj *syncobj; + +spin_lock (&file_private->syncobj_table_lock); + syncobj =idr_remove (&file_private->syncobj_idr, handle); +spin_unlock (&file_private->syncobj_table_lock); + +if (!syncobj) +return -ENOENT ; + +drm_syncobj_put (syncobj); +return 0 ; +} + +static int drm_syncobj_file_release (struct inode *inode,struct file *file) +{ +struct drm_syncobj *syncobj = file->private_data; + +drm_syncobj_put (syncobj); +return 0 ; +} + +static const struct file_operations drm_syncobj_file_fops = { + .release = drm_syncobj_file_release, +}; + +static int drm_syncobj_alloc_file (struct drm_syncobj *syncobj) +{ +struct file *file =anon_inode_getfile ("syncobj_file" , + &drm_syncobj_file_fops, + syncobj,0 ); +if (IS_ERR (file)) +return PTR_ERR (file); + +drm_syncobj_get (syncobj); +if (cmpxchg (&syncobj->file,NULL , file)) { +/* lost the race */ +fput (file); + } + +return 0 ; +} + +static int drm_syncobj_handle_to_fd (struct drm_file *file_private, +u32 handle,int *p_fd) +{ +struct drm_syncobj *syncobj =drm_syncobj_find (file_private, handle); +int ret; +int fd; + +if (!syncobj) +return -EINVAL ; + + fd =get_unused_fd_flags (O_CLOEXEC ); +if (fd <0 ) { +drm_syncobj_put (syncobj); +return fd; + } + +if (!syncobj->file) { + ret =drm_syncobj_alloc_file (syncobj); +if (ret) + goto out_put_fd; + } +fd_install (fd, syncobj->file); +drm_syncobj_put (syncobj); + *p_fd = fd; +return 0 ; +out_put_fd: +put_unused_fd (fd); +drm_syncobj_put (syncobj); +return ret; +} + +static struct drm_syncobj *drm_syncobj_fdget (int fd) +{ +struct file *file =fget (fd); + +if (!file) +return NULL ; +if (file->f_op != &drm_syncobj_file_fops) + goto err; + +return file->private_data; +err: +fput (file); +return NULL ; +}; + +static int drm_syncobj_fd_to_handle (struct drm_file *file_private, +int fd,u32 *handle) +{ +struct drm_syncobj *syncobj =drm_syncobj_fdget (fd); +int ret; + +if (!syncobj) +return -EINVAL ; + +/* take a reference to put in the idr */ +drm_syncobj_get (syncobj); + +idr_preload (GFP_KERNEL ); +spin_lock (&file_private->syncobj_table_lock); + ret =idr_alloc (&file_private->syncobj_idr, syncobj,1 ,0 ,GFP_NOWAIT ); +spin_unlock (&file_private->syncobj_table_lock); +idr_preload_end (); + +if (ret <0 ) { +fput (syncobj->file); +return ret; + } + *handle = ret; +return 0 ; +} + +/** +* drm_syncobj_open - initalizes syncobj file-private structures at devnode open time +* @dev: drm_device which is being opened by userspace +* @file_private: drm file-private structure to set up +* +* Called at device open time, sets up the structure for handling refcounting +* of sync objects. +*/ +void +drm_syncobj_open (struct drm_file *file_private) +{ +idr_init (&file_private->syncobj_idr); +spin_lock_init (&file_private->syncobj_table_lock); +} + +static int +drm_syncobj_release_handle (int id,void *ptr,void *data) +{ +struct drm_syncobj *syncobj = ptr; + +drm_syncobj_put (syncobj); +return 0 ; +} + +/** +* drm_syncobj_release - release file-private sync object resources +* @dev: drm_device which is being closed by userspace +* @file_private: drm file-private structure to clean up +* +* Called at close time when the filp is going away. +* +* Releases any remaining references on objects by this filp. +*/ +void +drm_syncobj_release (struct drm_file *file_private) +{ +idr_for_each (&file_private->syncobj_idr, + &drm_syncobj_release_handle, file_private); +idr_destroy (&file_private->syncobj_idr); +} + +int +drm_syncobj_create_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private) +{ +struct drm_syncobj_create *args = data; + +if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +return -ENODEV ; + +/* no valid flags yet */ +if (args->flags) +return -EINVAL ; + +return drm_syncobj_create (file_private, + &args->handle); +} + +int +drm_syncobj_destroy_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private) +{ +struct drm_syncobj_destroy *args = data; + +if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +return -ENODEV ; + +/* make sure padding is empty */ +if (args->pad) +return -EINVAL ; +return drm_syncobj_destroy (file_private, args->handle);
drivers/gpu/drm/drm_syncobj.c line 368
Quoted 10 lines
drivers/gpu/drm/drm_syncobj.c new file +10
-0+} + +int +drm_syncobj_handle_to_fd_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private) +{ +struct drm_syncobj_handle *args = data; + +if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +return -ENODEV ;
Validation for ->pad and ->flags missing.
drivers/gpu/drm/drm_syncobj.c line 381
Quoted 13 lines
drivers/gpu/drm/drm_syncobj.c new file +13
-0+ +return drm_syncobj_handle_to_fd (file_private, args->handle, + &args->fd); +} + +int +drm_syncobj_fd_to_handle_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private) +{ +struct drm_syncobj_handle *args = data; + +if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +return -ENODEV ;
Same. igts help with catching these :-)
Quoted 77 lines
drivers/gpu/drm/drm_syncobj.c new file +4
-0+ +return drm_syncobj_fd_to_handle (file_private, args->fd, + &args->handle); +}include/drm/drmP.h +5
-0@@ -405,6 +405,11 @@ struct drm_file {/** Lock for synchronization of access to object_idr. */ spinlock_t table_lock; +/** Mapping of sync object handles to object pointers. */ +struct idr syncobj_idr; +/** Lock for synchronization of access to syncobj_idr. */ +spinlock_t syncobj_table_lock; +struct file *filp;void *driver_priv;include/drm/drm_drv.h +1
-0@@ -53,6 +53,7 @@ struct drm_mode_create_dumb;#define DRIVER_RENDER 0x8000#define DRIVER_ATOMIC 0x10000#define DRIVER_KMS_LEGACY_CONTEXT 0x20000 +#define DRIVER_SYNCOBJ 0x40000/** * struct drm_driver - DRM driver structure include/drm/drm_syncobj.h new file +39
-0@@ -0,0 +1,87 @@ +/* +* Copyright © 2017 Red Hat +* +* Permission is hereby granted, free of charge, to any person obtaining a +* copy of this software and associated documentation files (the "Software"), +* to deal in the Software without restriction, including without limitation +* the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following conditions: +* +* The above copyright notice and this permission notice (including the next +* paragraph) shall be included in all copies or substantial portions of the +* Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +* IN THE SOFTWARE. +* +* Authors: +* +*/ +#ifndef __DRM_SYNCOBJ_H__ +#define __DRM_SYNCOBJ_H__ + +#include "linux/dma-fence.h" + +/** +* struct drm_syncobj - sync object. +* +* This structure defines a generic sync object which wraps a dma fence. +*/ +struct drm_syncobj { +/** +* @refcount: +*
include/drm/drm_syncobj.h line 49
Quoted 10 lines
include/drm/drm_syncobj.h new file +10
-0+* Reference count of this object. +*/ +struct kref refcount; +/** +* @fence: +* NULL or a pointer to the fence bound to this object. +*/ +struct dma_fence *fence; +/** +* @file
: missing at the end.
Quoted 95 lines
include/drm/drm_syncobj.h new file +38
-0+* a file backing for this syncobj. +*/ + struct file *file; +}; + +void drm_syncobj_free (struct kref *kref); + +/** +* drm_syncobj_get - acquire a syncobj reference +* @obj: sync object +* +* This acquires additional reference to @obj. It is illegal to call this +* without already holding a reference. No locks required. +*/ +static inline void +drm_syncobj_get (struct drm_syncobj *obj) +{ +kref_get (&obj->refcount); +} + +/** +* drm_syncobj_put - release a reference to a sync object. +* @obj: sync object. +*/ +static inline void +drm_syncobj_put (struct drm_syncobj *obj) +{ +kref_put (&obj->refcount, drm_syncobj_free); +} + +int drm_syncobj_fence_get (struct drm_file *file_private, +u32 handle, +struct dma_fence **fence); +int drm_syncobj_replace_fence (struct drm_file *file_private, +u32 handle, +struct dma_fence *fence); + +#endif include/uapi/drm/drm.h +25
-0@@ -647,6 +647,7 @@ struct drm_gem_open {#define DRM_CAP_CURSOR_HEIGHT 0x9#define DRM_CAP_ADDFB2_MODIFIERS 0x10#define DRM_CAP_PAGE_FLIP_TARGET 0x11 +#define DRM_CAP_SYNCOBJ 0x13/** DRM_IOCTL_GET_CAP ioctl argument type */ struct drm_get_cap {@@ -696,6 +697,25 @@ struct drm_prime_handle {__s32 fd; }; +struct drm_syncobj_create { +__u32 handle; +__u32 flags; +}; + +struct drm_syncobj_destroy { +__u32 handle; +__u32 pad; +}; + +struct drm_syncobj_handle { +__u32 handle; +/** Flags.. only applicable for handle->fd */ +__u32 flags; + +__s32 fd; +__u32 pad; +}; +#if defined(__cplusplus) }#endif @@ -814,6 +834,11 @@ extern "C" {#define DRM_IOCTL_MODE_CREATEPROPBLOB DRM_IOWR(0xBD, struct drm_mode_create_blob)#define DRM_IOCTL_MODE_DESTROYPROPBLOB DRM_IOWR(0xBE, struct drm_mode_destroy_blob) +#define DRM_IOCTL_SYNCOBJ_CREATE DRM_IOWR(0xBF, struct drm_syncobj_create) +#define DRM_IOCTL_SYNCOBJ_DESTROY DRM_IOWR(0xC0, struct drm_syncobj_destroy) +#define DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD DRM_IOWR(0xC1, struct drm_syncobj_handle) +#define DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE DRM_IOWR(0xC2, struct drm_syncobj_handle) +/** * Device specific ioctls should only be in their respective headers * The device specific ioctl range is from 0x40 to 0x9f. Signature · 6 lines
2.9.3 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Sean Paul
Reviewed-byRe: [PATCH 1/5] drm: introduce sync objects (v2)
In reply to a message not in the archiveQuoted 12 lines · Dave Airlie
From: Dave Airlie <[email protected]>
Sync objects are new toplevel drm object, that contain a pointer to a fence. This fence can be updated via command submission ioctls via drivers.
There is also a generic wait obj API modelled on the vulkan wait API (with code modelled on some amdgpu code).
These objects can be converted to an opaque fd that can be passes between processes.
Quoted 7 lines
v2: rename reference/unreference to put/get (Chris) fix leaked reference (David Zhou)
drop mutex in favour of cmpxchg (Chris) documentdrm_syncobj_fence_get
use ENOENT for syncobj lookup.Signed-off-by Dave Airlie <[email protected]>
With Daniel's comments addressed,
Quoted 713 lines
fixup
--- Documentation/gpu/drm-internals.rst | 3 + Documentation/gpu/drm-mm.rst | 6 + drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_fops.c | 8 + drivers/gpu/drm/drm_internal.h | 13 ++ drivers/gpu/drm/drm_ioctl.c | 12 ++ drivers/gpu/drm/drm_syncobj.c | 385 ++++++++++++++++++++++++++++++++++++ include/drm/drmP.h | 5 + include/drm/drm_drv.h | 1 + include/drm/drm_syncobj.h | 87 ++++++++ include/uapi/drm/drm.h | 25 +++ 11 files changed, 546 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_syncobj.c create mode 100644 include/drm/drm_syncobj.hDocumentation/gpu/drm-internals.rst +3
-0@@ -98,6 +98,9 @@ DRIVER_ATOMIC implement appropriate obj->atomic_get_property() vfuncs for any modeset objects with driver specific properties. +DRIVER_SYNCOBJ + Driver support drm sync objects. + Major, Minor and Patchlevel ~~~~~~~~~~~~~~~~~~~~~~~~~~~Documentation/gpu/drm-mm.rst +6
-0@@ -483,3 +483,9 @@ DRM Cache Handling .. kernel-doc:: drivers/gpu/drm/drm_cache.c :export: + +DRM Sync Objects +=========================== + +.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c + :export:drivers/gpu/drm/Makefile +1
-1@@ -16,7 +16,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \ drm_framebuffer.o drm_connector.o drm_blend.o \ drm_encoder.o drm_mode_object.o drm_property.o \ drm_plane.o drm_color_mgmt.o drm_print.o \ - drm_dumb_buffers.o drm_mode_config.o + drm_dumb_buffers.o drm_mode_config.o drm_syncobj.o drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o drm-$(CONFIG_DRM_VM) += drm_vm.odrivers/gpu/drm/drm_fops.c +8
-0@@ -219,6 +219,9 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)if (drm_core_check_feature (dev,DRIVER_GEM ))drm_gem_open (dev, priv); +if (drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +drm_syncobj_open (priv); +if (drm_core_check_feature (dev,DRIVER_PRIME ))drm_prime_init_file_private (&priv->prime);@@ -266,6 +269,8 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor) out_prime_destroy:if (drm_core_check_feature (dev,DRIVER_PRIME ))drm_prime_destroy_file_private (&priv->prime); +if (drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +drm_syncobj_release (priv);if (drm_core_check_feature (dev,DRIVER_GEM ))drm_gem_release (dev, priv);put_pid (priv->pid);@@ -400,6 +405,9 @@ int drm_release(struct inode *inode, struct file *filp)drm_property_destroy_user_blobs (dev, file_priv); } +if (drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +drm_syncobj_release (file_priv); +if (drm_core_check_feature (dev,DRIVER_GEM ))drm_gem_release (dev, file_priv);drivers/gpu/drm/drm_internal.h +13
-0@@ -142,4 +142,17 @@ static inline int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc) {return 0 ; } +#endif + +/* drm_syncobj.c */ +void drm_syncobj_open (struct drm_file *file_private); +void drm_syncobj_release (struct drm_file *file_private); +int drm_syncobj_create_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private); +int drm_syncobj_destroy_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private); +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);drivers/gpu/drm/drm_ioctl.c +12
-0@@ -240,6 +240,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ req->value |= dev->driver->prime_fd_to_handle ?DRM_PRIME_CAP_IMPORT :0 ; req->value |= dev->driver->prime_handle_to_fd ?DRM_PRIME_CAP_EXPORT :0 ;return 0 ; +case DRM_CAP_SYNCOBJ : + req->value =drm_core_check_feature (dev,DRIVER_SYNCOBJ ); +return 0 ; }/* Other caps only work with KMS drivers */ @@ -641,6 +644,15 @@ static const struct drm_ioctl_desc drm_ioctls[] = {DRM_IOCTL_DEF (DRM_IOCTL_MODE_ATOMIC , drm_mode_atomic_ioctl,DRM_MASTER |DRM_CONTROL_ALLOW |DRM_UNLOCKED ),DRM_IOCTL_DEF (DRM_IOCTL_MODE_CREATEPROPBLOB , drm_mode_createblob_ioctl,DRM_CONTROL_ALLOW |DRM_UNLOCKED ),DRM_IOCTL_DEF (DRM_IOCTL_MODE_DESTROYPROPBLOB , drm_mode_destroyblob_ioctl,DRM_CONTROL_ALLOW |DRM_UNLOCKED ), + +DRM_IOCTL_DEF (DRM_IOCTL_SYNCOBJ_CREATE , drm_syncobj_create_ioctl, +DRM_UNLOCKED |DRM_RENDER_ALLOW ), +DRM_IOCTL_DEF (DRM_IOCTL_SYNCOBJ_DESTROY , drm_syncobj_destroy_ioctl, +DRM_UNLOCKED |DRM_RENDER_ALLOW ), +DRM_IOCTL_DEF (DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD , drm_syncobj_handle_to_fd_ioctl, +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), };#define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )drivers/gpu/drm/drm_syncobj.c new file +385
-0@@ -0,0 +1,385 @@ +/* +* Copyright 2017 Red Hat +* +* Permission is hereby granted, free of charge, to any person obtaining a +* copy of this software and associated documentation files (the "Software"), +* to deal in the Software without restriction, including without limitation +* the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following conditions: +* +* The above copyright notice and this permission notice (including the next +* paragraph) shall be included in all copies or substantial portions of the +* Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +* IN THE SOFTWARE. +* +* Authors: +* +*/ + +/** +* DOC: Overview +* +* DRM synchronisation objects (syncobj) are a persistent objects, +* that contain an optional fence. The fence can be updated with a new +* fence, or be NULL. +* +* 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 +* +* Their primary use-case is to implement Vulkan fences and semaphores. +* +* syncobj have a kref reference count, but also have an optional file. +* The file is only created once the syncobj is exported. +* The file takes a reference on the kref. +*/ + +#include <drm/drmP.h> +#include <linux/file.h> +#include <linux/fs.h> +#include <linux/anon_inodes.h> + +#include "drm_internal.h" +#include <drm/drm_syncobj.h> + +static struct drm_syncobj *drm_syncobj_find (struct drm_file *file_private, +u32 handle) +{ +struct drm_syncobj *syncobj; + +spin_lock (&file_private->syncobj_table_lock); + +/* Check if we currently have a reference on the object */ + syncobj =idr_find (&file_private->syncobj_idr, handle); +if (syncobj) +drm_syncobj_get (syncobj); + +spin_unlock (&file_private->syncobj_table_lock); + +return syncobj; +} + +/** +* drm_syncobj_replace_fence - lookup and replace fence in a sync object. +* @file_private - drm file private pointer. +* @handle - syncobj handle to lookup +* @fence - fence to install in sync file. +* Returns: +* 0 on success, or -ENOENT when the handle doesn't point at a valid sem file. +* +* This looks up a sync object and replaces the fence on it, freeing +* the old one. +*/ +int drm_syncobj_replace_fence (struct drm_file *file_private, +u32 handle, +struct dma_fence *fence) +{ +struct drm_syncobj *syncobj =drm_syncobj_find (file_private, handle); +struct dma_fence *old_fence =NULL ; + +if (!syncobj) +return -ENOENT ; + +if (fence) +dma_fence_get (fence); + old_fence =xchg (&syncobj->fence, fence); + +dma_fence_put (old_fence); +drm_syncobj_put (syncobj); + +return 0 ; +} +EXPORT_SYMBOL (drm_syncobj_replace_fence); + +/** +* drm_syncobj_fence_get - lookup sync object and reference fence +* @file_private - drm file private pointer. +* @handle - syncobj handle to lookup +* @fence - returned referenced fence +* Returns: +* 0 on success, or -ENOENT where syncobj is invalid, +* or -EINVAL when the syncobj has no fence. +* +* This returns a reference to underlying fence backing the +* syncobj if one exists. +*/ +int drm_syncobj_fence_get (struct drm_file *file_private, +u32 handle, +struct dma_fence **fence) +{ +struct drm_syncobj *syncobj =drm_syncobj_find (file_private, handle); +int ret =0 ; + +if (!syncobj) +return -ENOENT ; + + *fence =dma_fence_get (syncobj->fence); +if (!*fence) { + ret = -EINVAL ; + } +drm_syncobj_put (syncobj); +return ret; +} +EXPORT_SYMBOL (drm_syncobj_fence_get); + +void drm_syncobj_free (struct kref *kref) +{ +struct drm_syncobj *syncobj =container_of (kref, + struct drm_syncobj, + refcount); +dma_fence_put (syncobj->fence); +kfree (syncobj); +} + +static int drm_syncobj_create (struct drm_file *file_private, +u32 *handle) +{ +int ret; +struct drm_syncobj *syncobj; + + syncobj =kzalloc (sizeof (struct drm_syncobj ),GFP_KERNEL ); +if (!syncobj) +return -ENOMEM ; + +kref_init (&syncobj->refcount); + +idr_preload (GFP_KERNEL ); +spin_lock (&file_private->syncobj_table_lock); + ret =idr_alloc (&file_private->syncobj_idr, syncobj,1 ,0 ,GFP_NOWAIT ); +spin_unlock (&file_private->syncobj_table_lock); + +idr_preload_end (); + +if (ret <0 ) { +drm_syncobj_put (syncobj); +return ret; + } + + *handle = ret; +return 0 ; +} + +static int drm_syncobj_destroy (struct drm_file *file_private, +u32 handle) +{ +struct drm_syncobj *syncobj; + +spin_lock (&file_private->syncobj_table_lock); + syncobj =idr_remove (&file_private->syncobj_idr, handle); +spin_unlock (&file_private->syncobj_table_lock); + +if (!syncobj) +return -ENOENT ; + +drm_syncobj_put (syncobj); +return 0 ; +} + +static int drm_syncobj_file_release (struct inode *inode,struct file *file) +{ +struct drm_syncobj *syncobj = file->private_data; + +drm_syncobj_put (syncobj); +return 0 ; +} + +static const struct file_operations drm_syncobj_file_fops = { + .release = drm_syncobj_file_release, +}; + +static int drm_syncobj_alloc_file (struct drm_syncobj *syncobj) +{ +struct file *file =anon_inode_getfile ("syncobj_file" , + &drm_syncobj_file_fops, + syncobj,0 ); +if (IS_ERR (file)) +return PTR_ERR (file); + +drm_syncobj_get (syncobj); +if (cmpxchg (&syncobj->file,NULL , file)) { +/* lost the race */ +fput (file); + } + +return 0 ; +} + +static int drm_syncobj_handle_to_fd (struct drm_file *file_private, +u32 handle,int *p_fd) +{ +struct drm_syncobj *syncobj =drm_syncobj_find (file_private, handle); +int ret; +int fd; + +if (!syncobj) +return -EINVAL ; + + fd =get_unused_fd_flags (O_CLOEXEC ); +if (fd <0 ) { +drm_syncobj_put (syncobj); +return fd; + } + +if (!syncobj->file) { + ret =drm_syncobj_alloc_file (syncobj); +if (ret) + goto out_put_fd; + } +fd_install (fd, syncobj->file); +drm_syncobj_put (syncobj); + *p_fd = fd; +return 0 ; +out_put_fd: +put_unused_fd (fd); +drm_syncobj_put (syncobj); +return ret; +} + +static struct drm_syncobj *drm_syncobj_fdget (int fd) +{ +struct file *file =fget (fd); + +if (!file) +return NULL ; +if (file->f_op != &drm_syncobj_file_fops) + goto err; + +return file->private_data; +err: +fput (file); +return NULL ; +}; + +static int drm_syncobj_fd_to_handle (struct drm_file *file_private, +int fd,u32 *handle) +{ +struct drm_syncobj *syncobj =drm_syncobj_fdget (fd); +int ret; + +if (!syncobj) +return -EINVAL ; + +/* take a reference to put in the idr */ +drm_syncobj_get (syncobj); + +idr_preload (GFP_KERNEL ); +spin_lock (&file_private->syncobj_table_lock); + ret =idr_alloc (&file_private->syncobj_idr, syncobj,1 ,0 ,GFP_NOWAIT ); +spin_unlock (&file_private->syncobj_table_lock); +idr_preload_end (); + +if (ret <0 ) { +fput (syncobj->file); +return ret; + } + *handle = ret; +return 0 ; +} + +/** +* drm_syncobj_open - initalizes syncobj file-private structures at devnode open time +* @dev: drm_device which is being opened by userspace +* @file_private: drm file-private structure to set up +* +* Called at device open time, sets up the structure for handling refcounting +* of sync objects. +*/ +void +drm_syncobj_open (struct drm_file *file_private) +{ +idr_init (&file_private->syncobj_idr); +spin_lock_init (&file_private->syncobj_table_lock); +} + +static int +drm_syncobj_release_handle (int id,void *ptr,void *data) +{ +struct drm_syncobj *syncobj = ptr; + +drm_syncobj_put (syncobj); +return 0 ; +} + +/** +* drm_syncobj_release - release file-private sync object resources +* @dev: drm_device which is being closed by userspace +* @file_private: drm file-private structure to clean up +* +* Called at close time when the filp is going away. +* +* Releases any remaining references on objects by this filp. +*/ +void +drm_syncobj_release (struct drm_file *file_private) +{ +idr_for_each (&file_private->syncobj_idr, + &drm_syncobj_release_handle, file_private); +idr_destroy (&file_private->syncobj_idr); +} + +int +drm_syncobj_create_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private) +{ +struct drm_syncobj_create *args = data; + +if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +return -ENODEV ; + +/* no valid flags yet */ +if (args->flags) +return -EINVAL ; + +return drm_syncobj_create (file_private, + &args->handle); +} + +int +drm_syncobj_destroy_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private) +{ +struct drm_syncobj_destroy *args = data; + +if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +return -ENODEV ; + +/* make sure padding is empty */ +if (args->pad) +return -EINVAL ; +return drm_syncobj_destroy (file_private, args->handle); +} + +int +drm_syncobj_handle_to_fd_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private) +{ +struct drm_syncobj_handle *args = data; + +if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +return -ENODEV ; + +return drm_syncobj_handle_to_fd (file_private, args->handle, + &args->fd); +} + +int +drm_syncobj_fd_to_handle_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_private) +{ +struct drm_syncobj_handle *args = data; + +if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ )) +return -ENODEV ; + +return drm_syncobj_fd_to_handle (file_private, args->fd, + &args->handle); +}include/drm/drmP.h +5
-0@@ -405,6 +405,11 @@ struct drm_file {/** Lock for synchronization of access to object_idr. */ spinlock_t table_lock; +/** Mapping of sync object handles to object pointers. */ +struct idr syncobj_idr; +/** Lock for synchronization of access to syncobj_idr. */ +spinlock_t syncobj_table_lock; +struct file *filp;void *driver_priv;include/drm/drm_drv.h +1
-0@@ -53,6 +53,7 @@ struct drm_mode_create_dumb;#define DRIVER_RENDER 0x8000#define DRIVER_ATOMIC 0x10000#define DRIVER_KMS_LEGACY_CONTEXT 0x20000 +#define DRIVER_SYNCOBJ 0x40000/** * struct drm_driver - DRM driver structure include/drm/drm_syncobj.h new file +87
-0@@ -0,0 +1,87 @@ +/* +* Copyright © 2017 Red Hat +* +* Permission is hereby granted, free of charge, to any person obtaining a +* copy of this software and associated documentation files (the "Software"), +* to deal in the Software without restriction, including without limitation +* the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following conditions: +* +* The above copyright notice and this permission notice (including the next +* paragraph) shall be included in all copies or substantial portions of the +* Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +* IN THE SOFTWARE. +* +* Authors: +* +*/ +#ifndef __DRM_SYNCOBJ_H__ +#define __DRM_SYNCOBJ_H__ + +#include "linux/dma-fence.h" + +/** +* struct drm_syncobj - sync object. +* +* This structure defines a generic sync object which wraps a dma fence. +*/ +struct drm_syncobj { +/** +* @refcount: +* +* Reference count of this object. +*/ +struct kref refcount; +/** +* @fence: +* NULL or a pointer to the fence bound to this object. +*/ +struct dma_fence *fence; +/** +* @file +* a file backing for this syncobj. +*/ +struct file *file; +}; + +void drm_syncobj_free (struct kref *kref); + +/** +* drm_syncobj_get - acquire a syncobj reference +* @obj: sync object +* +* This acquires additional reference to @obj. It is illegal to call this +* without already holding a reference. No locks required. +*/ +static inline void +drm_syncobj_get (struct drm_syncobj *obj) +{ +kref_get (&obj->refcount); +} + +/** +* drm_syncobj_put - release a reference to a sync object. +* @obj: sync object. +*/ +static inline void +drm_syncobj_put (struct drm_syncobj *obj) +{ +kref_put (&obj->refcount, drm_syncobj_free); +} + +int drm_syncobj_fence_get (struct drm_file *file_private, +u32 handle, +struct dma_fence **fence); +int drm_syncobj_replace_fence (struct drm_file *file_private, +u32 handle, +struct dma_fence *fence); + +#endif include/uapi/drm/drm.h +25
-0@@ -647,6 +647,7 @@ struct drm_gem_open {#define DRM_CAP_CURSOR_HEIGHT 0x9#define DRM_CAP_ADDFB2_MODIFIERS 0x10#define DRM_CAP_PAGE_FLIP_TARGET 0x11 +#define DRM_CAP_SYNCOBJ 0x13/** DRM_IOCTL_GET_CAP ioctl argument type */ struct drm_get_cap {@@ -696,6 +697,25 @@ struct drm_prime_handle {__s32 fd; }; +struct drm_syncobj_create { +__u32 handle; +__u32 flags; +}; + +struct drm_syncobj_destroy { +__u32 handle; +__u32 pad; +}; + +struct drm_syncobj_handle { +__u32 handle; +/** Flags.. only applicable for handle->fd */ +__u32 flags; + +__s32 fd; +__u32 pad; +}; +#if defined(__cplusplus) }#endif @@ -814,6 +834,11 @@ extern "C" {#define DRM_IOCTL_MODE_CREATEPROPBLOB DRM_IOWR(0xBD, struct drm_mode_create_blob)#define DRM_IOCTL_MODE_DESTROYPROPBLOB DRM_IOWR(0xBE, struct drm_mode_destroy_blob) +#define DRM_IOCTL_SYNCOBJ_CREATE DRM_IOWR(0xBF, struct drm_syncobj_create) +#define DRM_IOCTL_SYNCOBJ_DESTROY DRM_IOWR(0xC0, struct drm_syncobj_destroy) +#define DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD DRM_IOWR(0xC1, struct drm_syncobj_handle) +#define DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE DRM_IOWR(0xC2, struct drm_syncobj_handle) +/** * Device specific ioctls should only be in their respective headers * The device specific ioctl range is from 0x40 to 0x9f. Signature · 6 lines
2.9.3 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 5 lines
Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
PATCH 2/5 drm/syncobj: add sync obj wait interface. (v2)
In reply to a message not in the archiveFrom: Dave Airlie <[email protected]>
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.
--- 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(-)
drivers/gpu/drm/drm_internal.h +2 -0
@@ -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);
drivers/gpu/drm/drm_ioctl.c +2 -0
@@ -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 )
drivers/gpu/drm/drm_syncobj.c +138 -1
156 lines not on this page · raw message
include/uapi/drm/drm.h +12 -0
@@ -716,6 +716,17 @@ struct drm_syncobj_handle {__u32 pad; }; +/* timeout_ns is relative timeout in nanoseconds */ +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0) +struct drm_syncobj_wait { +__u64 handles; +__u64 timeout_ns; +__u32 count_handles; +__u32 flags; +__u32 out_status; +__u32 first_signaled;/* on valid when not waiting all */ +}; +#if defined(__cplusplus) }#endif
@@ -838,6 +849,7 @@ extern "C" {#define DRM_IOCTL_SYNCOBJ_DESTROY DRM_IOWR(0xC0, struct drm_syncobj_destroy)#define DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD DRM_IOWR(0xC1, struct drm_syncobj_handle)#define DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE DRM_IOWR(0xC2, struct drm_syncobj_handle) +#define DRM_IOCTL_SYNCOBJ_WAIT DRM_IOWR(0xC3, struct drm_syncobj_wait)/** * Device specific ioctls should only be in their respective headers
Signature · 6 lines
2.9.3 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/5] drm/syncobj: add sync obj wait interface. (v2)
Quoted 10 lines · Dave Airlie
From: Dave Airlie <[email protected]>
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 <[email protected]>
Quoted 230 lines
--- 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(-)drivers/gpu/drm/drm_internal.h +2
-0@@ -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);drivers/gpu/drm/drm_ioctl.c +2
-0@@ -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 )drivers/gpu/drm/drm_syncobj.c +138
-1@@ -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 ) +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: +if (ret >0 ) + wait->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; +}include/uapi/drm/drm.h +12
-0@@ -716,6 +716,17 @@ struct drm_syncobj_handle {__u32 pad; }; +/* timeout_ns is relative timeout in nanoseconds */ +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0) +struct drm_syncobj_wait { +__u64 handles; +__u64 timeout_ns; +__u32 count_handles; +__u32 flags; +__u32 out_status; +__u32 first_signaled;/* on valid when not waiting all */ +}; +#if defined(__cplusplus) }#endif @@ -838,6 +849,7 @@ extern "C" {#define DRM_IOCTL_SYNCOBJ_DESTROY DRM_IOWR(0xC0, struct drm_syncobj_destroy)#define DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD DRM_IOWR(0xC1, struct drm_syncobj_handle)#define DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE DRM_IOWR(0xC2, struct drm_syncobj_handle) +#define DRM_IOCTL_SYNCOBJ_WAIT DRM_IOWR(0xC3, struct drm_syncobj_wait)/** * Device specific ioctls should only be in their respective headers Signature · 6 lines
2.9.3 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
A message that is not in the archive (sent off-list or not yet received): 20170512003457.24936-3-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Re: [PATCH 2/5] drm/syncobj: add sync obj wait interface. (v2)
Quoted 11 lines · Dave Airlie
drivers/gpu/drm/drm_syncobj.c +11
-0+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;
drivers/gpu/drm/drm_syncobj.c drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, · line 412
Quoted 10 lines
drivers/gpu/drm/drm_syncobj.c +10
-0+ + 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
Signature · 5 lines
Chris Wilson, Intel Open Source Technology Centre _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): 20170512084951.GD12185-aII6DKEyn0pWYbfKqPwjAkR8Iwp7RQ6xAL8bYrjMMd8@public.gmane.org
Re: [PATCH 2/5] drm/syncobj: add sync obj wait interface. (v2)
Am 12.05.2017 um 10:49 schrieb Chris Wilson:
Quoted 17 lines · Dave Airlie
Quoted 16 lines
drivers/gpu/drm/drm_syncobj.c +16
-0+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;
Quoted 8 lines
drivers/gpu/drm/drm_syncobj.c drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, · line 412
Quoted 5 lines
drivers/gpu/drm/drm_syncobj.c +5
-0+ +if (!fence) +continue ; + + ret =dma_fence_wait_timeout (fence, true, timeout);Doesn't handle
-EINTRyet with timeout. If having adrmIoctl()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.
Quoted 2 lines
-Chris
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Sean Paul
Reviewed-byRe: [PATCH 2/5] drm/syncobj: add sync obj wait interface. (v2)
In reply to a message not in the archiveQuoted 10 lines · Dave Airlie
From: Dave Airlie <[email protected]>
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 <[email protected]>
Quoted 230 lines
--- 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(-)drivers/gpu/drm/drm_internal.h +2
-0@@ -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);drivers/gpu/drm/drm_ioctl.c +2
-0@@ -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 )drivers/gpu/drm/drm_syncobj.c +138
-1@@ -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 ) +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: +if (ret >0 ) + wait->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; +}include/uapi/drm/drm.h +12
-0@@ -716,6 +716,17 @@ struct drm_syncobj_handle {__u32 pad; }; +/* timeout_ns is relative timeout in nanoseconds */ +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0) +struct drm_syncobj_wait { +__u64 handles; +__u64 timeout_ns; +__u32 count_handles; +__u32 flags; +__u32 out_status; +__u32 first_signaled;/* on valid when not waiting all */ +}; +#if defined(__cplusplus) }#endif @@ -838,6 +849,7 @@ extern "C" {#define DRM_IOCTL_SYNCOBJ_DESTROY DRM_IOWR(0xC0, struct drm_syncobj_destroy)#define DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD DRM_IOWR(0xC1, struct drm_syncobj_handle)#define DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE DRM_IOWR(0xC2, struct drm_syncobj_handle) +#define DRM_IOCTL_SYNCOBJ_WAIT DRM_IOWR(0xC3, struct drm_syncobj_wait)/** * Device specific ioctls should only be in their respective headers Signature · 6 lines
2.9.3 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 5 lines
Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
PATCH 3/5 drm/syncobj: add sync_file interaction.
In reply to a message not in the archiveFrom: Dave Airlie <[email protected]>
This interface allows importing the fence from a sync_file into an existing drm sync object, or exporting the fence attached to an existing drm sync object into a new sync file object.
This should only be used to interact with sync files where necessary.
--- drivers/gpu/drm/drm_syncobj.c | 56 +++++++++++++++++++++++++++++++++++++++++++ include/uapi/drm/drm.h | 6 +++-- 2 files changed, 60 insertions(+), 2 deletions(-)
drivers/gpu/drm/drm_syncobj.c +56 -0
@@ -52,6 +52,7 @@#include <linux/file.h> #include <linux/fs.h> #include <linux/anon_inodes.h> +#include <linux/sync_file.h> #include "drm_internal.h" #include <drm/drm_syncobj.h>
@@ -290,6 +291,48 @@ static int drm_syncobj_fd_to_handle(struct drm_file *file_private,return 0 ; } +int drm_syncobj_import_sync_file_fence (struct drm_file *file_private, +int fd,int handle) +{ +struct dma_fence *fence =sync_file_get_fence (fd); +if (!fence) +return -EINVAL ; + +return drm_syncobj_replace_fence (file_private, handle, fence); +} + +int drm_syncobj_export_sync_file (struct drm_file *file_private, +int handle,int *p_fd) +{ +int ret; +struct dma_fence *fence; +struct sync_file *sync_file; +int fd =get_unused_fd_flags (O_CLOEXEC ); + +if (fd <0 ) +return fd; + + ret =drm_syncobj_fence_get (file_private, handle, &fence); +if (ret) + goto err_put_fd; + + sync_file =sync_file_create (fence); +if (!sync_file) { + ret = -EINVAL ; + goto err_fence_put; + } + +fd_install (fd, sync_file->file); + +dma_fence_put (fence); + *p_fd = fd; +return 0 ; +err_fence_put: +dma_fence_put (fence); +err_put_fd: +put_unused_fd (fd); +return ret; +}/** * drm_syncobj_open - initalizes syncobj file-private structures at devnode open time * @dev: drm_device which is being opened by userspace
@@ -372,6 +415,12 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ ))return -ENODEV ; +if (args->flags &DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_FENCE_SYNC_FILE ) +return drm_syncobj_export_sync_file (file_private, args->handle, + &args->fd); +else if (args->flags) +return -EINVAL ; +return drm_syncobj_handle_to_fd (file_private, args->handle, &args->fd); }
@@ -385,6 +434,13 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ ))return -ENODEV ; +if (args->flags &DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE_FENCE ) +return drm_syncobj_import_sync_file_fence (file_private, + args->fd, + args->handle); +else if (args->flags) +return -EINVAL ; +return drm_syncobj_fd_to_handle (file_private, args->fd, &args->handle); }
include/uapi/drm/drm.h +4 -2
@@ -707,13 +707,15 @@ struct drm_syncobj_destroy {__u32 pad; }; +#define DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE_FENCE (1 << 0) +#define DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_FENCE_SYNC_FILE (1 << 0)struct drm_syncobj_handle {__u32 handle;/** Flags.. only applicable for handle->fd */ -__u32 flags; +__u32 fd_flags;__s32 fd; -__u32 pad; +__u32 flags; };/* timeout_ns is relative timeout in nanoseconds */
Signature · 6 lines
2.9.3 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): 20170512003457.24936-4-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Re: [PATCH 3/5] drm/syncobj: add sync_file interaction.
Quoted 9 lines · Dave Airlie
From: Dave Airlie <[email protected]>
This interface allows importing the fence from a
sync_fileinto an existing drm sync object, or exporting the fence attached to an existing drm sync object into a new sync file object.This should only be used to interact with sync files where necessary.
Signed-off-by Dave Airlie <[email protected]>
A bunch of nits around ioctl validation and stuff. lgtm otherwise, ack with those addressed.
-Daniel
Quoted 28 lines
--- drivers/gpu/drm/drm_syncobj.c | 56 +++++++++++++++++++++++++++++++++++++++++++ include/uapi/drm/drm.h | 6 +++-- 2 files changed, 60 insertions(+), 2 deletions(-)drivers/gpu/drm/drm_syncobj.c +8
-0@@ -52,6 +52,7 @@#include <linux/file.h> #include <linux/fs.h> #include <linux/anon_inodes.h> +#include <linux/sync_file.h> #include "drm_internal.h" #include <drm/drm_syncobj.h> @@ -290,6 +291,48 @@ static int drm_syncobj_fd_to_handle(struct drm_file *file_private,return 0 ; } +int drm_syncobj_import_sync_file_fence (struct drm_file *file_private, +int fd,int handle) +{ +struct dma_fence *fence =sync_file_get_fence (fd); +if (!fence) +return -EINVAL ; +
drivers/gpu/drm/drm_syncobj.c static int drm_syncobj_fd_to_handle(struct drm_file *file_private, · line 310
Quoted 10 lines
drivers/gpu/drm/drm_syncobj.c +10
-0+return drm_syncobj_replace_fence (file_private, handle, fence); +} + +int drm_syncobj_export_sync_file (struct drm_file *file_private, +int handle,int *p_fd) +{ +int ret; +struct dma_fence *fence; +struct sync_file *sync_file; +int fd =get_unused_fd_flags (O_CLOEXEC );
I guess the idea was to use args->fd_flags here?
Quoted 29 lines
drivers/gpu/drm/drm_syncobj.c +25
-0+ +if (fd <0 ) +return fd; + + ret =drm_syncobj_fence_get (file_private, handle, &fence); +if (ret) + goto err_put_fd; + + sync_file =sync_file_create (fence); +if (!sync_file) { + ret = -EINVAL ; + goto err_fence_put; + } + +fd_install (fd, sync_file->file); + +dma_fence_put (fence); + *p_fd = fd; +return 0 ; +err_fence_put: +dma_fence_put (fence); +err_put_fd: +put_unused_fd (fd); +return ret; +}/** * drm_syncobj_open - initalizes syncobj file-private structures at devnode open time * @dev: drm_device which is being opened by userspace
drivers/gpu/drm/drm_syncobj.c drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data, · line 420
Quoted 6 lines
drivers/gpu/drm/drm_syncobj.c +3
-0@@ -372,6 +415,12 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ ))return -ENODEV ; +if (args->flags &DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_FENCE_SYNC_FILE ) +return drm_syncobj_export_sync_file (file_private, args->handle, + &args->fd);
Checks like in the wait ioctl that no other flags are set is missing in both callbacks.
Quoted 29 lines
drivers/gpu/drm/drm_syncobj.c +10
-0+else if (args->flags) +return -EINVAL ; +return drm_syncobj_handle_to_fd (file_private, args->handle, &args->fd); }@@ -385,6 +434,13 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ ))return -ENODEV ; +if (args->flags &DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE_FENCE ) +return drm_syncobj_import_sync_file_fence (file_private, + args->fd, + args->handle); +else if (args->flags) +return -EINVAL ; +return drm_syncobj_fd_to_handle (file_private, args->fd, &args->handle); }include/uapi/drm/drm.h +1
-0@@ -707,13 +707,15 @@ struct drm_syncobj_destroy {__u32 pad; }; +#define DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE_FENCE (1 << 0)
include/uapi/drm/drm.h struct drm_syncobj_destroy { · line 718
Quoted 10 lines
include/uapi/drm/drm.h +3
-2+#define DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_FENCE_SYNC_FILE (1 << 0)struct drm_syncobj_handle {__u32 handle;/** Flags.. only applicable for handle->fd */ -__u32 flags; +__u32 fd_flags;__s32 fd; -__u32 pad; +__u32 flags;
s/pad/fd_flags/ instead of the shuffle you've done here I presume?
Also I didn't spot any fd_flags validation anywhere.
Quoted 10 lines
include/uapi/drm/drm.h +0
-0};/* timeout_ns is relative timeout in nanoseconds */ Signature · 6 lines
2.9.3 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Sean Paul
Reviewed-byRe: [PATCH 3/5] drm/syncobj: add sync_file interaction.
In reply to a message not in the archiveQuoted 9 lines · Dave Airlie
From: Dave Airlie <[email protected]>
This interface allows importing the fence from a
sync_fileinto an existing drm sync object, or exporting the fence attached to an existing drm sync object into a new sync file object.This should only be used to interact with sync files where necessary.
Signed-off-by Dave Airlie <[email protected]>
With Daniel's comments taken into account,
Quoted 122 lines
--- drivers/gpu/drm/drm_syncobj.c | 56 +++++++++++++++++++++++++++++++++++++++++++ include/uapi/drm/drm.h | 6 +++-- 2 files changed, 60 insertions(+), 2 deletions(-)drivers/gpu/drm/drm_syncobj.c +56
-0@@ -52,6 +52,7 @@#include <linux/file.h> #include <linux/fs.h> #include <linux/anon_inodes.h> +#include <linux/sync_file.h> #include "drm_internal.h" #include <drm/drm_syncobj.h> @@ -290,6 +291,48 @@ static int drm_syncobj_fd_to_handle(struct drm_file *file_private,return 0 ; } +int drm_syncobj_import_sync_file_fence (struct drm_file *file_private, +int fd,int handle) +{ +struct dma_fence *fence =sync_file_get_fence (fd); +if (!fence) +return -EINVAL ; + +return drm_syncobj_replace_fence (file_private, handle, fence); +} + +int drm_syncobj_export_sync_file (struct drm_file *file_private, +int handle,int *p_fd) +{ +int ret; +struct dma_fence *fence; +struct sync_file *sync_file; +int fd =get_unused_fd_flags (O_CLOEXEC ); + +if (fd <0 ) +return fd; + + ret =drm_syncobj_fence_get (file_private, handle, &fence); +if (ret) + goto err_put_fd; + + sync_file =sync_file_create (fence); +if (!sync_file) { + ret = -EINVAL ; + goto err_fence_put; + } + +fd_install (fd, sync_file->file); + +dma_fence_put (fence); + *p_fd = fd; +return 0 ; +err_fence_put: +dma_fence_put (fence); +err_put_fd: +put_unused_fd (fd); +return ret; +}/** * drm_syncobj_open - initalizes syncobj file-private structures at devnode open time * @dev: drm_device which is being opened by userspace @@ -372,6 +415,12 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ ))return -ENODEV ; +if (args->flags &DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_FENCE_SYNC_FILE ) +return drm_syncobj_export_sync_file (file_private, args->handle, + &args->fd); +else if (args->flags) +return -EINVAL ; +return drm_syncobj_handle_to_fd (file_private, args->handle, &args->fd); }@@ -385,6 +434,13 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ ))return -ENODEV ; +if (args->flags &DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE_FENCE ) +return drm_syncobj_import_sync_file_fence (file_private, + args->fd, + args->handle); +else if (args->flags) +return -EINVAL ; +return drm_syncobj_fd_to_handle (file_private, args->fd, &args->handle); }include/uapi/drm/drm.h +4
-2@@ -707,13 +707,15 @@ struct drm_syncobj_destroy {__u32 pad; }; +#define DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE_FENCE (1 << 0) +#define DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_FENCE_SYNC_FILE (1 << 0)struct drm_syncobj_handle {__u32 handle;/** Flags.. only applicable for handle->fd */ -__u32 flags; +__u32 fd_flags;__s32 fd; -__u32 pad; +__u32 flags; };/* timeout_ns is relative timeout in nanoseconds */ Signature · 6 lines
2.9.3 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 5 lines
Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 3/5] drm/syncobj: add sync_file interaction.
In reply to a message not in the archiveQuoting Dave Airlie (2017-05-12 01:34:55)
drivers/gpu/drm/drm_syncobj.c drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, · line 442
Quoted 10 lines
drivers/gpu/drm/drm_syncobj.c +6
-0@@ -385,6 +434,13 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ ))return -ENODEV ; +if (args->flags &DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE_FENCE ) +return drm_syncobj_import_sync_file_fence (file_private, + args->fd, + args->handle); +else if (args->flags) +return -EINVAL ;
Argh, what I missed before was that importing from a sync_file reuses the handle, but importing from a syncobj fd creates a new handle.
Just venting my ocd. It would be nice if the interface was consistent, and I can see arguments for both approaches (just not a good argument as to why they should differ). A compromise would be a flag to create/reuse handle (or if handle=0, create, if handle!=0 resuse).
-Chris
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Dave Airlie
maintainerRe: [PATCH 3/5] drm/syncobj: add sync_file interaction.
Quoted 15 lines · Chris Wilson
Quoting Dave Airlie (2017-05-12 01:34:55)
drivers/gpu/drm/drm_syncobj.c drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, · line 442
Quoted 10 lines
drivers/gpu/drm/drm_syncobj.c +6
-0@@ -385,6 +434,13 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ ))return -ENODEV ; +if (args->flags &DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE_FENCE ) +return drm_syncobj_import_sync_file_fence (file_private, + args->fd, + args->handle); +else if (args->flags) +return -EINVAL ;Argh, what I missed before was that importing from a
sync_filereuses the handle, but importing from a syncobj fd creates a new handle.
Quoted 4 lines
Just venting my ocd. It would be nice if the interface was consistent, and I can see arguments for both approaches (just not a good argument as to why they should differ). A compromise would be a flag to create/reuse handle (or if handle=0, create, if handle!=0 resuse).
The interface is consistent for the objects.
With a sync_fd import you import the state into an existing syncobj.
With a syncobj import you import the object into the current process state (so get a new handle).
I can't think of a use case for anything else, maybe having a sync_fd state imported into a new syncobj? but not sure it really helps.
Userspace should be using different paths to get to these interfaces.
Dave.
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
A message that is not in the archive (sent off-list or not yet received): CAPM=9tw8kFhHWp1tVhmBRqBqX4WnaSvQtLd0OD_U=CNpDROsBQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Re: [PATCH 3/5] drm/syncobj: add sync_file interaction.
Quoting Dave Airlie (2017-08-04 00:01:10)
Quoted 21 lines · Chris Wilson
Quoted 15 lines
Quoting Dave Airlie (2017-05-12 01:34:55)
drivers/gpu/drm/drm_syncobj.c drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, · line 442
Quoted 10 lines
drivers/gpu/drm/drm_syncobj.c +6
-0@@ -385,6 +434,13 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,if (!drm_core_check_feature (dev,DRIVER_SYNCOBJ ))return -ENODEV ; +if (args->flags &DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE_FENCE ) +return drm_syncobj_import_sync_file_fence (file_private, + args->fd, + args->handle); +else if (args->flags) +return -EINVAL ;Argh, what I missed before was that importing from a
sync_filereuses the handle, but importing from a syncobj fd creates a new handle.Quoted 4 lines
Just venting my ocd. It would be nice if the interface was consistent, and I can see arguments for both approaches (just not a good argument as to why they should differ). A compromise would be a flag to create/reuse handle (or if handle=0, create, if handle!=0 resuse).
Quoted 6 lines
The interface is consistent for the objects.
With a
sync_fdimport you import the state into an existing syncobj.With a syncobj import you import the object into the current process state (so get a new handle).
You can say the same for either path though, e.g.
With a sync fd import you import the fence into the current process state (so get a new handle).
One process is using explict sync file fencing, presumably passing it to kms and other consumers,and passes it to a second process that prefers to use syncobj and so encapsulates it.
Quoted 2 lines
I can't think of a use case for anything else, maybe having a
sync_fdstate imported into a new syncobj? but not sure it really helps.
Yes, I expect taking a sync_file fd and generating a new syncobj is just as desired as taking a syncobj fd and generating a new synbocj. Conversely, importing a syncobj fd into a preallocated syncobj makes just as much sense as for a sync_file fd.
Quoted 1 line
Userspace should be using different paths to get to these interfaces.
Still the interfaces are different for no good reason, which certainly makes for a surprising API.
-Chris
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): 150180256673.14563.14290309660718108784-M6iVdVfohj6unts5RBS2dVaTQe2KTcn/@public.gmane.org
Re: [PATCH 3/5] drm/syncobj: add sync_file interaction.
Quoted 26 lines · Chris Wilson
Quoting Dave Airlie (2017-08-04 00:01:10)
Quoted 25 lines · Chris Wilson
Quoted 15 lines
Quoting Dave Airlie (2017-05-12 01:34:55)
Review drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, · line 442
Quoted 10 lines
(quoted diff) +6
-0@@ -385,6 +434,13 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) return -ENODEV; + if (args->flags & DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE_FENCE) + return drm_syncobj_import_sync_file_fence(file_private, + args->fd, + args->handle); + else if (args->flags) + return -EINVAL;Argh, what I missed before was that importing from a
sync_filereuses the handle, but importing from a syncobj fd creates a new handle.Quoted 4 lines
Just venting my ocd. It would be nice if the interface was consistent, and I can see arguments for both approaches (just not a good argument as to why they should differ). A compromise would be a flag to create/reuse handle (or if handle=0, create, if handle!=0 resuse).
The interface is consistent for the objects.
With a
sync_fdimport you import the state into an existing syncobj.
Quoted 7 lines
Quoted 2 lines
With a syncobj import you import the object into the current process state (so get a new handle).
You can say the same for either path though, e.g.
With a sync fd import you import the fence into the current process state (so get a new handle).
You import the fence, not the sync_fd. There is a difference here. A syncobj has a fence underlying it, it makes sense to import a new fence state into an existing object. It doesn't make sense to import a new syncobj into an existing object. It would be like importing a GEM object into a GEM object.
I expect document it and be done is the best answer unless we can come up with a userspace use case that really expects the other behaviour.
Dave.
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
PATCH 4/5 amdgpu/cs: split out fence dependency checking
In reply to a message not in the archiveFrom: Dave Airlie <[email protected]>
This just splits out the fence depenency checking into it's own function to make it easier to add semaphore dependencies.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 85 +++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 38 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +47 -38
@@ -963,56 +963,65 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,return 0 ; } -static int amdgpu_cs_dependencies (struct amdgpu_device *adev, -struct amdgpu_cs_parser *p) +static int amdgpu_process_fence_dep (struct amdgpu_cs_parser *p, +struct amdgpu_cs_chunk *chunk) {struct amdgpu_fpriv *fpriv = p->filp->driver_priv; -int i, j, r; - -for (i =0 ; i < p->nchunks; ++i) { -struct drm_amdgpu_cs_chunk_dep *deps; -struct amdgpu_cs_chunk *chunk; -unsigned num_deps; +unsigned num_deps; +int i, r; +struct drm_amdgpu_cs_chunk_dep *deps; - chunk = &p->chunks[i]; + deps = (struct drm_amdgpu_cs_chunk_dep *)chunk->kdata; + num_deps = chunk->length_dw *4 / +sizeof (struct drm_amdgpu_cs_chunk_dep ); -if (chunk->chunk_id !=AMDGPU_CHUNK_ID_DEPENDENCIES ) -continue ; +for (i =0 ; i < num_deps; ++i) { +struct amdgpu_ring *ring; +struct amdgpu_ctx *ctx; +struct dma_fence *fence; - deps = (struct drm_amdgpu_cs_chunk_dep *)chunk->kdata; - num_deps = chunk->length_dw *4 / -sizeof (struct drm_amdgpu_cs_chunk_dep ); + r =amdgpu_cs_get_ring (p->adev, deps[i].ip_type, + deps[i].ip_instance, + deps[i].ring, &ring); +if (r) +return r; -for (j =0 ; j < num_deps; ++j) { -struct amdgpu_ring *ring; -struct amdgpu_ctx *ctx; -struct dma_fence *fence; + ctx =amdgpu_ctx_get (fpriv, deps[i].ctx_id); +if (ctx ==NULL ) +return -EINVAL ; - r =amdgpu_cs_get_ring (adev, deps[j].ip_type, - deps[j].ip_instance, - deps[j].ring, &ring); + fence =amdgpu_ctx_get_fence (ctx, ring, + deps[i].handle); +if (IS_ERR (fence)) { + r =PTR_ERR (fence); +amdgpu_ctx_put (ctx); +return r; + }else if (fence) { + r =amdgpu_sync_fence (p->adev, &p->job->sync, + fence); +dma_fence_put (fence); +amdgpu_ctx_put (ctx);if (r)return r; + } + } +return 0 ; +} - ctx =amdgpu_ctx_get (fpriv, deps[j].ctx_id); -if (ctx ==NULL ) -return -EINVAL ; +static int amdgpu_cs_dependencies (struct amdgpu_device *adev, +struct amdgpu_cs_parser *p) +{ +int i, r; - fence =amdgpu_ctx_get_fence (ctx, ring, - deps[j].handle); -if (IS_ERR (fence)) { - r =PTR_ERR (fence); -amdgpu_ctx_put (ctx); -return r; +for (i =0 ; i < p->nchunks; ++i) { +struct amdgpu_cs_chunk *chunk; - }else if (fence) { - r =amdgpu_sync_fence (adev, &p->job->sync, - fence); -dma_fence_put (fence); -amdgpu_ctx_put (ctx); -if (r) -return r; - } + chunk = &p->chunks[i]; + +if (chunk->chunk_id ==AMDGPU_CHUNK_ID_DEPENDENCIES ) { + r =amdgpu_process_fence_dep (p, chunk); +if (r) +return r; } }
Signature · 6 lines
2.9.3 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
PATCH 5/5 amdgpu: use drm sync objects for shared semaphores (v4)
In reply to a message not in the archiveFrom: Dave Airlie <[email protected]>
This creates a new command submission chunk for amdgpu to add in and out sync objects around the submission.
Sync objects are managed via the drm syncobj ioctls.
The command submission interface is enhanced with two new chunks, one for syncobj pre submission dependencies, and one for post submission sync obj signalling, and just takes a list of handles for each.
This is based on work originally done by David Zhou at AMD, with input from Christian Konig on what things should look like.
In theory VkFences could be backed with sync objects and just get passed into the cs as syncobj handles as well.
NOTE: this interface addition needs a version bump to expose it to userspace.
v1.1: keep file reference on import.
v2: move to using syncobjs
v2.1: change some APIs to just use p pointer.
v3: make more robust against CS failures, we now add the wait sems but only remove them once the CS job has been submitted.
v4: rewrite names of API and base on new syncobj code.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 81 ++++++++++++++++++++++++++++++++- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +- include/uapi/drm/amdgpu_drm.h | 6 +++ 3 files changed, 87 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +80 -1
@@ -27,6 +27,7 @@#include <linux/pagemap.h> #include <drm/drmP.h> #include <drm/amdgpu_drm.h> +#include <drm/drm_syncobj.h> #include "amdgpu.h" #include "amdgpu_trace.h"
@@ -217,6 +218,8 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data)break ;case AMDGPU_CHUNK_ID_DEPENDENCIES : +case AMDGPU_CHUNK_ID_SYNCOBJ_IN : +case AMDGPU_CHUNK_ID_SYNCOBJ_OUT :break ;default :
@@ -1008,6 +1011,40 @@ static int amdgpu_process_fence_dep(struct amdgpu_cs_parser *p,return 0 ; } +static int amdgpu_syncobj_lookup_and_add_to_sync (struct amdgpu_cs_parser *p, +uint32_t handle) +{ +int r; +struct dma_fence *fence; + r =drm_syncobj_fence_get (p->filp, handle, &fence); +if (r) +return r; + + r =amdgpu_sync_fence (p->adev, &p->job->sync, fence); +dma_fence_put (fence); + +return r; +} + +static int amdgpu_process_syncobj_in_dep (struct amdgpu_cs_parser *p, +struct amdgpu_cs_chunk *chunk) +{ +unsigned num_deps; +int i, r; +struct drm_amdgpu_cs_chunk_sem *deps; + + deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; + num_deps = chunk->length_dw *4 / +sizeof (struct drm_amdgpu_cs_chunk_sem ); + +for (i =0 ; i < num_deps; ++i) { + r =amdgpu_syncobj_lookup_and_add_to_sync (p, deps[i].handle); +if (r) +return r; + } +return 0 ; +} +static int amdgpu_cs_dependencies (struct amdgpu_device *adev,struct amdgpu_cs_parser *p) {
@@ -1022,12 +1059,54 @@ static int amdgpu_cs_dependencies(struct amdgpu_device *adev, r =amdgpu_process_fence_dep (p, chunk);if (r)return r; + }else if (chunk->chunk_id ==AMDGPU_CHUNK_ID_SYNCOBJ_IN ) { + r =amdgpu_process_syncobj_in_dep (p, chunk); +if (r) +return r; } }return 0 ; } +static int amdgpu_process_syncobj_out_dep (struct amdgpu_cs_parser *p, +struct amdgpu_cs_chunk *chunk) +{ +unsigned num_deps; +int i, r; +struct drm_amdgpu_cs_chunk_sem *deps; + + deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; + num_deps = chunk->length_dw *4 / +sizeof (struct drm_amdgpu_cs_chunk_sem ); + +for (i =0 ; i < num_deps; ++i) { + r =drm_syncobj_replace_fence (p->filp, deps[i].handle, + p->fence); +if (r) +return r; + } +return 0 ; +} + +static int amdgpu_cs_post_dependencies (struct amdgpu_cs_parser *p) +{ +int i, r; + +for (i =0 ; i < p->nchunks; ++i) { +struct amdgpu_cs_chunk *chunk; + + chunk = &p->chunks[i]; + +if (chunk->chunk_id ==AMDGPU_CHUNK_ID_SYNCOBJ_OUT ) { + r =amdgpu_process_syncobj_out_dep (p, chunk); +if (r) +return r; + } + } +return 0 ; +} +static int amdgpu_cs_submit (struct amdgpu_cs_parser *p,union drm_amdgpu_cs *cs) {
@@ -1055,7 +1134,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,trace_amdgpu_cs_ioctl (job);amd_sched_entity_push_job (&job->base); -return 0 ; +return amdgpu_cs_post_dependencies (p); }int amdgpu_cs_ioctl (struct drm_device *dev,void *data,struct drm_file *filp)
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +1 -1
@@ -683,7 +683,7 @@ static struct drm_driver kms_driver = { .driver_features =DRIVER_USE_AGP |DRIVER_HAVE_IRQ |DRIVER_IRQ_SHARED |DRIVER_GEM | - DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET, + DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ, .load = amdgpu_driver_load_kms, .open = amdgpu_driver_open_kms, .preclose = amdgpu_driver_preclose_kms,
include/uapi/drm/amdgpu_drm.h +6 -0
@@ -390,6 +390,8 @@ struct drm_amdgpu_gem_va {#define AMDGPU_CHUNK_ID_IB 0x01#define AMDGPU_CHUNK_ID_FENCE 0x02#define AMDGPU_CHUNK_ID_DEPENDENCIES 0x03 +#define AMDGPU_CHUNK_ID_SYNCOBJ_IN 0x04 +#define AMDGPU_CHUNK_ID_SYNCOBJ_OUT 0x05struct drm_amdgpu_cs_chunk {__u32 chunk_id;
@@ -454,6 +456,10 @@ struct drm_amdgpu_cs_chunk_fence {__u32 offset; }; +struct drm_amdgpu_cs_chunk_sem { +__u32 handle; +}; +struct drm_amdgpu_cs_chunk_data {union {struct drm_amdgpu_cs_chunk_ib ib_data;
Signature · 6 lines
2.9.3 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): 20170512003457.24936-6-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Re: [PATCH 5/5] amdgpu: use drm sync objects for shared semaphores (v4)
1. generally, functions in amdgpu_cs.c should be with amdgpu_cs_ as prefix.
2. If I'm not wrong to your proposal, SYNCOBJ_IN is to semaphore wait while SYNCOBJ_OUT is to semaphore signal. SYNCOBJ_IN/OUT both are based on command submission ioctl, that means user space must generate CS when using semaphore? but with my understand, they should not be dependent with that, they can be used independently, right?
Anything I missed, if yes, pls correct me.
+ case AMDGPU_CHUNK_ID_SYNCOBJ_IN:
+ case AMDGPU_CHUNK_ID_SYNCOBJ_OUT:
Regards,
David Zhou
Quoted 189 lines · Dave Airlie
From: Dave Airlie <[email protected]>
This creates a new command submission chunk for amdgpu to add in and out sync objects around the submission.
Sync objects are managed via the drm syncobj ioctls.
The command submission interface is enhanced with two new chunks, one for syncobj pre submission dependencies, and one for post submission sync obj signalling, and just takes a list of handles for each.
This is based on work originally done by David Zhou at AMD, with input from Christian Konig on what things should look like.
In theory VkFences could be backed with sync objects and just get passed into the cs as syncobj handles as well.
NOTE: this interface addition needs a version bump to expose it to userspace.
v1.1: keep file reference on import.
v2: move to using syncobjs
v2.1: change some APIs to just use p pointer.
v3: make more robust against CS failures, we now add the wait sems but only remove them once the CS job has been submitted.
v4: rewrite names of API and base on new syncobj code.Signed-off-by Dave Airlie <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 81 ++++++++++++++++++++++++++++++++- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +- include/uapi/drm/amdgpu_drm.h | 6 +++ 3 files changed, 87 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +80
-1@@ -27,6 +27,7 @@#include <linux/pagemap.h> #include <drm/drmP.h> #include <drm/amdgpu_drm.h> +#include <drm/drm_syncobj.h> #include "amdgpu.h" #include "amdgpu_trace.h" @@ -217,6 +218,8 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data)break ;case AMDGPU_CHUNK_ID_DEPENDENCIES : +case AMDGPU_CHUNK_ID_SYNCOBJ_IN : +case AMDGPU_CHUNK_ID_SYNCOBJ_OUT :break ;default :@@ -1008,6 +1011,40 @@ static int amdgpu_process_fence_dep(struct amdgpu_cs_parser *p,return 0 ; } +static int amdgpu_syncobj_lookup_and_add_to_sync (struct amdgpu_cs_parser *p, +uint32_t handle) +{ +int r; +struct dma_fence *fence; + r =drm_syncobj_fence_get (p->filp, handle, &fence); +if (r) +return r; + + r =amdgpu_sync_fence (p->adev, &p->job->sync, fence); +dma_fence_put (fence); + +return r; +} + +static int amdgpu_process_syncobj_in_dep (struct amdgpu_cs_parser *p, +struct amdgpu_cs_chunk *chunk) +{ +unsigned num_deps; +int i, r; +struct drm_amdgpu_cs_chunk_sem *deps; + + deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; + num_deps = chunk->length_dw *4 / +sizeof (struct drm_amdgpu_cs_chunk_sem ); + +for (i =0 ; i < num_deps; ++i) { + r =amdgpu_syncobj_lookup_and_add_to_sync (p, deps[i].handle); +if (r) +return r; + } +return 0 ; +} +static int amdgpu_cs_dependencies (struct amdgpu_device *adev,struct amdgpu_cs_parser *p) {@@ -1022,12 +1059,54 @@ static int amdgpu_cs_dependencies(struct amdgpu_device *adev, r =amdgpu_process_fence_dep (p, chunk);if (r)return r; + }else if (chunk->chunk_id ==AMDGPU_CHUNK_ID_SYNCOBJ_IN ) { + r =amdgpu_process_syncobj_in_dep (p, chunk); +if (r) +return r; } }return 0 ; } +static int amdgpu_process_syncobj_out_dep (struct amdgpu_cs_parser *p, +struct amdgpu_cs_chunk *chunk) +{ +unsigned num_deps; +int i, r; +struct drm_amdgpu_cs_chunk_sem *deps; + + deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; + num_deps = chunk->length_dw *4 / +sizeof (struct drm_amdgpu_cs_chunk_sem ); + +for (i =0 ; i < num_deps; ++i) { + r =drm_syncobj_replace_fence (p->filp, deps[i].handle, + p->fence); +if (r) +return r; + } +return 0 ; +} + +static int amdgpu_cs_post_dependencies (struct amdgpu_cs_parser *p) +{ +int i, r; + +for (i =0 ; i < p->nchunks; ++i) { +struct amdgpu_cs_chunk *chunk; + + chunk = &p->chunks[i]; + +if (chunk->chunk_id ==AMDGPU_CHUNK_ID_SYNCOBJ_OUT ) { + r =amdgpu_process_syncobj_out_dep (p, chunk); +if (r) +return r; + } + } +return 0 ; +} +static int amdgpu_cs_submit (struct amdgpu_cs_parser *p,union drm_amdgpu_cs *cs) {@@ -1055,7 +1134,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,trace_amdgpu_cs_ioctl (job);amd_sched_entity_push_job (&job->base); -return 0 ; +return amdgpu_cs_post_dependencies (p); }int amdgpu_cs_ioctl (struct drm_device *dev,void *data,struct drm_file *filp)drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +1
-1@@ -683,7 +683,7 @@ static struct drm_driver kms_driver = { .driver_features =DRIVER_USE_AGP |DRIVER_HAVE_IRQ |DRIVER_IRQ_SHARED |DRIVER_GEM | - DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET, + DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ, .load = amdgpu_driver_load_kms, .open = amdgpu_driver_open_kms, .preclose = amdgpu_driver_preclose_kms,include/uapi/drm/amdgpu_drm.h +2
-0@@ -390,6 +390,8 @@ struct drm_amdgpu_gem_va {#define AMDGPU_CHUNK_ID_IB 0x01#define AMDGPU_CHUNK_ID_FENCE 0x02#define AMDGPU_CHUNK_ID_DEPENDENCIES 0x03 +#define AMDGPU_CHUNK_ID_SYNCOBJ_IN 0x04 +#define AMDGPU_CHUNK_ID_SYNCOBJ_OUT 0x05struct drm_amdgpu_cs_chunk {__u32 chunk_id;
include/uapi/drm/amdgpu_drm.h struct drm_amdgpu_cs_chunk_fence { · line 465
Quoted 10 lines
include/uapi/drm/amdgpu_drm.h +4
-0@@ -454,6 +456,10 @@ struct drm_amdgpu_cs_chunk_fence {__u32 offset; }; +struct drm_amdgpu_cs_chunk_sem { +__u32 handle; +}; +struct drm_amdgpu_cs_chunk_data {union {struct drm_amdgpu_cs_chunk_ib ib_data;
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH 5/5] amdgpu: use drm sync objects for shared semaphores (v4)
Quoted 1 line · On 12 May 2017 at 13:34, zhoucm1 <[email protected]> wrote:
1. generally, functions in
amdgpu_cs.cshould be withamdgpu_cs_as prefix.
Okay I've fixed this and previous patch up locally.
Quoted 5 lines
2. If I'm not wrong to your proposal,
SYNCOBJ_INis to semaphore wait whileSYNCOBJ_OUTis to semaphore signal. SYNCOBJ_IN/OUT both are based on command submission ioctl, that means user space must generate CS when using semaphore? but with my understand, they should not be dependent with that, they can be used independently, right?
Yes in is WAIT and out is signal, however OUT could also be used to write a syncobj as a fence if needed, hence why I moved away from semaphore naming.
The only place I can see them being used independently is a possible signal operation after present, due not being able to pass the semaphores over dri3 yet. I think I've said this before and Christian has confirmed that doing anything with semaphores not via the command submission ioctl is going to be messy as they have to queue jobs in the scheduler, so if we need to tune the command submission ioctl to take empty CS or add a flag to just do semaphore operations we should do so in the future when we have a clear use case for it (and we see the need to optimise for it).
Dave.
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): CAPM=9tz5A9toCZAk10nOWub-qfiV6+aPBy9FDZANvAywfp8Vhw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Re: [PATCH 5/5] amdgpu: use drm sync objects for shared semaphores (v4)
Quoted 13 lines · Dave Airlie
Quoted 1 line · On 12 May 2017 at 13:34, zhoucm1 <[email protected]> wrote:
1. generally, functions in
amdgpu_cs.cshould be withamdgpu_cs_as prefix.Okay I've fixed this and previous patch up locally.
Quoted 5 lines
2. If I'm not wrong to your proposal,
SYNCOBJ_INis to semaphore wait whileSYNCOBJ_OUTis to semaphore signal. SYNCOBJ_IN/OUT both are based on command submission ioctl, that means user space must generate CS when using semaphore? but with my understand, they should not be dependent with that, they can be used independently, right?Yes in is WAIT and out is signal, however OUT could also be used to write a syncobj as a fence if needed, hence why I moved away from semaphore naming.
Quoted 9 lines
The only place I can see them being used independently is a possible signal operation after present, due not being able to pass the semaphores over dri3 yet. I think I've said this before and Christian has confirmed that doing anything with semaphores not via the command submission ioctl is going to be messy as they have to queue jobs in the scheduler, so if we need to tune the command submission ioctl to take empty CS or add a flag to just do semaphore operations we should do so in the future when we have a clear use case for it (and we see the need to optimise for it).
I see.
+David Mao and Jacob to aware, they are expert of Vulkan, if they have no concern, It's ok.
Regards,
David Zhou
Quoted 2 lines
Dave.
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 5/5] amdgpu: use drm sync objects for shared semaphores (v4)
In reply to a message not in the archiveThe series looks ok to me so far, just the minor rebase nit on the first patch.
Let me take an look again tomorrow morning since it is 1am and I might've missed something.
Regards,
Andres
Quoted 190 lines · Dave Airlie
From: Dave Airlie <[email protected]>
This creates a new command submission chunk for amdgpu to add in and out sync objects around the submission.
Sync objects are managed via the drm syncobj ioctls.
The command submission interface is enhanced with two new chunks, one for syncobj pre submission dependencies, and one for post submission sync obj signalling, and just takes a list of handles for each.
This is based on work originally done by David Zhou at AMD, with input from Christian Konig on what things should look like.
In theory VkFences could be backed with sync objects and just get passed into the cs as syncobj handles as well.
NOTE: this interface addition needs a version bump to expose it to userspace.
v1.1: keep file reference on import.
v2: move to using syncobjs
v2.1: change some APIs to just use p pointer.
v3: make more robust against CS failures, we now add the wait sems but only remove them once the CS job has been submitted.
v4: rewrite names of API and base on new syncobj code.Signed-off-by Dave Airlie <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 81 ++++++++++++++++++++++++++++++++- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +- include/uapi/drm/amdgpu_drm.h | 6 +++ 3 files changed, 87 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +80
-1@@ -27,6 +27,7 @@#include <linux/pagemap.h> #include <drm/drmP.h> #include <drm/amdgpu_drm.h> +#include <drm/drm_syncobj.h> #include "amdgpu.h" #include "amdgpu_trace.h" @@ -217,6 +218,8 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data)break ;case AMDGPU_CHUNK_ID_DEPENDENCIES : +case AMDGPU_CHUNK_ID_SYNCOBJ_IN : +case AMDGPU_CHUNK_ID_SYNCOBJ_OUT :break ;default :@@ -1008,6 +1011,40 @@ static int amdgpu_process_fence_dep(struct amdgpu_cs_parser *p,return 0 ; } +static int amdgpu_syncobj_lookup_and_add_to_sync (struct amdgpu_cs_parser *p, +uint32_t handle) +{ +int r; +struct dma_fence *fence; + r =drm_syncobj_fence_get (p->filp, handle, &fence); +if (r) +return r; + + r =amdgpu_sync_fence (p->adev, &p->job->sync, fence); +dma_fence_put (fence); + +return r; +} + +static int amdgpu_process_syncobj_in_dep (struct amdgpu_cs_parser *p, +struct amdgpu_cs_chunk *chunk) +{ +unsigned num_deps; +int i, r; +struct drm_amdgpu_cs_chunk_sem *deps; + + deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; + num_deps = chunk->length_dw *4 / +sizeof (struct drm_amdgpu_cs_chunk_sem ); + +for (i =0 ; i < num_deps; ++i) { + r =amdgpu_syncobj_lookup_and_add_to_sync (p, deps[i].handle); +if (r) +return r; + } +return 0 ; +} +static int amdgpu_cs_dependencies (struct amdgpu_device *adev,struct amdgpu_cs_parser *p) {@@ -1022,12 +1059,54 @@ static int amdgpu_cs_dependencies(struct amdgpu_device *adev, r =amdgpu_process_fence_dep (p, chunk);if (r)return r; + }else if (chunk->chunk_id ==AMDGPU_CHUNK_ID_SYNCOBJ_IN ) { + r =amdgpu_process_syncobj_in_dep (p, chunk); +if (r) +return r; } }return 0 ; } +static int amdgpu_process_syncobj_out_dep (struct amdgpu_cs_parser *p, +struct amdgpu_cs_chunk *chunk) +{ +unsigned num_deps; +int i, r; +struct drm_amdgpu_cs_chunk_sem *deps; + + deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; + num_deps = chunk->length_dw *4 / +sizeof (struct drm_amdgpu_cs_chunk_sem ); + +for (i =0 ; i < num_deps; ++i) { + r =drm_syncobj_replace_fence (p->filp, deps[i].handle, + p->fence); +if (r) +return r; + } +return 0 ; +} + +static int amdgpu_cs_post_dependencies (struct amdgpu_cs_parser *p) +{ +int i, r; + +for (i =0 ; i < p->nchunks; ++i) { +struct amdgpu_cs_chunk *chunk; + + chunk = &p->chunks[i]; + +if (chunk->chunk_id ==AMDGPU_CHUNK_ID_SYNCOBJ_OUT ) { + r =amdgpu_process_syncobj_out_dep (p, chunk); +if (r) +return r; + } + } +return 0 ; +} +static int amdgpu_cs_submit (struct amdgpu_cs_parser *p,union drm_amdgpu_cs *cs) {@@ -1055,7 +1134,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,trace_amdgpu_cs_ioctl (job);amd_sched_entity_push_job (&job->base); -return 0 ; +return amdgpu_cs_post_dependencies (p); }int amdgpu_cs_ioctl (struct drm_device *dev,void *data,struct drm_file *filp)drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +1
-1@@ -683,7 +683,7 @@ static struct drm_driver kms_driver = { .driver_features =DRIVER_USE_AGP |DRIVER_HAVE_IRQ |DRIVER_IRQ_SHARED |DRIVER_GEM | - DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET, + DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ, .load = amdgpu_driver_load_kms, .open = amdgpu_driver_open_kms, .preclose = amdgpu_driver_preclose_kms,include/uapi/drm/amdgpu_drm.h +2
-0@@ -390,6 +390,8 @@ struct drm_amdgpu_gem_va {#define AMDGPU_CHUNK_ID_IB 0x01#define AMDGPU_CHUNK_ID_FENCE 0x02#define AMDGPU_CHUNK_ID_DEPENDENCIES 0x03 +#define AMDGPU_CHUNK_ID_SYNCOBJ_IN 0x04 +#define AMDGPU_CHUNK_ID_SYNCOBJ_OUT 0x05struct drm_amdgpu_cs_chunk {__u32 chunk_id;
include/uapi/drm/amdgpu_drm.h struct drm_amdgpu_cs_chunk_fence { · line 465
Quoted 10 lines
include/uapi/drm/amdgpu_drm.h +4
-0@@ -454,6 +456,10 @@ struct drm_amdgpu_cs_chunk_fence {__u32 offset; }; +struct drm_amdgpu_cs_chunk_sem { +__u32 handle; +}; +struct drm_amdgpu_cs_chunk_data {union {struct drm_amdgpu_cs_chunk_ib ib_data;
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 5/5] amdgpu: use drm sync objects for shared semaphores (v4)
In reply to Dave AirlieAm 12.05.2017 um 02:34 schrieb Dave Airlie:
Quoted 154 lines
From: Dave Airlie <[email protected]>
This creates a new command submission chunk for amdgpu to add in and out sync objects around the submission.
Sync objects are managed via the drm syncobj ioctls.
The command submission interface is enhanced with two new chunks, one for syncobj pre submission dependencies, and one for post submission sync obj signalling, and just takes a list of handles for each.
This is based on work originally done by David Zhou at AMD, with input from Christian Konig on what things should look like.
In theory VkFences could be backed with sync objects and just get passed into the cs as syncobj handles as well.
NOTE: this interface addition needs a version bump to expose it to userspace.
v1.1: keep file reference on import.
v2: move to using syncobjs
v2.1: change some APIs to just use p pointer.
v3: make more robust against CS failures, we now add the wait sems but only remove them once the CS job has been submitted.
v4: rewrite names of API and base on new syncobj code.Signed-off-by Dave Airlie <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 81 ++++++++++++++++++++++++++++++++- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +- include/uapi/drm/amdgpu_drm.h | 6 +++ 3 files changed, 87 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +79
-0@@ -27,6 +27,7 @@#include <linux/pagemap.h> #include <drm/drmP.h> #include <drm/amdgpu_drm.h> +#include <drm/drm_syncobj.h> #include "amdgpu.h" #include "amdgpu_trace.h" @@ -217,6 +218,8 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data)break ;case AMDGPU_CHUNK_ID_DEPENDENCIES : +case AMDGPU_CHUNK_ID_SYNCOBJ_IN : +case AMDGPU_CHUNK_ID_SYNCOBJ_OUT :break ;default :@@ -1008,6 +1011,40 @@ static int amdgpu_process_fence_dep(struct amdgpu_cs_parser *p,return 0 ; } +static int amdgpu_syncobj_lookup_and_add_to_sync (struct amdgpu_cs_parser *p, +uint32_t handle) +{ +int r; +struct dma_fence *fence; + r =drm_syncobj_fence_get (p->filp, handle, &fence); +if (r) +return r; + + r =amdgpu_sync_fence (p->adev, &p->job->sync, fence); +dma_fence_put (fence); + +return r; +} + +static int amdgpu_process_syncobj_in_dep (struct amdgpu_cs_parser *p, +struct amdgpu_cs_chunk *chunk) +{ +unsigned num_deps; +int i, r; +struct drm_amdgpu_cs_chunk_sem *deps; + + deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; + num_deps = chunk->length_dw *4 / +sizeof (struct drm_amdgpu_cs_chunk_sem ); + +for (i =0 ; i < num_deps; ++i) { + r =amdgpu_syncobj_lookup_and_add_to_sync (p, deps[i].handle); +if (r) +return r; + } +return 0 ; +} +static int amdgpu_cs_dependencies (struct amdgpu_device *adev,struct amdgpu_cs_parser *p) {@@ -1022,12 +1059,54 @@ static int amdgpu_cs_dependencies(struct amdgpu_device *adev, r =amdgpu_process_fence_dep (p, chunk);if (r)return r; + }else if (chunk->chunk_id ==AMDGPU_CHUNK_ID_SYNCOBJ_IN ) { + r =amdgpu_process_syncobj_in_dep (p, chunk); +if (r) +return r; } }return 0 ; } +static int amdgpu_process_syncobj_out_dep (struct amdgpu_cs_parser *p, +struct amdgpu_cs_chunk *chunk) +{ +unsigned num_deps; +int i, r; +struct drm_amdgpu_cs_chunk_sem *deps; + + deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; + num_deps = chunk->length_dw *4 / +sizeof (struct drm_amdgpu_cs_chunk_sem ); + +for (i =0 ; i < num_deps; ++i) { + r =drm_syncobj_replace_fence (p->filp, deps[i].handle, + p->fence); +if (r) +return r; + } +return 0 ; +} + +static int amdgpu_cs_post_dependencies (struct amdgpu_cs_parser *p) +{ +int i, r; + +for (i =0 ; i < p->nchunks; ++i) { +struct amdgpu_cs_chunk *chunk; + + chunk = &p->chunks[i]; + +if (chunk->chunk_id ==AMDGPU_CHUNK_ID_SYNCOBJ_OUT ) { + r =amdgpu_process_syncobj_out_dep (p, chunk); +if (r) +return r; + } + } +return 0 ; +} +static int amdgpu_cs_submit (struct amdgpu_cs_parser *p,union drm_amdgpu_cs *cs) {
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, · line 1137
Quoted 5 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +1
-1@@ -1055,7 +1134,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,trace_amdgpu_cs_ioctl (job);amd_sched_entity_push_job (&job->base); -return 0 ; +return amdgpu_cs_post_dependencies (p);
When userspace provided an invalid out sync handle we return an error here, but the job would have been pushed to the hardware already.
Better move this before amd_sched_entity_push_job() and abort gracefully when this happens.
Apart from that the patch looks good to me,
Christian.
Quoted 30 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +0
-0}int amdgpu_cs_ioctl (struct drm_device *dev,void *data,struct drm_file *filp)drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +1
-1@@ -683,7 +683,7 @@ static struct drm_driver kms_driver = { .driver_features =DRIVER_USE_AGP |DRIVER_HAVE_IRQ |DRIVER_IRQ_SHARED |DRIVER_GEM | - DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET, + DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ, .load = amdgpu_driver_load_kms, .open = amdgpu_driver_open_kms, .preclose = amdgpu_driver_preclose_kms,include/uapi/drm/amdgpu_drm.h +2
-0@@ -390,6 +390,8 @@ struct drm_amdgpu_gem_va {#define AMDGPU_CHUNK_ID_IB 0x01#define AMDGPU_CHUNK_ID_FENCE 0x02#define AMDGPU_CHUNK_ID_DEPENDENCIES 0x03 +#define AMDGPU_CHUNK_ID_SYNCOBJ_IN 0x04 +#define AMDGPU_CHUNK_ID_SYNCOBJ_OUT 0x05struct drm_amdgpu_cs_chunk {__u32 chunk_id;
include/uapi/drm/amdgpu_drm.h struct drm_amdgpu_cs_chunk_fence { · line 465
Quoted 10 lines
include/uapi/drm/amdgpu_drm.h +4
-0@@ -454,6 +456,10 @@ struct drm_amdgpu_cs_chunk_fence {__u32 offset; }; +struct drm_amdgpu_cs_chunk_sem { +__u32 handle; +}; +struct drm_amdgpu_cs_chunk_data {union {struct drm_amdgpu_cs_chunk_ib ib_data;
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel