| Message ID | 20191113194643.14831-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t] igt: Use COND_BBEND for busy spinning on gen9 | expand |
> -----Original Message----- > From: Chris Wilson <chris@chris-wilson.co.uk> > Sent: Wednesday, November 13, 2019 11:47 AM > To: intel-gfx@lists.freedesktop.org > Cc: Bloomfield, Jon <jon.bloomfield@intel.com>; Lahtinen, Joonas > <joonas.lahtinen@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; > Kuoppala, Mika <mika.kuoppala@intel.com>; Mika Kuoppala > <mika.kuoppala@linux.intel.com> > Subject: [PATCH i-g-t] igt: Use COND_BBEND for busy spinning on gen9 > > From: Jon Bloomfield <jon.bloomfield@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@intel.com> (v2) > Cc: Joonas Lahtinen <joonas.lahtinen@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@intel.com> > Signed-off-by: Mika Kuoppala <mika.kuoppala@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!
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);