Mesa (main): v3dv: Always wait on last_job_syncs if job->serialize

[GitLab Mirror]

Module: Mesa
Branch: main
Commit: 321f0b85f2e5d539144aa3dd91d80289cce3c2a0
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=321f0b85f2e5d539144aa3dd91d80289cce3c2a0

Author: Jason Ekstrand <jason.ekstrand at collabora.com>
Date:   Thu Apr  7 10:17:26 2022 -0500

v3dv: Always wait on last_job_syncs if job->serialize

Even if we're the first job on some queue, there may be no wait
semaphores but we still need to ensure things happen in-order.  (See
the "Implicit Synchronization Guarantees" section of the Vulkan spec.)
The client can submit back-to-back command buffers with no semaphores
between them and it needs to adt the same as if there were a semaphore.
If job->serialize is set because of a barrier or something, we still
need to synchronize across HW queues by waiting on last_job_syncs.

Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15704>

---

 src/broadcom/vulkan/v3dv_queue.c | 53 ++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/src/broadcom/vulkan/v3dv_queue.c b/src/broadcom/vulkan/v3dv_queue.c
index 51d2ae8cf56..d5501371388 100644
--- a/src/broadcom/vulkan/v3dv_queue.c
+++ b/src/broadcom/vulkan/v3dv_queue.c
@@ -744,11 +744,10 @@ set_in_syncs(struct v3dv_device *device,
    if (device->last_job_syncs.first[queue])
       n_sems = sems_info->wait_sem_count;
 
-   /* If we don't need to wait on wait semaphores but the serialize flag is
-    * set, this job waits for completion of all GPU jobs submitted in any
-    * queue V3DV_QUEUE_(CL/TFU/CSD) before running.
+   /* If the serialize flag is set, this job waits for completion of all GPU
+    * jobs submitted in any queue V3DV_QUEUE_(CL/TFU/CSD) before running.
     */
-   *count = n_sems == 0 && job->serialize ? 3 : n_sems;
+   *count = n_sems + (job->serialize ? 3 : 0);
 
    if (!*count)
       return NULL;
@@ -760,30 +759,30 @@ set_in_syncs(struct v3dv_device *device,
    if (!syncs)
       return NULL;
 
-   if (n_sems) {
-      for (int i = 0; i < *count; i++) {
-         struct v3dv_semaphore *sem =
-            v3dv_semaphore_from_handle(sems_info->wait_sems[i]);
-         syncs[i].handle = semaphore_get_sync(sem);
+   for (int i = 0; i < n_sems; i++) {
+      struct v3dv_semaphore *sem =
+         v3dv_semaphore_from_handle(sems_info->wait_sems[i]);
+      syncs[i].handle = semaphore_get_sync(sem);
 
-         /* From the Vulkan 1.0 spec:
-          *
-          *    "If the import is temporary, the implementation must restore
-          *     the semaphore to its prior permanent state after submitting
-          *     the next semaphore wait operation."
-          *
-          * We can't destroy the temporary sync until the kernel is done
-          * with it, this is why we need to have this 'has_temp' flag instead
-          * of checking temp_sync for 0 to know if we have a temporary
-          * payload. The temporary sync will be destroyed if we import into
-          * the semaphore again or if the semaphore is destroyed by the
-          * client.
-          */
-         sem->has_temp = false;
-      }
-   } else {
-      for (int i = 0; i < *count; i++)
-         syncs[i].handle = device->last_job_syncs.syncs[i];
+      /* From the Vulkan 1.0 spec:
+       *
+       *    "If the import is temporary, the implementation must restore
+       *     the semaphore to its prior permanent state after submitting
+       *     the next semaphore wait operation."
+       *
+       * We can't destroy the temporary sync until the kernel is done
+       * with it, this is why we need to have this 'has_temp' flag instead
+       * of checking temp_sync for 0 to know if we have a temporary
+       * payload. The temporary sync will be destroyed if we import into
+       * the semaphore again or if the semaphore is destroyed by the
+       * client.
+       */
+      sem->has_temp = false;
+   }
+
+   if (job->serialize) {
+      for (int i = 0; i < 3; i++)
+         syncs[n_sems + i].handle = device->last_job_syncs.syncs[i];
    }
 
    return syncs;