Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Quoting Mika Kuoppala (2019-11-13 15:49:08)
>> 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
>>
>> 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>
>> ---
>> lib/i830_reg.h | 2 +-
>> lib/igt_dummyload.c | 46 +++++++++++++++++++++++--
>> lib/igt_dummyload.h | 3 ++
>> lib/intel_reg.h | 3 ++
>> tests/i915/gem_double_irq_loop.c | 2 --
>> tests/i915/gem_write_read_ring_switch.c | 2 +-
>> 6 files changed, 52 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/i830_reg.h b/lib/i830_reg.h
>> index a57691c7..41020256 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 c079bd04..a88c8582 100644
>> --- a/lib/igt_dummyload.c
>> +++ b/lib/igt_dummyload.c
>> @@ -130,7 +130,15 @@ emit_recursive_batch(igt_spin_t *spin,
>> spin->poll_handle = poll->handle;
>> execbuf->buffer_count++;
>>
>> - if (opts->flags & IGT_SPIN_POLL_RUN) {
>> + /*
>> + * For gen9+ we use a conditional loop rather than an
>> + * infinite loop, because we are unable to modify the
>> + * BB's once they have been scanned by the cmdparser
>> + * We share the poll buffer for the conditional test
>> + * and is always the first buffer in the batch list
>> + */
>> +
>> + if (gen >= 9 || (opts->flags & IGT_SPIN_POLL_RUN)) {
>
> Nah, you could just sample the batch buffer rather than always adding
> the poll buffer (since the cmdparser implicitly creates the second buffer
> for you). Using the comparison value of MI_BATCH_BUFFER_END you wouldn't
> even have to worry about altering callers.
That is a neat trick! bb start is greater than bb end so
we have that opportunity. Might lead also to much less fixing
on mutables.
-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
