[Intel-gfx] [PATCH] drm/i915: Accurately track when we mark the hardware as idle/busy

[Chris Wilson]

On Tue, Feb 18, 2014 at 07:23:13PM -0300, Paulo Zanoni wrote:
> 2014-02-18 18:34 GMT-03:00 Paulo Zanoni <przanoni at gmail.com>:
> > 2014-02-18 16:25 GMT-03:00 Mika Kuoppala <mika.kuoppala at linux.intel.com>:
> >> Chris Wilson <chris at chris-wilson.co.uk> writes:
> >>
> >>> We currently call intel_mark_idle() too often, as we do so as a
> >>> side-effect of processing the request queue. However, we the calls to
> >>> intel_mark_idle() are expected to be paired with a call to
> >>> intel_mark_busy() (or else we try to idle the hardware by accessing
> >>> registers that are already disabled). Make the idle/busy tracking
> >>> explicit to prevent the multiple calls.
> >>>
> >>> Reported-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >
> > Thanks!
> >
> > I tested it and this patch + another local patch I have fix the
> > problem that can be reproduced by the "gem-idle" subtest of pm_pc8.c
> > (I still did not commit the subtest, but will do it soon).
> >
> > Also, I guess this patch deprecates dev_priv->pc8.gpu_idle. I had
> > plans to move it to dev_priv->pm.gpu_idle, but now I'll try to kill
> > it.
> >
> >>> ---
> >>>  drivers/gpu/drm/i915/i915_drv.h      |    8 ++++++++
> >>>  drivers/gpu/drm/i915/i915_gem.c      |    4 +---
> >>>  drivers/gpu/drm/i915/intel_display.c |   11 +++++++++++
> >>>  drivers/gpu/drm/i915/intel_drv.h     |    2 +-
> >>>  4 files changed, 21 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>> index 00222cc..8441c8a 100644
> >>> --- a/drivers/gpu/drm/i915/i915_drv.h
> >>> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >>> @@ -1134,6 +1134,14 @@ struct i915_gem_mm {
> >>>        */
> >>>       bool interruptible;
> >>>
> >>> +     /**
> >>> +      * Is the GPU currently considered idle, or busy executing userspace
> >>> +      * requests?  Whilst idle, we attempt to power down the hardware and
> >>> +      * display clocks. In order to reduce the effect on performance, there
> >>> +      * is a slight delay before we do so.
> >>> +      */
> >>> +     bool busy;
> 
> Hi
> 
> Also, don't we want to init this to true, since the first thing called
> seems to be intel_mark_idle?

As far as the state being tracked, it is logically .busy = false on
first load, until we process the first batch.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre