Re: [PATCH] drm/i915: add energy counter support for IVB

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Wed, 20 Jun 2012 09:38:25 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, Jun 20, 2012 at 09:20:39AM +0300, Jani Nikula wrote:
> > On Tue, 19 Jun 2012, Jesse Barnes <jbarnes@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@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