[Intel-gfx] [PATCH 01/19] drm/i915/execlists: Always clear ring_pause if we do not submit

[Mika Kuoppala]

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

> Quoting Mika Kuoppala (2019-06-24 10:03:48)
>> Chris Wilson <chris at chris-wilson.co.uk> writes:
>> 
>> > In the unlikely case (thank you CI!), we may find ourselves wanting to
>> > issue a preemption but having no runnable requests left. In this case,
>> > we set the semaphore before computing the preemption and so must unset
>> > it before forgetting (or else we leave the machine busywaiting until the
>> > next request comes along and so likely hang).
>> >
>> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>> > ---
>> >  drivers/gpu/drm/i915/gt/intel_lrc.c | 9 ++++++++-
>> >  1 file changed, 8 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > index c8a0c9b32764..efccc31887de 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > @@ -233,13 +233,18 @@ static inline u32 intel_hws_preempt_address(struct intel_engine_cs *engine)
>> >  static inline void
>> >  ring_set_paused(const struct intel_engine_cs *engine, int state)
>> >  {
>> > +     u32 *sema = &engine->status_page.addr[I915_GEM_HWS_PREEMPT];
>> > +
>> > +     if (*sema == state)
>> > +             return;
>> > +
>> 
>> So you want to avoid useless wmb, as I don't see other
>> benefit. Makes this look suspiciously racy but seems
>> to be just my usual paranoia.
>
> Instead of the readback,
> 	if (state)
> 		wmb();
> would also work, if we ditch one half the paranoia. That's better.

Ok, as unpausing should not be so critical. So both forms
of paranoia is fine with me.

For wmb one can think of it as a paranoia or one can think it of as
documentation. Or both.

Reviewed-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>

> -Chris