Mesa (staging/21.2): panfrost: Replace writers pointer with hash table

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Thu Aug 26 19:11:37 UTC 2021


Module: Mesa
Branch: staging/21.2
Commit: 4489684269989818330f34515168fdcf9abd7f62
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=4489684269989818330f34515168fdcf9abd7f62

Author: Alyssa Rosenzweig <alyssa at collabora.com>
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 <alyssa at collabora.com>
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;



More information about the mesa-commit mailing list