i965: Rename brw_bo::offset64 to gtt_offset.
authorChris Wilson <chris@chris-wilson.co.uk>
Wed, 30 Aug 2017 07:02:10 +0000 (00:02 -0700)
committerKenneth Graunke <kenneth@whitecape.org>
Fri, 1 Sep 2017 16:59:39 +0000 (09:59 -0700)
We can drop the meaningless "64" suffix - libdrm_intel originally had
an "offset" field that was an "unsigned long" which was the wrong size,
and we couldn't remove/alter that field without breaking ABI, so we had
to add a uint64_t "offset64" field.

"gtt_offset" is also more descriptive than "offset".

(Patch originally written by Ken, but Chris suggested a better name and
supplied the giant comment making up the bulk of the patch, so I changed
the authorship to him.)

Acked-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
src/mesa/drivers/dri/i965/brw_bufmgr.c
src/mesa/drivers/dri/i965/brw_bufmgr.h
src/mesa/drivers/dri/i965/intel_batchbuffer.c

index 5b4e784ae247b0a486616627b89c779bca5c36b6..5afc1585874f43ba46a5d7b5bf62f43494fadf4c 100644 (file)
@@ -517,7 +517,7 @@ brw_bo_gem_create_from_name(struct brw_bufmgr *bufmgr,
    p_atomic_set(&bo->refcount, 1);
 
    bo->size = open_arg.size;
-   bo->offset64 = 0;
+   bo->gtt_offset = 0;
    bo->bufmgr = bufmgr;
    bo->gem_handle = open_arg.handle;
    bo->name = name;
index 7423dde2d368000dab54aea108d6eabac13f8b9e..2c8a4add1721c5fc94c72f3b640c241eb8b3495a 100644 (file)
@@ -70,11 +70,33 @@ struct brw_bo {
    uint32_t gem_handle;
 
    /**
-    * Last seen card virtual address (offset from the beginning of the
-    * aperture) for the object.  This should be used to fill relocation
-    * entries when calling brw_bo_emit_reloc()
+    * Offset of the buffer inside the Graphics Translation Table.
+    *
+    * This is effectively our GPU address for the buffer and we use it
+    * as our base for all state pointers into the buffer. However, since the
+    * kernel may be forced to move it around during the course of the
+    * buffer's lifetime, we can only know where the buffer was on the last
+    * execbuf. We presume, and are usually right, that the buffer will not
+    * move and so we use that last offset for the next batch and by doing
+    * so we can avoid having the kernel perform a relocation fixup pass as
+    * our pointers inside the batch will be using the correct base offset.
+    *
+    * Since we do use it as a base address for the next batch of pointers,
+    * the kernel treats our offset as a request, and if possible will
+    * arrange the buffer to placed at that address (trying to balance
+    * the cost of buffer migration versus the cost of performing
+    * relocations). Furthermore, we can force the kernel to place the buffer,
+    * or report a failure if we specified a conflicting offset, at our chosen
+    * offset by specifying EXEC_OBJECT_PINNED.
+    *
+    * Note the GTT may be either per context, or shared globally across the
+    * system. On a shared system, our buffers have to contend for address
+    * space with both aperture mappings and framebuffers and so are more
+    * likely to be moved. On a full ppGTT system, each batch exists in its
+    * own GTT, and so each buffer may have their own offset within each
+    * context.
     */
-   uint64_t offset64;
+   uint64_t gtt_offset;
 
    /**
     * The validation list index for this buffer, or -1 when not in a batch.
index daed8526eae426c45394b36fe86b092392b473bf..370400ab484167c540e5640859487426a10ee3aa 100644 (file)
@@ -130,7 +130,7 @@ add_exec_bo(struct intel_batchbuffer *batch, struct brw_bo *bo)
       (struct drm_i915_gem_exec_object2) {
          .handle = bo->gem_handle,
          .alignment = bo->align,
-         .offset = bo->offset64,
+         .offset = bo->gtt_offset,
          .flags = bo->kflags,
       };
 
@@ -310,7 +310,7 @@ do_batch_dump(struct brw_context *brw)
 
    uint32_t *data = map ? map : batch->map;
    uint32_t *end = data + USED_BATCH(*batch);
-   uint32_t gtt_offset = map ? batch->bo->offset64 : 0;
+   uint32_t gtt_offset = map ? batch->bo->gtt_offset : 0;
    int length;
 
    bool color = INTEL_DEBUG & DEBUG_COLOR;
@@ -614,11 +614,12 @@ execbuffer(int fd,
       bo->idle = false;
       bo->index = -1;
 
-      /* Update brw_bo::offset64 */
-      if (batch->validation_list[i].offset != bo->offset64) {
+      /* Update brw_bo::gtt_offset */
+      if (batch->validation_list[i].offset != bo->gtt_offset) {
          DBG("BO %d migrated: 0x%" PRIx64 " -> 0x%llx\n",
-             bo->gem_handle, bo->offset64, batch->validation_list[i].offset);
-         bo->offset64 = batch->validation_list[i].offset;
+             bo->gem_handle, bo->gtt_offset,
+             batch->validation_list[i].offset);
+         bo->gtt_offset = batch->validation_list[i].offset;
       }
    }
 
@@ -652,7 +653,7 @@ do_flush_locked(struct brw_context *brw, int in_fence_fd, int *out_fence_fd)
       /* The requirement for using I915_EXEC_NO_RELOC are:
        *
        *   The addresses written in the objects must match the corresponding
-       *   reloc.presumed_offset which in turn must match the corresponding
+       *   reloc.gtt_offset which in turn must match the corresponding
        *   execobject.offset.
        *
        *   Any render targets written to in the batch must be flagged with