On Wed, May 31, 2017 at 05:01:53PM +0300, Mika Kuoppala wrote: > Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > > > On Tue, May 30, 2017 at 03:33:41PM +0300, Mika Kuoppala wrote: > >> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > >> > >> > As another precaution when testing whether the CS engine is actually > >> > idle, also inspect the ring's HEAD/TAIL registers, which should be equal > >> > when there are no commands left to execute by the GPU. > >> > > >> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > >> > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> > >> > --- > >> > drivers/gpu/drm/i915/intel_engine_cs.c | 5 +++++ > >> > 1 file changed, 5 insertions(+) > >> > > >> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > >> > index 699f2d3861c7..bc38bd128b76 100644 > >> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > >> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > >> > @@ -1212,6 +1212,11 @@ static bool ring_is_idle(struct intel_engine_cs *engine) > >> > > >> > intel_runtime_pm_get(dev_priv); > >> > > >> > + /* First check that no commands are left in the ring */ > >> > + if ((I915_READ_HEAD(engine) & HEAD_ADDR) != > >> > + (I915_READ_TAIL(engine) & TAIL_ADDR)) > >> > + idle = false; > >> > + > >> > >> You are already certain that is not idle so why not goto out? > > > > In this case I could argue that extra path for the jump is not worth it. > > It saves a mmio read, yes, but will any one notice? > and one write :P > > > It boils down to is it easier to read as: > > > > Sold. It is easier to read as is. 3/3 is > > Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> Thanks, now pushed. Hopefully it never spots an error! -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx