On Fri, Feb 21, 2014 at 02:46:55PM -0300, Paulo Zanoni wrote:
> 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.
I prefer explicit get/put calls instead of magic wrappers. runtime pm
get/put calls can be rather heavy-weight functions when we need to
actually bring up a domain again and restore piles of registers. So being
explicit is imo a feature.
Even more so with our explosion of power domains - I expect that some of
the debugfs files need to grab multiple different domains all over the
place.
-Daniel
>
> 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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
