[igt-dev] [Intel-gfx] [PATCH i-g-t 3/3] igt: Use COND_BBEND for busy spinning on gen9

[Chris Wilson]

Quoting Chris Wilson (2019-11-13 18:28:08)
> From: Jon Bloomfield <jon.bloomfield at intel.com>
> 
> gen9+ introduces a cmdparser for the BLT engine which copies the
> incoming BB to a kmd owned buffer for submission (to prevent changes
> being made after the bb has been safely scanned). This breaks the
> spin functionality because it relies on changing the submitted spin
> buffers in order to terminate them.
> 
> Instead, for gen9+, we change the semantics by introducing a COND_BB_END
> into the infinite loop, to wait until a memory flag (in anothe bo) is
> cleared.
> 
> v2: Correct nop length to avoid overwriting bb_end instr when using
>     a dependency bo (cork)
> v3: fix conflicts on igt_dummyload (Mika)
> v4: s/bool running/uint32_t running, fix r->delta (Mika)
> v5: remove overzealous assert (Mika)
> v6: rebase on top of lib changes (Mika)
> v7: rework on top of public igt lib changes (Mika)
> v8: rebase
> v9: simplify by using bb end as conditional (Chris)
> 
> Signed-off-by: Jon Bloomfield <jon.bloomfield at intel.com> (v2)
> Cc: Joonas Lahtinen <joonas.lahtinen at intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> ---
>  lib/i830_reg.h                          |  2 +-
>  lib/igt_dummyload.c                     | 17 ++++++++++++++++-
>  lib/intel_reg.h                         |  3 +++
>  tests/i915/gem_double_irq_loop.c        |  2 --
>  tests/i915/gem_write_read_ring_switch.c |  2 +-
>  5 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/i830_reg.h b/lib/i830_reg.h
> index a57691c7e..410202567 100644
> --- a/lib/i830_reg.h
> +++ b/lib/i830_reg.h
> @@ -43,7 +43,7 @@
>  /* broadwater flush bits */
>  #define BRW_MI_GLOBAL_SNAPSHOT_RESET   (1 << 3)
>  
> -#define MI_COND_BATCH_BUFFER_END       (0x36<<23 | 1)
> +#define MI_COND_BATCH_BUFFER_END       (0x36 << 23)
>  #define MI_DO_COMPARE                  (1<<21)
>  
>  #define MI_BATCH_BUFFER_END    (0xA << 23)
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index c079bd045..652d469a0 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -208,7 +208,22 @@ emit_recursive_batch(igt_spin_t *spin,
>          * trouble. See https://bugs.freedesktop.org/show_bug.cgi?id=102262
>          */
>         if (!(opts->flags & IGT_SPIN_FAST))
> -               cs += 1000;
> +               cs += 960;
> +
> +       if (gen >= 8) {

If we push this back to gen>7 it will allow us to use it with its
cmdparser as well.

Hmm? I guess we could just do gen>6 so that there's some systematic
cutoff point.

> +               r = &relocs[batch->relocation_count++];
> +
> +               r->presumed_offset = 0;
> +               r->target_handle = batch->handle;
> +               r->offset = (cs + 2 - batch_start) * sizeof(*cs);
> +               r->read_domains = I915_GEM_DOMAIN_COMMAND;
> +               r->delta = (spin->condition - batch_start) * sizeof(*cs);
> +
> +               *cs++ = MI_COND_BATCH_BUFFER_END | 1 << 21 | 5 << 12 | 2;
> +               *cs++ = MI_BATCH_BUFFER_END;
> +               *cs++ = r->delta;
> +               *cs++ = 0;

I think this approach should do the trick even for my attempts to
frustrate you.

With consensus on when to use COND_BB_END and verification I haven't
caused havoc...

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris