[Intel-gfx] [PATCH] drm/i915: leave rc6 enabled at suspend time v4

[Imre Deak]

On Wed, 2014-06-11 at 15:24 -0700, Jesse Barnes wrote:
> On Wed, 11 Jun 2014 15:21:16 -0700
> Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> 
> > On Tue, 10 Jun 2014 17:26:45 +0200
> > Daniel Vetter <daniel at ffwll.ch> wrote:
> > 
> > > 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 at linux.intel.com>
> > > > > > > Signed-off-by: Jesse Barnes <jbarnes at 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.
> > 
> > Seems like we could just flush the enable if it was outstanding?
> 
> Oh and I see we already have the flush in v4...

Right, I was just blind and I guess also thinking in terms of timers
that don't have a similar flush API. Anyway this is fine and my comment
about it above can be ignored.

> do you think we might not actually arm the deferred enable work before
> we get here? 

I haven't thought about this. But now that you mentioned I looked into
the device and dpm core and can't see what would prevent our system
suspend handler to be called while we are still running our driver load
function. But this is a separate issue (if true at all) that exists
regardless of this patch and also not a really real life scenario. So I
suggest we investigate this as a follow-up.

> I think the runtime_resume and suspend calls should be serialized, but
> if not we'd need some locking I guess...

Yes, this is taken care of the RPM framework. Similarly system
suspend/resume handlers are serialized wrt. the runtime suspend/resume
handlers.

This patch is:
Reviewed-by: Imre Deak <imre.deak at intel.com>

Btw, now all 4 patches in the patchset are reviewed.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20140612/6d5d80ce/attachment.sig>