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