Re: [PATCH 2/4] drm/i915: leave rc6 enabled at suspend time

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Fri, 30 May 2014 15:54:37 +0300
Imre Deak <imre.deak@intel.com> wrote:

> On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote:
> > From: Kristen Carlson Accardi <kristen@linux.intel.com>
> > 
> > This allows the system to enter the lowest power mode during system freeze.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c  |  3 ---
> >  drivers/gpu/drm/i915/intel_drv.h |  1 +
> >  drivers/gpu/drm/i915/intel_pm.c  | 16 +++++++++++-----
> >  3 files changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 66c6ffb..433bdfa 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -521,8 +521,6 @@ static int i915_drm_freeze(struct drm_device *dev)
> >  		drm_irq_uninstall(dev);
> >  		dev_priv->enable_hotplug_processing = false;
> >  
> > -		intel_disable_gt_powersave(dev);
> > -
> 
> I wonder what was the reason for this call. One possibility is that
> i915_save_state() depends on it to save the correct registers, but it
> would be good to clarify this.
> 
> It also cancels some deferred works which we do need here. But we could
> also add that to intel_enable_gt_powersave_sync() in this patch.

Yeah I was worried about that too, but then we do the reset on resume
anyway, and I didn't see anything in my logs in testing...

But I can split that out if there's a reason to.  Seems like we do a
bit too much teardown at suspend these days (like tearing down opregion
state), I'd like to trim it back if possible and share between runtime
and system suspend/freeze.

I'll look into the forcewake bits.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center