From cecb889481db23dc2b945dc3904f58f41a45fdfc Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Wed, 12 May 2021 18:23:19 +0200 Subject: [PATCH] panfrost: Do tracking of resources, not BOs Squashed together with commits from Boris's original dependency tracking cleanup series. Signed-off-by: Boris Brezillon Signed-off-by: Alyssa Rosenzweig Part-of: --- 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 | 255 +++++-------------- 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, 80 insertions(+), 224 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); - - 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; - - 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; - } - } - } - panfrost_bo_unreference(bo); } + util_dynarray_foreach(&batch->resources, struct panfrost_resource *, rsrc) { + BITSET_CLEAR((*rsrc)->track.users, batch_idx); + + if ((*rsrc)->track.writer == batch) + (*rsrc)->track.writer = NULL; + + 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; + uint32_t batch_idx = panfrost_batch_idx(batch); + struct panfrost_batch *writer = rsrc->track.writer; - 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; + if (unlikely(!BITSET_TEST(rsrc->track.users, batch_idx))) { + BITSET_SET(rsrc->track.users, batch_idx); + + /* Reference the resource on the batch */ + struct pipe_resource **dst = util_dynarray_grow(&batch->resources, + struct pipe_resource *, 1); + + *dst = NULL; + pipe_resource_reference(dst, &rsrc->base); } - assert(access); - - if (writes && !old_writes) { - assert(!access->writer); - - /* 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; - + /* 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); + panfrost_batch_submit(&ctx->batches.slots[i], 0, 0); } - - /* 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); - } - } 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;