Re: [PATCH 4/4] drm/i915: make sure PC8 is enabled on suspend and disabled on resume

From: Daniel Vetter <daniel@ffwll.ch>
To: Imre Deak <imre.deak@intel.com>
Cc: intel-gfx@lists.freedesktop.org, kristen@linux.intel.com
Subject: Re: [PATCH 4/4] drm/i915: make sure PC8 is enabled on suspend and disabled on resume
Date: Mon, 2 Jun 2014 18:05:04 +0200	[thread overview]
Message-ID: <20140602160504.GC19050@phenom.ffwll.local> (raw)
In-Reply-To: <1401724638.1590.26.camel@intelbox>

On Mon, Jun 02, 2014 at 06:57:18PM +0300, Imre Deak wrote:
> On Mon, 2014-06-02 at 17:32 +0200, Daniel Vetter wrote:
> > On Mon, Jun 02, 2014 at 02:37:35PM +0300, Imre Deak wrote:
> > > On Mon, 2014-06-02 at 10:45 +0200, Daniel Vetter wrote:
> > > > On Thu, May 29, 2014 at 02:11:37PM -0700, Jesse Barnes wrote:
> > > > > From: Kristen Carlson Accardi <kristen@linux.intel.com>
> > > > > 
> > > > > This matches the runtime suspend paths and allows the system to enter
> > > > > the lowest power mode at freeze time.
> > > > > 
> > > > > Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
> > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > > 
> > > > pc8 is fully subsumed into runtime pm by now. Do we _really_ still need
> > > > this?
> > > 
> > > Yes, since the system suspend/resume handlers are called with an RPM ref
> > > held and thus PC8 disabled.
> > 
> > But doesn't patch 1 try to fix that?
> 
> That only disables the display side, but we won't disable PC8 until the
> RPM suspend handler is called. And that won't happen because the last
> RPM ref is held by the DPM framework for the duration of system
> suspend/resume handlers.

Yeah, there's discussion going on to make system suspend be optionally
based upon runtime pm in the core. Atm that's not possible since the
system wakes up everyone to suspend them.

> > Imo we should have this here but instead go through highl-level the runtime
> > pm functions to shut off the chip on all platforms.
> 
> After the planned refactoring we could have a low-level function that we
> can call both from here and the runtime PM path, but until that happens
> we need to do this here directly.

Yeah, that's what I'm actually aiming for - we should be able to call the
runtime pm suspend code from the system suspend code and share pretty much
all the code. The sequence I'm thinking of would be for system suspend:

1. Stop everything we need to stop (gpu, display, rps, ...) to be able to
enter runtime pm.

2. Check for leaked references. This might be tricky because audio.

3. Call runtime pm suspend hooks.

Resume would be the same, but inverted.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch