* [PATCH 1/7] drm/syncobj: Rename fence_get to find_fence
2017-08-16 4:13 [PATCH 0/7] drm/syncobj: Add full-featured wait support (v3) Jason Ekstrand
@ 2017-08-16 4:13 ` Jason Ekstrand
2017-08-16 4:13 ` [PATCH 2/7] drm/syncobj: Add a race-free drm_syncobj_fence_get helper (v2) Jason Ekstrand
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Jason Ekstrand @ 2017-08-16 4:13 UTC (permalink / raw)
To: dri-devel; +Cc: Jason Ekstrand, Jason Ekstrand
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 3378951..dcfb056 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1038,7 +1038,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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/7] drm/syncobj: Add a race-free drm_syncobj_fence_get helper (v2)
2017-08-16 4:13 [PATCH 0/7] drm/syncobj: Add full-featured wait support (v3) Jason Ekstrand
2017-08-16 4:13 ` [PATCH 1/7] drm/syncobj: Rename fence_get to find_fence Jason Ekstrand
@ 2017-08-16 4:13 ` Jason Ekstrand
2017-08-16 4:13 ` [PATCH 3/7] i915: Use drm_syncobj_fence_get Jason Ekstrand
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Jason Ekstrand @ 2017-08-16 4:13 UTC (permalink / raw)
To: dri-devel; +Cc: Jason Ekstrand, Jason Ekstrand
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/7] i915: Use drm_syncobj_fence_get
2017-08-16 4:13 [PATCH 0/7] drm/syncobj: Add full-featured wait support (v3) Jason Ekstrand
2017-08-16 4:13 ` [PATCH 1/7] drm/syncobj: Rename fence_get to find_fence Jason Ekstrand
2017-08-16 4:13 ` [PATCH 2/7] drm/syncobj: Add a race-free drm_syncobj_fence_get helper (v2) Jason Ekstrand
@ 2017-08-16 4:13 ` Jason Ekstrand
2017-08-16 4:13 ` [PATCH 4/7] drm/syncobj: add sync obj wait interface. (v8) Jason Ekstrand
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Jason Ekstrand @ 2017-08-16 4:13 UTC (permalink / raw)
To: dri-devel; +Cc: Jason Ekstrand, Jason Ekstrand
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 8e8bc7a..0a85e32 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -2215,9 +2215,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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/7] drm/syncobj: add sync obj wait interface. (v8)
2017-08-16 4:13 [PATCH 0/7] drm/syncobj: Add full-featured wait support (v3) Jason Ekstrand
` (2 preceding siblings ...)
2017-08-16 4:13 ` [PATCH 3/7] i915: Use drm_syncobj_fence_get Jason Ekstrand
@ 2017-08-16 4:13 ` Jason Ekstrand
2017-08-16 4:13 ` [PATCH 5/7] drm/syncobj: Add a reset ioctl Jason Ekstrand
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Jason Ekstrand @ 2017-08-16 4:13 UTC (permalink / raw)
To: dri-devel; +Cc: Dave Airlie
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 8bfeb32..2ab0ff90 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 5/7] drm/syncobj: Add a reset ioctl
2017-08-16 4:13 [PATCH 0/7] drm/syncobj: Add full-featured wait support (v3) Jason Ekstrand
` (3 preceding siblings ...)
2017-08-16 4:13 ` [PATCH 4/7] drm/syncobj: add sync obj wait interface. (v8) Jason Ekstrand
@ 2017-08-16 4:13 ` Jason Ekstrand
2017-08-16 4:13 ` [PATCH 6/7] drm/syncobj: Add a callback mechanism for replace_fence (v2) Jason Ekstrand
2017-08-16 4:13 ` [PATCH 7/7] drm/syncobj: Allow wait for submit and signal behavior (v5) Jason Ekstrand
6 siblings, 0 replies; 8+ messages in thread
From: Jason Ekstrand @ 2017-08-16 4:13 UTC (permalink / raw)
To: dri-devel; +Cc: Jason Ekstrand, Jason Ekstrand
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.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-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 | 24 ++++++++++++++++++++++++
include/uapi/drm/drm.h | 6 ++++++
4 files changed, 34 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 2ab0ff90..4185483 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 4e8563c..755af5f 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -589,3 +589,27 @@ 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_reset *args = data;
+ struct drm_syncobj *syncobj;
+
+ if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+ return -ENODEV;
+
+ if (args->flags != 0)
+ return -EINVAL;
+
+ syncobj = drm_syncobj_find(file_private, args->handle);
+ if (!syncobj)
+ return -ENOENT;
+
+ drm_syncobj_replace_fence(syncobj, NULL);
+
+ drm_syncobj_put(syncobj);
+
+ return 0;
+}
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 0757c1a..4b301b4 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -729,6 +729,11 @@ struct drm_syncobj_wait {
__u32 pad;
};
+struct drm_syncobj_reset {
+ __u32 handle;
+ __u32 flags;
+};
+
#if defined(__cplusplus)
}
#endif
@@ -852,6 +857,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_reset)
/**
* Device specific ioctls should only be in their respective headers
--
2.5.0.400.gff86faf
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 6/7] drm/syncobj: Add a callback mechanism for replace_fence (v2)
2017-08-16 4:13 [PATCH 0/7] drm/syncobj: Add full-featured wait support (v3) Jason Ekstrand
` (4 preceding siblings ...)
2017-08-16 4:13 ` [PATCH 5/7] drm/syncobj: Add a reset ioctl Jason Ekstrand
@ 2017-08-16 4:13 ` Jason Ekstrand
2017-08-16 4:13 ` [PATCH 7/7] drm/syncobj: Allow wait for submit and signal behavior (v5) Jason Ekstrand
6 siblings, 0 replies; 8+ messages in thread
From: Jason Ekstrand @ 2017-08-16 4:13 UTC (permalink / raw)
To: dri-devel; +Cc: Jason Ekstrand, Jason Ekstrand
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 755af5f..24a5286 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 7/7] drm/syncobj: Allow wait for submit and signal behavior (v5)
2017-08-16 4:13 [PATCH 0/7] drm/syncobj: Add full-featured wait support (v3) Jason Ekstrand
` (5 preceding siblings ...)
2017-08-16 4:13 ` [PATCH 6/7] drm/syncobj: Add a callback mechanism for replace_fence (v2) Jason Ekstrand
@ 2017-08-16 4:13 ` Jason Ekstrand
6 siblings, 0 replies; 8+ messages in thread
From: Jason Ekstrand @ 2017-08-16 4:13 UTC (permalink / raw)
To: dri-devel
Cc: Jason Ekstrand, Dave Airlie, Christian König, Jason Ekstrand
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 24a5286..7dfdb98 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 4b301b4..f8ec8fe 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread