2014-02-21 14:41 GMT-03:00 Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>:
> On Fri, 21 Feb 2014 13:52:23 -0300
> Paulo Zanoni <przanoni@xxxxxxxxx> wrote:
>
>> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>>
>> These are places where we read (not write) registers while we're
>> runtime suspended.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>> ---
>> 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...
Well, at least the deubgfs-read test from pm_pc8 doesn't complain on
my HSW and SNB. But yeah, I thought about the wrapper too. I imagine
people adding new debugfs files will always forget about the runtime
PM refcounts, so maybe the wrapper is safer. But I could do this in a
separate patch.
I also thought about wrapping the connector->detect functions.
Thanks for the reviews!
>
> But these bits look correct, if not sufficient, so:
>
> Reviewed-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
>
> --
> Jesse Barnes, Intel Open Source Technology Center
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
