Quoting Kenneth Graunke (2018-05-04 02:12:37)
> @@ -933,6 +945,14 @@ emit_reloc(struct intel_batchbuffer *batch,
> {
> assert(target != NULL);
>
> + if (target->kflags & EXEC_OBJECT_PINNED) {
> + brw_use_pinned_bo(batch, target, reloc_flags & RELOC_WRITE);
> + return target->gtt_offset + target_offset;
> + }
> +
> + unsigned int index = add_exec_bo(batch, target);
> + struct drm_i915_gem_exec_object2 *entry = &batch->validation_list[index];
> +
> if (rlist->reloc_count == rlist->reloc_array_size) {
> rlist->reloc_array_size *= 2;
> rlist->relocs = realloc(rlist->relocs,
> @@ -940,9 +960,6 @@ emit_reloc(struct intel_batchbuffer *batch,
> sizeof(struct drm_i915_gem_relocation_entry));
> }
>
> - unsigned int index = add_exec_bo(batch, target);
> - struct drm_i915_gem_exec_object2 *entry = &batch->validation_list[index];
> -
> if (reloc_flags & RELOC_32BIT) {
> /* Restrict this buffer to the low 32 bits of the address space.
> *
> @@ -976,6 +993,21 @@ emit_reloc(struct intel_batchbuffer *batch,
> return entry->offset + target_offset;
> }
>
> +void
> +brw_use_pinned_bo(struct intel_batchbuffer *batch, struct brw_bo *bo,
> + unsigned writable_flag)
> +{
> + assert(bo->kflags & EXEC_OBJECT_PINNED);
> + assert((writable_flag & ~EXEC_OBJECT_WRITE) == 0);
> +
> + unsigned int index = add_exec_bo(batch, bo);
> + struct drm_i915_gem_exec_object2 *entry = &batch->validation_list[index];
> + assert(entry->offset == bo->gtt_offset);
> +
> + if (writable_flag)
> + entry->flags |= EXEC_OBJECT_WRITE;
> +}
I'm not fond of this (at least the emit_reloc() perspective). In
emit_reloc() we were very careful to order the writes to ensure the
validation object was always consistent with the batchbuffer entry,
and this throws it all away (granted it's not a concern, my worry is
just that the code looks dangerous).
My preference would be something like:
static uint64_t
emit_reloc(struct intel_batchbuffer *batch,
struct brw_reloc_list *rlist, uint32_t offset,
struct brw_bo *target, int32_t target_offset,
unsigned int reloc_flags)
{
assert(target != NULL);
unsigned int index = add_exec_bo(batch, target);
struct drm_i915_gem_exec_object2 *entry = &batch->validation_list[index];
if (target->kflags & EXEC_OBJECT_PINNED) {
assert(!(reloc_flags & ~EXEC_OBJECT_WRITE));
goto skip_relocs;
}
if (rlist->reloc_count == rlist->reloc_array_size) {
rlist->reloc_array_size *= 2;
rlist->relocs = realloc(rlist->relocs,
rlist->reloc_array_size *
sizeof(struct drm_i915_gem_relocation_entry));
}
if (reloc_flags & RELOC_32BIT) {
/* Restrict this buffer to the low 32 bits of the address space.
*
* Altering the validation list flags restricts it for this batch,
* but we also alter the BO's kflags to restrict it permanently
* (until the BO is destroyed and put back in the cache). Buffers
* may stay bound across batches, and we want keep it constrained.
*/
target->kflags &= ~EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
entry->flags &= ~EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
/* RELOC_32BIT is not an EXEC_OBJECT_* flag, so get rid of it. */
reloc_flags &= ~RELOC_32BIT;
}
rlist->relocs[rlist->reloc_count++] =
(struct drm_i915_gem_relocation_entry) {
.offset = offset,
.delta = target_offset,
.target_handle = batch->use_batch_first ? index : target->gem_handle,
.presumed_offset = entry->offset,
};
skip_relocs:
if (reloc_flags)
entry->flags |= reloc_flags & batch->valid_reloc_flags;
/* Using the old buffer offset, write in what the right data would be, in
* case the buffer doesn't move and we can short-circuit the relocation
* processing in the kernel
*/
return entry->offset + target_offset;
}
The relationship between validation object entry and the batch buffer
contents is much easier to verify.
-Chris
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
