From: Rob Clark <robdclark@gmail.com>
To: Faith Ekstrand <faith@gfxstrand.net>
Cc: dri-devel@lists.freedesktop.org,
"Rob Clark" <robdclark@chromium.org>,
"Daniel Vetter" <daniel@ffwll.ch>,
"Christian König" <ckoenig.leichtzumerken@gmail.com>,
intel-gfx@lists.freedesktop.org,
"Bas Nieuwenhuizen" <bas@basnieuwenhuizen.nl>,
"open list" <linux-kernel@vger.kernel.org>,
"Maxime Ripard" <mripard@kernel.org>,
"David Airlie" <airlied@gmail.com>,
"Luben Tuikov" <luben.tuikov@amd.com>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
"Matt Turner" <mattst88@gmail.com>,
freedreno@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v10 09/15] drm/syncobj: Add deadline support for syncobj waits
Date: Fri, 17 Mar 2023 12:38:22 -0700 [thread overview]
Message-ID: <CAF6AEGv5Bo6sbT+en7=C+QG9+m+Wn0gauQ1qhdzBv6AEkChu7A@mail.gmail.com> (raw)
In-Reply-To: <CAOFGe944_xJOJ3a-uJDVyca_1_+aYTqat4=Qc3CC1wUubxw3XQ@mail.gmail.com>
On Fri, Mar 17, 2023 at 12:08 PM Faith Ekstrand <faith@gfxstrand.net> wrote:
>
>
> On Wed, Mar 8, 2023 at 9:54 AM Rob Clark <robdclark@gmail.com> wrote:
>>
>> From: Rob Clark <robdclark@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@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.
Yes, it was intentional.. it gives userspace a way to probe whether
the kernel supports the new flag..
BR,
-R
>> 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 prev parent reply other threads:[~2023-03-17 19:38 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-08 15:52 [PATCH v10 00/15] dma-fence: Deadline awareness Rob Clark
2023-03-08 15:52 ` [PATCH v10 01/15] dma-buf/dma-fence: Add deadline awareness Rob Clark
2023-03-10 15:45 ` Jonas Ådahl
2023-03-10 17:38 ` Rob Clark
2023-03-15 13:53 ` Jonas Ådahl
2023-03-15 16:19 ` Rob Clark
2023-03-16 9:26 ` Jonas Ådahl
2023-03-16 16:28 ` Rob Clark
2023-03-16 22:22 ` Sebastian Wick
2023-03-16 22:59 ` Rob Clark
2023-03-17 15:07 ` Sebastian Wick
2023-03-17 9:10 ` Michel Dänzer
2023-03-17 10:23 ` Jonas Ådahl
2023-03-17 15:59 ` Rob Clark
2023-03-21 13:24 ` Jonas Ådahl
2023-03-21 14:34 ` Rob Clark
2023-03-08 15:52 ` [PATCH v10 02/15] dma-buf/fence-array: Add fence deadline support Rob Clark
2023-03-08 15:52 ` [PATCH v10 03/15] dma-buf/fence-chain: " Rob Clark
2023-03-08 15:52 ` [PATCH v10 04/15] dma-buf/dma-resv: Add a way to set fence deadline Rob Clark
2023-03-08 15:52 ` [PATCH v10 05/15] dma-buf/sync_file: Surface sync-file uABI Rob Clark
2023-03-08 15:52 ` [PATCH v10 06/15] dma-buf/sync_file: Add SET_DEADLINE ioctl Rob Clark
2023-03-08 15:52 ` [PATCH v10 07/15] dma-buf/sw_sync: Add fence deadline support Rob Clark
2023-03-28 13:57 ` Tvrtko Ursulin
2023-03-08 15:52 ` [PATCH v10 08/15] drm/scheduler: " Rob Clark
2023-03-08 15:53 ` [PATCH v10 09/15] drm/syncobj: Add deadline support for syncobj waits Rob Clark
[not found] ` <CAOFGe944_xJOJ3a-uJDVyca_1_+aYTqat4=Qc3CC1wUubxw3XQ@mail.gmail.com>
2023-03-17 19:38 ` Rob Clark [this message]
2023-03-18 16:07 ` [Intel-gfx] " Rob Clark
2023-03-28 14:24 ` Tvrtko Ursulin
2023-03-08 15:53 ` [PATCH v10 10/15] drm/vblank: Add helper to get next vblank time Rob Clark
2023-03-08 15:53 ` [PATCH v10 11/15] drm/atomic-helper: Set fence deadline for vblank Rob Clark
2023-03-31 20:44 ` Nathan Chancellor
2023-03-31 22:14 ` Rob Clark
2023-03-31 23:30 ` Nathan Chancellor
2023-04-01 15:39 ` Rob Clark
2023-04-04 17:22 ` Dmitry Baryshkov
2023-04-04 19:16 ` Daniel Vetter
2023-04-04 21:53 ` Dmitry Baryshkov
2023-04-05 7:58 ` Daniel Vetter
2023-03-08 15:53 ` [PATCH v10 12/15] drm/msm: Add deadline based boost support Rob Clark
2023-03-08 15:53 ` [PATCH v10 13/15] drm/msm: Add wait-boost support Rob Clark
2023-03-08 15:53 ` [PATCH v10 14/15] drm/msm/atomic: Switch to vblank_start helper Rob Clark
2023-03-08 15:53 ` [PATCH v10 15/15] drm/i915: Add deadline based boost support Rob Clark
2023-03-09 10:21 ` [PATCH v10 00/15] dma-fence: Deadline awareness Pekka Paalanen
2023-03-16 21:22 ` Rob Clark
2023-03-27 19:05 ` Matt Turner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAF6AEGv5Bo6sbT+en7=C+QG9+m+Wn0gauQ1qhdzBv6AEkChu7A@mail.gmail.com' \
--to=robdclark@gmail.com \
--cc=airlied@gmail.com \
--cc=bas@basnieuwenhuizen.nl \
--cc=ckoenig.leichtzumerken@gmail.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=faith@gfxstrand.net \
--cc=freedreno@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luben.tuikov@amd.com \
--cc=mattst88@gmail.com \
--cc=mripard@kernel.org \
--cc=robdclark@chromium.org \
--cc=rodrigo.vivi@intel.com \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).