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

[Jesse Barnes <jbarnes@virtuousgeek.org>]

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);
>>  }
>>  
>>  #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. 
-- 
Jesse Barnes, Intel Open Source Technology Center