Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Quoting Mika Kuoppala (2019-06-24 10:03:48)
>> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
>> > ---
>> > 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@xxxxxxxxxxxxxxx>
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
