From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter 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 Message-ID: <20140602160504.GC19050@phenom.ffwll.local> References: <1401397897-4655-1-git-send-email-jbarnes@virtuousgeek.org> <1401397897-4655-4-git-send-email-jbarnes@virtuousgeek.org> <20140602084522.GM19050@phenom.ffwll.local> <1401709055.1590.11.camel@intelbox> <20140602153217.GX19050@phenom.ffwll.local> <1401724638.1590.26.camel@intelbox> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wg0-f41.google.com (mail-wg0-f41.google.com [74.125.82.41]) by gabe.freedesktop.org (Postfix) with ESMTP id 13A966E5F0 for ; Mon, 2 Jun 2014 09:05:10 -0700 (PDT) Received: by mail-wg0-f41.google.com with SMTP id z12so5403015wgg.12 for ; Mon, 02 Jun 2014 09:05:09 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1401724638.1590.26.camel@intelbox> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Imre Deak Cc: intel-gfx@lists.freedesktop.org, kristen@linux.intel.com List-Id: intel-gfx@lists.freedesktop.org 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 > > > > > > > > > > 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 > > > > > Signed-off-by: Jesse Barnes > > > > > > > > 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