From: Emma Anholt Date: Wed, 30 Jun 2021 23:33:22 +0000 (-0700) Subject: freedreno: Use a BO bitset for faster checks for resource referenced. X-Git-Tag: upstream/22.3.5~17823 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=3ade94df862ccc9562fd82ff8baa14b62b8be031;p=platform%2Fupstream%2Fmesa.git freedreno: Use a BO bitset for faster checks for resource referenced. When moving the batch cache to the context, I added hash table lookups from batch to rsc for "is this resource in use" because we could no longer store data in the rsc bo under the batch cache's lock. We can save that cost by tracking a bitfield of resources referenced by the batch, which gives us very cheap checks in the draw path at a minor cost in memory. We can just use the GEM BO handle, since it's a nice small integer already (we can't use the TC buffer ID, because the frontend changes that, and we're in the driver thread). This required moving the !pending() assert up in resource shadowing, since the BO swap meant we were checking pending on the wrong resource. Part-of: --- diff --git a/src/freedreno/drm/freedreno_bo.c b/src/freedreno/drm/freedreno_bo.c index 7be0c78..0c4d54f 100644 --- a/src/freedreno/drm/freedreno_bo.c +++ b/src/freedreno/drm/freedreno_bo.c @@ -403,6 +403,19 @@ fd_bo_handle(struct fd_bo *bo) return bo->handle; } +/** + * Returns a small integer ID for the BO valid for the lifetime of the fd_bo. + * + * It happens to be the GEM handle, but don't use it as one since this getter + * doesn't do any flushing or marking the BO as uncacheable like you would need + * for most cases of using a GEM handle. + */ +uint32_t +fd_bo_id(struct fd_bo *bo) +{ + return bo->handle; +} + int fd_bo_dmabuf(struct fd_bo *bo) { diff --git a/src/freedreno/drm/freedreno_drmif.h b/src/freedreno/drm/freedreno_drmif.h index 67b784a..4050295 100644 --- a/src/freedreno/drm/freedreno_drmif.h +++ b/src/freedreno/drm/freedreno_drmif.h @@ -201,6 +201,7 @@ struct fd_bo *fd_bo_ref(struct fd_bo *bo); void fd_bo_del(struct fd_bo *bo); int fd_bo_get_name(struct fd_bo *bo, uint32_t *name); uint32_t fd_bo_handle(struct fd_bo *bo); +uint32_t fd_bo_id(struct fd_bo *bo); int fd_bo_dmabuf(struct fd_bo *bo); uint32_t fd_bo_size(struct fd_bo *bo); void *fd_bo_map(struct fd_bo *bo); diff --git a/src/gallium/drivers/freedreno/freedreno_batch.c b/src/gallium/drivers/freedreno/freedreno_batch.c index f8ada70..17a5363 100644 --- a/src/gallium/drivers/freedreno/freedreno_batch.c +++ b/src/gallium/drivers/freedreno/freedreno_batch.c @@ -282,6 +282,9 @@ batch_reset_resources(struct fd_batch *batch) * rather than having any deleted keys. */ _mesa_set_clear(batch->resources, NULL); + + free(batch->bos); + batch->bos = NULL; } static void @@ -421,13 +424,27 @@ fd_batch_add_dep(struct fd_batch *batch, struct fd_batch *dep) static void fd_batch_add_resource(struct fd_batch *batch, struct fd_resource *rsc) { - bool found = false; + if (fd_batch_references(batch, rsc)) + return; + + ASSERTED bool found = false; _mesa_set_search_or_add_pre_hashed(batch->resources, rsc->hash, rsc, &found); - if (!found) { - struct pipe_resource *table_ref = NULL; - pipe_resource_reference(&table_ref, &rsc->b.b); - p_atomic_inc(&rsc->batch_references); + assert(!found); + + struct pipe_resource *table_ref = NULL; + pipe_resource_reference(&table_ref, &rsc->b.b); + p_atomic_inc(&rsc->batch_references); + + uint32_t handle = fd_bo_id(rsc->bo); + if (batch->bos_size <= handle) { + uint32_t new_size = MAX2(BITSET_WORDBITS, + util_next_power_of_two(handle + 1)); + batch->bos = realloc(batch->bos, new_size / 8); + memset(&batch->bos[batch->bos_size / BITSET_WORDBITS], 0, + (new_size - batch->bos_size) / 8); + batch->bos_size = new_size; } + BITSET_SET(batch->bos, handle); } void @@ -443,6 +460,13 @@ fd_batch_resource_write(struct fd_batch *batch, struct fd_resource *rsc) */ rsc->valid = true; + /* This has to happen before the early out, because + * fd_bc_invalidate_resource() may not have been called on our context to + * clear our writer when reallocating the BO, and otherwise we could end up + * with our batch writing the BO but returning !fd_batch_references(rsc). + */ + fd_batch_add_resource(batch, rsc); + struct fd_batch *prev_writer = fd_bc_writer(ctx, rsc); if (prev_writer == batch) return; @@ -467,8 +491,6 @@ fd_batch_resource_write(struct fd_batch *batch, struct fd_resource *rsc) pipe_resource_reference(&table_rsc_ref, &rsc->b.b); _mesa_hash_table_insert_pre_hashed(cache->written_resources, rsc->hash, rsc, batch); - - 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 c74e5e8..6123016 100644 --- a/src/gallium/drivers/freedreno/freedreno_batch.h +++ b/src/gallium/drivers/freedreno/freedreno_batch.h @@ -246,6 +246,9 @@ struct fd_batch { */ struct set *resources; + BITSET_WORD *bos; + uint32_t bos_size; + /** key in batch-cache (if not null): */ struct fd_batch_key *key; uint32_t hash; diff --git a/src/gallium/drivers/freedreno/freedreno_resource.c b/src/gallium/drivers/freedreno/freedreno_resource.c index b139d0e..5313669 100644 --- a/src/gallium/drivers/freedreno/freedreno_resource.c +++ b/src/gallium/drivers/freedreno/freedreno_resource.c @@ -432,17 +432,6 @@ fd_try_shadow_resource(struct fd_context *ctx, struct fd_resource *rsc, DBG("shadow: %p (%d) -> %p (%d)", rsc, rsc->b.b.reference.count, shadow, shadow->b.b.reference.count); - swap(rsc->bo, shadow->bo); - swap(rsc->valid, shadow->valid); - - /* swap() doesn't work because you can't typeof() the bitfield. */ - bool temp = shadow->needs_ubwc_clear; - shadow->needs_ubwc_clear = rsc->needs_ubwc_clear; - rsc->needs_ubwc_clear = temp; - - swap(rsc->layout, shadow->layout); - rsc->seqno = p_atomic_inc_return(&ctx->screen->rsc_seqno); - /* At this point, the newly created shadow buffer is not referenced by any * batches, but the existing rsc may be as a reader (we flushed writers * above). We want to remove the old reader references to rsc so that we @@ -465,6 +454,17 @@ fd_try_shadow_resource(struct fd_context *ctx, struct fd_resource *rsc, } } + swap(rsc->bo, shadow->bo); + swap(rsc->valid, shadow->valid); + + /* swap() doesn't work because you can't typeof() the bitfield. */ + bool temp = shadow->needs_ubwc_clear; + shadow->needs_ubwc_clear = rsc->needs_ubwc_clear; + rsc->needs_ubwc_clear = temp; + + swap(rsc->layout, shadow->layout); + rsc->seqno = p_atomic_inc_return(&ctx->screen->rsc_seqno); + struct pipe_blit_info blit = {}; blit.dst.resource = prsc; blit.dst.format = prsc->format; diff --git a/src/gallium/drivers/freedreno/freedreno_resource.h b/src/gallium/drivers/freedreno/freedreno_resource.h index ba2825a..64184cb 100644 --- a/src/gallium/drivers/freedreno/freedreno_resource.h +++ b/src/gallium/drivers/freedreno/freedreno_resource.h @@ -27,6 +27,7 @@ #ifndef FREEDRENO_RESOURCE_H_ #define FREEDRENO_RESOURCE_H_ +#include "util/bitset.h" #include "util/list.h" #include "util/set.h" #include "util/simple_mtx.h" @@ -143,7 +144,11 @@ fd_memory_object(struct pipe_memory_object *pmemobj) static inline bool fd_batch_references(struct fd_batch *batch, struct fd_resource *rsc) { - return _mesa_set_search_pre_hashed(batch->resources, rsc->hash, rsc) != NULL; + /* Currently each rsc has an individual BO, so we can use the bo handle as a + * unique index for the resource. + */ + uint32_t handle = fd_bo_id(rsc->bo); + return handle < batch->bos_size && BITSET_TEST(batch->bos, handle); } static inline struct fd_batch *