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 <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12848>

diff --git a/src/gallium/drivers/iris/iris_batch.c b/src/gallium/drivers/iris/iris_batch.c
index d6f005d..d21e2ec 100644 (file)
--- 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 b039fe4..2803ae1 100644 (file)
--- 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;