[PATCH] drm/syncobj: Add documentation for timeline syncobj

Lionel Landwerlin lionel.g.landwerlin at intel.com
Thu Aug 22 22:28:36 UTC 2019


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 <mailto: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
>     <mailto:lionel.g.landwerlin at intel.com>>
>     Cc: Christian Koenig <Christian.Koenig at amd.com
>     <mailto:Christian.Koenig at amd.com>>
>     Cc: Jason Ekstrand <jason at jlekstrand.net
>     <mailto:jason at jlekstrand.net>>
>     Cc: David(ChunMing) Zhou <David1.Zhou at amd.com
>     <mailto: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).


>     + * 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.


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 haven't thought of all the implications that might have though... 
Should we allow reset on a timeline syncobj?


-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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20190823/e4a06829/attachment-0001.html>


More information about the dri-devel mailing list