Module: Mesa Branch: staging/21.2 Commit: 4489684269989818330f34515168fdcf9abd7f62 URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=4489684269989818330f34515168fdcf9abd7f62
Author: Alyssa Rosenzweig <[email protected]> Date: Tue Aug 17 17:16:54 2021 +0000 panfrost: Replace writers pointer with hash table This ensures each context can have a separate batch writing a resource and we don't race trying to flush each other's batches. Unfortunately the extra hash table operations regress draw-overhead numbers by about 8% but I'd rather eat the overhead and have an obviously correct implementation than leave known buggy code in tree. Signed-off-by: Alyssa Rosenzweig <[email protected]> Cc: mesa-stable Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12528> (cherry picked from commit 8503cab2e0032dd4d4bd1548919d3c359db82547) Conflicts: src/gallium/drivers/panfrost/pan_job.c --- .pick_status.json | 2 +- src/gallium/drivers/panfrost/pan_context.c | 5 +++++ src/gallium/drivers/panfrost/pan_context.h | 3 +++ src/gallium/drivers/panfrost/pan_job.c | 21 ++++++++++++++------- src/gallium/drivers/panfrost/pan_resource.c | 2 +- src/gallium/drivers/panfrost/pan_resource.h | 7 +++++-- 6 files changed, 29 insertions(+), 11 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 8f10bbec674..b5226fd6a8b 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -1129,7 +1129,7 @@ "description": "panfrost: Replace writers pointer with hash table", "nominated": true, "nomination_type": 0, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": null }, diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c index 55b9f511ed6..b617c41241b 100644 --- a/src/gallium/drivers/panfrost/pan_context.c +++ b/src/gallium/drivers/panfrost/pan_context.c @@ -794,6 +794,8 @@ panfrost_destroy(struct pipe_context *pipe) { struct panfrost_context *panfrost = pan_context(pipe); + _mesa_hash_table_destroy(panfrost->writers, NULL); + if (panfrost->blitter) util_blitter_destroy(panfrost->blitter); @@ -1126,6 +1128,9 @@ panfrost_create_context(struct pipe_screen *screen, void *priv, unsigned flags) ctx->blitter = util_blitter_create(gallium); + ctx->writers = _mesa_hash_table_create(gallium, _mesa_hash_pointer, + _mesa_key_pointer_equal); + assert(ctx->blitter); /* Prepare for render! */ diff --git a/src/gallium/drivers/panfrost/pan_context.h b/src/gallium/drivers/panfrost/pan_context.h index 41f013c66c8..6febeb8d4cf 100644 --- a/src/gallium/drivers/panfrost/pan_context.h +++ b/src/gallium/drivers/panfrost/pan_context.h @@ -146,6 +146,9 @@ struct panfrost_context { BITSET_DECLARE(active, PAN_MAX_BATCHES); } batches; + /* Map from resources to panfrost_batches */ + struct hash_table *writers; + /* Bound job batch */ struct panfrost_batch *batch; diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c index 736f93e2e50..d84d604361e 100644 --- a/src/gallium/drivers/panfrost/pan_job.c +++ b/src/gallium/drivers/panfrost/pan_job.c @@ -132,8 +132,10 @@ panfrost_batch_cleanup(struct panfrost_batch *batch) set_foreach_remove(batch->resources, entry) { struct panfrost_resource *rsrc = (void *) entry->key; - if (rsrc->track.writer == batch) - rsrc->track.writer = NULL; + if (_mesa_hash_table_search(ctx->writers, rsrc)) { + _mesa_hash_table_remove_key(ctx->writers, rsrc); + rsrc->track.nr_writers--; + } rsrc->track.nr_users--; @@ -272,7 +274,8 @@ panfrost_batch_update_access(struct panfrost_batch *batch, { struct panfrost_context *ctx = batch->ctx; uint32_t batch_idx = panfrost_batch_idx(batch); - struct panfrost_batch *writer = rsrc->track.writer; + struct hash_entry *entry = _mesa_hash_table_search(ctx->writers, rsrc); + struct panfrost_batch *writer = entry ? entry->data : NULL; bool found = false; _mesa_set_search_or_add(batch->resources, rsrc, &found); @@ -301,8 +304,10 @@ panfrost_batch_update_access(struct panfrost_batch *batch, } } - if (writes) - rsrc->track.writer = batch; + if (writes) { + _mesa_hash_table_insert(ctx->writers, rsrc, batch); + rsrc->track.nr_writers++; + } } static void @@ -933,8 +938,10 @@ void panfrost_flush_writer(struct panfrost_context *ctx, struct panfrost_resource *rsrc) { - if (rsrc->track.writer) { - panfrost_batch_submit(rsrc->track.writer, ctx->syncobj, ctx->syncobj); + struct hash_entry *entry = _mesa_hash_table_search(ctx->writers, rsrc); + + if (entry) { + panfrost_batch_submit(entry->data, ctx->syncobj, ctx->syncobj); } } diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c index 9917cfa3a03..131699ac8d9 100644 --- a/src/gallium/drivers/panfrost/pan_resource.c +++ b/src/gallium/drivers/panfrost/pan_resource.c @@ -870,7 +870,7 @@ panfrost_ptr_map(struct pipe_context *pctx, bool valid = BITSET_TEST(rsrc->valid.data, level); - if ((usage & PIPE_MAP_READ) && (valid || rsrc->track.writer)) { + if ((usage & PIPE_MAP_READ) && (valid || rsrc->track.nr_writers > 0)) { pan_blit_to_staging(pctx, transfer); panfrost_flush_writer(ctx, staging); panfrost_bo_wait(staging->image.data.bo, INT64_MAX, false); diff --git a/src/gallium/drivers/panfrost/pan_resource.h b/src/gallium/drivers/panfrost/pan_resource.h index de0597a48f0..3102229f9ca 100644 --- a/src/gallium/drivers/panfrost/pan_resource.h +++ b/src/gallium/drivers/panfrost/pan_resource.h @@ -48,11 +48,14 @@ struct panfrost_resource { } damage; struct { - struct panfrost_batch *writer; - /** Number of batches accessing this resource. Used to check if * a resource is in use. */ _Atomic unsigned nr_users; + + /** Number of batches writing this resource. Note that only one + * batch per context may write a resource, so this is the + * number of contexts that have an active writer. */ + _Atomic unsigned nr_writers; } track; struct renderonly_scanout *scanout;
