[PATCH 01/10] drm/syncobj: Rename fence_get to find_fence

List overview All Threads
Download

newer

older

[Bug 83023] A problem related to...

[PATCH] drm/syncobj: Fix the...

Jason Ekstrand

25 Aug 2017 25 Aug '17

5:52 p.m.

The function has far more in common with drm_syncobj_find than with any in the get/put functions.

Signed-off-by: Jason Ekstrand jason@jlekstrand.net Acked-by: Christian König christian.koenig@amd.com (v1) --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +- drivers/gpu/drm/drm_syncobj.c | 10 +++++----- include/drm/drm_syncobj.h | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 15d4a28..269b835 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1035,7 +1035,7 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p, { int r; struct dma_fence *fence; - r = drm_syncobj_fence_get(p->filp, handle, &fence); + r = drm_syncobj_find_fence(p->filp, handle, &fence); if (r) return r;

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index a5b38a8..0412b0b 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -95,9 +95,9 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, } EXPORT_SYMBOL(drm_syncobj_replace_fence);

-int drm_syncobj_fence_get(struct drm_file *file_private, - u32 handle, - struct dma_fence **fence) +int drm_syncobj_find_fence(struct drm_file *file_private, + u32 handle, + struct dma_fence **fence) { struct drm_syncobj *syncobj = drm_syncobj_find(file_private, handle); int ret = 0; @@ -112,7 +112,7 @@ int drm_syncobj_fence_get(struct drm_file *file_private, drm_syncobj_put(syncobj); return ret; } -EXPORT_SYMBOL(drm_syncobj_fence_get); +EXPORT_SYMBOL(drm_syncobj_find_fence);

/** * drm_syncobj_free - free a sync object. @@ -307,7 +307,7 @@ int drm_syncobj_export_sync_file(struct drm_file *file_private, if (fd < 0) return fd;

- ret = drm_syncobj_fence_get(file_private, handle, &fence); + ret = drm_syncobj_find_fence(file_private, handle, &fence); if (ret) goto err_put_fd;

diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 89976da..7d4ad77 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -81,9 +81,9 @@ struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private, u32 handle); void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, struct dma_fence *fence); -int drm_syncobj_fence_get(struct drm_file *file_private, - u32 handle, - struct dma_fence **fence); +int drm_syncobj_find_fence(struct drm_file *file_private, + u32 handle, + struct dma_fence **fence); void drm_syncobj_free(struct kref *kref);

#endif

-- 2.5.0.400.gff86faf

Show replies by date

Jason Ekstrand

25 Aug 25 Aug

5:52 p.m.

New subject: [PATCH 02/10] drm/syncobj: Add a race-free drm_syncobj_fence_get helper (v2)

The atomic exchange operation in drm_syncobj_replace_fence is sufficient for the case where it races with itself. However, if you have a race between a replace_fence and dma_fence_get(syncobj->fence), you may end up with the entire replace_fence happening between the point in time where the one thread gets the syncobj->fence pointer and when it calls dma_fence_get() on it. If this happens, then the reference may be dropped before we get a chance to get a new one. The new helper uses dma_fence_get_rcu_safe to get rid of the race.

This is also needed because it allows us to do a bit more than just get a reference in drm_syncobj_fence_get should we wish to do so.

v2: - RCU isn't that scary - Call rcu_read_lock/unlock - Don't rename fence to _fence - Make the helper static inline

Signed-off-by: Jason Ekstrand jason@jlekstrand.net Acked-by: Christian König christian.koenig@amd.com (v1) --- drivers/gpu/drm/drm_syncobj.c | 2 +- include/drm/drm_syncobj.h | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 0412b0b..eea38d8 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -105,7 +105,7 @@ int drm_syncobj_find_fence(struct drm_file *file_private, if (!syncobj) return -ENOENT;

- *fence = dma_fence_get(syncobj->fence); + *fence = drm_syncobj_fence_get(syncobj); if (!*fence) { ret = -EINVAL; } diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 7d4ad77..ce94d14 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -77,6 +77,18 @@ drm_syncobj_put(struct drm_syncobj *obj) kref_put(&obj->refcount, drm_syncobj_free); }

+static inline struct dma_fence * +drm_syncobj_fence_get(struct drm_syncobj *syncobj) +{ + struct dma_fence *fence; + + rcu_read_lock(); + fence = dma_fence_get_rcu_safe(&syncobj->fence); + rcu_read_unlock(); + + return fence; +} + struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private, u32 handle); void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,

-- 2.5.0.400.gff86faf

Jason Ekstrand

5:52 p.m.

New subject: [PATCH 03/10] i915: Use drm_syncobj_fence_get

Signed-off-by: Jason Ekstrand jason@jlekstrand.net --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 3d74f3a..4c20162 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -2129,9 +2129,7 @@ await_fence_array(struct i915_execbuffer *eb, if (!(flags & I915_EXEC_FENCE_WAIT)) continue;

- rcu_read_lock(); - fence = dma_fence_get_rcu_safe(&syncobj->fence); - rcu_read_unlock(); + fence = drm_syncobj_fence_get(syncobj); if (!fence) return -EINVAL;

-- 2.5.0.400.gff86faf

Jason Ekstrand

5:52 p.m.

New subject: [PATCH 04/10] drm/syncobj: add sync obj wait interface. (v8)

From: Dave Airlie airlied@redhat.com

This interface will allow sync object to be used to back Vulkan fences. This API is pretty much the vulkan fence waiting API, and I've ported the code from amdgpu.

v2: accept relative timeout, pass remaining time back to userspace. v3: return to absolute timeouts. v4: absolute zero = poll, rewrite any/all code to have same operation for arrays return -EINVAL for 0 fences. v4.1: fixup fences allocation check, use u64_to_user_ptr v5: move to sec/nsec, and use timespec64 for calcs. v6: use -ETIME and drop the out status flag. (-ETIME is suggested by ickle, I can feel a shed painting) v7: talked to Daniel/Arnd, use ktime and ns everywhere. v8: be more careful in the timeout calculations use uint32_t for counter variables so we don't overflow graciously handle -ENOINT being returned from dma_fence_wait_timeout

Signed-off-by: Dave Airlie airlied@redhat.com Reviewed-by: Jason Ekstrand jason@jlekstrand.net Acked-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/drm_internal.h | 2 + drivers/gpu/drm/drm_ioctl.c | 2 + drivers/gpu/drm/drm_syncobj.c | 142 +++++++++++++++++++++++++++++++++++++++++ include/uapi/drm/drm.h | 12 ++++ 4 files changed, 158 insertions(+)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 4e906b8..534e5ac 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -167,3 +167,5 @@ int drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); +int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index d920b21..b4f4434 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -657,6 +657,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE, drm_syncobj_fd_to_handle_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), };

#define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index eea38d8..4e8563c 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1,5 +1,7 @@ /* * Copyright 2017 Red Hat + * Parts ported from amdgpu (fence wait code). + * Copyright 2016 Advanced Micro Devices, Inc. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -31,6 +33,9 @@ * that contain an optional fence. The fence can be updated with a new * fence, or be NULL. * + * syncobj's can be waited upon, where it will wait for the underlying + * fence. + * * syncobj's can be export to fd's and back, these fd's are opaque and * have no other use case, except passing the syncobj between processes. * @@ -447,3 +452,140 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, return drm_syncobj_fd_to_handle(file_private, args->fd, &args->handle); } + +/** + * drm_timeout_abs_to_jiffies - calculate jiffies timeout from absolute value + * + * @timeout_nsec: timeout nsec component in ns, 0 for poll + * + * Calculate the timeout in jiffies from an absolute time in sec/nsec. + */ +static signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec) +{ + ktime_t abs_timeout, now; + u64 timeout_ns, timeout_jiffies64; + + /* make 0 timeout means poll - absolute 0 doesn't seem valid */ + if (timeout_nsec == 0) + return 0; + + abs_timeout = ns_to_ktime(timeout_nsec); + now = ktime_get(); + + if (!ktime_after(abs_timeout, now)) + return 0; + + timeout_ns = ktime_to_ns(ktime_sub(abs_timeout, now)); + + timeout_jiffies64 = nsecs_to_jiffies64(timeout_ns); + /* clamp timeout to avoid infinite timeout */ + if (timeout_jiffies64 >= MAX_SCHEDULE_TIMEOUT - 1) + return MAX_SCHEDULE_TIMEOUT - 1; + + return timeout_jiffies64 + 1; +} + +static int drm_syncobj_wait_fences(struct drm_device *dev, + struct drm_file *file_private, + struct drm_syncobj_wait *wait, + struct dma_fence **fences) +{ + signed long timeout = drm_timeout_abs_to_jiffies(wait->timeout_nsec); + signed long ret = 0; + uint32_t first = ~0; + + if (wait->flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL) { + uint32_t i; + for (i = 0; i < wait->count_handles; i++) { + ret = dma_fence_wait_timeout(fences[i], true, timeout); + + /* Various dma_fence wait callbacks will return + * ENOENT to indicate that the fence has already + * been signaled. We need to sanitize this to 0 so + * we don't return early and the client doesn't see + * an unexpected error. + */ + if (ret == -ENOENT) + ret = 0; + + if (ret < 0) + return ret; + if (ret == 0) + break; + timeout = ret; + } + first = 0; + } else { + ret = dma_fence_wait_any_timeout(fences, + wait->count_handles, + true, timeout, + &first); + } + + if (ret < 0) + return ret; + + wait->first_signaled = first; + if (ret == 0) + return -ETIME; + return 0; +} + +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; + struct dma_fence **fences; + int ret = 0; + uint32_t i; + + 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 -EINVAL; + + /* 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, + u64_to_user_ptr(args->handles), + sizeof(uint32_t) * args->count_handles)) { + ret = -EFAULT; + goto err_free_handles; + } + + fences = kcalloc(args->count_handles, + sizeof(struct dma_fence *), GFP_KERNEL); + if (!fences) { + ret = -ENOMEM; + goto err_free_handles; + } + + for (i = 0; i < args->count_handles; i++) { + ret = drm_syncobj_find_fence(file_private, handles[i], + &fences[i]); + if (ret) + goto err_free_fence_array; + } + + ret = drm_syncobj_wait_fences(dev, file_private, + args, fences); + +err_free_fence_array: + for (i = 0; i < args->count_handles; i++) + dma_fence_put(fences[i]); + kfree(fences); +err_free_handles: + kfree(handles); + + return ret; +} diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 101593a..0757c1a 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -718,6 +718,17 @@ struct drm_syncobj_handle { __u32 pad; };

+#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0) +struct drm_syncobj_wait { + __u64 handles; + /* absolute timeout */ + __s64 timeout_nsec; + __u32 count_handles; + __u32 flags; + __u32 first_signaled; /* only valid when not waiting all */ + __u32 pad; +}; + #if defined(__cplusplus) } #endif @@ -840,6 +851,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

-- 2.5.0.400.gff86faf

Jason Ekstrand

5:52 p.m.

New subject: [PATCH 05/10] drm/syncobj: Add a callback mechanism for replace_fence (v2)

It is useful in certain circumstances to know when the fence is replaced in a syncobj. Specifically, it may be useful to know when the fence goes from NULL to something valid. This does make syncobj_replace_fence a little more expensive because it has to take a lock but, in the common case where there is no callback list, it spends a very short amount of time inside the lock.

v2: - Don't lock in drm_syncobj_fence_get. We only really need to lock around fence_replace to make the callback work.

Signed-off-by: Jason Ekstrand jason@jlekstrand.net --- drivers/gpu/drm/drm_syncobj.c | 60 +++++++++++++++++++++++++++++++++++++++++-- include/drm/drm_syncobj.h | 39 ++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 4e8563c..bade497 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -80,6 +80,46 @@ struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private, } EXPORT_SYMBOL(drm_syncobj_find);

+static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj, + struct drm_syncobj_cb *cb, + drm_syncobj_func_t func) +{ + cb->func = func; + list_add_tail(&cb->node, &syncobj->cb_list); +} + +/** + * drm_syncobj_add_callback - adds a callback to syncobj::cb_list + * @syncobj: Sync object to which to add the callback + * @cb: Callback to add + * @func: Func to use when initializing the drm_syncobj_cb struct + * + * This adds a callback to be called next time the fence is replaced + */ +void drm_syncobj_add_callback(struct drm_syncobj *syncobj, + struct drm_syncobj_cb *cb, + drm_syncobj_func_t func) +{ + spin_lock(&syncobj->lock); + drm_syncobj_add_callback_locked(syncobj, cb, func); + spin_unlock(&syncobj->lock); +} +EXPORT_SYMBOL(drm_syncobj_add_callback); + +/** + * drm_syncobj_add_callback - removes a callback to syncobj::cb_list + * @syncobj: Sync object from which to remove the callback + * @cb: Callback to remove + */ +void drm_syncobj_remove_callback(struct drm_syncobj *syncobj, + struct drm_syncobj_cb *cb) +{ + spin_lock(&syncobj->lock); + list_del_init(&cb->node); + spin_unlock(&syncobj->lock); +} +EXPORT_SYMBOL(drm_syncobj_remove_callback); + /** * drm_syncobj_replace_fence - replace fence in a sync object. * @syncobj: Sync object to replace fence in @@ -91,10 +131,24 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, struct dma_fence *fence) { struct dma_fence *old_fence; + struct drm_syncobj_cb *cur, *tmp;

if (fence) dma_fence_get(fence); - old_fence = xchg(&syncobj->fence, fence); + + spin_lock(&syncobj->lock); + + old_fence = syncobj->fence; + syncobj->fence = fence; + + if (fence != old_fence) { + list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) { + list_del_init(&cur->node); + cur->func(syncobj, cur); + } + } + + spin_unlock(&syncobj->lock);

dma_fence_put(old_fence); } @@ -130,7 +184,7 @@ void drm_syncobj_free(struct kref *kref) struct drm_syncobj *syncobj = container_of(kref, struct drm_syncobj, refcount); - dma_fence_put(syncobj->fence); + drm_syncobj_replace_fence(syncobj, NULL); kfree(syncobj); } EXPORT_SYMBOL(drm_syncobj_free); @@ -146,6 +200,8 @@ static int drm_syncobj_create(struct drm_file *file_private, return -ENOMEM;

kref_init(&syncobj->refcount); + INIT_LIST_HEAD(&syncobj->cb_list); + spin_lock_init(&syncobj->lock);

idr_preload(GFP_KERNEL); spin_lock(&file_private->syncobj_table_lock); diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index ce94d14..aef7c10 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -28,6 +28,8 @@

#include "linux/dma-fence.h"

+struct drm_syncobj_cb; + /** * struct drm_syncobj - sync object. * @@ -43,15 +45,47 @@ struct drm_syncobj { /** * @fence: * NULL or a pointer to the fence bound to this object. + * + * This field should not be used directly. Use drm_syncobj_fence_get + * and drm_syncobj_replace_fence instead. */ struct dma_fence *fence; /** + * @list: + * List of callbacks to call when the fence gets replaced + */ + struct list_head cb_list; + /** + * @lock: + * locks cb_list and write-locks fence. + */ + spinlock_t lock; + /** * @file: * a file backing for this syncobj. */ struct file *file; };

+typedef void (*drm_syncobj_func_t)(struct drm_syncobj *syncobj, + struct drm_syncobj_cb *cb); + +/** + * struct drm_syncobj_cb - callback for drm_syncobj_add_callback + * @node: used by drm_syncob_add_callback to append this struct to + * syncobj::cb_list + * @func: drm_syncobj_func_t to call + * + * This struct will be initialized by drm_syncobj_add_callback, additional + * data can be passed along by embedding drm_syncobj_cb in another struct. + * The callback will get called the next time drm_syncobj_replace_fence is + * called. + */ +struct drm_syncobj_cb { + struct list_head node; + drm_syncobj_func_t func; +}; + void drm_syncobj_free(struct kref *kref);

/** @@ -91,6 +125,11 @@ drm_syncobj_fence_get(struct drm_syncobj *syncobj)

struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private, u32 handle); +void drm_syncobj_add_callback(struct drm_syncobj *syncobj, + struct drm_syncobj_cb *cb, + drm_syncobj_func_t func); +void drm_syncobj_remove_callback(struct drm_syncobj *syncobj, + struct drm_syncobj_cb *cb); void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, struct dma_fence *fence); int drm_syncobj_find_fence(struct drm_file *file_private,

-- 2.5.0.400.gff86faf

kbuild test robot

27 Aug 27 Aug

7:34 p.m.

New subject: [PATCH 05/10] drm/syncobj: Add a callback mechanism for replace_fence (v2)

Hi Jason,

[auto build test WARNING on drm/drm-next] [also build test WARNING on next-20170825] [cannot apply to v4.13-rc6] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Jason-Ekstrand/drm-syncobj-Rename-f... base: git://people.freedesktop.org/~airlied/linux.git drm-next reproduce: make htmldocs

All warnings (new ones prefixed by >>):

WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org) include/linux/init.h:1: warning: no structured comments found include/linux/mod_devicetable.h:687: warning: Excess struct/union/enum/typedef member 'ver_major' description in 'fsl_mc_device_id' include/linux/mod_devicetable.h:687: warning: Excess struct/union/enum/typedef member 'ver_minor' description in 'fsl_mc_device_id' kernel/sys.c:1: warning: no structured comments found include/linux/device.h:968: warning: No description found for parameter 'dma_ops' drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found include/linux/sync_file.h:51: warning: No description found for parameter 'flags' include/linux/iio/iio.h:603: warning: No description found for parameter 'trig_readonly' include/linux/iio/trigger.h:151: warning: No description found for parameter 'indio_dev' include/linux/iio/trigger.h:151: warning: No description found for parameter 'trig' include/linux/device.h:969: warning: No description found for parameter 'dma_ops' arch/s390/include/asm/cmb.h:1: warning: no structured comments found drivers/scsi/scsi_lib.c:1116: warning: No description found for parameter 'rq' drivers/scsi/constants.c:1: warning: no structured comments found include/linux/usb/gadget.h:230: warning: No description found for parameter 'claimed' include/linux/usb/gadget.h:230: warning: No description found for parameter 'enabled' include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_altset_not_supp' include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_stall_not_supp' include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_zlp_not_supp' fs/inode.c:1666: warning: No description found for parameter 'rcu' include/linux/jbd2.h:443: warning: No description found for parameter 'i_transaction' include/linux/jbd2.h:443: warning: No description found for parameter 'i_next_transaction' include/linux/jbd2.h:443: warning: No description found for parameter 'i_list' include/linux/jbd2.h:443: warning: No description found for parameter 'i_vfs_inode' include/linux/jbd2.h:443: warning: No description found for parameter 'i_flags' include/linux/jbd2.h:497: warning: No description found for parameter 'h_rsv_handle' include/linux/jbd2.h:497: warning: No description found for parameter 'h_reserved' include/linux/jbd2.h:497: warning: No description found for parameter 'h_type' include/linux/jbd2.h:497: warning: No description found for parameter 'h_line_no' include/linux/jbd2.h:497: warning: No description found for parameter 'h_start_jiffies' include/linux/jbd2.h:497: warning: No description found for parameter 'h_requested_credits' include/linux/jbd2.h:497: warning: No description found for parameter 'saved_alloc_context' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_chkpt_bhs' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_devname' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_average_commit_time' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_min_batch_time' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_max_batch_time' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_commit_callback' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_failed_commit' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_chksum_driver' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_csum_seed' fs/jbd2/transaction.c:511: warning: No description found for parameter 'type' fs/jbd2/transaction.c:511: warning: No description found for parameter 'line_no' fs/jbd2/transaction.c:641: warning: No description found for parameter 'gfp_mask' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_pin' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_unpin' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_res_obj' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_get_sg_table' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_import_sg_table' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_vmap' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_vunmap' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_mmap' include/drm/drm_mode_config.h:771: warning: No description found for parameter 'modifiers_property' include/drm/drm_mode_config.h:771: warning: Excess struct/union/enum/typedef member 'modifiers' description in 'drm_mode_config' include/drm/drm_plane.h:544: warning: No description found for parameter 'modifiers' include/drm/drm_plane.h:544: warning: No description found for parameter 'modifier_count'

...

...
include/drm/drm_syncobj.h:69: warning: No description found for parameter 'cb_list' include/drm/drm_syncobj.h:69: warning: Excess struct/union/enum/typedef member 'list' description in 'drm_syncobj'

drivers/gpu/host1x/bus.c:50: warning: No description found for parameter 'driver' Documentation/doc-guide/sphinx.rst:121: ERROR: Unknown target name: "sphinx c domain". kernel/sched/fair.c:7584: WARNING: Inline emphasis start-string without end-string. kernel/time/timer.c:1200: ERROR: Unexpected indentation. kernel/time/timer.c:1202: ERROR: Unexpected indentation. kernel/time/timer.c:1203: WARNING: Block quote ends without a blank line; unexpected unindent. include/linux/wait.h:108: WARNING: Block quote ends without a blank line; unexpected unindent. include/linux/wait.h:111: ERROR: Unexpected indentation. include/linux/wait.h:113: WARNING: Block quote ends without a blank line; unexpected unindent. kernel/time/hrtimer.c:991: WARNING: Block quote ends without a blank line; unexpected unindent. kernel/signal.c:323: WARNING: Inline literal start-string without end-string. kernel/rcu/tree.c:3187: ERROR: Unexpected indentation. kernel/rcu/tree.c:3214: ERROR: Unexpected indentation. kernel/rcu/tree.c:3215: WARNING: Bullet list ends without a blank line; unexpected unindent. include/linux/iio/iio.h:219: ERROR: Unexpected indentation. include/linux/iio/iio.h:220: WARNING: Block quote ends without a blank line; unexpected unindent. include/linux/iio/iio.h:226: WARNING: Definition list ends without a blank line; unexpected unindent. drivers/iio/industrialio-core.c:633: ERROR: Unknown target name: "iio_val". drivers/iio/industrialio-core.c:640: ERROR: Unknown target name: "iio_val". drivers/ata/libata-core.c:5906: ERROR: Unknown target name: "hw". drivers/message/fusion/mptbase.c:5051: WARNING: Definition list ends without a blank line; unexpected unindent. drivers/tty/serial/serial_core.c:1897: WARNING: Definition list ends without a blank line; unexpected unindent. drivers/pci/pci.c:3470: ERROR: Unexpected indentation. include/linux/regulator/driver.h:271: ERROR: Unknown target name: "regulator_regmap_x_voltage". include/linux/spi/spi.h:373: ERROR: Unexpected indentation. drivers/w1/w1_io.c:196: WARNING: Definition list ends without a blank line; unexpected unindent. block/bio.c:404: ERROR: Unknown target name: "gfp". sound/soc/soc-core.c:2703: ERROR: Unknown target name: "snd_soc_daifmt". sound/core/jack.c:312: ERROR: Unknown target name: "snd_jack_btn". Documentation/virtual/kvm/vcpu-requests.rst:: WARNING: document isn't included in any toctree Documentation/dev-tools/kselftest.rst:15: WARNING: Could not lex literal_block as "c". Highlighting skipped. Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 133: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 164: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 193: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 133: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 164: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 193: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 133: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 164: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 193: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 133: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 164: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 193: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 133: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 164: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 193: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 133: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 164: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 193: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 133: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 164: Having multiple values in <test> isn't supported and may not work as expected

vim +/cb_list +69 include/drm/drm_syncobj.h

e9083420 Dave Airlie 2017-04-04 @69

:::::: The code at line 69 was first introduced by commit :::::: e9083420bbacce27e43d418064d0d2dfb4b37aaa drm: introduce sync objects (v4)

:::::: TO: Dave Airlie airlied@redhat.com :::::: CC: Dave Airlie airlied@redhat.com

--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation

Jason Ekstrand

28 Aug 28 Aug

2:39 p.m.

New subject: [PATCH 5/10] drm/syncobj: Add a callback mechanism for replace_fence (v3)

v2: - Don't lock in drm_syncobj_fence_get. We only really need to lock around fence_replace to make the callback work. v3: - Fix the cb_list comment to make kbuild happy

kref_init(&syncobj->refcount); + INIT_LIST_HEAD(&syncobj->cb_list); + spin_lock_init(&syncobj->lock);

idr_preload(GFP_KERNEL); spin_lock(&file_private->syncobj_table_lock); diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index ce94d14..c00fee5 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -28,6 +28,8 @@

#include "linux/dma-fence.h"

+struct drm_syncobj_cb; + /** * struct drm_syncobj - sync object. * @@ -43,15 +45,47 @@ struct drm_syncobj { /** * @fence: * NULL or a pointer to the fence bound to this object. + * + * This field should not be used directly. Use drm_syncobj_fence_get + * and drm_syncobj_replace_fence instead. */ struct dma_fence *fence; /** + * @cb_list: + * List of callbacks to call when the fence gets replaced + */ + struct list_head cb_list; + /** + * @lock: + * locks cb_list and write-locks fence. + */ + spinlock_t lock; + /** * @file: * a file backing for this syncobj. */ struct file *file; };

/** @@ -91,6 +125,11 @@ drm_syncobj_fence_get(struct drm_syncobj *syncobj)

-- 2.5.0.400.gff86faf

Jason Ekstrand

25 Aug 25 Aug

5:52 p.m.

New subject: [PATCH 06/10] drm/syncobj: Allow wait for submit and signal behavior (v5)

Vulkan VkFence semantics require that the application be able to perform a CPU wait on work which may not yet have been submitted. This is perfectly safe because the CPU wait has a timeout which will get triggered eventually if no work is ever submitted. This behavior is advantageous for multi-threaded workloads because, so long as all of the threads agree on what fences to use up-front, you don't have the extra cross-thread synchronization cost of thread A telling thread B that it has submitted its dependent work and thread B is now free to wait.

Within a single process, this can be implemented in the userspace driver by doing exactly the same kind of tracking the app would have to do using posix condition variables or similar. However, in order for this to work cross-process (as is required by VK_KHR_external_fence), we need to handle this in the kernel.

This commit adds a WAIT_FOR_SUBMIT flag to DRM_IOCTL_SYNCOBJ_WAIT which instructs the IOCTL to wait for the syncobj to have a non-null fence and then wait on the fence. Combined with DRM_IOCTL_SYNCOBJ_RESET, you can easily get the Vulkan behavior.

v2: - Fix a bug in the invalid syncobj error path - Unify the wait-all and wait-any cases v3: - Unify the timeout == 0 case a bit with the timeout > 0 case - Use wait_event_interruptible_timeout v4: - Use proxy fence v5: - Revert to a combination of v2 and v3 - Don't use proxy fences - Don't use wait_event_interruptible_timeout because it just adds an extra layer of callbacks

Signed-off-by: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@redhat.com Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Christian König christian.koenig@amd.com --- drivers/gpu/drm/drm_syncobj.c | 252 ++++++++++++++++++++++++++++++++++-------- include/uapi/drm/drm.h | 1 + 2 files changed, 208 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index bade497..615c04e 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -51,6 +51,7 @@ #include <linux/fs.h> #include <linux/anon_inodes.h> #include <linux/sync_file.h> +#include <linux/sched/signal.h>

#include "drm_internal.h" #include <drm/drm_syncobj.h> @@ -88,6 +89,35 @@ static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj, list_add_tail(&cb->node, &syncobj->cb_list); }

+static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj, + struct dma_fence **fence, + struct drm_syncobj_cb *cb, + drm_syncobj_func_t func) +{ + int ret; + + *fence = drm_syncobj_fence_get(syncobj); + if (*fence) + return 1; + + spin_lock(&syncobj->lock); + /* We've already tried once to get a fence and failed. Now that we + * have the lock, try one more time just to be sure we don't add a + * callback when a fence has already been set. + */ + if (syncobj->fence) { + *fence = dma_fence_get(syncobj->fence); + ret = 1; + } else { + *fence = NULL; + drm_syncobj_add_callback_locked(syncobj, cb, func); + ret = 0; + } + spin_unlock(&syncobj->lock); + + return ret; +} + /** * drm_syncobj_add_callback - adds a callback to syncobj::cb_list * @syncobj: Sync object to which to add the callback @@ -509,6 +539,160 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, &args->handle); }

+struct syncobj_wait_entry { + struct task_struct *task; + struct dma_fence *fence; + struct dma_fence_cb fence_cb; + struct drm_syncobj_cb syncobj_cb; +}; + +static void syncobj_wait_fence_func(struct dma_fence *fence, + struct dma_fence_cb *cb) +{ + struct syncobj_wait_entry *wait = + container_of(cb, struct syncobj_wait_entry, fence_cb); + + wake_up_process(wait->task); +} + +static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj, + struct drm_syncobj_cb *cb) +{ + struct syncobj_wait_entry *wait = + container_of(cb, struct syncobj_wait_entry, syncobj_cb); + + /* This happens inside the syncobj lock */ + wait->fence = dma_fence_get(syncobj->fence); + wake_up_process(wait->task); +} + +static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, + uint32_t count, + uint32_t flags, + signed long timeout, + uint32_t *idx) +{ + struct syncobj_wait_entry *entries; + struct dma_fence *fence; + signed long ret; + uint32_t signaled_count, i; + + entries = kcalloc(count, sizeof(*entries), GFP_KERNEL); + if (!entries) + return -ENOMEM; + + /* Walk the list of sync objects and initialize entries. We do + * this up-front so that we can properly return -EINVAL if there is + * a syncobj with a missing fence and then never have the chance of + * returning -EINVAL again. + */ + signaled_count = 0; + for (i = 0; i < count; ++i) { + entries[i].task = current; + entries[i].fence = drm_syncobj_fence_get(syncobjs[i]); + if (!entries[i].fence) { + if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { + continue; + } else { + ret = -EINVAL; + goto cleanup_entries; + } + } + + if (dma_fence_is_signaled(entries[i].fence)) { + if (signaled_count == 0 && idx) + *idx = i; + signaled_count++; + } + } + + /* Initialize ret to the max of timeout and 1. That way, the + * default return value indicates a successful wait and not a + * timeout. + */ + ret = max_t(signed long, timeout, 1); + + if (signaled_count == count || + (signaled_count > 0 && + !(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL))) + goto cleanup_entries; + + /* There's a very annoying laxness in the dma_fence API here, in + * that backends are not required to automatically report when a + * fence is signaled prior to fence->ops->enable_signaling() being + * called. So here if we fail to match signaled_count, we need to + * fallthough and try a 0 timeout wait! + */ + + if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { + for (i = 0; i < count; ++i) { + drm_syncobj_fence_get_or_add_callback(syncobjs[i], + &entries[i].fence, + &entries[i].syncobj_cb, + syncobj_wait_syncobj_func); + } + } + + do { + set_current_state(TASK_INTERRUPTIBLE); + + signaled_count = 0; + for (i = 0; i < count; ++i) { + fence = entries[i].fence; + if (!fence) + continue; + + if (dma_fence_is_signaled(fence) || + (!entries[i].fence_cb.func && + dma_fence_add_callback(fence, + &entries[i].fence_cb, + syncobj_wait_fence_func))) { + /* The fence has been signaled */ + if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL) { + signaled_count++; + } else { + if (idx) + *idx = i; + goto done_waiting; + } + } + } + + if (signaled_count == count) + goto done_waiting; + + if (timeout == 0) { + /* If we are doing a 0 timeout wait and we got + * here, then we just timed out. + */ + ret = 0; + goto done_waiting; + } + + ret = schedule_timeout(ret); + + if (ret > 0 && signal_pending(current)) + ret = -ERESTARTSYS; + } while (ret > 0); + +done_waiting: + __set_current_state(TASK_RUNNING); + +cleanup_entries: + for (i = 0; i < count; ++i) { + if (entries[i].syncobj_cb.func) + drm_syncobj_remove_callback(syncobjs[i], + &entries[i].syncobj_cb); + if (entries[i].fence_cb.func) + dma_fence_remove_callback(entries[i].fence, + &entries[i].fence_cb); + dma_fence_put(entries[i].fence); + } + kfree(entries); + + return ret; +} + /** * drm_timeout_abs_to_jiffies - calculate jiffies timeout from absolute value * @@ -541,43 +725,19 @@ static signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec) return timeout_jiffies64 + 1; }

-static int drm_syncobj_wait_fences(struct drm_device *dev, - struct drm_file *file_private, - struct drm_syncobj_wait *wait, - struct dma_fence **fences) +static int drm_syncobj_array_wait(struct drm_device *dev, + struct drm_file *file_private, + struct drm_syncobj_wait *wait, + struct drm_syncobj **syncobjs) { signed long timeout = drm_timeout_abs_to_jiffies(wait->timeout_nsec); signed long ret = 0; uint32_t first = ~0;

- if (wait->flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL) { - uint32_t i; - for (i = 0; i < wait->count_handles; i++) { - ret = dma_fence_wait_timeout(fences[i], true, timeout); - - /* Various dma_fence wait callbacks will return - * ENOENT to indicate that the fence has already - * been signaled. We need to sanitize this to 0 so - * we don't return early and the client doesn't see - * an unexpected error. - */ - if (ret == -ENOENT) - ret = 0; - - if (ret < 0) - return ret; - if (ret == 0) - break; - timeout = ret; - } - first = 0; - } else { - ret = dma_fence_wait_any_timeout(fences, - wait->count_handles, - true, timeout, - &first); - } - + ret = drm_syncobj_array_wait_timeout(syncobjs, + wait->count_handles, + wait->flags, + timeout, &first); if (ret < 0) return ret;

@@ -593,14 +753,15 @@ drm_syncobj_wait_ioctl(struct drm_device *dev, void *data, { struct drm_syncobj_wait *args = data; uint32_t *handles; - struct dma_fence **fences; + struct drm_syncobj **syncobjs; int ret = 0; uint32_t i;

if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) return -ENODEV;

- if (args->flags != 0 && args->flags != DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL) + if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL | + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)) return -EINVAL;

if (args->count_handles == 0) @@ -619,27 +780,28 @@ drm_syncobj_wait_ioctl(struct drm_device *dev, void *data, goto err_free_handles; }

- fences = kcalloc(args->count_handles, - sizeof(struct dma_fence *), GFP_KERNEL); - if (!fences) { + syncobjs = kcalloc(args->count_handles, + sizeof(struct drm_syncobj *), GFP_KERNEL); + if (!syncobjs) { ret = -ENOMEM; goto err_free_handles; }

for (i = 0; i < args->count_handles; i++) { - ret = drm_syncobj_find_fence(file_private, handles[i], - &fences[i]); - if (ret) + syncobjs[i] = drm_syncobj_find(file_private, handles[i]); + if (!syncobjs[i]) { + ret = -ENOENT; goto err_free_fence_array; + } }

- ret = drm_syncobj_wait_fences(dev, file_private, - args, fences); + ret = drm_syncobj_array_wait(dev, file_private, + args, syncobjs);

err_free_fence_array: - for (i = 0; i < args->count_handles; i++) - dma_fence_put(fences[i]); - kfree(fences); + while (i-- > 0) + drm_syncobj_put(syncobjs[i]); + kfree(syncobjs); err_free_handles: kfree(handles);

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 0757c1a..6db408d 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -719,6 +719,7 @@ struct drm_syncobj_handle { };

#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0) +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1) struct drm_syncobj_wait { __u64 handles; /* absolute timeout */

-- 2.5.0.400.gff86faf

Jason Ekstrand

5:52 p.m.

New subject: [PATCH 07/10] drm/syncobj: Add a CREATE_SIGNALED flag

This requests that the driver create the sync object such that it already has a signaled dma_fence attached. Because we don't need anything in particular (just something signaled), we use a dummy null fence. This is useful for Vulkan which has a similar flag that can be passed to vkCreateFence.

Signed-off-by: Jason Ekstrand jason@jlekstrand.net --- drivers/gpu/drm/drm_syncobj.c | 57 ++++++++++++++++++++++++++++++++++++++++--- include/uapi/drm/drm.h | 1 + 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 615c04e..cccd3bd 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -184,6 +184,49 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, } EXPORT_SYMBOL(drm_syncobj_replace_fence);

+struct drm_syncobj_null_fence { + struct dma_fence base; + spinlock_t lock; +}; + +static const char *drm_syncobj_null_fence_get_name(struct dma_fence *fence) +{ + return "syncobjnull"; +} + +static bool drm_syncobj_null_fence_enable_signaling(struct dma_fence *fence) +{ + dma_fence_enable_sw_signaling(fence); + return !dma_fence_is_signaled(fence); +} + +static const struct dma_fence_ops drm_syncobj_null_fence_ops = { + .get_driver_name = drm_syncobj_null_fence_get_name, + .get_timeline_name = drm_syncobj_null_fence_get_name, + .enable_signaling = drm_syncobj_null_fence_enable_signaling, + .wait = dma_fence_default_wait, + .release = NULL, +}; + +static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) +{ + struct drm_syncobj_null_fence *fence; + fence = kzalloc(sizeof(*fence), GFP_KERNEL); + if (fence == NULL) + return -ENOMEM; + + spin_lock_init(&fence->lock); + dma_fence_init(&fence->base, &drm_syncobj_null_fence_ops, + &fence->lock, 0, 0); + dma_fence_signal(&fence->base); + + drm_syncobj_replace_fence(syncobj, &fence->base); + + dma_fence_put(&fence->base); + + return 0; +} + int drm_syncobj_find_fence(struct drm_file *file_private, u32 handle, struct dma_fence **fence) @@ -220,7 +263,7 @@ void drm_syncobj_free(struct kref *kref) EXPORT_SYMBOL(drm_syncobj_free);

static int drm_syncobj_create(struct drm_file *file_private, - u32 *handle) + u32 *handle, uint32_t flags) { int ret; struct drm_syncobj *syncobj; @@ -233,6 +276,14 @@ static int drm_syncobj_create(struct drm_file *file_private, INIT_LIST_HEAD(&syncobj->cb_list); spin_lock_init(&syncobj->lock);

+ if (flags & DRM_SYNCOBJ_CREATE_SIGNALED) { + ret = drm_syncobj_assign_null_handle(syncobj); + if (ret < 0) { + drm_syncobj_put(syncobj); + return ret; + } + } + idr_preload(GFP_KERNEL); spin_lock(&file_private->syncobj_table_lock); ret = idr_alloc(&file_private->syncobj_idr, syncobj, 1, 0, GFP_NOWAIT); @@ -468,11 +519,11 @@ drm_syncobj_create_ioctl(struct drm_device *dev, void *data, return -ENODEV;

/* no valid flags yet */ - if (args->flags) + if (args->flags & ~DRM_SYNCOBJ_CREATE_SIGNALED) return -EINVAL;

return drm_syncobj_create(file_private, - &args->handle); + &args->handle, args->flags); }

int diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 6db408d..4c74659 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -700,6 +700,7 @@ struct drm_prime_handle {

struct drm_syncobj_create { __u32 handle; +#define DRM_SYNCOBJ_CREATE_SIGNALED (1 << 0) __u32 flags; };

-- 2.5.0.400.gff86faf

Jason Ekstrand

5:52 p.m.

New subject: [PATCH 08/10] drm/syncobj: Add a syncobj_array_find helper

The wait ioctl has a bunch of code to read an syncobj handle array from userspace and turn it into an array of syncobj pointers. We're about to add two new IOCTLs which will need to work with arrays of syncobj handles so let's make some helpers.

Signed-off-by: Jason Ekstrand jason@jlekstrand.net --- drivers/gpu/drm/drm_syncobj.c | 89 ++++++++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index cccd3bd..15e74ca 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -798,58 +798,43 @@ static int drm_syncobj_array_wait(struct drm_device *dev, return 0; }

-int -drm_syncobj_wait_ioctl(struct drm_device *dev, void *data, - struct drm_file *file_private) +static int drm_syncobj_array_find(struct drm_file *file_private, + void *user_handles, uint32_t count_handles, + struct drm_syncobj ***syncobjs_out) { - struct drm_syncobj_wait *args = data; - uint32_t *handles; + uint32_t i, *handles; struct drm_syncobj **syncobjs; - int ret = 0; - uint32_t i; - - if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) - return -ENODEV; - - if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL | - DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)) - return -EINVAL; - - if (args->count_handles == 0) - return -EINVAL; + int ret;

- /* Get the handles from userspace */ - handles = kmalloc_array(args->count_handles, sizeof(uint32_t), - GFP_KERNEL); + handles = kmalloc_array(count_handles, sizeof(*handles), GFP_KERNEL); if (handles == NULL) return -ENOMEM;

- if (copy_from_user(handles, - u64_to_user_ptr(args->handles), - sizeof(uint32_t) * args->count_handles)) { + if (copy_from_user(handles, user_handles, + sizeof(uint32_t) * count_handles)) { ret = -EFAULT; goto err_free_handles; }

- syncobjs = kcalloc(args->count_handles, - sizeof(struct drm_syncobj *), GFP_KERNEL); - if (!syncobjs) { + syncobjs = kmalloc_array(count_handles, sizeof(*syncobjs), GFP_KERNEL); + if (syncobjs == NULL) { ret = -ENOMEM; goto err_free_handles; }

- for (i = 0; i < args->count_handles; i++) { + for (i = 0; i < count_handles; i++) { syncobjs[i] = drm_syncobj_find(file_private, handles[i]); if (!syncobjs[i]) { ret = -ENOENT; - goto err_free_fence_array; + goto err_put_syncobjs; } }

- ret = drm_syncobj_array_wait(dev, file_private, - args, syncobjs); + kfree(handles); + *syncobjs_out = syncobjs; + return 0;

-err_free_fence_array: +err_put_syncobjs: while (i-- > 0) drm_syncobj_put(syncobjs[i]); kfree(syncobjs); @@ -858,3 +843,45 @@ drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,

return ret; } + +static void drm_syncobj_array_free(struct drm_syncobj **syncobjs, + uint32_t count) +{ + uint32_t i; + for (i = 0; i < count; i++) + drm_syncobj_put(syncobjs[i]); + kfree(syncobjs); +} + +int +drm_syncobj_wait_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private) +{ + struct drm_syncobj_wait *args = data; + struct drm_syncobj **syncobjs; + int ret = 0; + + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) + return -ENODEV; + + if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL | + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)) + return -EINVAL; + + if (args->count_handles == 0) + return -EINVAL; + + ret = drm_syncobj_array_find(file_private, + u64_to_user_ptr(args->handles), + args->count_handles, + &syncobjs); + if (ret < 0) + return ret; + + ret = drm_syncobj_array_wait(dev, file_private, + args, syncobjs); + + drm_syncobj_array_free(syncobjs, args->count_handles); + + return ret; +}

-- 2.5.0.400.gff86faf

Jason Ekstrand

5:52 p.m.

New subject: [PATCH 09/10] drm/syncobj: Add a reset ioctl (v2)

This just resets the dma_fence to NULL so it looks like it's never been signaled. This will be useful once we add the new wait API for allowing wait on "submit and signal" behavior.

v2: - Take an array of sync objects (Dave Airlie)

Signed-off-by: Jason Ekstrand jason@jlekstrand.net Reviewed-by: Christian König christian.koenig@amd.com (v1) --- drivers/gpu/drm/drm_internal.h | 2 ++ drivers/gpu/drm/drm_ioctl.c | 2 ++ drivers/gpu/drm/drm_syncobj.c | 30 ++++++++++++++++++++++++++++++ include/uapi/drm/drm.h | 7 +++++++ 4 files changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 534e5ac..83f1615 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -169,3 +169,5 @@ 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); +int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index b4f4434..16c5d51 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -659,6 +659,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), };

#define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 15e74ca..57f8b6c 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -885,3 +885,33 @@ drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,

return ret; } + +int +drm_syncobj_reset_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private) +{ + struct drm_syncobj_array *args = data; + struct drm_syncobj **syncobjs; + uint32_t i; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) + return -ENODEV; + + if (args->count_handles == 0) + return -EINVAL; + + ret = drm_syncobj_array_find(file_private, + u64_to_user_ptr(args->handles), + args->count_handles, + &syncobjs); + if (ret < 0) + return ret; + + for (i = 0; i < args->count_handles; i++) + drm_syncobj_replace_fence(syncobjs[i], NULL); + + drm_syncobj_array_free(syncobjs, args->count_handles); + + return 0; +} diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 4c74659..b037fdf 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -731,6 +731,12 @@ struct drm_syncobj_wait { __u32 pad; };

+struct drm_syncobj_array { + __u64 handles; + __u32 count_handles; + __u32 pad; +}; + #if defined(__cplusplus) } #endif @@ -854,6 +860,7 @@ extern "C" { #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) +#define DRM_IOCTL_SYNCOBJ_RESET DRM_IOWR(0xC4, struct drm_syncobj_array)

/** * Device specific ioctls should only be in their respective headers

-- 2.5.0.400.gff86faf

Jason Ekstrand

5:52 p.m.

New subject: [PATCH 10/10] drm/syncobj: Add a signal ioctl (v2)

This IOCTL provides a mechanism for userspace to trigger a sync object directly. There are other ways that userspace can trigger a syncobj such as submitting a dummy batch somewhere or hanging on to a triggered sync_file and doing an import. This just provides an easy way to manually trigger the sync object without weird hacks.

The motivation for this IOCTL is Vulkan fences. Vulkan lets you create a fence already in the signaled state so that you can wait on it immediatly without stalling. We could also handle this with a new create flag to ask the driver to create a syncobj that is already signaled but the IOCTL seemed a bit cleaner and more generic.

v2: - Take an array of sync objects (Dave Airlie)

Signed-off-by: Jason Ekstrand jason@jlekstrand.net --- drivers/gpu/drm/drm_internal.h | 2 ++ drivers/gpu/drm/drm_ioctl.c | 2 ++ drivers/gpu/drm/drm_syncobj.c | 33 +++++++++++++++++++++++++++++++++ include/uapi/drm/drm.h | 1 + 4 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 83f1615..fbc3f30 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -171,3 +171,5 @@ int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); +int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 16c5d51..a9ae6dd 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -661,6 +661,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), };

#define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 57f8b6c..040ac97 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -915,3 +915,36 @@ drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,

return 0; } + +int +drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private) +{ + struct drm_syncobj_array *args = data; + struct drm_syncobj **syncobjs; + uint32_t i; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) + return -ENODEV; + + if (args->count_handles == 0) + return -EINVAL; + + ret = drm_syncobj_array_find(file_private, + u64_to_user_ptr(args->handles), + args->count_handles, + &syncobjs); + if (ret < 0) + return ret; + + for (i = 0; i < args->count_handles; i++) { + ret = drm_syncobj_assign_null_handle(syncobjs[i]); + if (ret < 0) + break; + } + + drm_syncobj_array_free(syncobjs, args->count_handles); + + return ret; +} diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index b037fdf..97677cd 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -861,6 +861,7 @@ extern "C" { #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) #define DRM_IOCTL_SYNCOBJ_RESET DRM_IOWR(0xC4, struct drm_syncobj_array) +#define DRM_IOCTL_SYNCOBJ_SIGNAL DRM_IOWR(0xC5, struct drm_syncobj_array)

/** * Device specific ioctls should only be in their respective headers

-- 2.5.0.400.gff86faf

2815

Age (days ago)

2818

Last active (days ago)

dri-devel@lists.freedesktop.org

11 comments

2 participants

tags (0)

participants (2)

Jason Ekstrand
kbuild test robot