Re: [igt-dev] [PATCH i-g-t 2/7] igt: Use COND_BBEND for busy spinning on gen9

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-11-13 15:49: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
>> 
>> 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                     | 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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev