From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 2/4] drm/i915: leave rc6 enabled at suspend time Date: Fri, 30 May 2014 08:32:20 -0700 Message-ID: <20140530083220.08b9b541@jbarnes-desktop> References: <1401397897-4655-1-git-send-email-jbarnes@virtuousgeek.org> <1401397897-4655-2-git-send-email-jbarnes@virtuousgeek.org> <1401454477.24060.32.camel@intelbox> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-pa0-f43.google.com (mail-pa0-f43.google.com [209.85.220.43]) by gabe.freedesktop.org (Postfix) with ESMTP id 1F6426EAE4 for ; Fri, 30 May 2014 08:32:02 -0700 (PDT) Received: by mail-pa0-f43.google.com with SMTP id hz1so1821151pad.16 for ; Fri, 30 May 2014 08:32:02 -0700 (PDT) In-Reply-To: <1401454477.24060.32.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@intel.com Cc: intel-gfx@lists.freedesktop.org, kristen@linux.intel.com List-Id: intel-gfx@lists.freedesktop.org On Fri, 30 May 2014 15:54:37 +0300 Imre Deak wrote: > On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote: > > From: Kristen Carlson Accardi > > > > This allows the system to enter the lowest power mode during system freeze. > > > > Signed-off-by: Jesse Barnes > > --- > > 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