From 3e424bcdfcef19682f9b651f7c1a04e32f18be5c Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Mon, 11 May 2020 13:28:58 -0700 Subject: [PATCH] freedreno: Split the fd_batch_resource_used by read vs write. This is for an optimization I plan in a following commit. I found I had to add likely()s to avoid a perf regression from branch prediction. On the drawoverhead 8 UBOs test, the HW can't quite keep up with the CPU, but if I set nohw then this change is 1.32023% +/- 0.373053% (n=10). Part-of: --- .../drivers/freedreno/a6xx/fd6_blitter.c | 4 +- .../drivers/freedreno/freedreno_batch.c | 104 +++++++++++------- .../drivers/freedreno/freedreno_batch.h | 3 +- .../drivers/freedreno/freedreno_draw.c | 4 +- .../drivers/freedreno/freedreno_query_acc.c | 2 +- 5 files changed, 70 insertions(+), 47 deletions(-) diff --git a/src/gallium/drivers/freedreno/a6xx/fd6_blitter.c b/src/gallium/drivers/freedreno/a6xx/fd6_blitter.c index 82b0aff3ae9..9b5231d55e3 100644 --- a/src/gallium/drivers/freedreno/a6xx/fd6_blitter.c +++ b/src/gallium/drivers/freedreno/a6xx/fd6_blitter.c @@ -643,8 +643,8 @@ handle_rgba_blit(struct fd_context *ctx, const struct pipe_blit_info *info) fd_screen_lock(ctx->screen); - fd_batch_resource_used(batch, fd_resource(info->src.resource), false); - fd_batch_resource_used(batch, fd_resource(info->dst.resource), true); + fd_batch_resource_read(batch, fd_resource(info->src.resource)); + fd_batch_resource_write(batch, fd_resource(info->dst.resource)); fd_screen_unlock(ctx->screen); diff --git a/src/gallium/drivers/freedreno/freedreno_batch.c b/src/gallium/drivers/freedreno/freedreno_batch.c index fcb11546b81..a653e0d91a8 100644 --- a/src/gallium/drivers/freedreno/freedreno_batch.c +++ b/src/gallium/drivers/freedreno/freedreno_batch.c @@ -392,65 +392,87 @@ flush_write_batch(struct fd_resource *rsc) fd_batch_reference_locked(&b, NULL); } +static bool +fd_batch_references_resource(struct fd_batch *batch, struct fd_resource *rsc) +{ + return rsc->batch_mask & (1 << batch->idx); +} + +static void +fd_batch_add_resource(struct fd_batch *batch, struct fd_resource *rsc) +{ + + if (likely(fd_batch_references_resource(batch, rsc))) { + debug_assert(_mesa_set_search(batch->resources, rsc)); + return; + } + + debug_assert(!_mesa_set_search(batch->resources, rsc)); + + _mesa_set_add(batch->resources, rsc); + rsc->batch_mask |= (1 << batch->idx); +} + void -fd_batch_resource_used(struct fd_batch *batch, struct fd_resource *rsc, bool write) +fd_batch_resource_write(struct fd_batch *batch, struct fd_resource *rsc) { fd_screen_assert_locked(batch->ctx->screen); if (rsc->stencil) - fd_batch_resource_used(batch, rsc->stencil, write); + fd_batch_resource_write(batch, rsc->stencil); - DBG("%p: %s %p", batch, write ? "write" : "read", rsc); + DBG("%p: write %p", batch, rsc); - if (write) - rsc->valid = true; + rsc->valid = true; /* note, invalidate write batch, to avoid further writes to rsc * resulting in a write-after-read hazard. */ + /* if we are pending read or write by any other batch: */ + if (unlikely(rsc->batch_mask & ~(1 << batch->idx))) { + struct fd_batch_cache *cache = &batch->ctx->screen->batch_cache; + struct fd_batch *dep; - if (write) { - /* if we are pending read or write by any other batch: */ - if (rsc->batch_mask & ~(1 << batch->idx)) { - struct fd_batch_cache *cache = &batch->ctx->screen->batch_cache; - struct fd_batch *dep; - - if (rsc->write_batch && rsc->write_batch != batch) - flush_write_batch(rsc); - - foreach_batch(dep, cache, rsc->batch_mask) { - struct fd_batch *b = NULL; - if (dep == batch) - continue; - /* note that batch_add_dep could flush and unref dep, so - * we need to hold a reference to keep it live for the - * fd_bc_invalidate_batch() - */ - fd_batch_reference(&b, dep); - fd_batch_add_dep(batch, b); - fd_bc_invalidate_batch(b, false); - fd_batch_reference_locked(&b, NULL); - } - } - fd_batch_reference_locked(&rsc->write_batch, batch); - } else { - /* If reading a resource pending a write, go ahead and flush the - * writer. This avoids situations where we end up having to - * flush the current batch in _resource_used() - */ if (rsc->write_batch && rsc->write_batch != batch) flush_write_batch(rsc); - } - if (rsc->batch_mask & (1 << batch->idx)) { - debug_assert(_mesa_set_search(batch->resources, rsc)); - return; + foreach_batch(dep, cache, rsc->batch_mask) { + struct fd_batch *b = NULL; + if (dep == batch) + continue; + /* note that batch_add_dep could flush and unref dep, so + * we need to hold a reference to keep it live for the + * fd_bc_invalidate_batch() + */ + fd_batch_reference(&b, dep); + fd_batch_add_dep(batch, b); + fd_bc_invalidate_batch(b, false); + fd_batch_reference_locked(&b, NULL); + } } + fd_batch_reference_locked(&rsc->write_batch, batch); - debug_assert(!_mesa_set_search(batch->resources, rsc)); + fd_batch_add_resource(batch, rsc); +} - _mesa_set_add(batch->resources, rsc); - rsc->batch_mask |= (1 << batch->idx); +void +fd_batch_resource_read(struct fd_batch *batch, struct fd_resource *rsc) +{ + fd_screen_assert_locked(batch->ctx->screen); + + if (rsc->stencil) + fd_batch_resource_read(batch, rsc->stencil); + + DBG("%p: read %p", batch, rsc); + + /* If reading a resource pending a write, go ahead and flush the + * writer. This avoids situations where we end up having to + * flush the current batch in _resource_used() + */ + if (unlikely(rsc->write_batch && rsc->write_batch != batch)) + flush_write_batch(rsc); + + fd_batch_add_resource(batch, rsc); } void diff --git a/src/gallium/drivers/freedreno/freedreno_batch.h b/src/gallium/drivers/freedreno/freedreno_batch.h index 479d78d5eca..8ec3700b7ad 100644 --- a/src/gallium/drivers/freedreno/freedreno_batch.h +++ b/src/gallium/drivers/freedreno/freedreno_batch.h @@ -256,7 +256,8 @@ struct fd_batch * fd_batch_create(struct fd_context *ctx, bool nondraw); void fd_batch_reset(struct fd_batch *batch); void fd_batch_flush(struct fd_batch *batch); void fd_batch_add_dep(struct fd_batch *batch, struct fd_batch *dep); -void fd_batch_resource_used(struct fd_batch *batch, struct fd_resource *rsc, bool write); +void fd_batch_resource_write(struct fd_batch *batch, struct fd_resource *rsc); +void fd_batch_resource_read(struct fd_batch *batch, struct fd_resource *rsc); void fd_batch_check_size(struct fd_batch *batch); /* not called directly: */ diff --git a/src/gallium/drivers/freedreno/freedreno_draw.c b/src/gallium/drivers/freedreno/freedreno_draw.c index b36e89dcb01..d3d68a14131 100644 --- a/src/gallium/drivers/freedreno/freedreno_draw.c +++ b/src/gallium/drivers/freedreno/freedreno_draw.c @@ -47,7 +47,7 @@ resource_read(struct fd_batch *batch, struct pipe_resource *prsc) { if (!prsc) return; - fd_batch_resource_used(batch, fd_resource(prsc), false); + fd_batch_resource_read(batch, fd_resource(prsc)); } static void @@ -55,7 +55,7 @@ resource_written(struct fd_batch *batch, struct pipe_resource *prsc) { if (!prsc) return; - fd_batch_resource_used(batch, fd_resource(prsc), true); + fd_batch_resource_write(batch, fd_resource(prsc)); } static void diff --git a/src/gallium/drivers/freedreno/freedreno_query_acc.c b/src/gallium/drivers/freedreno/freedreno_query_acc.c index 89312ed14c1..888f7763d85 100644 --- a/src/gallium/drivers/freedreno/freedreno_query_acc.c +++ b/src/gallium/drivers/freedreno/freedreno_query_acc.c @@ -88,7 +88,7 @@ fd_acc_query_resume(struct fd_acc_query *aq, struct fd_batch *batch) p->resume(aq, aq->batch); fd_screen_lock(batch->ctx->screen); - fd_batch_resource_used(batch, fd_resource(aq->prsc), true); + fd_batch_resource_write(batch, fd_resource(aq->prsc)); fd_screen_unlock(batch->ctx->screen); } -- 2.30.2