[PATCH] drm/syncobj: Add documentation for timeline syncobj
Daniel Vetter
daniel at ffwll.ch
Tue Aug 27 16:27:32 UTC 2019
On Mon, Aug 26, 2019 at 07:30:08AM +0300, Lionel Landwerlin wrote:
> On 26/08/2019 00:01, Daniel Vetter wrote:
> > On Fri, Aug 23, 2019 at 8:53 PM Jason Ekstrand <jason at jlekstrand.net> wrote:
> > >
> > > On Thu, Aug 22, 2019 at 5:28 PM Lionel Landwerlin <lionel.g.landwerlin at intel.com> wrote:
> > > > On 22/08/2019 21:24, Jason Ekstrand wrote:
> > > >
> > > > On Thu, Aug 22, 2019 at 9:55 AM Lionel Landwerlin <lionel.g.landwerlin at intel.com> wrote:
> > > > > We've added a set of new APIs to manipulate syncobjs holding timelines
> > > > > of dma_fence. This adds a bit of documentation about how this works.
> > > > >
> > > > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> > > > > Cc: Christian Koenig <Christian.Koenig at amd.com>
> > > > > Cc: Jason Ekstrand <jason at jlekstrand.net>
> > > > > Cc: David(ChunMing) Zhou <David1.Zhou at amd.com>
> > > > > ---
> > > > > drivers/gpu/drm/drm_syncobj.c | 87 +++++++++++++++++++++++++++++------
> > > > > 1 file changed, 74 insertions(+), 13 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> > > > > index b5ad73330a48..32ffded6d2c0 100644
> > > > > --- a/drivers/gpu/drm/drm_syncobj.c
> > > > > +++ b/drivers/gpu/drm/drm_syncobj.c
> > > > > @@ -43,27 +43,66 @@
> > > > > * - Signal a syncobj (set a trivially signaled fence)
> > > > > * - Wait for a syncobj's fence to appear and be signaled
> > > > > *
> > > > > + * The syncobj userspace API also provides operations to manipulate a syncobj
> > > > > + * in terms of a timeline of struct &dma_fence rather than a single struct
> > > > > + * &dma_fence, through the following operations:
> > > > > + *
> > > > > + * - Signal a given point on the timeline
> > > > > + * - Wait for a given point to appear and/or be signaled
> > > > > + * - Import and export from/to a given point of a timeline
> > > > > + *
> > > > > * At it's core, a syncobj is simply a wrapper around a pointer to a struct
> > > > > * &dma_fence which may be NULL.
> > > > > * When a syncobj is first created, its pointer is either NULL or a pointer
> > > > > * to an already signaled fence depending on whether the
> > > > > * &DRM_SYNCOBJ_CREATE_SIGNALED flag is passed to
> > > > > * &DRM_IOCTL_SYNCOBJ_CREATE.
> > > > > - * When GPU work which signals a syncobj is enqueued in a DRM driver,
> > > > > - * the syncobj fence is replaced with a fence which will be signaled by the
> > > > > - * completion of that work.
> > > > > - * When GPU work which waits on a syncobj is enqueued in a DRM driver, the
> > > > > - * driver retrieves syncobj's current fence at the time the work is enqueued
> > > > > - * waits on that fence before submitting the work to hardware.
> > > > > - * If the syncobj's fence is NULL, the enqueue operation is expected to fail.
> > > > > - * All manipulation of the syncobjs's fence happens in terms of the current
> > > > > - * fence at the time the ioctl is called by userspace regardless of whether
> > > > > - * that operation is an immediate host-side operation (signal or reset) or
> > > > > - * or an operation which is enqueued in some driver queue.
> > > > > - * &DRM_IOCTL_SYNCOBJ_RESET and &DRM_IOCTL_SYNCOBJ_SIGNAL can be used to
> > > > > - * manipulate a syncobj from the host by resetting its pointer to NULL or
> > > > > + *
> > > > > + * If the syncobj is considered as a binary (signal/unsignaled) primitive,
> > > >
> > > > What does "considered as a binary" mean? Is it an inherent property of the syncobj given at create time? Is it a state the syncobj can be in? Or is it a property of how the submit ioctl in the DRM driver references it? I'm really hoping it's either 1 or 3....
> > > >
> > > >
> > > > 3: you either use it binary/legacy apis, or timeline apis. timeline apis also provide some binary compatibility with the point 0 (in particular for wait).
> > >
> > > Right. Maybe we should say something like "When GPU work is enqueued which signals a non-zero time point" or something like that? I guess that implies a certain unification across drivers that maybe we don't want....
> > [Just jumping in on this comment here]
> >
> > I thought the point of syncobj is that you can share them across
> > drivers (not just within drivers)? Otherwise not much sense in the
> > common infrastructure. Hence I'd say we should spec all these things.
> > Concern from someone who's seen way too many cross-driver apis that
> > turned out the be decidedly cross-driver than planned ...
>
>
> The sharing of a timeline semaphore/syncobj between 2 apps/drivers implies
> that they both know they're dealing with a timeline semaphore.
>
> I see that at the same level as sharing a file descriptor and knowing it
> represents a syncfd or a syncobj.
>
> There has to be some kind of understanding, otherwise nothing works.
>
>
> If the shared semantic between the 2 clients is a binary (signal/unsignaled)
> semaphore, then both drivers should share the existing syncobj type, that is
> a syncobj that will only ever contain a single dma-fence.
>
> You can build that out of the timeline by exporting a particular point into
> another syncobj (transfer ioctl).
Oh this is just stating that apps need to agree on old syncobj or timeline
syncobj mode? I guess if it's all there is that should be a given, still
worth maybe putting in words.
-Daniel
>
>
> -Lionel
>
> >
> > Cheers, Daniel
> >
> >
> > > >
> > > > > + * when GPU work is enqueued in a DRM driver to signal the syncobj, the fence
> > > > > + * is replaced with a fence which will be signaled by the completion of that
> > > > > + * work.
> > > > > + * If the syncobj is considered as a timeline primitive, when GPU work is
> > > > > + * enqueued in a DRM driver to signal the a given point of the syncobj, a new
> > > > > + * struct &dma_fence_chain pointing to the DRM driver's fence and also
> > > > > + * pointing to the previous fence that was in the syncobj. The new struct
> > > > > + * &dma_fence_chain fence put into the syncobj will be signaled by completion
> > > > > + * of the DRM driver's work and also any work associated with the fence
> > > > > + * previously in the syncobj.
> > > > > + *
> > > > > + * When GPU work which waits on a syncobj is enqueued in a DRM driver, at the
> > > > > + * time the work is enqueued, it waits on the fence coming from the syncobj
> > > > > + * before submitting the work to hardware. That fence is either :
> > > > > + *
> > > > > + * - The syncobj's current fence if the syncobj is considered as a binary
> > > > > + * primitive.
> > > > > + * - The struct &dma_fence associated with a given point if the syncobj is
> > > > > + * considered as a timeline primitive.
> > > > > + *
> > > > > + * If the syncobj's fence is NULL or not present in the syncobj's timeline,
> > > > > + * the enqueue operation is expected to fail.
> > > > > + *
> > > > > + * With binary syncobj, all manipulation of the syncobjs's fence happens in
> > > > > + * terms of the current fence at the time the ioctl is called by userspace
> > > > > + * regardless of whether that operation is an immediate host-side operation
> > > > > + * (signal or reset) or or an operation which is enqueued in some driver
> > > > > + * queue. &DRM_IOCTL_SYNCOBJ_RESET and &DRM_IOCTL_SYNCOBJ_SIGNAL can be used
> > > > > + * to manipulate a syncobj from the host by resetting its pointer to NULL or
> > > > > * setting its pointer to a fence which is already signaled.
> > > > > *
> > > > > + * With timeline syncobj, all manipulation of the timeline fences happens in
> > > > > + * terms of the fence referred to in the timeline. See
> > > > > + * dma_fence_chain_find_seqno() to see how a given point is found in the
> > > > > + * timeline.
> > > > > + *
> > > > > + * Note that applications should be careful to always use timeline set of
> > > > > + * ioctl() when dealing with syncobj considered as timeline. Using a binary
> > > > > + * set of ioctl() with a syncobj considered as timeline could result incorrect
> > > > > + * synchronization. The use of binary syncobj is supported through the
> > > > > + * timeline set of ioctl() by using a point value of 0, this will reproduce
> > > > > + * the behavior of the binary set of ioctl() (for example replace the
> > > > > + * syncobj's fence when signaling).
> > > >
> > > > I know I've asked this before but I feel compelled to ask it again. Why do we allow them to mix and match? Why not just have a create flag and enforce meaningful behavior? I'm a bit concerned that userspace is going to start relying on the subtlties of the interaction between timeline and binary syncobjs which are neither documented nor properly tested in IGT.
> > > >
> > > >
> > > > For one, you might have to mix both types of syncobjs in a given wait/signal operation. So 0 ensures we can do that.
> > >
> > > Right, that sounds like a useful feature.
> > >
> > > > Second, drm-syncobj is a container and its payload is an interface (dma_fence) which has several implementations.
> > > >
> > > > The kernel primitive is just less restrictive than the Vulkan API here.
> > > >
> > > > I guess we could add a flag at creation to ensure the replacement of the fence in a timeline syncobj cannot happen.
> > >
> > > I would be in favor of that but I'd be interested to hear what Christian or David think.
> > >
> > > > I haven't thought of all the implications that might have though... Should we allow reset on a timeline syncobj?
> > >
> > > Good question. I'm inclined to say "yes" because it's pretty well-defined what such a reset means. However, it's not really needed.
> > >
> > > > -Lionel
> > > >
> > > >
> > > >
> > > > > + *
> > > > > *
> > > > > * Host-side wait on syncobjs
> > > > > * --------------------------
> > > > > @@ -87,6 +126,16 @@
> > > > > * synchronize between the two.
> > > > > * This requirement is inherited from the Vulkan fence API.
> > > > > *
> > > > > + * 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.
> > > > > + *
> > > > > + * &DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT also adds the ability to wait for a given
> > > > > + * fence to materialize on the timeline without waiting for the fence to be
> > > > > + * signaled by using the &DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE flag. This
> > > > > + * requirement is inherited from the wait-before-signal behavior required by
> > > > > + * the Vulkan timeline semaphore API.
> > > > > + *
> > > > > *
> > > > > * Import/export of syncobjs
> > > > > * -------------------------
> > > > > @@ -120,6 +169,18 @@
> > > > > * Because sync files are immutable, resetting or signaling the syncobj
> > > > > * will not affect any sync files whose fences have been imported into the
> > > > > * syncobj.
> > > > > + *
> > > > > + *
> > > > > + * Import/export of timeline points in timeline syncobjs
> > > > > + * -----------------------------------------------------
> > > > > + *
> > > > > + * &DRM_IOCTL_SYNCOBJ_TRANSFER provides a mechanism to transfer a struct
> > > > > + * &dma_fence of at a given point from a timeline syncobj to another point
> > > > > + * into another timeline syncobj.
> > > > > + *
> > > > > + * Note that if you want to transfer a struct &dma_fence from a given point on
> > > > > + * a timeline syncobj from/into a binary syncobj, you can use the point 0 to
> > > > > + * mean take/replace the fence in the syncobj.
> > > > > */
> > > > >
> > > > > #include <linux/anon_inodes.h>
> > > > > --
> > > > > 2.23.0
> > > > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list