[igt-dev] [PATCH i-g-t v21 35/35] tests/gem_linear_blits: Use intel allocator

[Chris Wilson]

Quoting Zbigniew Kempczyński (2021-03-01 16:14:03)
> From: Dominik Grzegorzek <dominik.grzegorzek at intel.com>
> 
> Use intel allocator directly, without intel-bb infrastructure.
> 
> v2: for relocations suggests use incremented offsets  instead of 0.
> 
> Signed-off-by: Dominik Grzegorzek <dominik.grzegorzek at intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  tests/i915/gem_linear_blits.c | 117 +++++++++++++++++++++++++---------
>  1 file changed, 87 insertions(+), 30 deletions(-)
> 
> diff --git a/tests/i915/gem_linear_blits.c b/tests/i915/gem_linear_blits.c
> index cae42d52a..d351463fc 100644
> --- a/tests/i915/gem_linear_blits.c
> +++ b/tests/i915/gem_linear_blits.c
> @@ -56,7 +56,8 @@ IGT_TEST_DESCRIPTION("Test doing many blits with a working set larger than the"
>  static uint32_t linear[WIDTH*HEIGHT];
>  
>  static void
> -copy(int fd, uint32_t dst, uint32_t src)
> +copy(int fd, uint64_t ahnd, uint32_t dst, uint32_t src,
> +     uint64_t dst_offset, uint64_t src_offset, bool do_relocs)
>  {
>         uint32_t batch[12];
>         struct drm_i915_gem_relocation_entry reloc[2];
> @@ -77,41 +78,58 @@ copy(int fd, uint32_t dst, uint32_t src)
>                   WIDTH*4;
>         batch[i++] = 0; /* dst x1,y1 */
>         batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
> -       batch[i++] = 0; /* dst reloc */
> +       batch[i++] = dst_offset;
>         if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> -               batch[i++] = 0;
> +               batch[i++] = dst_offset >> 32;
>         batch[i++] = 0; /* src x1,y1 */
>         batch[i++] = WIDTH*4;
> -       batch[i++] = 0; /* src reloc */
> +       batch[i++] = src_offset;
>         if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> -               batch[i++] = 0;
> +               batch[i++] = src_offset >> 32;
>         batch[i++] = MI_BATCH_BUFFER_END;
>         batch[i++] = MI_NOOP;
>  
> -       memset(reloc, 0, sizeof(reloc));
> -       reloc[0].target_handle = dst;
> -       reloc[0].delta = 0;
> -       reloc[0].offset = 4 * sizeof(batch[0]);
> -       reloc[0].presumed_offset = 0;
> -       reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
> -       reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
> -
> -       reloc[1].target_handle = src;
> -       reloc[1].delta = 0;
> -       reloc[1].offset = 7 * sizeof(batch[0]);
> -       if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> -               reloc[1].offset += sizeof(batch[0]);
> -       reloc[1].presumed_offset = 0;
> -       reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
> -       reloc[1].write_domain = 0;
> -
>         memset(obj, 0, sizeof(obj));
>         obj[0].handle = dst;
>         obj[1].handle = src;
>         obj[2].handle = gem_create(fd, 4096);
>         gem_write(fd, obj[2].handle, 0, batch, i * sizeof(batch[0]));
> -       obj[2].relocation_count = 2;
> -       obj[2].relocs_ptr = to_user_pointer(reloc);
> +
> +       if (do_relocs) {
> +               memset(reloc, 0, sizeof(reloc));
> +               reloc[0].target_handle = dst;
> +               reloc[0].delta = 0;
> +               reloc[0].offset = 4 * sizeof(batch[0]);
> +               reloc[0].presumed_offset = dst_offset;

presumed_offset should match obj[].offset as well.

> +               reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
> +               reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
> +
> +               reloc[1].target_handle = src;
> +               reloc[1].delta = 0;
> +               reloc[1].offset = 7 * sizeof(batch[0]);
> +               if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> +                       reloc[1].offset += sizeof(batch[0]);
> +               reloc[1].presumed_offset = src_offset;
> +               reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
> +               reloc[1].write_domain = 0;
> +
> +               obj[2].relocation_count = 2;
> +               obj[2].relocs_ptr = to_user_pointer(reloc);
> +       } else {
> +               obj[0].offset = CANONICAL(dst_offset);
> +               obj[0].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_WRITE |
> +                              EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> +
> +               obj[1].offset = CANONICAL(src_offset);
> +               obj[1].flags = EXEC_OBJECT_PINNED |
> +                              EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> +
> +               obj[2].offset = intel_allocator_alloc(ahnd, obj[2].handle,
> +                                                     sizeof(linear), 0);
> +               obj[2].offset = CANONICAL(obj[2].offset);
> +               obj[2].flags = EXEC_OBJECT_PINNED |
> +                              EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> +       }

I was (am?) hoping that this would become something more like

obj[0].offset = CANONICAL(dst_offset); * see notes
obj[0].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_WRITE | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;

obj[1].offset = CANONICAL(src_offset);
obj[1].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;

obj[2].offset = intel_allocator_alloc(ahnd, obj[2].handle, sizeof(linear), 0);
obj[2].offset = CANONICAL(obj[2].offset);
obj[2].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;

if (do_relocs) {
	memset(reloc, 0, sizeof(reloc));

	reloc[0].target_handle = dst;
	reloc[0].delta = 0;
	reloc[0].offset = 4 * sizeof(batch[0]);
	reloc[0].presumed_offset = obj[0].offset;
	reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
	reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;

	reloc[1].target_handle = src;
	reloc[1].delta = 0;
	reloc[1].offset = 7 * sizeof(batch[0]);
	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
		reloc[1].offset += sizeof(batch[0]);
	reloc[1].presumed_offset = obj[1].offset;
	reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
	reloc[1].write_domain = 0;

	obj[0].flags &= ~EXEC_OBJECT_PINNED;
	obj[1].flags &= ~EXEC_OBJECT_PINNED;
	obj[2].flags &= ~EXEC_OBJECT_PINNED;
	obj[2].relocation_count = ARRAY_SIZE(reloc);
	obj[2].relocs_ptr = to_user_pointer(reloc);
}

Note the batch should also be using the canonicalised addresses or it is
pretty pointless...
-Chris