From e4c3d3efc7a2fa9f975b1f9211f7d61aa7d31d94 Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Wed, 4 Aug 2021 11:05:13 -0700 Subject: [PATCH] iris: Defer construction of the validation (exec_object2) list When I wrote this code originally, I decided to try and construct the validation list up front, rather than at submission time. That worked okay, but it's not really necessary. It's a fair amount of data to store (struct drm_i915_gem_exec_object2 is 56 bytes per object), when we can easily construct it on the fly. More importantly, with suballocation, batch->exec_bos[i] may have multiple entries corresponding to a single validation list entry. Rather than tracking two lists with an awkward mapping between them, we choose to just store the BO list and generate the other on the fly. Reviewed-by: Paulo Zanoni Part-of: --- src/gallium/drivers/iris/iris_batch.c | 80 +++++++++++++-------------- src/gallium/drivers/iris/iris_batch.h | 3 +- 2 files changed, 41 insertions(+), 42 deletions(-) diff --git a/src/gallium/drivers/iris/iris_batch.c b/src/gallium/drivers/iris/iris_batch.c index d6f005d7ddf..d21e2ec27c5 100644 --- a/src/gallium/drivers/iris/iris_batch.c +++ b/src/gallium/drivers/iris/iris_batch.c @@ -97,19 +97,19 @@ dump_fence_list(struct iris_batch *batch) * Debugging code to dump the validation list, used by INTEL_DEBUG=submit. */ static void -dump_validation_list(struct iris_batch *batch) +dump_validation_list(struct iris_batch *batch, + struct drm_i915_gem_exec_object2 *validation_list) { fprintf(stderr, "Validation list (length %d):\n", batch->exec_count); for (int i = 0; i < batch->exec_count; i++) { - uint64_t flags = batch->validation_list[i].flags; - assert(batch->validation_list[i].handle == - batch->exec_bos[i]->gem_handle); + uint64_t flags = validation_list[i].flags; + assert(validation_list[i].handle == batch->exec_bos[i]->gem_handle); fprintf(stderr, "[%2d]: %2d %-14s @ 0x%"PRIx64" (%"PRIu64"B)\t %2d refs %s\n", i, - batch->validation_list[i].handle, + validation_list[i].handle, batch->exec_bos[i]->name, - (uint64_t)batch->validation_list[i].offset, + (uint64_t)validation_list[i].offset, batch->exec_bos[i]->size, batch->exec_bos[i]->refcount, (flags & EXEC_OBJECT_WRITE) ? " (write)" : ""); @@ -199,8 +199,6 @@ iris_init_batch(struct iris_context *ice, batch->exec_array_size = 128; batch->exec_bos = malloc(batch->exec_array_size * sizeof(batch->exec_bos[0])); - batch->validation_list = - malloc(batch->exec_array_size * sizeof(batch->validation_list[0])); batch->bos_written = rzalloc_array(NULL, BITSET_WORD, BITSET_WORDS(batch->exec_array_size)); @@ -261,9 +259,6 @@ ensure_exec_obj_space(struct iris_batch *batch, uint32_t count) batch->exec_bos = realloc(batch->exec_bos, batch->exec_array_size * sizeof(batch->exec_bos[0])); - batch->validation_list = - realloc(batch->validation_list, - batch->exec_array_size * sizeof(batch->validation_list[0])); batch->bos_written = rerzalloc(NULL, batch->bos_written, BITSET_WORD, BITSET_WORDS(old_size), @@ -274,15 +269,8 @@ ensure_exec_obj_space(struct iris_batch *batch, uint32_t count) static void add_bo_to_batch(struct iris_batch *batch, struct iris_bo *bo, bool writable) { - uint64_t extra_flags = 0; - assert(batch->exec_array_size > batch->exec_count); - if (writable) - extra_flags |= EXEC_OBJECT_WRITE; - if (!iris_bo_is_external(bo)) - extra_flags |= EXEC_OBJECT_ASYNC; - iris_bo_reference(bo); batch->exec_bos[batch->exec_count] = bo; @@ -290,13 +278,6 @@ add_bo_to_batch(struct iris_batch *batch, struct iris_bo *bo, bool writable) if (writable) BITSET_SET(batch->bos_written, batch->exec_count); - batch->validation_list[batch->exec_count] = - (struct drm_i915_gem_exec_object2) { - .handle = bo->gem_handle, - .offset = bo->address, - .flags = bo->kflags | extra_flags, - }; - bo->index = batch->exec_count; batch->exec_count++; batch->aperture_space += bo->size; @@ -334,10 +315,8 @@ iris_use_pinned_bo(struct iris_batch *batch, if (existing_index != -1) { /* The BO is already in the list; mark it writable */ - if (writable) { + if (writable) BITSET_SET(batch->bos_written, existing_index); - batch->validation_list[existing_index].flags |= EXEC_OBJECT_WRITE; - } return; } @@ -451,7 +430,6 @@ iris_batch_free(struct iris_batch *batch) iris_bo_unreference(batch->exec_bos[i]); } free(batch->exec_bos); - free(batch->validation_list); ralloc_free(batch->bos_written); ralloc_free(batch->exec_fences.mem_ctx); @@ -749,8 +727,7 @@ update_batch_syncobjs(struct iris_batch *batch) for (int i = 0; i < batch->exec_count; i++) { struct iris_bo *bo = batch->exec_bos[i]; - struct drm_i915_gem_exec_object2 *exec_obj = &batch->validation_list[i]; - bool write = exec_obj->flags & EXEC_OBJECT_WRITE; + bool write = BITSET_TEST(batch->bos_written, i); if (bo == batch->screen->workaround_bo) continue; @@ -768,6 +745,35 @@ submit_batch(struct iris_batch *batch) { iris_bo_unmap(batch->bo); + struct drm_i915_gem_exec_object2 *validation_list = + malloc(batch->exec_count * sizeof(*validation_list)); + + for (int i = 0; i < batch->exec_count; i++) { + struct iris_bo *bo = batch->exec_bos[i]; + bool written = BITSET_TEST(batch->bos_written, i); + unsigned extra_flags = 0; + + if (written) + extra_flags |= EXEC_OBJECT_WRITE; + if (!iris_bo_is_external(bo)) + extra_flags |= EXEC_OBJECT_ASYNC; + + validation_list[i] = (struct drm_i915_gem_exec_object2) { + .handle = bo->gem_handle, + .offset = bo->address, + .flags = bo->kflags | extra_flags, + }; + } + + if (INTEL_DEBUG & (DEBUG_BATCH | DEBUG_SUBMIT)) { + dump_fence_list(batch); + dump_validation_list(batch, validation_list); + } + + if (INTEL_DEBUG & DEBUG_BATCH) { + decode_batch(batch); + } + /* The requirement for using I915_EXEC_NO_RELOC are: * * The addresses written in the objects must match the corresponding @@ -781,7 +787,7 @@ submit_batch(struct iris_batch *batch) * address of that object within the active context. */ struct drm_i915_gem_execbuffer2 execbuf = { - .buffers_ptr = (uintptr_t) batch->validation_list, + .buffers_ptr = (uintptr_t) validation_list, .buffer_count = batch->exec_count, .batch_start_offset = 0, /* This must be QWord aligned. */ @@ -814,6 +820,8 @@ submit_batch(struct iris_batch *batch) iris_bo_unreference(bo); } + free(validation_list); + return ret; } @@ -859,14 +867,6 @@ _iris_batch_flush(struct iris_batch *batch, const char *file, int line) batch->exec_count, (float) batch->aperture_space / (1024 * 1024)); - if (INTEL_DEBUG & (DEBUG_BATCH | DEBUG_SUBMIT)) { - dump_fence_list(batch); - dump_validation_list(batch); - } - - if (INTEL_DEBUG & DEBUG_BATCH) { - decode_batch(batch); - } } int ret = submit_batch(batch); diff --git a/src/gallium/drivers/iris/iris_batch.h b/src/gallium/drivers/iris/iris_batch.h index b039fe48c7f..2803ae18a84 100644 --- a/src/gallium/drivers/iris/iris_batch.h +++ b/src/gallium/drivers/iris/iris_batch.h @@ -81,8 +81,7 @@ struct iris_batch { uint32_t hw_ctx_id; - /** The validation list */ - struct drm_i915_gem_exec_object2 *validation_list; + /** A list of all BOs referenced by this batch */ struct iris_bo **exec_bos; int exec_count; int exec_array_size;