[Intel-gfx] [PATCH v10 09/15] drm/syncobj: Add deadline support for syncobj waits
Faith Ekstrand
faith at gfxstrand.net
Fri Mar 17 19:08:39 UTC 2023
On Wed, Mar 8, 2023 at 9:54 AM Rob Clark <robdclark at gmail.com> wrote:
> From: Rob Clark <robdclark at chromium.org>
>
> Add a new flag to let userspace provide a deadline as a hint for syncobj
> and timeline waits. This gives a hint to the driver signaling the
> backing fences about how soon userspace needs it to compete work, so it
> can addjust GPU frequency accordingly. An immediate deadline can be
> given to provide something equivalent to i915 "wait boost".
>
> v2: Use absolute u64 ns value for deadline hint, drop cap and driver
> feature flag in favor of allowing count_handles==0 as a way for
> userspace to probe kernel for support of new flag
> v3: More verbose comments about UAPI
>
> Signed-off-by: Rob Clark <robdclark at chromium.org>
> ---
> drivers/gpu/drm/drm_syncobj.c | 64 ++++++++++++++++++++++++++++-------
> include/uapi/drm/drm.h | 17 ++++++++++
> 2 files changed, 68 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 0c2be8360525..a85e9464f07b 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -126,6 +126,11 @@
> * synchronize between the two.
> * This requirement is inherited from the Vulkan fence API.
> *
> + * If &DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE is set, the ioctl will also
> set
> + * a fence deadline hint on the backing fences before waiting, to provide
> the
> + * fence signaler with an appropriate sense of urgency. The deadline is
> + * specified as an absolute &CLOCK_MONOTONIC value in units of ns.
> + *
> * Similarly, &DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT takes an array of syncobj
> * handles as well as an array of u64 points and does a host-side wait on
> all
> * of syncobj fences at the given points simultaneously.
> @@ -973,7 +978,8 @@ static signed long
> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
> uint32_t count,
> uint32_t flags,
> signed long timeout,
> - uint32_t *idx)
> + uint32_t *idx,
> + ktime_t *deadline)
> {
> struct syncobj_wait_entry *entries;
> struct dma_fence *fence;
> @@ -1053,6 +1059,15 @@ static signed long
> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
> drm_syncobj_fence_add_wait(syncobjs[i],
> &entries[i]);
> }
>
> + if (deadline) {
> + for (i = 0; i < count; ++i) {
> + fence = entries[i].fence;
> + if (!fence)
> + continue;
> + dma_fence_set_deadline(fence, *deadline);
> + }
> + }
> +
> do {
> set_current_state(TASK_INTERRUPTIBLE);
>
> @@ -1151,7 +1166,8 @@ static int drm_syncobj_array_wait(struct drm_device
> *dev,
> struct drm_file *file_private,
> struct drm_syncobj_wait *wait,
> struct drm_syncobj_timeline_wait
> *timeline_wait,
> - struct drm_syncobj **syncobjs, bool
> timeline)
> + struct drm_syncobj **syncobjs, bool
> timeline,
> + ktime_t *deadline)
> {
> signed long timeout = 0;
> uint32_t first = ~0;
> @@ -1162,7 +1178,8 @@ static int drm_syncobj_array_wait(struct drm_device
> *dev,
> NULL,
>
> wait->count_handles,
> wait->flags,
> - timeout, &first);
> + timeout, &first,
> + deadline);
> if (timeout < 0)
> return timeout;
> wait->first_signaled = first;
> @@ -1172,7 +1189,8 @@ static int drm_syncobj_array_wait(struct drm_device
> *dev,
>
> u64_to_user_ptr(timeline_wait->points),
>
> timeline_wait->count_handles,
>
> timeline_wait->flags,
> - timeout, &first);
> + timeout, &first,
> + deadline);
> if (timeout < 0)
> return timeout;
> timeline_wait->first_signaled = first;
> @@ -1243,17 +1261,22 @@ drm_syncobj_wait_ioctl(struct drm_device *dev,
> void *data,
> {
> struct drm_syncobj_wait *args = data;
> struct drm_syncobj **syncobjs;
> + unsigned possible_flags;
> + ktime_t t, *tp = NULL;
> int ret = 0;
>
> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> return -EOPNOTSUPP;
>
> - if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
> - DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT))
> + possible_flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE;
> +
> + if (args->flags & ~possible_flags)
> return -EINVAL;
>
> if (args->count_handles == 0)
> - return -EINVAL;
> + return 0;
>
Did you intend this change? If so, why? What does waiting with no handles
gain us? I mean, it's probably fine but it seems unrelated to this change.
ret = drm_syncobj_array_find(file_private,
> u64_to_user_ptr(args->handles),
> @@ -1262,8 +1285,13 @@ drm_syncobj_wait_ioctl(struct drm_device *dev, void
> *data,
> if (ret < 0)
> return ret;
>
> + if (args->flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE) {
> + t = ns_to_ktime(args->deadline_ns);
> + tp = &t;
> + }
> +
> ret = drm_syncobj_array_wait(dev, file_private,
> - args, NULL, syncobjs, false);
> + args, NULL, syncobjs, false, tp);
>
> drm_syncobj_array_free(syncobjs, args->count_handles);
>
> @@ -1276,18 +1304,23 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device
> *dev, void *data,
> {
> struct drm_syncobj_timeline_wait *args = data;
> struct drm_syncobj **syncobjs;
> + unsigned possible_flags;
> + ktime_t t, *tp = NULL;
> int ret = 0;
>
> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
> return -EOPNOTSUPP;
>
> - if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
> - DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
> - DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE))
> + possible_flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE |
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE;
> +
> + if (args->flags & ~possible_flags)
> return -EINVAL;
>
> if (args->count_handles == 0)
> - return -EINVAL;
> + return -0;
>
Did you intend this change? -0 is a pretty weird integer.
> ret = drm_syncobj_array_find(file_private,
> u64_to_user_ptr(args->handles),
> @@ -1296,8 +1329,13 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device
> *dev, void *data,
> if (ret < 0)
> return ret;
>
> + if (args->flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE) {
> + t = ns_to_ktime(args->deadline_ns);
> + tp = &t;
> + }
> +
> ret = drm_syncobj_array_wait(dev, file_private,
> - NULL, args, syncobjs, true);
> + NULL, args, syncobjs, true, tp);
>
> drm_syncobj_array_free(syncobjs, args->count_handles);
>
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 642808520d92..bff0509ac8b6 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -887,6 +887,7 @@ struct drm_syncobj_transfer {
> #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0)
> #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1)
> #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE (1 << 2) /* wait for time
> point to become available */
> +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE (1 << 3) /* set fence
> deadline based to deadline_ns */
> struct drm_syncobj_wait {
> __u64 handles;
> /* absolute timeout */
> @@ -895,6 +896,14 @@ struct drm_syncobj_wait {
> __u32 flags;
> __u32 first_signaled; /* only valid when not waiting all */
> __u32 pad;
> + /**
> + * @deadline_ns - fence deadline hint
> + *
> + * Deadline hint, in absolute CLOCK_MONOTONIC, to set on backing
> + * fence(s) if the DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE flag is
> + * set.
> + */
> + __u64 deadline_ns;
> };
>
> struct drm_syncobj_timeline_wait {
> @@ -907,6 +916,14 @@ struct drm_syncobj_timeline_wait {
> __u32 flags;
> __u32 first_signaled; /* only valid when not waiting all */
> __u32 pad;
> + /**
> + * @deadline_ns - fence deadline hint
> + *
> + * Deadline hint, in absolute CLOCK_MONOTONIC, to set on backing
> + * fence(s) if the DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE flag is
> + * set.
> + */
> + __u64 deadline_ns;
> };
>
>
> --
> 2.39.2
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20230317/cc235f54/attachment-0001.htm>
More information about the Intel-gfx
mailing list