On Thu, Aug 22, 2019 at 5:28 PM Lionel Landwerlin < lionel.g.landwer...@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.landwer...@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.landwer...@intel.com> >> Cc: Christian Koenig <christian.koe...@amd.com> >> Cc: Jason Ekstrand <ja...@jlekstrand.net> >> Cc: David(ChunMing) Zhou <david1.z...@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.... > > >> + * 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
