Mesa (staging/22.1): mesa: fix SignalSemaphoreEXT behavior

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Tue Jul 12 17:20:15 UTC 2022


Module: Mesa
Branch: staging/22.1
Commit: 1678100c3c64724ac2462739c10fe92976f8a40b
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=1678100c3c64724ac2462739c10fe92976f8a40b

Author: Mike Blumenkrantz <michael.blumenkrantz at gmail.com>
Date:   Wed Jul  6 12:21:34 2022 -0400

mesa: fix SignalSemaphoreEXT behavior

the EXT_external_object spec originally was underspecified with regards
to this function, leaving room for synchronization errors where:
* app calls SignalSemaphoreEXT to signal a semaphore
* mesa defers pipe_context::fence_server_signal with threaded context
* driver defers gpu submission
* SignalSemaphoreEXT has long since returned, app submits vk cmdbuf waiting on semaphore
* spec violation / device lost

to prevent this, the spec is being changed to:
1) require an implicit flush when calling SignalSemaphoreEXT
2) require that this implicit flush also forces GPU submission before SignalSemaphoreEXT returns

all affected drivers have been updated

fixes #6568

cc: mesa-stable

Reviewed-by: Marek Olšák <marek.olsak at amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17376>
(cherry picked from commit 21b3a234048a270e7999f8e70e25091c599dd3eb)

---

 .pick_status.json                                     |  2 +-
 src/gallium/auxiliary/util/u_threaded_context.c       | 19 +++----------------
 src/gallium/auxiliary/util/u_threaded_context_calls.h |  1 -
 src/gallium/drivers/crocus/crocus_fence.c             |  2 ++
 src/gallium/drivers/iris/iris_fence.c                 |  2 ++
 src/gallium/drivers/radeonsi/si_fence.c               |  2 +-
 src/gallium/drivers/zink/zink_fence.c                 |  7 +++++--
 src/mesa/main/externalobjects.c                       |  2 +-
 8 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/.pick_status.json b/.pick_status.json
index f3fa8795e32..e373ce2fe45 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -535,7 +535,7 @@
         "description": "mesa: fix SignalSemaphoreEXT behavior",
         "nominated": true,
         "nomination_type": 0,
-        "resolution": 0,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": null
     },
diff --git a/src/gallium/auxiliary/util/u_threaded_context.c b/src/gallium/auxiliary/util/u_threaded_context.c
index b12140923a1..73b3e864b29 100644
--- a/src/gallium/auxiliary/util/u_threaded_context.c
+++ b/src/gallium/auxiliary/util/u_threaded_context.c
@@ -2835,27 +2835,14 @@ tc_fence_server_sync(struct pipe_context *_pipe,
    screen->fence_reference(screen, &call->fence, fence);
 }
 
-static uint16_t
-tc_call_fence_server_signal(struct pipe_context *pipe, void *call, uint64_t *last)
-{
-   struct pipe_fence_handle *fence = to_call(call, tc_fence_call)->fence;
-
-   pipe->fence_server_signal(pipe, fence);
-   pipe->screen->fence_reference(pipe->screen, &fence, NULL);
-   return call_size(tc_fence_call);
-}
-
 static void
 tc_fence_server_signal(struct pipe_context *_pipe,
                            struct pipe_fence_handle *fence)
 {
    struct threaded_context *tc = threaded_context(_pipe);
-   struct pipe_screen *screen = tc->pipe->screen;
-   struct tc_fence_call *call = tc_add_call(tc, TC_CALL_fence_server_signal,
-                                            tc_fence_call);
-
-   call->fence = NULL;
-   screen->fence_reference(screen, &call->fence, fence);
+   struct pipe_context *pipe = tc->pipe;
+   tc_sync(tc);
+   pipe->fence_server_signal(pipe, fence);
 }
 
 static struct pipe_video_codec *
diff --git a/src/gallium/auxiliary/util/u_threaded_context_calls.h b/src/gallium/auxiliary/util/u_threaded_context_calls.h
index ab78d3de3ae..2dbdd885a59 100644
--- a/src/gallium/auxiliary/util/u_threaded_context_calls.h
+++ b/src/gallium/auxiliary/util/u_threaded_context_calls.h
@@ -1,7 +1,6 @@
 CALL(flush)
 CALL(callback)
 CALL(fence_server_sync)
-CALL(fence_server_signal)
 CALL(destroy_query)
 CALL(begin_query)
 CALL(end_query)
diff --git a/src/gallium/drivers/crocus/crocus_fence.c b/src/gallium/drivers/crocus/crocus_fence.c
index 2e3ad4ba6c6..7d7617c08c8 100644
--- a/src/gallium/drivers/crocus/crocus_fence.c
+++ b/src/gallium/drivers/crocus/crocus_fence.c
@@ -551,6 +551,8 @@ crocus_fence_signal(struct pipe_context *ctx, struct pipe_fence_handle *fence)
          crocus_batch_add_syncobj(&ice->batches[b], fine->syncobj,
                                   I915_EXEC_FENCE_SIGNAL);
       }
+      if (ice->batches[b].contains_fence_signal)
+         crocus_batch_flush(&ice->batches[b]);
    }
 }
 
diff --git a/src/gallium/drivers/iris/iris_fence.c b/src/gallium/drivers/iris/iris_fence.c
index 935232407f7..c6e13663eb2 100644
--- a/src/gallium/drivers/iris/iris_fence.c
+++ b/src/gallium/drivers/iris/iris_fence.c
@@ -604,6 +604,8 @@ iris_fence_signal(struct pipe_context *ctx,
          batch->contains_fence_signal = true;
          iris_batch_add_syncobj(batch, fine->syncobj, I915_EXEC_FENCE_SIGNAL);
       }
+      if (batch->contains_fence_signal)
+         iris_batch_flush(batch);
    }
 }
 
diff --git a/src/gallium/drivers/radeonsi/si_fence.c b/src/gallium/drivers/radeonsi/si_fence.c
index e6e4dca082c..e6d6dc43c97 100644
--- a/src/gallium/drivers/radeonsi/si_fence.c
+++ b/src/gallium/drivers/radeonsi/si_fence.c
@@ -560,7 +560,7 @@ static void si_fence_server_signal(struct pipe_context *ctx, struct pipe_fence_h
       si_add_syncobj_signal(sctx, sfence->gfx);
 
    /**
-    * The spec does not require a flush here. We insert a flush
+    * The spec requires a flush here. We insert a flush
     * because syncobj based signals are not directly placed into
     * the command stream. Instead the signal happens when the
     * submission associated with the syncobj finishes execution.
diff --git a/src/gallium/drivers/zink/zink_fence.c b/src/gallium/drivers/zink/zink_fence.c
index a47398af865..3ba2bd1096c 100644
--- a/src/gallium/drivers/zink/zink_fence.c
+++ b/src/gallium/drivers/zink/zink_fence.c
@@ -204,10 +204,13 @@ zink_fence_server_signal(struct pipe_context *pctx, struct pipe_fence_handle *pf
    struct zink_tc_fence *mfence = (struct zink_tc_fence *)pfence;
 
    assert(!ctx->batch.state->signal_semaphore);
-   /* this is a deferred flush to reduce overhead */
    ctx->batch.state->signal_semaphore = mfence->sem;
    ctx->batch.has_work = true;
-   pctx->flush(pctx, NULL, PIPE_FLUSH_ASYNC);
+   struct zink_batch_state *bs = ctx->batch.state;
+   /* this must produce a synchronous flush that completes before the function returns */
+   pctx->flush(pctx, NULL, 0);
+   if (zink_screen(ctx->base.screen)->threaded)
+      util_queue_fence_wait(&bs->flush_completed);
 }
 
 void
diff --git a/src/mesa/main/externalobjects.c b/src/mesa/main/externalobjects.c
index 0462df4b757..002b77d66ae 100644
--- a/src/mesa/main/externalobjects.c
+++ b/src/mesa/main/externalobjects.c
@@ -696,7 +696,7 @@ server_signal_semaphore(struct gl_context *ctx,
          pipe->flush_resource(pipe, texObj->pt);
    }
 
-   /* The driver is allowed to flush during fence_server_signal, be prepared */
+   /* The driver must flush during fence_server_signal, be prepared */
    st_flush_bitmap_cache(st);
    pipe->fence_server_signal(pipe, semObj->fence);
 }



More information about the mesa-commit mailing list