Re: [PATCH libdrm 4/5] wrap syncobj timeline query/wait APIs for amdgpu v3

[Christian König]

Am 30.11.18 um 08:35 schrieb zhoucm1:


On 2018年11月28日 22:50, Christian König wrote:
From: Chunming Zhou <david1.z...@amd.com>

v2: symbos are stored in lexical order.
v3: drop export/import and extra query indirection

Signed-off-by: Chunming Zhou <david1.z...@amd.com>
Signed-off-by: Christian König <christian.koe...@amd.com>
---
  amdgpu/amdgpu-symbol-check |  2 ++
  amdgpu/amdgpu.h            | 39 
+++++++++++++++++++++++++++++++++++++++
  amdgpu/amdgpu_cs.c         | 23 +++++++++++++++++++++++
  3 files changed, 64 insertions(+)

diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check
index 6f5e0f95..4553736f 100755
--- a/amdgpu/amdgpu-symbol-check
+++ b/amdgpu/amdgpu-symbol-check
@@ -49,8 +49,10 @@ amdgpu_cs_submit
  amdgpu_cs_submit_raw
  amdgpu_cs_syncobj_export_sync_file
  amdgpu_cs_syncobj_import_sync_file
+amdgpu_cs_syncobj_query
  amdgpu_cs_syncobj_reset
  amdgpu_cs_syncobj_signal
+amdgpu_cs_syncobj_timeline_wait
  amdgpu_cs_syncobj_wait
  amdgpu_cs_wait_fences
  amdgpu_cs_wait_semaphore
diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
index dc51659a..330658a0 100644
--- a/amdgpu/amdgpu.h
+++ b/amdgpu/amdgpu.h
@@ -1489,6 +1489,45 @@ int 
amdgpu_cs_syncobj_wait(amdgpu_device_handle dev,
                 int64_t timeout_nsec, unsigned flags,
                 uint32_t *first_signaled);
  +/**
+ *  Wait for one or all sync objects on their points to signal.
+ *
+ * \param   dev        - \c [in] self-explanatory
+ * \param   handles - \c [in] array of sync object handles
+ * \param   points - \c [in] array of sync points to wait
+ * \param   num_handles - \c [in] self-explanatory
+ * \param   timeout_nsec - \c [in] self-explanatory
+ * \param   flags   - \c [in] a bitmask of DRM_SYNCOBJ_WAIT_FLAGS_*
+ * \param   first_signaled - \c [in] self-explanatory
+ *
+ * \return   0 on success\n
+ *          -ETIME - Timeout
+ *          <0 - Negative POSIX Error code
+ *
+ */
+int amdgpu_cs_syncobj_timeline_wait(amdgpu_device_handle dev,
+                    uint32_t *handles, uint64_t *points,
+                    unsigned num_handles,
+                    int64_t timeout_nsec, unsigned flags,
+                    uint32_t *first_signaled);
+/**
+ *  Query sync objects payloads.
+ *
+ * \param   dev        - \c [in] self-explanatory
+ * \param   handles - \c [in] array of sync object handles
+ * \param   points - \c [out] array of sync points returned, which 
presents
+ * syncobj payload.
+ * \param   num_handles - \c [in] self-explanatory
+ *
+ * \return   0 on success\n
+ *          -ETIME - Timeout
+ *          <0 - Negative POSIX Error code
+ *
+ */
+int amdgpu_cs_syncobj_query(amdgpu_device_handle dev,
+                uint32_t *handles, uint64_t *points,
+                unsigned num_handles);
+
  /**
   *  Export kernel sync object to shareable fd.
   *
diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
index 3b8231aa..e4a547c6 100644
--- a/amdgpu/amdgpu_cs.c
+++ b/amdgpu/amdgpu_cs.c
@@ -661,6 +661,29 @@ drm_public int 
amdgpu_cs_syncobj_wait(amdgpu_device_handle dev,
                    flags, first_signaled);
  }
  +drm_public int 
amdgpu_cs_syncobj_timeline_wait(amdgpu_device_handle dev,
+                           uint32_t *handles, uint64_t *points,
+                           unsigned num_handles,
+                           int64_t timeout_nsec, unsigned flags,
+                           uint32_t *first_signaled)
+{
+    if (NULL == dev)
+        return -EINVAL;
+
+    return drmSyncobjTimelineWait(dev->fd, handles, points, 
num_handles,
+                      timeout_nsec, flags, first_signaled);
+}
+
+drm_public int amdgpu_cs_syncobj_query(amdgpu_device_handle dev,
+                       uint32_t *handles, uint64_t *points,
This interfaces is public to umd, I think they like "uint64_t 
**points" for batch query, I've verified before, it works well and 
more convenience.
If removing num_handles, that means only one syncobj to query, I agree 
with "uint64_t *point".

"handles" as well as "points" are an array of objects. If the UMD wants 
to write the points to separate locations it can do so manually after 
calling the function.

It doesn't make any sense that libdrm or the kernel does the extra 
indirection, the transferred pointers are 64bit as well (even on a 32bit 
system) so the overhead is identical.

Adding another indirection just makes the implementation unnecessary 
complex.

Christian.


-David
+                       unsigned num_handles)
+{
+    if (NULL == dev)
+        return -EINVAL;
+
+    return drmSyncobjQuery(dev->fd, handles, points, num_handles);
+}
+
  drm_public int amdgpu_cs_export_syncobj(amdgpu_device_handle dev,
                      uint32_t handle,
                      int *shared_fd)


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