| Message ID | 20191113182808.10780-3-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t,1/3] i915: Skip if secure batches is not available | expand |
Quoting Chris Wilson (2019-11-13 18:28:08) > 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 | 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@chris-wilson.co.uk> -Chris
Quoting Chris Wilson (2019-11-13 18:42:06) > Quoting Chris Wilson (2019-11-13 18:28:08) > > 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 | 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. Oh, well, it appears the magic compare mode doesn't exist until Skylake. Fortunately, it exists! -Chris
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) { + 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);