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?
It boils down to is it easier to read as:
static bool ring_is_idle(struct intel_engine_cs *engine)
{
struct drm_i915_private *dev_priv = engine->i915;
bool idle = false;
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))
goto busy;
/* No bit for gen2, so assume the CS parser is idle */
if (INTEL_GEN(dev_priv) > 2 && !(I915_READ_MODE(engine) & MODE_IDLE))
goto busy;
idle = true;
busy:
intel_runtime_pm_put(dev_priv);
return idle;
}
First instinct would be no because of the goto.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
