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

[Chris Wilson]

Quoting Mika Kuoppala (2019-11-13 15:49: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
> 
> 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                     | 46 +++++++++++++++++++++++--
>  lib/igt_dummyload.h                     |  3 ++
>  lib/intel_reg.h                         |  3 ++
>  tests/i915/gem_double_irq_loop.c        |  2 --
>  tests/i915/gem_write_read_ring_switch.c |  2 +-
>  6 files changed, 52 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/i830_reg.h b/lib/i830_reg.h
> index a57691c7..41020256 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 c079bd04..a88c8582 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -130,7 +130,15 @@ emit_recursive_batch(igt_spin_t *spin,
>         spin->poll_handle = poll->handle;
>         execbuf->buffer_count++;
>  
> -       if (opts->flags & IGT_SPIN_POLL_RUN) {
> +       /*
> +        * For gen9+ we use a conditional loop rather than an
> +        * infinite loop, because we are unable to modify the
> +        * BB's once they have been scanned by the cmdparser
> +        * We share the poll buffer for the conditional test
> +        * and is always the first buffer in the batch list
> +        */
> +
> +       if (gen >= 9 || (opts->flags & IGT_SPIN_POLL_RUN)) {

Nah, you could just sample the batch buffer rather than always adding
the poll buffer (since the cmdparser implicitly creates the second buffer
for you). Using the comparison value of MI_BATCH_BUFFER_END you wouldn't
even have to worry about altering callers.
-Chris