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

[Ben Widawsky <ben@bwidawsk.net>]

On Tue, 19 Jun 2012 17:59:13 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> Ben Widawsky <ben@bwidawsk.net> wrote:
> 
> >On Tue, 19 Jun 2012 13:20:50 -0700
> >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.
> >
> >I'd like some more explanation of why this belongs in i915.
> >Furthermore,
> >it seems like something which belongs in sysfs, not debugfs.
> >
> >> 
> >> 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);
> >> +	else
> >> +		diff = val - dev_priv->last_secp;
> >> +	dev_priv->last_secp = val;
> >> +	seq_printf(m, "Energy since last read: %u uJ\n", diff * units);
> >> +	mutex_unlock(&dev->struct_mutex);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >>  /* As the drm_debugfs_init() routines are called before
> >dev->dev_private is
> >>   * allocated we need to hook into the minor for release. */
> >>  static int
> >> @@ -2035,6 +2070,7 @@ static struct drm_info_list i915_debugfs_list[]
> >= {
> >>  	{"i915_swizzle_info", i915_swizzle_info, 0},
> >>  	{"i915_ppgtt_info", i915_ppgtt_info, 0},
> >>  	{"i915_dpio", i915_dpio_info, 0},
> >> +	{"i915_energy", i915_energy_status, 0},
> >>  };
> >>  #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
> >>  
> >> @@ -2102,6 +2138,8 @@ void i915_debugfs_cleanup(struct drm_minor
> >*minor)
> >>  				 1, minor);
> >>  	drm_debugfs_remove_files((struct drm_info_list *)
> >&i915_ring_stop_fops,
> >>  				 1, minor);
> >> +	drm_debugfs_remove_files((struct drm_info_list *)
> >&i915_error_state_fops,
> >> +				 1, minor);

What is up with this hunk?

> >>  }
> >>  
> >>  #endif /* CONFIG_DEBUG_FS */
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >b/drivers/gpu/drm/i915/i915_drv.h
> >> index 11c7a6a..fb07415 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -802,6 +802,8 @@ typedef struct drm_i915_private {
> >>  	u8 corr;
> >>  	spinlock_t *mchdev_lock;
> >>  
> >> +	u32 last_secp;
> >> +
> >>  	enum no_fbc_reason no_fbc_reason;
> >>  
> >>  	struct drm_mm_node *compressed_fb;
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >b/drivers/gpu/drm/i915/i915_reg.h
> >> index 0f45a18..b7d1310 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -1219,6 +1219,8 @@
> >>  #define CLKCFG_MEM_800					(3 << 4)
> >>  #define CLKCFG_MEM_MASK					(7 << 4)
> >>  
> >> +#define SECP_NRG_STTS			(MCHBAR_MIRROR_BASE_SNB + 0x592c)
> >> +
> >>  #define TSC1			0x11001
> >>  #define   TSE			(1<<0)
> >>  #define TR1			0x11006
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
> >
> >-- 
> >Ben Widawsky, Intel Open Source Technology Center
> 
> The power reported here is for the GT, so I think i915 is appropriate. As for sysfs vs debugfs, I'm a wimp and don't want to define a new ABI. 

Ah, okay the MSR just give the units. I apologize for reading over it
too quickly.

-- 
Ben Widawsky, Intel Open Source Technology Center