From 598503e285f9bf49e09132e064cc07a9ab0ae64c Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 30 Aug 2017 00:02:10 -0700 Subject: [PATCH] i965: Rename brw_bo::offset64 to gtt_offset. 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 Reviewed-by: Chris Wilson --- src/mesa/drivers/dri/i965/brw_bufmgr.c | 2 +- src/mesa/drivers/dri/i965/brw_bufmgr.h | 30 +++++++++++++++++++++++---- src/mesa/drivers/dri/i965/intel_batchbuffer.c | 15 +++++++------- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c b/src/mesa/drivers/dri/i965/brw_bufmgr.c index 5b4e784..5afc158 100644 --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c @@ -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; diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h b/src/mesa/drivers/dri/i965/brw_bufmgr.h index 7423dde..2c8a4ad 100644 --- a/src/mesa/drivers/dri/i965/brw_bufmgr.h +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h @@ -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. diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c index daed852..370400a 100644 --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c @@ -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 -- 2.7.4