Re: [igt-dev] [PATCH i-g-t 7/7] tests/gem_exec_schedule: Use store_dword_plug again

[Jason Ekstrand <jason@jlekstrand.net>]

Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

On Tue, Jun 8, 2021 at 4:39 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> In
>
> commit 2884f91dd6d7682ea738ef6f0943cc591643dda2
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Fri Jul 20 12:29:30 2018 +0100
>
>     igt/gem_exec_schedule: Trim deep runtime
>
> this was open-coded, I guess to avoid having to allocate a ton of
> batchbuffers. Unfortunately this relies on being able to rewrite
> batchbuffer relocations while the batch is in-flight (otherwise
> everything stalls and our setup is for nothing), and we're removing
> gpu relocations from upstream.
>
> This problem was realized for the testcase in general in
>
> commit f1e62e330a6e2de7b3cbf7cf02d71ae00cc6adcc
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Tue May 26 15:22:38 2020 +0100
>
>     i915/gem_exec_schedule: Reduce relocation risk for store-dword
>
> but the fix in there is only stochastic, there's still a chance of
> failure and hence unsightly noise in CI. The proper fix is to use
> softpin for this testcase unconditionally (not just when relocations
> aren't available), so that we clearly disentangle the concerns here
> and really only test the scheduler. And not some legacy features that
> are on the way out to their deserved retirement like relocations.
>
> Zbyscek and Ashotush cc'ed so they're aware that this testcase must be
> switched over to softping uncondtionally, for correctness reasons.
>
> But that's a pile more work, so as an interim solution go back to
> __store_dword helper.
>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Dixit Ashutosh <ashutosh.dixit@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  tests/i915/gem_exec_schedule.c | 52 +++-------------------------------
>  1 file changed, 4 insertions(+), 48 deletions(-)
>
> diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
> index 9585059d8ded..b5de1ab4eeae 100644
> --- a/tests/i915/gem_exec_schedule.c
> +++ b/tests/i915/gem_exec_schedule.c
> @@ -1846,55 +1846,11 @@ static void deep(int fd, unsigned ring)
>
>         /* Create a deep dependency chain, with a few branches */
>         for (n = 0; n < nreq && igt_seconds_elapsed(&tv) < 2; n++) {
> -               const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
> -               struct drm_i915_gem_exec_object2 obj[3];
> -               struct drm_i915_gem_relocation_entry reloc;
> -               struct drm_i915_gem_execbuffer2 eb = {
> -                       .buffers_ptr = to_user_pointer(obj),
> -                       .buffer_count = 3,
> -                       .flags = ring | (gen < 6 ? I915_EXEC_SECURE : 0),
> -                       .rsvd1 = ctx[n % MAX_CONTEXTS],
> -               };
> -               uint32_t batch[16];
> -               int i;
> -
> -               memset(obj, 0, sizeof(obj));
> -               obj[0].handle = plug;
> -
> -               memset(&reloc, 0, sizeof(reloc));
> -               reloc.presumed_offset = 0;
> -               reloc.offset = sizeof(uint32_t);
> -               reloc.delta = sizeof(uint32_t) * n;
> -               reloc.read_domains = I915_GEM_DOMAIN_RENDER;
> -               reloc.write_domain = I915_GEM_DOMAIN_RENDER;
> -               obj[2].handle = gem_create(fd, 4096);
> -               obj[2].relocs_ptr = to_user_pointer(&reloc);
> -               obj[2].relocation_count = 1;
> -
> -               i = 0;
> -               batch[i] = MI_STORE_DWORD_IMM | (gen < 6 ? 1 << 22 : 0);
> -               if (gen >= 8) {
> -                       batch[++i] = reloc.delta;
> -                       batch[++i] = 0;
> -               } else if (gen >= 4) {
> -                       batch[++i] = 0;
> -                       batch[++i] = reloc.delta;
> -                       reloc.offset += sizeof(uint32_t);
> -               } else {
> -                       batch[i]--;
> -                       batch[++i] = reloc.delta;
> -               }
> -               batch[++i] = eb.rsvd1;
> -               batch[++i] = MI_BATCH_BUFFER_END;
> -               gem_write(fd, obj[2].handle, 0, batch, sizeof(batch));
> +               uint32_t context = ctx[n % MAX_CONTEXTS];
> +               gem_context_set_priority(fd, context, MAX_PRIO - nreq + n);
>
> -               gem_context_set_priority(fd, eb.rsvd1, MAX_PRIO - nreq + n);
> -               for (int m = 0; m < XS; m++) {
> -                       obj[1].handle = dep[m];
> -                       reloc.target_handle = obj[1].handle;
> -                       gem_execbuf(fd, &eb);
> -               }
> -               gem_close(fd, obj[2].handle);
> +               for (int m = 0; m < XS; m++)
> +                       store_dword_plug(fd, context, ring, dep[m], 4*n, context, plug, I915_GEM_DOMAIN_INSTRUCTION);
>         }
>         igt_info("First deptree: %d requests [%.3fs]\n",
>                  n * XS, 1e-9*igt_nsec_elapsed(&tv));
> --
> 2.24.1
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev