Re: [PATCH 06/11] drm/i915: get/put runtime PM in more places at i915_debugfs.c

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Fri, 21 Feb 2014 13:52:23 -0300
Paulo Zanoni <przanoni@gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> These are places where we read (not write) registers while we're
> runtime suspended.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  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 <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center