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

[Bloomfield, Jon]

> -----Original Message-----
> From: Chris Wilson <chris at chris-wilson.co.uk>
> Sent: Wednesday, November 13, 2019 11:47 AM
> To: intel-gfx at lists.freedesktop.org
> Cc: Bloomfield, Jon <jon.bloomfield at intel.com>; Lahtinen, Joonas
> <joonas.lahtinen at intel.com>; Vivi, Rodrigo <rodrigo.vivi at intel.com>;
> Kuoppala, Mika <mika.kuoppala at intel.com>; Mika Kuoppala
> <mika.kuoppala at linux.intel.com>
> Subject: [PATCH i-g-t] igt: Use COND_BBEND for busy spinning on gen9
> 
> 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                     | 24 +++++++++++++++++++++++-
>  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, 28 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..93b8b6bc6 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -208,7 +208,29 @@ 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;
> +
> +	/*
> +	 * When using a cmdparser, the batch is copied into a read only
> location
> +	 * and validated. We are then unable to alter the executing batch,
> +	 * breaking the older *spin->condition = MI_BB_END termination.
> +	 * Instead we can use a conditional MI_BB_END here that looks at
> +	 * the user's copy of the batch and terminates when they modified it.
> +	 */
> +	if (gen >= 9) {
> +		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;
> +	}
> 
>  	/* recurse */
>  	r = &relocs[batch->relocation_count++];
> diff --git a/lib/intel_reg.h b/lib/intel_reg.h
> index 069440cb7..10ca760a2 100644
> --- a/lib/intel_reg.h
> +++ b/lib/intel_reg.h
> @@ -2593,6 +2593,9 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN
> THE SOFTWARE.
>  #define MI_BATCH_BUFFER		((0x30 << 23) | 1)
>  #define MI_BATCH_BUFFER_START	(0x31 << 23)
>  #define MI_BATCH_BUFFER_END	(0xA << 23)
> +#define MI_COND_BATCH_BUFFER_END	(0x36 << 23)
> +#define MI_DO_COMPARE                   (1<<21)
> +
>  #define MI_BATCH_NON_SECURE		(1)
>  #define MI_BATCH_NON_SECURE_I965	(1 << 8)
>  #define MI_BATCH_NON_SECURE_HSW		(1<<13) /* Additional
> bit for RCS */
> diff --git a/tests/i915/gem_double_irq_loop.c
> b/tests/i915/gem_double_irq_loop.c
> index b326fc58a..f17f61c19 100644
> --- a/tests/i915/gem_double_irq_loop.c
> +++ b/tests/i915/gem_double_irq_loop.c
> @@ -52,8 +52,6 @@ static drm_intel_bo *target_buffer, *blt_bo;
>  IGT_TEST_DESCRIPTION("Basic check for missed IRQs on blt ring.");
> 
> 
> -#define MI_COND_BATCH_BUFFER_END	(0x36<<23 | 1)
> -#define MI_DO_COMPARE			(1<<21)
>  static void
>  dummy_reloc_loop(void)
>  {
> diff --git a/tests/i915/gem_write_read_ring_switch.c
> b/tests/i915/gem_write_read_ring_switch.c
> index ef229cc59..095c13c34 100644
> --- a/tests/i915/gem_write_read_ring_switch.c
> +++ b/tests/i915/gem_write_read_ring_switch.c
> @@ -115,7 +115,7 @@ static void run_test(int ring)
>  	 * otherwise the obj->last_write_seqno will be updated. */
>  	if (ring == I915_EXEC_RENDER) {
>  		BEGIN_BATCH(4, 1);
> -		OUT_BATCH(MI_COND_BATCH_BUFFER_END |
> MI_DO_COMPARE);
> +		OUT_BATCH(MI_COND_BATCH_BUFFER_END |
> MI_DO_COMPARE | 1);
>  		OUT_BATCH(0xffffffff); /* compare dword */
>  		OUT_RELOC(target_bo, I915_GEM_DOMAIN_RENDER, 0, 0);
>  		OUT_BATCH(MI_NOOP);
> --
> 2.24.0

Nice!