[igt-dev] [PATCH i-g-t] igt: Use COND_BBEND for busy spinning on gen9
Chris Wilson
chris at chris-wilson.co.uk
Wed Nov 13 21:47:37 UTC 2019
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 | 3 --
lib/igt_dummyload.c | 39 +++++++++++++++++++++++--
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, 41 insertions(+), 8 deletions(-)
diff --git a/lib/i830_reg.h b/lib/i830_reg.h
index a57691c7e..b8ad2ac00 100644
--- a/lib/i830_reg.h
+++ b/lib/i830_reg.h
@@ -43,9 +43,6 @@
/* broadwater flush bits */
#define BRW_MI_GLOBAL_SNAPSHOT_RESET (1 << 3)
-#define MI_COND_BATCH_BUFFER_END (0x36<<23 | 1)
-#define MI_DO_COMPARE (1<<21)
-
#define MI_BATCH_BUFFER_END (0xA << 23)
/* Noop */
diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index b9e239db3..cfd0e75a8 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,42 @@ 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 >= 8) { /* arbitrary cutoff between ring/execlists submission */
+ r = &relocs[obj[BATCH].relocation_count++];
+
+ /*
+ * On Sandybridge+ the comparison is a strict greater-than:
+ * if the value at spin->condition is greater than BB_END,
+ * we loop back to the beginning.
+ * Beginning with Kabylake, we can select the comparison mode
+ * and loop back to the beginning if spin->condition != BB_END
+ * (using 5 << 12).
+ * For simplicity, we try to stick to a one-size fits all.
+ */
+ spin->condition = batch + BATCH_SIZE / sizeof(*batch) - 1;
+ *spin->condition = 0xffffffff;
+
+ 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 | MI_DO_COMPARE | 2;
+ *cs++ = MI_BATCH_BUFFER_END;
+ *cs++ = r->delta;
+ *cs++ = 0;
+ }
/* recurse */
r = &relocs[obj[BATCH].relocation_count++];
diff --git a/lib/intel_reg.h b/lib/intel_reg.h
index 069440cb7..7b11fedd9 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
More information about the igt-dev
mailing list