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

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jun 10, 2014 at 05:41:49PM +0300, Imre Deak wrote:
> On Tue, 2014-06-10 at 15:57 +0200, Daniel Vetter wrote:
> > On Tue, Jun 10, 2014 at 04:42:50PM +0300, Imre Deak wrote:
> > > On Wed, 2014-06-04 at 13:45 -0700, Jesse Barnes wrote:
> > > > This allows the system to enter the lowest power mode during system freeze.
> > > > 
> > > > v2: delete force wake timer at suspend (Imre)
> > > > v3: add GT work suspend function (Imre)
> > > > v4: use uncore forcewake reset (Daniel)
> > > > 
> > > > Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
> > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_drv.c     |  4 ++--
> > > >  drivers/gpu/drm/i915/i915_drv.h     |  1 +
> > > >  drivers/gpu/drm/i915/intel_drv.h    |  1 +
> > > >  drivers/gpu/drm/i915/intel_pm.c     | 20 ++++++++++++++++++++
> > > >  drivers/gpu/drm/i915/intel_uncore.c |  2 +-
> > > >  5 files changed, 25 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > > index 66c6ffb..7148eac 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > @@ -521,7 +521,7 @@ static int i915_drm_freeze(struct drm_device *dev)
> > > >  		drm_irq_uninstall(dev);
> > > >  		dev_priv->enable_hotplug_processing = false;
> > > >  
> > > > -		intel_disable_gt_powersave(dev);
> > > > +		intel_suspend_gt_powersave(dev);
> > > 
> > > I realized now that we actually do need to enable RC6 explicitly. If we
> > > get here right after runtime resume, the deferred RC6 enabling might be
> > > still pending and so RC6 might not be enabled.
> > 
> > Hm, for runtime pm we might schedule the delayed rps work with a 0 delay,
> > just to get the gpu going faster. Just an aside.
> >
> > Also some runtime pm vs system suspend tests in igt would be good if we
> > don't have them yet. And if we have them some analysis why this didn't
> > blow up in testing (and something to address that gap if feasible).
> 
> To clarify the above is about the new functionality to enable deeper
> system wide power states. Atm, we don't have a bug related to the above
> runtime resume->system suspend sequence, since we explicitly disable gt
> power saving/RC6. For deeper power states we have to do the opposite and
> leave RC6 enabled and that's what I commented on.

Well I just want to make sure that we have test coverage. If Jesse has run
all the rpm tests with piglit run -t rpm and it didn't blow up we need to
fix this gap.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch