Quoting Chris Wilson (2019-11-13 20:20:31) > From: Jon Bloomfield <jon.bloomfield@xxxxxxxxx> > > 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@xxxxxxxxx> (v2) > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxx> > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> > Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> > --- > Compile after rebase! > --- > lib/i830_reg.h | 2 +- > lib/igt_dummyload.c | 27 +++++++++++++++++++++++-- > 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, 30 insertions(+), 6 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 b9e239db3..563a451da 100644 > --- a/lib/igt_dummyload.c > +++ b/lib/igt_dummyload.c > @@ -75,7 +75,7 @@ emit_recursive_batch(igt_spin_t *spin, > #define SCRATCH 0 > #define BATCH IGT_SPIN_BATCH > const int gen = intel_gen(intel_get_drm_devid(fd)); > - struct drm_i915_gem_relocation_entry relocs[2], *r; > + struct drm_i915_gem_relocation_entry relocs[3], *r; > struct drm_i915_gem_execbuffer2 *execbuf; > struct drm_i915_gem_exec_object2 *obj; > unsigned int flags[GEM_MAX_ENGINES]; > @@ -205,7 +205,30 @@ 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, > + * no matter how they modify it (from either the GPU or CPU). > + */ > + if (gen >= 9) { > + r = &relocs[obj[BATCH].relocation_count++]; > + > + r->presumed_offset = 0; > + r->target_handle = obj[BATCH].handle; > + r->offset = (cs + 2 - batch) * sizeof(*cs); > + r->read_domains = I915_GEM_DOMAIN_COMMAND; > + r->delta = (spin->condition - batch) * sizeof(*cs); > + > + *cs++ = MI_COND_BATCH_BUFFER_END | 1 << 21 | 5 << 12 | 2; Great the magic 5<<12 worked on kbl in testing, but we did not fare so lucky on skl! One slight adjustment later... -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
