[igt-dev] [PATCH i-g-t v18 24/31] tests/gem_linear_blits: Use intel allocator

[Chris Wilson]

Quoting Zbigniew Kempczyński (2021-01-29 14:44:47)
> From: Dominik Grzegorzek <dominik.grzegorzek at intel.com>
> 
> Use intel allocator directly, without intel-bb infrastructure.
> 
> 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>

So I did not spot anything in these test changes that make me thing the
underlying coverage was changed.

> ---
>  tests/i915/gem_linear_blits.c | 103 ++++++++++++++++++++++++----------
>  1 file changed, 73 insertions(+), 30 deletions(-)
> 
> diff --git a/tests/i915/gem_linear_blits.c b/tests/i915/gem_linear_blits.c
> index cae42d52a..372d6ad81 100644
> --- a/tests/i915/gem_linear_blits.c
> +++ b/tests/i915/gem_linear_blits.c
> @@ -56,12 +56,14 @@ 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)
>  {
>         uint32_t batch[12];
>         struct drm_i915_gem_relocation_entry reloc[2];
>         struct drm_i915_gem_exec_object2 obj[3];
>         struct drm_i915_gem_execbuffer2 exec;
> +       bool do_relocs = false;
>         int i = 0;
>  
>         batch[i++] = XY_SRC_COPY_BLT_CMD |
> @@ -77,41 +79,59 @@ 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 (src_offset == 0 && dst_offset == 0) {
> +               do_relocs = true;
> +               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;

Is now dst_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 = 0;

src_offset. Yes they may be zero, but semantics matter.

Why leave them as zero though? You can seed them with sensible
placements. Sell me on some benefits of this allocator.
-Chris