<div dir="ltr"><div dir="ltr"><br></div><div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Aug 22, 2019 at 5:28 PM Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com" target="_blank">lionel.g.landwerlin@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<div class="gmail-m_-5272467495377520358gmail-m_-1548110483521490893moz-cite-prefix">On 22/08/2019 21:24, Jason Ekstrand
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, Aug 22, 2019 at 9:55
AM Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com" target="_blank">lionel.g.landwerlin@intel.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">We've added a set of new
APIs to manipulate syncobjs holding timelines<br>
of dma_fence. This adds a bit of documentation about how
this works.<br>
<br>
Signed-off-by: Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com" target="_blank">lionel.g.landwerlin@intel.com</a>><br>
Cc: Christian Koenig <<a href="mailto:Christian.Koenig@amd.com" target="_blank">Christian.Koenig@amd.com</a>><br>
Cc: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>><br>
Cc: David(ChunMing) Zhou <<a href="mailto:David1.Zhou@amd.com" target="_blank">David1.Zhou@amd.com</a>><br>
---<br>
drivers/gpu/drm/drm_syncobj.c | 87
+++++++++++++++++++++++++++++------<br>
1 file changed, 74 insertions(+), 13 deletions(-)<br>
<br>
diff --git a/drivers/gpu/drm/drm_syncobj.c
b/drivers/gpu/drm/drm_syncobj.c<br>
index b5ad73330a48..32ffded6d2c0 100644<br>
--- a/drivers/gpu/drm/drm_syncobj.c<br>
+++ b/drivers/gpu/drm/drm_syncobj.c<br>
@@ -43,27 +43,66 @@<br>
* - Signal a syncobj (set a trivially signaled fence)<br>
* - Wait for a syncobj's fence to appear and be signaled<br>
*<br>
+ * The syncobj userspace API also provides operations to
manipulate a syncobj<br>
+ * in terms of a timeline of struct &dma_fence rather
than a single struct<br>
+ * &dma_fence, through the following operations:<br>
+ *<br>
+ * - Signal a given point on the timeline<br>
+ * - Wait for a given point to appear and/or be signaled<br>
+ * - Import and export from/to a given point of a
timeline<br>
+ *<br>
* At it's core, a syncobj is simply a wrapper around a
pointer to a struct<br>
* &dma_fence which may be NULL.<br>
* When a syncobj is first created, its pointer is either
NULL or a pointer<br>
* to an already signaled fence depending on whether the<br>
* &DRM_SYNCOBJ_CREATE_SIGNALED flag is passed to<br>
* &DRM_IOCTL_SYNCOBJ_CREATE.<br>
- * When GPU work which signals a syncobj is enqueued in a
DRM driver,<br>
- * the syncobj fence is replaced with a fence which will be
signaled by the<br>
- * completion of that work.<br>
- * When GPU work which waits on a syncobj is enqueued in a
DRM driver, the<br>
- * driver retrieves syncobj's current fence at the time the
work is enqueued<br>
- * waits on that fence before submitting the work to
hardware.<br>
- * If the syncobj's fence is NULL, the enqueue operation is
expected to fail.<br>
- * All manipulation of the syncobjs's fence happens in
terms of the current<br>
- * fence at the time the ioctl is called by userspace
regardless of whether<br>
- * that operation is an immediate host-side operation
(signal or reset) or<br>
- * or an operation which is enqueued in some driver queue.<br>
- * &DRM_IOCTL_SYNCOBJ_RESET and
&DRM_IOCTL_SYNCOBJ_SIGNAL can be used to<br>
- * manipulate a syncobj from the host by resetting its
pointer to NULL or<br>
+ *<br>
+ * If the syncobj is considered as a binary
(signal/unsignaled) primitive,<br>
</blockquote>
<div><br>
</div>
<div>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....<br>
</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>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).<br>
</p>
</div></blockquote><div><br></div><div>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....<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ * when GPU work is enqueued in a DRM driver to signal the
syncobj, the fence<br>
+ * is replaced with a fence which will be signaled by the
completion of that<br>
+ * work.<br>
+ * If the syncobj is considered as a timeline primitive,
when GPU work is<br>
+ * enqueued in a DRM driver to signal the a given point of
the syncobj, a new<br>
+ * struct &dma_fence_chain pointing to the DRM driver's
fence and also<br>
+ * pointing to the previous fence that was in the syncobj.
The new struct<br>
+ * &dma_fence_chain fence put into the syncobj will be
signaled by completion<br>
+ * of the DRM driver's work and also any work associated
with the fence<br>
+ * previously in the syncobj.<br>
+ *<br>
+ * When GPU work which waits on a syncobj is enqueued in a
DRM driver, at the<br>
+ * time the work is enqueued, it waits on the fence coming
from the syncobj<br>
+ * before submitting the work to hardware. That fence is
either :<br>
+ *<br>
+ * - The syncobj's current fence if the syncobj is
considered as a binary<br>
+ * primitive.<br>
+ * - The struct &dma_fence associated with a given
point if the syncobj is<br>
+ * considered as a timeline primitive.<br>
+ *<br>
+ * If the syncobj's fence is NULL or not present in the
syncobj's timeline,<br>
+ * the enqueue operation is expected to fail.<br>
+ *<br>
+ * With binary syncobj, all manipulation of the syncobjs's
fence happens in<br>
+ * terms of the current fence at the time the ioctl is
called by userspace<br>
+ * regardless of whether that operation is an immediate
host-side operation<br>
+ * (signal or reset) or or an operation which is enqueued
in some driver<br>
+ * queue. &DRM_IOCTL_SYNCOBJ_RESET and
&DRM_IOCTL_SYNCOBJ_SIGNAL can be used<br>
+ * to manipulate a syncobj from the host by resetting its
pointer to NULL or<br>
* setting its pointer to a fence which is already
signaled.<br>
*<br>
+ * With timeline syncobj, all manipulation of the timeline
fences happens in<br>
+ * terms of the fence referred to in the timeline. See<br>
+ * dma_fence_chain_find_seqno() to see how a given point is
found in the<br>
+ * timeline.<br>
+ *<br>
+ * Note that applications should be careful to always use
timeline set of<br>
+ * ioctl() when dealing with syncobj considered as
timeline. Using a binary<br>
+ * set of ioctl() with a syncobj considered as timeline
could result incorrect<br>
+ * synchronization. The use of binary syncobj is supported
through the<br>
+ * timeline set of ioctl() by using a point value of 0,
this will reproduce<br>
+ * the behavior of the binary set of ioctl() (for example
replace the<br>
+ * syncobj's fence when signaling).<br>
</blockquote>
<div> </div>
<div>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.<br>
</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>For one, you might have to mix both types of syncobjs in a given
wait/signal operation. So 0 ensures we can do that.</p></div></blockquote><div><br></div><div>Right, that sounds like a useful feature.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF"><p>Second, drm-syncobj is a container and its payload is an
interface (dma_fence) which has several implementations.</p>
<p>The kernel primitive is just less restrictive than the Vulkan API
here.<br>
</p>
<p>I guess we could add a flag at creation to ensure the replacement
of the fence in a timeline syncobj cannot happen.</p></div></blockquote><div><br></div><div>I would be in favor of that but I'd be interested to hear what Christian or David think.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<p>I haven't thought of all the implications that might have
though... Should we allow reset on a timeline syncobj?</p></div></blockquote><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF"><p>-Lionel<br>
</p>
<p><br>
</p>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div> <br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ *<br>
*<br>
* Host-side wait on syncobjs<br>
* --------------------------<br>
@@ -87,6 +126,16 @@<br>
* synchronize between the two.<br>
* This requirement is inherited from the Vulkan fence API.<br>
*<br>
+ * Similarly, &DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT takes an
array of syncobj<br>
+ * handles as well as an array of u64 points and does a
host-side wait on all<br>
+ * of syncobj fences at the given points simultaneously.<br>
+ *<br>
+ * &DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT also adds the
ability to wait for a given<br>
+ * fence to materialize on the timeline without waiting for
the fence to be<br>
+ * signaled by using the
&DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE flag. This<br>
+ * requirement is inherited from the wait-before-signal
behavior required by<br>
+ * the Vulkan timeline semaphore API.<br>
+ *<br>
*<br>
* Import/export of syncobjs<br>
* -------------------------<br>
@@ -120,6 +169,18 @@<br>
* Because sync files are immutable, resetting or signaling
the syncobj<br>
* will not affect any sync files whose fences have been
imported into the<br>
* syncobj.<br>
+ *<br>
+ *<br>
+ * Import/export of timeline points in timeline syncobjs<br>
+ * -----------------------------------------------------<br>
+ *<br>
+ * &DRM_IOCTL_SYNCOBJ_TRANSFER provides a mechanism to
transfer a struct<br>
+ * &dma_fence of at a given point from a timeline
syncobj to another point<br>
+ * into another timeline syncobj.<br>
+ *<br>
+ * Note that if you want to transfer a struct
&dma_fence from a given point on<br>
+ * a timeline syncobj from/into a binary syncobj, you can
use the point 0 to<br>
+ * mean take/replace the fence in the syncobj.<br>
*/<br>
<br>
#include <linux/anon_inodes.h><br>
-- <br>
2.23.0<br>
<br>
</blockquote>
</div>
</div>
</blockquote>
<p><br>
</p>
</div>
</blockquote></div></div>
</div>