[PATCH] drm/syncobj: Add documentation for timeline syncobj
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>>
> 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
> 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)
> 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
> + * 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
> + * 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
> + * 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
> + * 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
> + * regardless of whether that operation is an immediate host-side
> + * (signal or reset) or or an operation which is enqueued in some
> + * 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
> + * 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?
> + *
> * 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
> + * 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
> * 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
> + * &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>
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the dri-devel