[Intel-gfx] [PATCH 08/10] drm/i915: Defer enabling rc6 til after we submit the first batch/context

[Chris Wilson]

On Mon, Jul 11, 2016 at 05:14:22PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > Some hardware requires a valid render context before it can initiate
> > rc6 power gating of the GPU; the default state of the GPU is not
> > sufficient and may lead to undefined behaviour. The first execution of
> > any batch will load the "golden render state", at which point it is safe
> > to enable rc6. As we do not forcibly load the kernel context at resume,
> > we have to hook into the batch submission to be sure that the render
> > state is setup before enabling rc6.
> >
> > However, since we don't enable powersaving until that first batch, we
> > queued a delayed task in order to guarantee that the batch is indeed
> > submitted.
> >
> > v2: Rearrange intel_disable_gt_powersave() to match.
> > v3: Apply user specified cur_freq (or idle_freq if not set).
> > v4: Give in, and supply a delayed work to autoenable rc6
> > v5: Mika suggested a couple of better names for delayed_resume_work
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c      |   7 +-
> >  drivers/gpu/drm/i915/i915_drv.h      |   2 +-
> >  drivers/gpu/drm/i915/i915_gem.c      |   1 +
> >  drivers/gpu/drm/i915/intel_display.c |   2 +-
> >  drivers/gpu/drm/i915/intel_drv.h     |   2 +
> >  drivers/gpu/drm/i915/intel_pm.c      | 122 +++++++++++++++++++++++------------
> >  drivers/gpu/drm/i915/intel_uncore.c  |   2 +-
> >  7 files changed, 89 insertions(+), 49 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index b9a811750ca8..ff3342b78a74 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1343,7 +1343,7 @@ void i915_driver_unload(struct drm_device *dev)
> >  	i915_destroy_error_state(dev);
> >  
> >  	/* Flush any outstanding unpin_work. */
> > -	flush_workqueue(dev_priv->wq);
> > +	drain_workqueue(dev_priv->wq);
> >
> 
> For this to make sense, should we also use the dev priv workqueue
> for the gt autoenabling?

Sure. (That change was a bit of paranoia that stuck.) There's nothing
stopping the autoenable work from using the dev_priv->wq and that would
indeed make both it semantically cleaner.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre