From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 06/11] drm/i915: get/put runtime PM in more places at i915_debugfs.c Date: Fri, 21 Feb 2014 09:41:48 -0800 Message-ID: <20140221094148.729c35da@jbarnes-desktop> References: <1393001548-2883-1-git-send-email-przanoni@gmail.com> <1393001548-2883-7-git-send-email-przanoni@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy13-pub.mail.unifiedlayer.com (oproxy13-pub.mail.unifiedlayer.com [69.89.16.30]) by gabe.freedesktop.org (Postfix) with SMTP id CD4ACFA293 for ; Fri, 21 Feb 2014 09:41:33 -0800 (PST) In-Reply-To: <1393001548-2883-7-git-send-email-przanoni@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Paulo Zanoni Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Fri, 21 Feb 2014 13:52:23 -0300 Paulo Zanoni wrote: > From: Paulo Zanoni > > These are places where we read (not write) registers while we're > runtime suspended. > > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/i915_debugfs.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index d90a707..34e347f 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1348,6 +1348,8 @@ static int i915_fbc_status(struct seq_file *m, void *unused) > return 0; > } > > + intel_runtime_pm_get(dev_priv); > + > if (intel_fbc_enabled(dev)) { > seq_puts(m, "FBC enabled\n"); > } else { > @@ -1391,6 +1393,9 @@ static int i915_fbc_status(struct seq_file *m, void *unused) > } > seq_putc(m, '\n'); > } > + > + intel_runtime_pm_put(dev_priv); > + > return 0; > } > > @@ -1405,11 +1410,15 @@ static int i915_ips_status(struct seq_file *m, void *unused) > return 0; > } > > + intel_runtime_pm_get(dev_priv); > + > if (IS_BROADWELL(dev) || I915_READ(IPS_CTL) & IPS_ENABLE) > seq_puts(m, "enabled\n"); > else > seq_puts(m, "disabled\n"); > > + intel_runtime_pm_put(dev_priv); > + > return 0; > } > > @@ -1420,6 +1429,8 @@ static int i915_sr_status(struct seq_file *m, void *unused) > drm_i915_private_t *dev_priv = dev->dev_private; > bool sr_enabled = false; > > + intel_runtime_pm_get(dev_priv); > + > if (HAS_PCH_SPLIT(dev)) > sr_enabled = I915_READ(WM1_LP_ILK) & WM1_LP_SR_EN; > else if (IS_CRESTLINE(dev) || IS_I945G(dev) || IS_I945GM(dev)) > @@ -1429,6 +1440,8 @@ static int i915_sr_status(struct seq_file *m, void *unused) > else if (IS_PINEVIEW(dev)) > sr_enabled = I915_READ(DSPFW3) & PINEVIEW_SELF_REFRESH_EN; > > + intel_runtime_pm_put(dev_priv); > + > seq_printf(m, "self-refresh: %s\n", > sr_enabled ? "enabled" : "disabled"); > > @@ -1972,12 +1985,16 @@ static int i915_energy_uJ(struct seq_file *m, void *data) > if (INTEL_INFO(dev)->gen < 6) > return -ENODEV; > > + intel_runtime_pm_get(dev_priv); > + > rdmsrl(MSR_RAPL_POWER_UNIT, power); > power = (power & 0x1f00) >> 8; > units = 1000000 / (1 << power); /* convert to uJ */ > power = I915_READ(MCH_SECP_NRG_STTS); > power *= units; > > + intel_runtime_pm_put(dev_priv); > + > seq_printf(m, "%llu", (long long unsigned)power); > > return 0; Looks like there are more places we need this too.. wonder if it would be better to put the get into some wrapper around our sysfs files... But these bits look correct, if not sufficient, so: Reviewed-by: Jesse Barnes -- Jesse Barnes, Intel Open Source Technology Center