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

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

[Lionel Landwerlin]

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@intel.com>
Cc: Christian Koenig <Christian.Koenig@amd.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: David(ChunMing) Zhou <David1.Zhou@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,
+ * 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).
+ *
  *
  * 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

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

[Jason Ekstrand]

[-- Attachment #1.1: Type: text/plain, Size: 7608 bytes --]

On Thu, Aug 22, 2019 at 9:55 AM Lionel Landwerlin <
lionel.g.landwerlin@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@intel.com>
> Cc: Christian Koenig <Christian.Koenig@amd.com>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: David(ChunMing) Zhou <David1.Zhou@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....


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

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

[-- Attachment #1.2: Type: text/html, Size: 9059 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Lionel Landwerlin]

[-- Attachment #1.1: Type: text/plain, Size: 9431 bytes --]

On 22/08/2019 21:24, Jason Ekstrand wrote:
> On Thu, Aug 22, 2019 at 9:55 AM Lionel Landwerlin 
> <lionel.g.landwerlin@intel.com <mailto:lionel.g.landwerlin@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@intel.com
>     <mailto:lionel.g.landwerlin@intel.com>>
>     Cc: Christian Koenig <Christian.Koenig@amd.com
>     <mailto:Christian.Koenig@amd.com>>
>     Cc: Jason Ekstrand <jason@jlekstrand.net
>     <mailto:jason@jlekstrand.net>>
>     Cc: David(ChunMing) Zhou <David1.Zhou@amd.com
>     <mailto:David1.Zhou@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
>


[-- Attachment #1.2: Type: text/html, Size: 13731 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Jason Ekstrand]

[-- Attachment #1.1: Type: text/plain, Size: 9210 bytes --]

On Thu, Aug 22, 2019 at 5:28 PM Lionel Landwerlin <
lionel.g.landwerlin@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@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@intel.com>
>> Cc: Christian Koenig <Christian.Koenig@amd.com>
>> Cc: Jason Ekstrand <jason@jlekstrand.net>
>> Cc: David(ChunMing) Zhou <David1.Zhou@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
>>
>>
>

[-- Attachment #1.2: Type: text/html, Size: 15162 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Fri, Aug 23, 2019 at 8:53 PM Jason Ekstrand <jason@jlekstrand.net> wrote:
>
>
> On Thu, Aug 22, 2019 at 5:28 PM Lionel Landwerlin <lionel.g.landwerlin@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@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@intel.com>
>>> Cc: Christian Koenig <Christian.Koenig@amd.com>
>>> Cc: Jason Ekstrand <jason@jlekstrand.net>
>>> Cc: David(ChunMing) Zhou <David1.Zhou@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 ...

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@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Lionel Landwerlin]

On 26/08/2019 00:01, Daniel Vetter wrote:
> On Fri, Aug 23, 2019 at 8:53 PM Jason Ekstrand <jason@jlekstrand.net> wrote:
>>
>> On Thu, Aug 22, 2019 at 5:28 PM Lionel Landwerlin <lionel.g.landwerlin@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@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@intel.com>
>>>> Cc: Christian Koenig <Christian.Koenig@amd.com>
>>>> Cc: Jason Ekstrand <jason@jlekstrand.net>
>>>> Cc: David(ChunMing) Zhou <David1.Zhou@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).


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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

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@jlekstrand.net> wrote:
> > > 
> > > On Thu, Aug 22, 2019 at 5:28 PM Lionel Landwerlin <lionel.g.landwerlin@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@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@intel.com>
> > > > > Cc: Christian Koenig <Christian.Koenig@amd.com>
> > > > > Cc: Jason Ekstrand <jason@jlekstrand.net>
> > > > > Cc: David(ChunMing) Zhou <David1.Zhou@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@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Lionel Landwerlin]

On 27/08/2019 19:27, Daniel Vetter wrote:
> 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@jlekstrand.net> wrote:
>>>> On Thu, Aug 22, 2019 at 5:28 PM Lionel Landwerlin <lionel.g.landwerlin@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@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@intel.com>
>>>>>> Cc: Christian Koenig <Christian.Koenig@amd.com>
>>>>>> Cc: Jason Ekstrand <jason@jlekstrand.net>
>>>>>> Cc: David(ChunMing) Zhou <David1.Zhou@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


Thanks, there is a note a bit further down in this patch.

It was worded along the lines of with a semaphore within single app, but 
it applies to shared semaphores too.


-Lionel


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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel