Mesa (master): freedreno: fence_server_sync() fixes

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Thu Sep 3 00:20:01 UTC 2020


Module: Mesa
Branch: master
Commit: bf23ff83e69a76424ea083c6641b952868e4b87c
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=bf23ff83e69a76424ea083c6641b952868e4b87c

Author: Rob Clark <robdclark at chromium.org>
Date:   Tue Sep  1 15:24:38 2020 -0700

freedreno: fence_server_sync() fixes

Two potential problems, batch re-ordering doesn't really play nicely
with fence_server_sync(), so when we switch away from one batch, detect
the case that we need to sync, and if so flush.  The alternative of
trying to track that later batches depend on an earlier batch that had
an in-fence is hairy, and the normal use-case would be to sync at the
beginning of the frame.

But this brings up the second problem, which is that typically we'll get
told to sync on an in-fence before the first draw, which means before
mesa/st flushes down the framebuffer state to the driver.  Which means
we don't yet have the correct batch to attach the fence to.  So we need
to track the in-fence on the context, and transfer it to the batch
before draws, etc.

Signed-off-by: Rob Clark <robdclark at chromium.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6575>

---

 .../drivers/freedreno/freedreno_batch_cache.c      | 10 +++++
 src/gallium/drivers/freedreno/freedreno_context.c  |  5 +++
 src/gallium/drivers/freedreno/freedreno_context.h  | 46 ++++++++++++++++++++++
 src/gallium/drivers/freedreno/freedreno_fence.c    |  3 +-
 src/gallium/drivers/freedreno/freedreno_state.c    |  2 +
 5 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/freedreno/freedreno_batch_cache.c b/src/gallium/drivers/freedreno/freedreno_batch_cache.c
index 448e701bbc6..c407cc5e33d 100644
--- a/src/gallium/drivers/freedreno/freedreno_batch_cache.c
+++ b/src/gallium/drivers/freedreno/freedreno_batch_cache.c
@@ -317,6 +317,13 @@ fd_bc_alloc_batch(struct fd_batch_cache *cache, struct fd_context *ctx, bool non
 	struct fd_batch *batch;
 	uint32_t idx;
 
+	/* For normal draw batches, pctx->set_framebuffer_state() handles
+	 * this, but for nondraw batches, this is a nice central location
+	 * to handle them all.
+	 */
+	if (nondraw)
+		fd_context_switch_from(ctx);
+
 	fd_screen_lock(ctx->screen);
 
 	while ((idx = ffs(~cache->batch_mask)) == 0) {
@@ -382,6 +389,9 @@ fd_bc_alloc_batch(struct fd_batch_cache *cache, struct fd_context *ctx, bool non
 	debug_assert(cache->batches[idx] == NULL);
 	cache->batches[idx] = batch;
 
+	if (nondraw)
+		fd_context_switch_to(ctx, batch);
+
 out:
 	fd_screen_unlock(ctx->screen);
 
diff --git a/src/gallium/drivers/freedreno/freedreno_context.c b/src/gallium/drivers/freedreno/freedreno_context.c
index e783a8ac715..8cbcd1cb52f 100644
--- a/src/gallium/drivers/freedreno/freedreno_context.c
+++ b/src/gallium/drivers/freedreno/freedreno_context.c
@@ -220,6 +220,9 @@ fd_context_destroy(struct pipe_context *pctx)
 
 	fd_fence_ref(&ctx->last_fence, NULL);
 
+	if (ctx->in_fence_fd != -1)
+		close(ctx->in_fence_fd);
+
 	util_copy_framebuffer_state(&ctx->framebuffer, NULL);
 	fd_batch_reference(&ctx->batch, NULL);  /* unref current batch */
 	fd_bc_invalidate_context(ctx);
@@ -400,6 +403,8 @@ fd_context_init(struct fd_context *ctx, struct pipe_screen *pscreen,
 	ctx->screen = screen;
 	ctx->pipe = fd_pipe_new2(screen->dev, FD_PIPE_3D, prio);
 
+	ctx->in_fence_fd = -1;
+
 	if (fd_device_version(screen->dev) >= FD_VERSION_ROBUSTNESS) {
 		ctx->context_reset_count = fd_get_reset_count(ctx, true);
 		ctx->global_reset_count = fd_get_reset_count(ctx, false);
diff --git a/src/gallium/drivers/freedreno/freedreno_context.h b/src/gallium/drivers/freedreno/freedreno_context.h
index e3ee25db022..0f1e8992c9c 100644
--- a/src/gallium/drivers/freedreno/freedreno_context.h
+++ b/src/gallium/drivers/freedreno/freedreno_context.h
@@ -27,6 +27,8 @@
 #ifndef FREEDRENO_CONTEXT_H_
 #define FREEDRENO_CONTEXT_H_
 
+#include <libsync.h>
+
 #include "pipe/p_context.h"
 #include "indices/u_primconvert.h"
 #include "util/u_blitter.h"
@@ -259,6 +261,21 @@ struct fd_context {
 	 */
 	struct pipe_fence_handle *last_fence;
 
+	/* Fence fd we are told to wait on via ->fence_server_sync() (or -1
+	 * if none).  The in-fence is transferred over to the batch on the
+	 * next draw/blit/grid.
+	 *
+	 * The reason for this extra complexity is that apps will typically
+	 * do eglWaitSyncKHR()/etc at the beginning of the frame, before the
+	 * first draw.  But mesa/st doesn't flush down framebuffer state
+	 * change until we hit a draw, so at ->fence_server_sync() time, we
+	 * don't yet have the correct batch.  If we created a batch at that
+	 * point, it would be the wrong one, and we'd have to flush it pre-
+	 * maturely, causing us to stall early in the frame where we could
+	 * be building up cmdstream.
+	 */
+	int in_fence_fd;
+
 	/* track last known reset status globally and per-context to
 	 * determine if more resets occurred since then.  If global reset
 	 * count increases, it means some other context crashed.  If
@@ -478,6 +495,34 @@ fd_supported_prim(struct fd_context *ctx, unsigned prim)
 	return (1 << prim) & ctx->primtype_mask;
 }
 
+/**
+ * If we have a pending fence_server_sync() (GPU side sync), flush now.
+ * The alternative to try to track this with batch dependencies gets
+ * hairy quickly.
+ *
+ * Call this before switching to a different batch, to handle this case.
+ */
+static inline void
+fd_context_switch_from(struct fd_context *ctx)
+{
+	if (ctx->batch && (ctx->batch->in_fence_fd != -1))
+		fd_batch_flush(ctx->batch);
+}
+
+/**
+ * If there is a pending fence-fd that we need to sync on, this will
+ * transfer the reference to the next batch we are going to render
+ * to.
+ */
+static inline void
+fd_context_switch_to(struct fd_context *ctx, struct fd_batch *batch)
+{
+	if (ctx->in_fence_fd != -1) {
+		sync_accumulate("freedreno", &batch->in_fence_fd, ctx->in_fence_fd);
+		ctx->in_fence_fd = -1;
+	}
+}
+
 static inline struct fd_batch *
 fd_context_batch(struct fd_context *ctx)
 {
@@ -488,6 +533,7 @@ fd_context_batch(struct fd_context *ctx)
 		ctx->batch = batch;
 		fd_context_all_dirty(ctx);
 	}
+	fd_context_switch_to(ctx, ctx->batch);
 	return ctx->batch;
 }
 
diff --git a/src/gallium/drivers/freedreno/freedreno_fence.c b/src/gallium/drivers/freedreno/freedreno_fence.c
index 29ee6627e2d..0cce0867779 100644
--- a/src/gallium/drivers/freedreno/freedreno_fence.c
+++ b/src/gallium/drivers/freedreno/freedreno_fence.c
@@ -155,7 +155,6 @@ void fd_fence_server_sync(struct pipe_context *pctx,
 		struct pipe_fence_handle *fence)
 {
 	struct fd_context *ctx = fd_context(pctx);
-	struct fd_batch *batch = fd_context_batch(ctx);
 
 	fence_flush(fence);
 
@@ -163,7 +162,7 @@ void fd_fence_server_sync(struct pipe_context *pctx,
 	if (fence->fence_fd == -1)
 		return;
 
-	if (sync_accumulate("freedreno", &batch->in_fence_fd, fence->fence_fd)) {
+	if (sync_accumulate("freedreno", &ctx->in_fence_fd, fence->fence_fd)) {
 		/* error */
 	}
 }
diff --git a/src/gallium/drivers/freedreno/freedreno_state.c b/src/gallium/drivers/freedreno/freedreno_state.c
index 1e0f6f40b18..9e1c1a1776c 100644
--- a/src/gallium/drivers/freedreno/freedreno_state.c
+++ b/src/gallium/drivers/freedreno/freedreno_state.c
@@ -220,6 +220,8 @@ fd_set_framebuffer_state(struct pipe_context *pctx,
 		framebuffer->width, framebuffer->height,
 		framebuffer->layers, framebuffer->samples);
 
+	fd_context_switch_from(ctx);
+
 	cso = &ctx->framebuffer;
 
 	if (util_framebuffer_state_equal(cso, framebuffer))



More information about the mesa-commit mailing list