On Wed, 20 Jun 2012 09:38:25 +0200
Daniel Vetter <daniel at ffwll.ch> wrote:
> On Wed, Jun 20, 2012 at 09:20:39AM +0300, Jani Nikula wrote:
> > On Tue, 19 Jun 2012, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> > > On SNB and IVB, there's an MSR (also exposed through MCHBAR) we can use
> > > to read out the amount of energy used over time. Expose this in debugfs
> > > to make it easy to do power comparisons with different configurations.
> > >
> > > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index c0b7688..c8998b4 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -1897,6 +1897,41 @@ static const struct file_operations i915_cache_sharing_fops = {
> > > .llseek = default_llseek,
> > > };
> > >
> > > +#define MSR_IA32_PACKAGE_POWER_SKU_UNIT 0x00000606
> > > +
> > > +static int
> > > +i915_energy_status(struct seq_file *m, void *data)
> > > +{
> > > + struct drm_info_node *node = (struct drm_info_node *) m->private;
> > > + struct drm_device *dev = node->minor->dev;
> > > + struct drm_i915_private *dev_priv = dev->dev_private;
> > > + u64 ppsu;
> > > + u32 val, diff, units;
> > > +
> > > + if (!(IS_GEN6(dev) || IS_GEN7(dev))) {
> > > + seq_printf(m, "Unsupported platform\n");
> > > + return 0;
> > > + }
> > > +
> > > + rdmsrl(MSR_IA32_PACKAGE_POWER_SKU_UNIT, ppsu);
> > > +
> > > + ppsu = (ppsu & 0x1f00) >> 8;
> > > +
> > > + units = 1000000 / (1 << ppsu); /* convert to uJ */
> > > +
> > > + mutex_lock(&dev->struct_mutex);
> > > + val = I915_READ(SECP_NRG_STTS);
> > > + if (val < dev_priv->last_secp)
> > > + diff = val + (0xffffffff - dev_priv->last_secp);
> >
> > From the nitpickery dept.: I think that's off-by-one. But nobody will
> > ever notice from the output. ;)
>
> And from the bikeshed departement: Can't we just print a running number? I
> know, substraction is bloody hard, but for anything else than total power
> consumption (e.g. graphing power over time) the running thing is imo
> simpler. We've had the same discussion for the rc6 sysfs residency timers
> and concluded (after Arjan yelled at us) that doing the substraction in
> userspace is better, least it allows multiple userspace tools to read
> this.
>
Yeah that's a good point; this way happened to be simpler for what I
was doing, but just exposing the cooked register value (converted to
ujoules) is better. Will fix.
I'll also drop the bug fix hunk that Ben noticed.
--
Jesse Barnes, Intel Open Source Technology Center
