Mesa (main): panfrost: Do tracking of resources, not BOs

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Tue Jul 6 19:10:18 UTC 2021


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

Author: Boris Brezillon <boris.brezillon at collabora.com>
Date:   Wed May 12 18:23:19 2021 +0200

panfrost: Do tracking of resources, not BOs

Squashed together with commits from Boris's original dependency tracking
cleanup series.

Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
Signed-off-by: Alyssa Rosenzweig <alyssa at rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11077>

---

 src/gallium/drivers/panfrost/pan_cmdstream.c |   2 +-
 src/gallium/drivers/panfrost/pan_context.c   |   6 +-
 src/gallium/drivers/panfrost/pan_context.h   |   5 -
 src/gallium/drivers/panfrost/pan_job.c       | 249 ++++++---------------------
 src/gallium/drivers/panfrost/pan_job.h       |  12 +-
 src/gallium/drivers/panfrost/pan_resource.c  |  18 +-
 src/gallium/drivers/panfrost/pan_resource.h  |   6 +
 7 files changed, 77 insertions(+), 221 deletions(-)

diff --git a/src/gallium/drivers/panfrost/pan_cmdstream.c b/src/gallium/drivers/panfrost/pan_cmdstream.c
index eb64fbecd57..0293293db80 100644
--- a/src/gallium/drivers/panfrost/pan_cmdstream.c
+++ b/src/gallium/drivers/panfrost/pan_cmdstream.c
@@ -1132,7 +1132,7 @@ panfrost_map_constant_buffer_cpu(struct panfrost_context *ctx,
 
         if (rsrc) {
                 panfrost_bo_mmap(rsrc->image.data.bo);
-                panfrost_flush_batches_accessing_bo(ctx, rsrc->image.data.bo, false);
+                panfrost_flush_batches_accessing_rsrc(ctx, rsrc, false);
                 panfrost_bo_wait(rsrc->image.data.bo, INT64_MAX, false);
 
                 return rsrc->image.data.bo->ptr.cpu + cb->buffer_offset;
diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
index 2630f17b28f..c0d398484a4 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -1821,7 +1821,7 @@ panfrost_get_query_result(struct pipe_context *pipe,
         case PIPE_QUERY_OCCLUSION_COUNTER:
         case PIPE_QUERY_OCCLUSION_PREDICATE:
         case PIPE_QUERY_OCCLUSION_PREDICATE_CONSERVATIVE:
-                panfrost_flush_batches_accessing_bo(ctx, rsrc->image.data.bo, false);
+                panfrost_flush_batches_accessing_rsrc(ctx, rsrc, false);
                 panfrost_bo_wait(rsrc->image.data.bo, INT64_MAX, false);
 
                 /* Read back the query results */
@@ -2036,10 +2036,6 @@ panfrost_create_context(struct pipe_screen *screen, void *priv, unsigned flags)
 
         /* Prepare for render! */
 
-        ctx->accessed_bos =
-                _mesa_hash_table_create(ctx, _mesa_hash_pointer,
-                                        _mesa_key_pointer_equal);
-
         /* By default mask everything on */
         ctx->sample_mask = ~0;
         ctx->active_queries = true;
diff --git a/src/gallium/drivers/panfrost/pan_context.h b/src/gallium/drivers/panfrost/pan_context.h
index 7213bcfa5d3..0b460a10785 100644
--- a/src/gallium/drivers/panfrost/pan_context.h
+++ b/src/gallium/drivers/panfrost/pan_context.h
@@ -119,8 +119,6 @@ struct panfrost_streamout {
         unsigned num_targets;
 };
 
-#define PAN_MAX_BATCHES 32
-
 struct panfrost_context {
         /* Gallium context */
         struct pipe_context base;
@@ -148,9 +146,6 @@ struct panfrost_context {
         /* Bound job batch */
         struct panfrost_batch *batch;
 
-        /* panfrost_bo -> panfrost_bo_access */
-        struct hash_table *accessed_bos;
-
         /* Within a launch_grid call.. */
         const struct pipe_grid_info *compute_grid;
 
diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c
index 7175505911b..c0cacaacb58 100644
--- a/src/gallium/drivers/panfrost/pan_job.c
+++ b/src/gallium/drivers/panfrost/pan_job.c
@@ -42,27 +42,11 @@
 #include "decode.h"
 #include "panfrost-quirks.h"
 
-/* panfrost_bo_access is here to help us keep track of batch accesses to BOs
- * and build a proper dependency graph such that batches can be pipelined for
- * better GPU utilization.
- *
- * Each accessed BO has a corresponding entry in the ->accessed_bos hash table.
- * A BO is either being written or read at any time (see last_is_write).
- * When the last access is a write, the batch writing the BO might have read
- * dependencies (readers that have not been executed yet and want to read the
- * previous BO content), and when the last access is a read, all readers might
- * depend on another batch to push its results to memory. That's what the
- * readers/writers keep track off.
- * There can only be one writer at any given time, if a new batch wants to
- * write to the same BO, a dependency will be added between the new writer and
- * the old writer (at the batch level), and panfrost_bo_access->writer will be
- * updated to point to the new writer.
- */
-struct panfrost_bo_access {
-        struct util_dynarray readers;
-        struct panfrost_batch *writer;
-        bool last_is_write;
-};
+static unsigned
+panfrost_batch_idx(struct panfrost_batch *batch)
+{
+        return batch - batch->ctx->batches.slots;
+}
 
 static void
 panfrost_batch_init(struct panfrost_context *ctx,
@@ -83,6 +67,7 @@ panfrost_batch_init(struct panfrost_context *ctx,
         batch->maxx = batch->maxy = 0;
 
         util_copy_framebuffer_state(&batch->key, key);
+        util_dynarray_init(&batch->resources, NULL);
 
         /* Preallocate the main pool, since every batch has at least one job
          * structure so it will be used */
@@ -130,6 +115,8 @@ panfrost_batch_cleanup(struct panfrost_batch *batch)
         if (ctx->batch == batch)
                 ctx->batch = NULL;
 
+        unsigned batch_idx = panfrost_batch_idx(batch);
+
         for (int i = batch->first_bo; i <= batch->last_bo; i++) {
                 uint32_t *flags = util_sparse_array_get(&batch->bos, i);
 
@@ -137,35 +124,19 @@ panfrost_batch_cleanup(struct panfrost_batch *batch)
                         continue;
 
                 struct panfrost_bo *bo = pan_lookup_bo(dev, i);
+                panfrost_bo_unreference(bo);
+        }
 
-                if (!(*flags & PAN_BO_ACCESS_SHARED)) {
-                        panfrost_bo_unreference(bo);
-                        continue;
-                }
-
-                struct hash_entry *access_entry =
-                        _mesa_hash_table_search(ctx->accessed_bos, bo);
-
-                assert(access_entry && access_entry->data);
-
-                struct panfrost_bo_access *access = access_entry->data;
+        util_dynarray_foreach(&batch->resources, struct panfrost_resource *, rsrc) {
+                BITSET_CLEAR((*rsrc)->track.users, batch_idx);
 
-                if (*flags & PAN_BO_ACCESS_WRITE) {
-                        assert(access->writer == batch);
-                        access->writer = NULL;
-                } else if (*flags & PAN_BO_ACCESS_READ) {
-                        util_dynarray_foreach(&access->readers,
-                                              struct panfrost_batch *, reader) {
-                                if (*reader == batch) {
-                                        *reader = NULL;
-                                        break;
-                                }
-                        }
-                }
+                if ((*rsrc)->track.writer == batch)
+                        (*rsrc)->track.writer = NULL;
 
-                panfrost_bo_unreference(bo);
+                pipe_resource_reference((struct pipe_resource **) rsrc, NULL);
         }
 
+        util_dynarray_fini(&batch->resources);
         panfrost_pool_cleanup(&batch->pool);
         panfrost_pool_cleanup(&batch->invisible_pool);
 
@@ -288,104 +259,38 @@ panfrost_get_fresh_batch_for_fbo(struct panfrost_context *ctx)
 }
 
 static void
-panfrost_batch_update_bo_access(struct panfrost_batch *batch,
-                                struct panfrost_bo *bo, bool writes)
+panfrost_batch_update_access(struct panfrost_batch *batch,
+                             struct panfrost_resource *rsrc, bool writes)
 {
         struct panfrost_context *ctx = batch->ctx;
-        struct panfrost_bo_access *access;
-        bool old_writes = false;
-        struct hash_entry *entry;
-
-        entry = _mesa_hash_table_search(ctx->accessed_bos, bo);
-        access = entry ? entry->data : NULL;
-        if (access) {
-                old_writes = access->last_is_write;
-        } else {
-                access = rzalloc(ctx, struct panfrost_bo_access);
-                util_dynarray_init(&access->readers, access);
-                _mesa_hash_table_insert(ctx->accessed_bos, bo, access);
-                /* We are the first to access this BO, let's initialize
-                 * old_writes to our own access type in that case.
-                 */
-                old_writes = writes;
-        }
+        uint32_t batch_idx = panfrost_batch_idx(batch);
+        struct panfrost_batch *writer = rsrc->track.writer;
 
-        assert(access);
+        if (unlikely(!BITSET_TEST(rsrc->track.users, batch_idx))) {
+                BITSET_SET(rsrc->track.users, batch_idx);
 
-        if (writes && !old_writes) {
-                assert(!access->writer);
+                /* Reference the resource on the batch */
+                struct pipe_resource **dst = util_dynarray_grow(&batch->resources,
+                                struct pipe_resource *, 1);
 
-                /* Previous access was a read and we want to write this BO.
-                 * We need to flush readers.
-                 */
-                util_dynarray_foreach(&access->readers,
-                                      struct panfrost_batch *, reader) {
-                        if (!*reader)
-                                continue;
+                *dst = NULL;
+                pipe_resource_reference(dst, &rsrc->base);
+        }
 
+        /* Flush users if required */
+        if (writes || ((writer != NULL) && (writer != batch))) {
+                unsigned i;
+                BITSET_FOREACH_SET(i, rsrc->track.users, PAN_MAX_BATCHES) {
                         /* Skip the entry if this our batch. */
-                        if (*reader == batch) {
-                                *reader = NULL;
+                        if (i == batch_idx)
                                 continue;
-                        }
-
-                        panfrost_batch_submit(*reader, 0, 0);
-                        assert(!*reader);
-                }
-
-                /* We now are the new writer. */
-                access->writer = batch;
-
-                /* Reset the readers array. */
-                util_dynarray_clear(&access->readers);
-        } else if (writes && old_writes) {
-                /* First check if we were the previous writer, in that case
-                 * there's nothing to do. Otherwise we need flush the previous
-                 * writer.
-                 */
-		if (access->writer != batch) {
-                        if (access->writer) {
-                                panfrost_batch_submit(access->writer, 0, 0);
-                                assert(!access->writer);
-                        }
-
-                        access->writer = batch;
-                }
-        } else if (!writes && old_writes) {
-                /* First check if we were the previous writer, in that case
-                 * we want to keep the access type unchanged, as a write is
-                 * more constraining than a read.
-                 */
-                if (access->writer != batch) {
-                        /* Flush the previous writer. */
-                        if (access->writer) {
-                                panfrost_batch_submit(access->writer, 0, 0);
-                                assert(!access->writer);
-                        }
-
-                        /* The previous access was a write, there's no reason
-                         * to have entries in the readers array.
-                         */
-                        assert(!util_dynarray_num_elements(&access->readers,
-                                                           struct panfrost_batch *));
 
-                        /* Add ourselves to the readers array. */
-                        util_dynarray_append(&access->readers,
-                                             struct panfrost_batch *,
-                                             batch);
+                        panfrost_batch_submit(&ctx->batches.slots[i], 0, 0);
                 }
-        } else {
-                assert(!access->writer);
-
-                /* Previous access was a read and we want to read this BO.
-                 * Add ourselves to the readers array.
-                 */
-                util_dynarray_append(&access->readers,
-                                     struct panfrost_batch *,
-                                     batch);
         }
 
-        access->last_is_write = writes;
+        if (writes)
+                rsrc->track.writer = batch;
 }
 
 static void
@@ -410,21 +315,6 @@ panfrost_batch_add_bo_old(struct panfrost_batch *batch,
 
         flags |= old_flags;
         *entry = flags;
-
-        /* If this is not a shared BO, we don't really care about dependency
-         * tracking.
-         */
-        if (!(flags & PAN_BO_ACCESS_SHARED))
-                return;
-
-        /* RW flags didn't change since our last access, no need to update the
-         * BO access entry.
-         */
-        if ((old_flags & PAN_BO_ACCESS_RW) == (flags & PAN_BO_ACCESS_RW))
-                return;
-
-        assert(flags & PAN_BO_ACCESS_RW);
-        panfrost_batch_update_bo_access(batch, bo, flags & PAN_BO_ACCESS_WRITE);
 }
 
 static uint32_t
@@ -447,9 +337,7 @@ panfrost_batch_read_rsrc(struct panfrost_batch *batch,
                          struct panfrost_resource *rsrc,
                          enum pipe_shader_type stage)
 {
-        uint32_t access =
-                PAN_BO_ACCESS_SHARED |
-                PAN_BO_ACCESS_READ |
+        uint32_t access = PAN_BO_ACCESS_READ |
                 panfrost_access_for_stage(stage);
 
         panfrost_batch_add_bo_old(batch, rsrc->image.data.bo, access);
@@ -459,6 +347,8 @@ panfrost_batch_read_rsrc(struct panfrost_batch *batch,
 
         if (rsrc->separate_stencil)
                 panfrost_batch_add_bo_old(batch, rsrc->separate_stencil->image.data.bo, access);
+
+        panfrost_batch_update_access(batch, rsrc, false);
 }
 
 void
@@ -466,9 +356,7 @@ panfrost_batch_write_rsrc(struct panfrost_batch *batch,
                          struct panfrost_resource *rsrc,
                          enum pipe_shader_type stage)
 {
-        uint32_t access =
-                PAN_BO_ACCESS_SHARED |
-                PAN_BO_ACCESS_WRITE |
+        uint32_t access = PAN_BO_ACCESS_WRITE |
                 panfrost_access_for_stage(stage);
 
         panfrost_batch_add_bo_old(batch, rsrc->image.data.bo, access);
@@ -478,6 +366,8 @@ panfrost_batch_write_rsrc(struct panfrost_batch *batch,
 
         if (rsrc->separate_stencil)
                 panfrost_batch_add_bo_old(batch, rsrc->separate_stencil->image.data.bo, access);
+
+        panfrost_batch_update_access(batch, rsrc, true);
 }
 
 /* Adds the BO backing surface to a batch if the surface is non-null */
@@ -1015,59 +905,28 @@ panfrost_flush_all_batches(struct panfrost_context *ctx)
         }
 }
 
-bool
-panfrost_pending_batches_access_bo(struct panfrost_context *ctx,
-                                   const struct panfrost_bo *bo)
-{
-        struct panfrost_bo_access *access;
-        struct hash_entry *hentry;
-
-        hentry = _mesa_hash_table_search(ctx->accessed_bos, bo);
-        access = hentry ? hentry->data : NULL;
-        if (!access)
-                return false;
-
-        if (access->writer)
-                return true;
-
-        util_dynarray_foreach(&access->readers, struct panfrost_batch *, reader) {
-                if (*reader)
-                        return true;
-        }
-
-        return false;
-}
-
 /* We always flush writers. We might also need to flush readers */
 
 void
-panfrost_flush_batches_accessing_bo(struct panfrost_context *ctx,
-                                    struct panfrost_bo *bo,
-                                    bool flush_readers)
+panfrost_flush_batches_accessing_rsrc(struct panfrost_context *ctx,
+                                      struct panfrost_resource *rsrc,
+                                      bool flush_readers)
 {
-        struct panfrost_bo_access *access;
-        struct hash_entry *hentry;
-
-        hentry = _mesa_hash_table_search(ctx->accessed_bos, bo);
-        access = hentry ? hentry->data : NULL;
-        if (!access)
-                return;
-
-        if (access->writer) {
-                panfrost_batch_submit(access->writer, ctx->syncobj, ctx->syncobj);
-                assert(!access->writer);
+        if (rsrc->track.writer) {
+                panfrost_batch_submit(rsrc->track.writer, ctx->syncobj, ctx->syncobj);
+                rsrc->track.writer = NULL;
         }
 
         if (!flush_readers)
                 return;
 
-        util_dynarray_foreach(&access->readers, struct panfrost_batch *,
-                              reader) {
-                if (*reader) {
-                        panfrost_batch_submit(*reader, ctx->syncobj, ctx->syncobj);
-                        assert(!*reader);
-                }
+        unsigned i;
+        BITSET_FOREACH_SET(i, rsrc->track.users, PAN_MAX_BATCHES) {
+                panfrost_batch_submit(&ctx->batches.slots[i],
+                                      ctx->syncobj, ctx->syncobj);
         }
+
+        assert(!BITSET_COUNT(rsrc->track.users));
 }
 
 void
diff --git a/src/gallium/drivers/panfrost/pan_job.h b/src/gallium/drivers/panfrost/pan_job.h
index 8871b1fa9b9..aca798f547e 100644
--- a/src/gallium/drivers/panfrost/pan_job.h
+++ b/src/gallium/drivers/panfrost/pan_job.h
@@ -129,6 +129,9 @@ struct panfrost_batch {
         mali_ptr attrib_bufs[PIPE_SHADER_TYPES];
         mali_ptr uniform_buffers[PIPE_SHADER_TYPES];
         mali_ptr push_uniforms[PIPE_SHADER_TYPES];
+
+        /* Referenced resources for cleanup */
+        struct util_dynarray resources;
 };
 
 /* Functions for managing the above */
@@ -169,13 +172,10 @@ panfrost_batch_create_bo(struct panfrost_batch *batch, size_t size,
 void
 panfrost_flush_all_batches(struct panfrost_context *ctx);
 
-bool
-panfrost_pending_batches_access_bo(struct panfrost_context *ctx,
-                                   const struct panfrost_bo *bo);
-
 void
-panfrost_flush_batches_accessing_bo(struct panfrost_context *ctx,
-                                    struct panfrost_bo *bo, bool flush_readers);
+panfrost_flush_batches_accessing_rsrc(struct panfrost_context *ctx,
+                                      struct panfrost_resource *rsrc,
+                                      bool flush_readers);
 
 void
 panfrost_batch_adjust_stack_size(struct panfrost_batch *batch);
diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
index a29b65fdb33..199608c1c1d 100644
--- a/src/gallium/drivers/panfrost/pan_resource.c
+++ b/src/gallium/drivers/panfrost/pan_resource.c
@@ -843,11 +843,11 @@ panfrost_ptr_map(struct pipe_context *pctx,
                 /* TODO: Eliminate this flush. It's only there to determine if
                  * we're initialized or not, when the initialization could come
                  * from a pending batch XXX */
-                panfrost_flush_batches_accessing_bo(ctx, rsrc->image.data.bo, true);
+                panfrost_flush_batches_accessing_rsrc(ctx, rsrc, true);
 
                 if ((usage & PIPE_MAP_READ) && BITSET_TEST(rsrc->valid.data, level)) {
                         pan_blit_to_staging(pctx, transfer);
-                        panfrost_flush_batches_accessing_bo(ctx, staging->image.data.bo, true);
+                        panfrost_flush_batches_accessing_rsrc(ctx, staging, true);
                         panfrost_bo_wait(staging->image.data.bo, INT64_MAX, false);
                 }
 
@@ -869,14 +869,14 @@ panfrost_ptr_map(struct pipe_context *pctx,
             (usage & PIPE_MAP_WRITE) &&
             !(resource->target == PIPE_BUFFER
               && !util_ranges_intersect(&rsrc->valid_buffer_range, box->x, box->x + box->width)) &&
-            panfrost_pending_batches_access_bo(ctx, bo)) {
+            BITSET_COUNT(rsrc->track.users) != 0) {
 
                 /* When a resource to be modified is already being used by a
                  * pending batch, it is often faster to copy the whole BO than
                  * to flush and split the frame in two.
                  */
 
-                panfrost_flush_batches_accessing_bo(ctx, bo, false);
+                panfrost_flush_batches_accessing_rsrc(ctx, rsrc, false);
                 panfrost_bo_wait(bo, INT64_MAX, false);
 
                 create_new_bo = true;
@@ -888,7 +888,7 @@ panfrost_ptr_map(struct pipe_context *pctx,
                  * not ready yet (still accessed by one of the already flushed
                  * batches), we try to allocate a new one to avoid waiting.
                  */
-                if (panfrost_pending_batches_access_bo(ctx, bo) ||
+                if (BITSET_COUNT(rsrc->track.users) ||
                     !panfrost_bo_wait(bo, 0, true)) {
                         /* We want the BO to be MMAPed. */
                         uint32_t flags = bo->flags & ~PAN_BO_DELAY_MMAP;
@@ -919,7 +919,7 @@ panfrost_ptr_map(struct pipe_context *pctx,
                                 /* Allocation failed or was impossible, let's
                                  * fall back on a flush+wait.
                                  */
-                                panfrost_flush_batches_accessing_bo(ctx, bo, true);
+                                panfrost_flush_batches_accessing_rsrc(ctx, rsrc, true);
                                 panfrost_bo_wait(bo, INT64_MAX, true);
                         }
                 }
@@ -929,10 +929,10 @@ panfrost_ptr_map(struct pipe_context *pctx,
                 /* No flush for writes to uninitialized */
         } else if (!(usage & PIPE_MAP_UNSYNCHRONIZED)) {
                 if (usage & PIPE_MAP_WRITE) {
-                        panfrost_flush_batches_accessing_bo(ctx, bo, true);
+                        panfrost_flush_batches_accessing_rsrc(ctx, rsrc, true);
                         panfrost_bo_wait(bo, INT64_MAX, true);
                 } else if (usage & PIPE_MAP_READ) {
-                        panfrost_flush_batches_accessing_bo(ctx, bo, false);
+                        panfrost_flush_batches_accessing_rsrc(ctx, rsrc, false);
                         panfrost_bo_wait(bo, INT64_MAX, false);
                 }
         }
@@ -1100,7 +1100,7 @@ panfrost_ptr_unmap(struct pipe_context *pctx,
                                 panfrost_bo_reference(prsrc->image.data.bo);
                         } else {
                                 pan_blit_from_staging(pctx, trans);
-                                panfrost_flush_batches_accessing_bo(pan_context(pctx), pan_resource(trans->staging.rsrc)->image.data.bo, true);
+                                panfrost_flush_batches_accessing_rsrc(pan_context(pctx), pan_resource(trans->staging.rsrc), true);
                         }
                 }
 
diff --git a/src/gallium/drivers/panfrost/pan_resource.h b/src/gallium/drivers/panfrost/pan_resource.h
index 6b680f518d2..bac29823d51 100644
--- a/src/gallium/drivers/panfrost/pan_resource.h
+++ b/src/gallium/drivers/panfrost/pan_resource.h
@@ -34,6 +34,7 @@
 #include "util/u_range.h"
 
 #define LAYOUT_CONVERT_THRESHOLD 8
+#define PAN_MAX_BATCHES 32
 
 struct panfrost_resource {
         struct pipe_resource base;
@@ -47,6 +48,11 @@ struct panfrost_resource {
                 } tile_map;
         } damage;
 
+        struct {
+                struct panfrost_batch *writer;
+                BITSET_DECLARE(users, PAN_MAX_BATCHES);
+        } track;
+
         struct renderonly_scanout *scanout;
 
         struct panfrost_resource *separate_stencil;



More information about the mesa-commit mailing list