[PATCH 0/7] moar sysfs stuff

[PATCH 0/7] moar sysfs stuff

[Ben Widawsky]

Not very thoroughly tested yet. I'll be adding igt tests ASAP.

Ben Widawsky (6):
  drm/i915: Enable some sysfs stuff without CONFIG_PM
  drm/i915: variable renames
  drm/i915: #define gpu freq multipler
  drm/i915: Add current GPU freq to sysfs
  drm/i915: Add setters for min/max frequency
  drm/i915: Show render P state thresholds in sysfs

Jesse Barnes (1):
  drm/i915: expose energy counter on SNB and IVB

 drivers/gpu/drm/i915/i915_debugfs.c |  22 ++--
 drivers/gpu/drm/i915/i915_drv.h     |   2 +
 drivers/gpu/drm/i915/i915_reg.h     |   2 +
 drivers/gpu/drm/i915/i915_sysfs.c   | 237 ++++++++++++++++++++++++++++++++----
 4 files changed, 230 insertions(+), 33 deletions(-)

-- 
1.7.12

[PATCH 1/7] drm/i915: Enable some sysfs stuff without CONFIG_PM

[Ben Widawsky]

The original patch was actually incorrect in stubbing out the sysfs for
l3 parity.
commit 5ab3633d6907018b0b830a720e877c3884d679c3
Author: Hunt Xu <mhuntxu@gmail.com>
Date:   Sun Jul 1 03:45:07 2012 +0000

    drm/i915: make rc6 in sysfs functions conditional

Unfortunately Hunt didn't respond to my review comments, and Daniel
sucked in the patch again ignoring. Worst of all, I'm too lazy to write
the patch for what I originally wanted, which was to keep rc6 sysfs even
without CONFIG_PM. This simpler patch does enough to enable us to add
more sysfs entries though.

NOTE: This is already picked up in dinq, but is here in case anyone is
looking to test this series on it's own.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_sysfs.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index c5ee7ee..da733a3 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -93,6 +93,7 @@ static struct attribute_group rc6_attr_group = {
 	.name = power_group_name,
 	.attrs =  rc6_attrs
 };
+#endif
 
 static int l3_access_valid(struct drm_device *dev, loff_t offset)
 {
@@ -206,13 +207,14 @@ void i915_setup_sysfs(struct drm_device *dev)
 {
 	int ret;
 
+#ifdef CONFIG_PM
 	if (INTEL_INFO(dev)->gen >= 6) {
 		ret = sysfs_merge_group(&dev->primary->kdev.kobj,
 					&rc6_attr_group);
 		if (ret)
 			DRM_ERROR("RC6 residency sysfs setup failed\n");
 	}
-
+#endif
 	if (HAS_L3_GPU_CACHE(dev)) {
 		ret = device_create_bin_file(&dev->primary->kdev, &dpf_attrs);
 		if (ret)
@@ -225,14 +227,3 @@ void i915_teardown_sysfs(struct drm_device *dev)
 	device_remove_bin_file(&dev->primary->kdev,  &dpf_attrs);
 	sysfs_unmerge_group(&dev->primary->kdev.kobj, &rc6_attr_group);
 }
-#else
-void i915_setup_sysfs(struct drm_device *dev)
-{
-	return;
-}
-
-void i915_teardown_sysfs(struct drm_device *dev)
-{
-	return;
-}
-#endif /* CONFIG_PM */
-- 
1.7.12

[PATCH 2/7] drm/i915: variable renames

[Ben Widawsky]

From: Ben Widawsky <benjamin.widawsky@intel.com>

Name variables a bit better for copy-pasters. This got turned up as part
of review for upcoming sysfs patches.

CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_sysfs.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index da733a3..c701a17 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -46,32 +46,32 @@ static u32 calc_residency(struct drm_device *dev, const u32 reg)
 }
 
 static ssize_t
-show_rc6_mask(struct device *dev, struct device_attribute *attr, char *buf)
+show_rc6_mask(struct device *kdev, struct device_attribute *attr, char *buf)
 {
-	struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
+	struct drm_minor *dminor = container_of(kdev, struct drm_minor, kdev);
 	return snprintf(buf, PAGE_SIZE, "%x", intel_enable_rc6(dminor->dev));
 }
 
 static ssize_t
-show_rc6_ms(struct device *dev, struct device_attribute *attr, char *buf)
+show_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf)
 {
-	struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
+	struct drm_minor *dminor = container_of(kdev, struct drm_minor, kdev);
 	u32 rc6_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6);
 	return snprintf(buf, PAGE_SIZE, "%u", rc6_residency);
 }
 
 static ssize_t
-show_rc6p_ms(struct device *dev, struct device_attribute *attr, char *buf)
+show_rc6p_ms(struct device *kdev, struct device_attribute *attr, char *buf)
 {
-	struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
+	struct drm_minor *dminor = container_of(kdev, struct drm_minor, kdev);
 	u32 rc6p_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6p);
 	return snprintf(buf, PAGE_SIZE, "%u", rc6p_residency);
 }
 
 static ssize_t
-show_rc6pp_ms(struct device *dev, struct device_attribute *attr, char *buf)
+show_rc6pp_ms(struct device *kdev, struct device_attribute *attr, char *buf)
 {
-	struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
+	struct drm_minor *dminor = container_of(kdev, struct drm_minor, kdev);
 	u32 rc6pp_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6pp);
 	return snprintf(buf, PAGE_SIZE, "%u", rc6pp_residency);
 }
-- 
1.7.12

[PATCH 3/7] drm/i915: #define gpu freq multipler

[Ben Widawsky]

Magic numbers are bad mmmkay. In this case in particular the value is
especially weird because the docs say multiple things. We'll need this
value for sysfs, so extracting it is useful for that as well.

CC: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 22 +++++++++++-----------
 drivers/gpu/drm/i915/i915_drv.h     |  2 ++
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3d886af..73d1f98 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -930,7 +930,7 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
 		seq_printf(m, "Render p-state limit: %d\n",
 			   rp_state_limits & 0xff);
 		seq_printf(m, "CAGF: %dMHz\n", ((rpstat & GEN6_CAGF_MASK) >>
-						GEN6_CAGF_SHIFT) * 50);
+						GEN6_CAGF_SHIFT) * GT_FREQUENCY_MULTIPLIER);
 		seq_printf(m, "RP CUR UP EI: %dus\n", rpupei &
 			   GEN6_CURICONT_MASK);
 		seq_printf(m, "RP CUR UP: %dus\n", rpcurup &
@@ -946,15 +946,15 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
 
 		max_freq = (rp_state_cap & 0xff0000) >> 16;
 		seq_printf(m, "Lowest (RPN) frequency: %dMHz\n",
-			   max_freq * 50);
+			   max_freq * GT_FREQUENCY_MULTIPLIER);
 
 		max_freq = (rp_state_cap & 0xff00) >> 8;
 		seq_printf(m, "Nominal (RP1) frequency: %dMHz\n",
-			   max_freq * 50);
+			   max_freq * GT_FREQUENCY_MULTIPLIER);
 
 		max_freq = rp_state_cap & 0xff;
 		seq_printf(m, "Max non-overclocked (RP0) frequency: %dMHz\n",
-			   max_freq * 50);
+			   max_freq * GT_FREQUENCY_MULTIPLIER);
 	} else {
 		seq_printf(m, "no P-state info available\n");
 	}
@@ -1308,7 +1308,7 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
 			continue;
 		}
 		ia_freq = I915_READ(GEN6_PCODE_DATA);
-		seq_printf(m, "%d\t\t%d\n", gpu_freq * 50, ia_freq * 100);
+		seq_printf(m, "%d\t\t%d\n", gpu_freq * GT_FREQUENCY_MULTIPLIER, ia_freq * 100);
 	}
 
 	mutex_unlock(&dev->struct_mutex);
@@ -1735,7 +1735,7 @@ i915_max_freq_read(struct file *filp,
 		return ret;
 
 	len = snprintf(buf, sizeof(buf),
-		       "max freq: %d\n", dev_priv->rps.max_delay * 50);
+		       "max freq: %d\n", dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER);
 	mutex_unlock(&dev->struct_mutex);
 
 	if (len > sizeof(buf))
@@ -1778,9 +1778,9 @@ i915_max_freq_write(struct file *filp,
 	/*
 	 * Turbo will still be enabled, but won't go above the set value.
 	 */
-	dev_priv->rps.max_delay = val / 50;
+	dev_priv->rps.max_delay = val / GT_FREQUENCY_MULTIPLIER;
 
-	gen6_set_rps(dev, val / 50);
+	gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
 	mutex_unlock(&dev->struct_mutex);
 
 	return cnt;
@@ -1811,7 +1811,7 @@ i915_min_freq_read(struct file *filp, char __user *ubuf, size_t max,
 		return ret;
 
 	len = snprintf(buf, sizeof(buf),
-		       "min freq: %d\n", dev_priv->rps.min_delay * 50);
+		       "min freq: %d\n", dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER);
 	mutex_unlock(&dev->struct_mutex);
 
 	if (len > sizeof(buf))
@@ -1852,9 +1852,9 @@ i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt,
 	/*
 	 * Turbo will still be enabled, but won't go below the set value.
 	 */
-	dev_priv->rps.min_delay = val / 50;
+	dev_priv->rps.min_delay = val / GT_FREQUENCY_MULTIPLIER;
 
-	gen6_set_rps(dev, val / 50);
+	gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
 	mutex_unlock(&dev->struct_mutex);
 
 	return cnt;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f16ab5e..51af543 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1162,6 +1162,8 @@ struct drm_i915_file_private {
 
 #define HAS_L3_GPU_CACHE(dev) (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
 
+#define GT_FREQUENCY_MULTIPLIER 50
+
 #include "i915_trace.h"
 
 /**
-- 
1.7.12

Re: [PATCH 3/7] drm/i915: #define gpu freq multipler

[Jesse Barnes]

On Thu,  6 Sep 2012 13:54:06 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:

> Magic numbers are bad mmmkay. In this case in particular the value is
> especially weird because the docs say multiple things. We'll need this
> value for sysfs, so extracting it is useful for that as well.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 4/7 v2] drm/i915: Add current GPU freq to sysfs

[Ben Widawsky]

Userspace applications such as PowerTOP are interesting in being able to
read the current GPU frequency. The patch itself sets up a generic array
for gen6 attributes so we can easily add other items in the future (and
it also happens to be just about the cleanest way to do this).

The patch is a nice addition to
commit 1ac02185dff3afac146d745ba220dc6672d1d162
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Aug 30 13:26:48 2012 +0200

    drm/i915: add a tracepoint for gpu frequency changes

Reading the GPU frequncy can be done by reading a file like:
/sys/class/drm/card0/render_frequency_mhz

v2: rename the sysfs file to gt_cur_freq_mhz (Daniel)
Use kdev naming (Chris)
Use DEVICE_ATTR over __ATTR (Ben)
Add min/max freq (Ben/Daniel)
user the new #define for frequency multiplier

CC: Arjan van de Ven <arjan@linux.intel.com>
CC: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_sysfs.c | 71 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index c701a17..6c7f905 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -203,6 +203,70 @@ static struct bin_attribute dpf_attrs = {
 	.mmap = NULL
 };
 
+static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+	struct drm_device *dev = minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret;
+
+	ret = i915_mutex_lock_interruptible(dev);
+	if (ret)
+		return ret;
+
+	ret = dev_priv->rps.cur_delay * GT_FREQUENCY_MULTIPLIER;
+	mutex_unlock(&dev->struct_mutex);
+
+	return snprintf(buf, PAGE_SIZE, "%d", ret);
+}
+
+static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+	struct drm_device *dev = minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret;
+
+	ret = i915_mutex_lock_interruptible(dev);
+	if (ret)
+		return ret;
+
+	ret = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER;
+	mutex_unlock(&dev->struct_mutex);
+
+	return snprintf(buf, PAGE_SIZE, "%d", ret);
+}
+
+static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+	struct drm_device *dev = minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret;
+
+	ret = i915_mutex_lock_interruptible(dev);
+	if (ret)
+		return ret;
+
+	ret = dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER;
+	mutex_unlock(&dev->struct_mutex);
+
+	return snprintf(buf, PAGE_SIZE, "%d", ret);
+}
+
+static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, NULL);
+static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, NULL);
+static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, NULL);
+
+static const struct attribute *gen6_attrs[] = {
+	&dev_attr_gt_cur_freq_mhz.attr,
+	&dev_attr_gt_max_freq_mhz.attr,
+	&dev_attr_gt_min_freq_mhz.attr,
+	NULL,
+};
+
+
 void i915_setup_sysfs(struct drm_device *dev)
 {
 	int ret;
@@ -220,10 +284,17 @@ void i915_setup_sysfs(struct drm_device *dev)
 		if (ret)
 			DRM_ERROR("l3 parity sysfs setup failed\n");
 	}
+
+	if (INTEL_INFO(dev)->gen >= 6) {
+		ret = sysfs_create_files(&dev->primary->kdev.kobj, gen6_attrs);
+		if (ret)
+			DRM_ERROR("gen6 sysfs setup failed\n");
+	}
 }
 
 void i915_teardown_sysfs(struct drm_device *dev)
 {
+	sysfs_remove_files(&dev->primary->kdev.kobj, gen6_attrs);
 	device_remove_bin_file(&dev->primary->kdev,  &dpf_attrs);
 	sysfs_unmerge_group(&dev->primary->kdev.kobj, &rc6_attr_group);
 }
-- 
1.7.12

Re: [PATCH 4/7 v2] drm/i915: Add current GPU freq to sysfs

[Ben Widawsky]

On 2012-09-06 14:09, Michael Larabel wrote:
> Have you discussed this with Radeon/Nouveau to see if they would
> adopt a common sysfs interface? In the past, Eugeni Dodonov and I had
> talked about this with a desire to have the open-source drivers 
> expose
> the useful information for monitoring in a similar manner for helping
> userspace (current/max/min clock speeds, voltage, etc). [My personal
> interest is from monitoring this information in a more standardized
> way from the Phoronix Test Suite system monitoring components rather
> than writing driver-specific paths like it is now.]
>
> Martin (CCed) has also expressed interest from the Nouveau side in
> calling for some standardization.
>
> -- Michael

We can... I did very briefly look at what Nouveau does with the hwmon 
interface, and what DRM provided, and thought it'd be more work than I 
wanted to sign up for (which is just about nothing sysfs other than 
display). Thanks for bringing him into the thread though.

>
> On 09/06/2012 03:54 PM, Ben Widawsky wrote:
>> Userspace applications such as PowerTOP are interesting in being 
>> able to
>> read the current GPU frequency. The patch itself sets up a generic 
>> array
>> for gen6 attributes so we can easily add other items in the future 
>> (and
>> it also happens to be just about the cleanest way to do this).
>>
>> The patch is a nice addition to
>> commit 1ac02185dff3afac146d745ba220dc6672d1d162
>> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Date:   Thu Aug 30 13:26:48 2012 +0200
>>
>>      drm/i915: add a tracepoint for gpu frequency changes
>>
>> Reading the GPU frequncy can be done by reading a file like:
>> /sys/class/drm/card0/render_frequency_mhz
>>
>> v2: rename the sysfs file to gt_cur_freq_mhz (Daniel)
>> Use kdev naming (Chris)
>> Use DEVICE_ATTR over __ATTR (Ben)
>> Add min/max freq (Ben/Daniel)
>> user the new #define for frequency multiplier
>>
>> CC: Arjan van de Ven <arjan@linux.intel.com>
>> CC: Jacob Pan <jacob.jun.pan@linux.intel.com>
>> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
>> ---
>>   drivers/gpu/drm/i915/i915_sysfs.c | 71 
>> +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 71 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
>> b/drivers/gpu/drm/i915/i915_sysfs.c
>> index c701a17..6c7f905 100644
>> --- a/drivers/gpu/drm/i915/i915_sysfs.c
>> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
>> @@ -203,6 +203,70 @@ static struct bin_attribute dpf_attrs = {
>>   	.mmap = NULL
>>   };
>>   +static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
>> +				    struct device_attribute *attr, char *buf)
>> +{
>> +	struct drm_minor *minor = container_of(kdev, struct drm_minor, 
>> kdev);
>> +	struct drm_device *dev = minor->dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	int ret;
>> +
>> +	ret = i915_mutex_lock_interruptible(dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = dev_priv->rps.cur_delay * GT_FREQUENCY_MULTIPLIER;
>> +	mutex_unlock(&dev->struct_mutex);
>> +
>> +	return snprintf(buf, PAGE_SIZE, "%d", ret);
>> +}
>> +
>> +static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct 
>> device_attribute *attr, char *buf)
>> +{
>> +	struct drm_minor *minor = container_of(kdev, struct drm_minor, 
>> kdev);
>> +	struct drm_device *dev = minor->dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	int ret;
>> +
>> +	ret = i915_mutex_lock_interruptible(dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER;
>> +	mutex_unlock(&dev->struct_mutex);
>> +
>> +	return snprintf(buf, PAGE_SIZE, "%d", ret);
>> +}
>> +
>> +static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct 
>> device_attribute *attr, char *buf)
>> +{
>> +	struct drm_minor *minor = container_of(kdev, struct drm_minor, 
>> kdev);
>> +	struct drm_device *dev = minor->dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	int ret;
>> +
>> +	ret = i915_mutex_lock_interruptible(dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER;
>> +	mutex_unlock(&dev->struct_mutex);
>> +
>> +	return snprintf(buf, PAGE_SIZE, "%d", ret);
>> +}
>> +
>> +static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, 
>> NULL);
>> +static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, 
>> gt_max_freq_mhz_show, NULL);
>> +static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, 
>> gt_min_freq_mhz_show, NULL);
>> +
>> +static const struct attribute *gen6_attrs[] = {
>> +	&dev_attr_gt_cur_freq_mhz.attr,
>> +	&dev_attr_gt_max_freq_mhz.attr,
>> +	&dev_attr_gt_min_freq_mhz.attr,
>> +	NULL,
>> +};
>> +
>> +
>>   void i915_setup_sysfs(struct drm_device *dev)
>>   {
>>   	int ret;
>> @@ -220,10 +284,17 @@ void i915_setup_sysfs(struct drm_device *dev)
>>   		if (ret)
>>   			DRM_ERROR("l3 parity sysfs setup failed\n");
>>   	}
>> +
>> +	if (INTEL_INFO(dev)->gen >= 6) {
>> +		ret = sysfs_create_files(&dev->primary->kdev.kobj, gen6_attrs);
>> +		if (ret)
>> +			DRM_ERROR("gen6 sysfs setup failed\n");
>> +	}
>>   }
>>   void i915_teardown_sysfs(struct drm_device *dev)
>>   {
>> +	sysfs_remove_files(&dev->primary->kdev.kobj, gen6_attrs);
>>   	device_remove_bin_file(&dev->primary->kdev,  &dpf_attrs);
>>   	sysfs_unmerge_group(&dev->primary->kdev.kobj, &rc6_attr_group);
>>   }

-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH 5/7] drm/i915: Add setters for min/max frequency

[Ben Widawsky]

Provide a standardized sysfs interface for setting min, and max
frequencies.  The code which reads the limits were lifted from the
debugfs files. As a brief explanation, the limits are similar to the CPU
p-states. We have 3 states:

RP0 - ie. max frequency
RP1 - ie. "local min" frequency
RPn - seriously lowest frequency

Initially Daniel asked me to clamp the writes to supported values, but
in conforming to the way the cpufreq drivers seem to work, instead
return -EINVAL (noticed by Jesse in discussion).

The values can be used by userspace wishing to control the limits of the
GPU (see the CC list for people who care).

CC: Arjan van de Ven <arjan@linux.intel.com>
CC: Jacob Pan <jacob.jun.pan@linux.intel.com>
CC: Daniel Vetter <daniel.vetter@ffwll.ch>
CC: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_sysfs.c | 78 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 76 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 6c7f905..5a5e610 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -238,6 +238,43 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute
 	return snprintf(buf, PAGE_SIZE, "%d", ret);
 }
 
+static ssize_t gt_max_freq_mhz_store(struct device *kdev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+	struct drm_device *dev = minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val, rp_state_cap, hw_max;
+	ssize_t ret;
+
+	ret = kstrtou32(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+	hw_max = (rp_state_cap & 0xff) * GT_FREQUENCY_MULTIPLIER;
+
+	if (val > hw_max)
+		return -EINVAL;
+
+	if (val < dev_priv->rps.min_delay)
+		val = dev_priv->rps.min_delay;
+
+	if (dev_priv->rps.cur_delay > val)
+		gen6_set_rps(dev_priv->dev, val);
+
+	dev_priv->rps.max_delay = val / GT_FREQUENCY_MULTIPLIER;
+
+	mutex_unlock(&dev->struct_mutex);
+
+	return count;
+}
+
 static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
 {
 	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
@@ -255,9 +292,46 @@ static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute
 	return snprintf(buf, PAGE_SIZE, "%d", ret);
 }
 
+static ssize_t gt_min_freq_mhz_store(struct device *kdev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+	struct drm_device *dev = minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val, rp_state_cap, hw_min;
+	ssize_t ret;
+
+	ret = kstrtou32(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+	hw_min = ((rp_state_cap & 0xff0000) >> 16) * GT_FREQUENCY_MULTIPLIER;
+
+	if (val < hw_min)
+		return -EINVAL;
+
+	if (val > dev_priv->rps.max_delay)
+		val = dev_priv->rps.max_delay;
+
+	if (dev_priv->rps.cur_delay < val)
+		gen6_set_rps(dev_priv->dev, val);
+
+	dev_priv->rps.min_delay = val / GT_FREQUENCY_MULTIPLIER;
+
+	mutex_unlock(&dev->struct_mutex);
+
+	return count;
+}
+
 static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, NULL);
-static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, NULL);
-static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, NULL);
+static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, gt_max_freq_mhz_store);
+static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, gt_min_freq_mhz_store);
 
 static const struct attribute *gen6_attrs[] = {
 	&dev_attr_gt_cur_freq_mhz.attr,
-- 
1.7.12

Re: [PATCH 5/7] drm/i915: Add setters for min/max frequency

[Jesse Barnes]

On Thu,  6 Sep 2012 13:54:08 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:

> Provide a standardized sysfs interface for setting min, and max
> frequencies.  The code which reads the limits were lifted from the
> debugfs files. As a brief explanation, the limits are similar to the CPU
> p-states. We have 3 states:
> 
> RP0 - ie. max frequency
> RP1 - ie. "local min" frequency

I'd say "preferred min" or "vmin" here instead.

> RPn - seriously lowest frequency
> 
> Initially Daniel asked me to clamp the writes to supported values, but
> in conforming to the way the cpufreq drivers seem to work, instead
> return -EINVAL (noticed by Jesse in discussion).
> 
> The values can be used by userspace wishing to control the limits of the
> GPU (see the CC list for people who care).

Can you add this and any other i915 bits that are missing to the sysfs
documentation under Documentation/?

Thanks for moving to -EINVAL, it makes a lot more sense to me.

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 5/7 v2] drm/i915: Add setters for min/max frequency

[Ben Widawsky]

Provide a standardized sysfs interface for setting min, and max
frequencies.  The code which reads the limits were lifted from the
debugfs files. As a brief explanation, the limits are similar to the CPU
p-states. We have 3 states:

RP0 - ie. max frequency
RP1 - ie. "preferred min" frequency
RPn - seriously lowest frequency

Initially Daniel asked me to clamp the writes to supported values, but
in conforming to the way the cpufreq drivers seem to work, instead
return -EINVAL (noticed by Jesse in discussion).

The values can be used by userspace wishing to control the limits of the
GPU (see the CC list for people who care).

v2: add the dropped mutex_unlock in error cases (Chris)
EINVAL on both too min, or too max (Daniel)

CC: Arjan van de Ven <arjan@linux.intel.com>
CC: Jacob Pan <jacob.jun.pan@linux.intel.com>
CC: Daniel Vetter <daniel.vetter@ffwll.ch>
CC: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_sysfs.c | 84 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 6c7f905..4f470cf 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -238,6 +238,46 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute
 	return snprintf(buf, PAGE_SIZE, "%d", ret);
 }
 
+static ssize_t gt_max_freq_mhz_store(struct device *kdev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+	struct drm_device *dev = minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val, rp_state_cap, hw_max, hw_min;
+	ssize_t ret;
+
+	ret = kstrtou32(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+	hw_max = (rp_state_cap & 0xff) * GT_FREQUENCY_MULTIPLIER;
+	hw_min = ((rp_state_cap & 0xff0000) >> 16) * GT_FREQUENCY_MULTIPLIER;
+
+	if (val < hw_min || val > hw_max) {
+		mutex_unlock(&dev->struct_mutex);
+		return -EINVAL;
+	}
+
+	if (val < dev_priv->rps.min_delay)
+		val = dev_priv->rps.min_delay;
+
+	if (dev_priv->rps.cur_delay > val)
+		gen6_set_rps(dev_priv->dev, val);
+
+	dev_priv->rps.max_delay = val / GT_FREQUENCY_MULTIPLIER;
+
+	mutex_unlock(&dev->struct_mutex);
+
+	return count;
+}
+
 static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
 {
 	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
@@ -255,9 +295,49 @@ static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute
 	return snprintf(buf, PAGE_SIZE, "%d", ret);
 }
 
+static ssize_t gt_min_freq_mhz_store(struct device *kdev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+	struct drm_device *dev = minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val, rp_state_cap, hw_max, hw_min;
+	ssize_t ret;
+
+	ret = kstrtou32(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+	hw_max = (rp_state_cap & 0xff) * GT_FREQUENCY_MULTIPLIER;
+	hw_min = ((rp_state_cap & 0xff0000) >> 16) * GT_FREQUENCY_MULTIPLIER;
+
+	if (val < hw_min || val > hw_max) {
+		mutex_unlock(&dev->struct_mutex);
+		return -EINVAL;
+	}
+
+	if (val > dev_priv->rps.max_delay)
+		val = dev_priv->rps.max_delay;
+
+	if (dev_priv->rps.cur_delay < val)
+		gen6_set_rps(dev_priv->dev, val);
+
+	dev_priv->rps.min_delay = val / GT_FREQUENCY_MULTIPLIER;
+
+	mutex_unlock(&dev->struct_mutex);
+
+	return count;
+}
+
 static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, NULL);
-static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, NULL);
-static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, NULL);
+static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, gt_max_freq_mhz_store);
+static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, gt_min_freq_mhz_store);
 
 static const struct attribute *gen6_attrs[] = {
 	&dev_attr_gt_cur_freq_mhz.attr,
-- 
1.7.12

Re: [PATCH 5/7 v2] drm/i915: Add setters for min/max frequency

[Chris Wilson]

The only bit of the series I don't like is the conversion to uJ which
looks like it can overflow. Otherwise,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 6/7] drm/i915: Show render P state thresholds in sysfs

[Ben Widawsky]

This is useful for userspace utilities which wish to use the previous
interface, specifically for micromanaging the increase/decrease steps by
setting min == max.

CC: Jacob Pan <jacob.jun.pan@linux.intel.com>
CC: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_sysfs.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 5a5e610..e3a31ae 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -333,10 +333,46 @@ static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, NULL);
 static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, gt_max_freq_mhz_store);
 static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, gt_min_freq_mhz_store);
 
+
+static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf);
+static DEVICE_ATTR(gt_RP0_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
+static DEVICE_ATTR(gt_RP1_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
+static DEVICE_ATTR(gt_RPn_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
+
+/* For now we have a static number of RP states */
+static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
+	struct drm_device *dev = minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val, rp_state_cap;
+	ssize_t ret;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+	mutex_unlock(&dev->struct_mutex);
+
+	if (attr == &dev_attr_gt_RP0_freq_mhz) {
+		val = ((rp_state_cap & 0x0000ff) >> 0) * GT_FREQUENCY_MULTIPLIER;
+	} else if (attr == &dev_attr_gt_RP1_freq_mhz) {
+		val = ((rp_state_cap & 0x00ff00) >> 8) * GT_FREQUENCY_MULTIPLIER;
+	} else if (attr == &dev_attr_gt_RPn_freq_mhz) {
+		val = ((rp_state_cap & 0xff0000) >> 16) * GT_FREQUENCY_MULTIPLIER;
+	} else {
+		BUG();
+	}
+	return snprintf(buf, PAGE_SIZE, "%d", val);
+}
+
 static const struct attribute *gen6_attrs[] = {
 	&dev_attr_gt_cur_freq_mhz.attr,
 	&dev_attr_gt_max_freq_mhz.attr,
 	&dev_attr_gt_min_freq_mhz.attr,
+	&dev_attr_gt_RP0_freq_mhz.attr,
+	&dev_attr_gt_RP1_freq_mhz.attr,
+	&dev_attr_gt_RPn_freq_mhz.attr,
 	NULL,
 };
 
-- 
1.7.12

Re: [PATCH 6/7] drm/i915: Show render P state thresholds in sysfs

[Jesse Barnes]

On Thu,  6 Sep 2012 13:54:09 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:

> This is useful for userspace utilities which wish to use the previous
> interface, specifically for micromanaging the increase/decrease steps by
> setting min == max.
> 
> CC: Jacob Pan <jacob.jun.pan@linux.intel.com>
> CC: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_sysfs.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index 5a5e610..e3a31ae 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -333,10 +333,46 @@ static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, NULL);
>  static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, gt_max_freq_mhz_store);
>  static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, gt_min_freq_mhz_store);
>  
> +
> +static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf);
> +static DEVICE_ATTR(gt_RP0_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> +static DEVICE_ATTR(gt_RP1_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> +static DEVICE_ATTR(gt_RPn_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> +
> +/* For now we have a static number of RP states */
> +static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
> +{
> +	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
> +	struct drm_device *dev = minor->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u32 val, rp_state_cap;
> +	ssize_t ret;
> +
> +	ret = mutex_lock_interruptible(&dev->struct_mutex);
> +	if (ret)
> +		return ret;
> +	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> +	mutex_unlock(&dev->struct_mutex);
> +
> +	if (attr == &dev_attr_gt_RP0_freq_mhz) {
> +		val = ((rp_state_cap & 0x0000ff) >> 0) * GT_FREQUENCY_MULTIPLIER;
> +	} else if (attr == &dev_attr_gt_RP1_freq_mhz) {
> +		val = ((rp_state_cap & 0x00ff00) >> 8) * GT_FREQUENCY_MULTIPLIER;
> +	} else if (attr == &dev_attr_gt_RPn_freq_mhz) {
> +		val = ((rp_state_cap & 0xff0000) >> 16) * GT_FREQUENCY_MULTIPLIER;
> +	} else {
> +		BUG();
> +	}
> +	return snprintf(buf, PAGE_SIZE, "%d", val);
> +}
> +
>  static const struct attribute *gen6_attrs[] = {
>  	&dev_attr_gt_cur_freq_mhz.attr,
>  	&dev_attr_gt_max_freq_mhz.attr,
>  	&dev_attr_gt_min_freq_mhz.attr,
> +	&dev_attr_gt_RP0_freq_mhz.attr,
> +	&dev_attr_gt_RP1_freq_mhz.attr,
> +	&dev_attr_gt_RPn_freq_mhz.attr,
>  	NULL,
>  };
>  

If you add documentation:
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 7/7] drm/i915: expose energy counter on SNB and IVB

[Ben Widawsky]

From: Jesse Barnes <jbarnes@virtuousgeek.org>

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 sysfs
to make it easy to do power comparisons with different configurations.

If the platform supports it, the file will show up under the
drm/card0/power subdirectory of the PCI device in sysfs as gt_energy_uJ.
The value in the file is a running total of energy (in microjoules)
consumed by the graphics device.

v2: move to sysfs (Ben, Daniel)
    expose a simple value (Chris)
    drop unrelated hunk (Ben)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

v3: by Ben
Tied it into existing rc6  sysfs entries and named that a more generic
"power attrs." Fixed rebase conflicts.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_reg.h   |  2 ++
 drivers/gpu/drm/i915/i915_sysfs.c | 25 +++++++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d0b60f2..0e34f5f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1242,6 +1242,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
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index e3a31ae..6bbc9af 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -76,22 +76,43 @@ show_rc6pp_ms(struct device *kdev, struct device_attribute *attr, char *buf)
 	return snprintf(buf, PAGE_SIZE, "%u", rc6pp_residency);
 }
 
+#define MSR_IA32_PACKAGE_POWER_SKU_UNIT		0x00000606
+
+static ssize_t
+show_gt_energy_uJ(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
+	struct drm_i915_private *dev_priv = dminor->dev->dev_private;
+	u64 ppsu;
+	u32 val, units;
+
+	rdmsrl(MSR_IA32_PACKAGE_POWER_SKU_UNIT, ppsu);
+
+	ppsu = (ppsu & 0x1f00) >> 8;
+	units = 1000000 / (1 << ppsu); /* convert to uJ */
+	val = I915_READ(SECP_NRG_STTS);
+
+	return snprintf(buf, PAGE_SIZE, "%u", val * units);
+}
+
 static DEVICE_ATTR(rc6_enable, S_IRUGO, show_rc6_mask, NULL);
 static DEVICE_ATTR(rc6_residency_ms, S_IRUGO, show_rc6_ms, NULL);
 static DEVICE_ATTR(rc6p_residency_ms, S_IRUGO, show_rc6p_ms, NULL);
 static DEVICE_ATTR(rc6pp_residency_ms, S_IRUGO, show_rc6pp_ms, NULL);
+static DEVICE_ATTR(gt_energy_uJ, S_IRUGO, show_gt_energy_uJ, NULL);
 
-static struct attribute *rc6_attrs[] = {
+static struct attribute *power_attrs[] = {
 	&dev_attr_rc6_enable.attr,
 	&dev_attr_rc6_residency_ms.attr,
 	&dev_attr_rc6p_residency_ms.attr,
 	&dev_attr_rc6pp_residency_ms.attr,
+	&dev_attr_gt_energy_uJ.attr,
 	NULL
 };
 
 static struct attribute_group rc6_attr_group = {
 	.name = power_group_name,
-	.attrs =  rc6_attrs
+	.attrs =  power_attrs
 };
 #endif
 
-- 
1.7.12

Re: [PATCH 7/7] drm/i915: expose energy counter on SNB and IVB

[Daniel Vetter]

On Thu, Sep 06, 2012 at 01:54:10PM -0700, Ben Widawsky wrote:
> From: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> 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 sysfs
> to make it easy to do power comparisons with different configurations.
> 
> If the platform supports it, the file will show up under the
> drm/card0/power subdirectory of the PCI device in sysfs as gt_energy_uJ.
> The value in the file is a running total of energy (in microjoules)
> consumed by the graphics device.
> 
> v2: move to sysfs (Ben, Daniel)
>     expose a simple value (Chris)
>     drop unrelated hunk (Ben)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> v3: by Ben
> Tied it into existing rc6  sysfs entries and named that a more generic
> "power attrs." Fixed rebase conflicts.
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_reg.h   |  2 ++
>  drivers/gpu/drm/i915/i915_sysfs.c | 25 +++++++++++++++++++++++--
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d0b60f2..0e34f5f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1242,6 +1242,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
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index e3a31ae..6bbc9af 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -76,22 +76,43 @@ show_rc6pp_ms(struct device *kdev, struct device_attribute *attr, char *buf)
>  	return snprintf(buf, PAGE_SIZE, "%u", rc6pp_residency);
>  }
>  
> +#define MSR_IA32_PACKAGE_POWER_SKU_UNIT		0x00000606
> +
> +static ssize_t
> +show_gt_energy_uJ(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
> +	struct drm_i915_private *dev_priv = dminor->dev->dev_private;
> +	u64 ppsu;
> +	u32 val, units;
> +
> +	rdmsrl(MSR_IA32_PACKAGE_POWER_SKU_UNIT, ppsu);
> +
> +	ppsu = (ppsu & 0x1f00) >> 8;
> +	units = 1000000 / (1 << ppsu); /* convert to uJ */
> +	val = I915_READ(SECP_NRG_STTS);
> +
> +	return snprintf(buf, PAGE_SIZE, "%u", val * units);

Besides that this could overflow it also introduces rounding errors that
accumulate. I guess we need a 64 mul+div here, like in the residency
counters.
-Daniel

> +}
> +
>  static DEVICE_ATTR(rc6_enable, S_IRUGO, show_rc6_mask, NULL);
>  static DEVICE_ATTR(rc6_residency_ms, S_IRUGO, show_rc6_ms, NULL);
>  static DEVICE_ATTR(rc6p_residency_ms, S_IRUGO, show_rc6p_ms, NULL);
>  static DEVICE_ATTR(rc6pp_residency_ms, S_IRUGO, show_rc6pp_ms, NULL);
> +static DEVICE_ATTR(gt_energy_uJ, S_IRUGO, show_gt_energy_uJ, NULL);
>  
> -static struct attribute *rc6_attrs[] = {
> +static struct attribute *power_attrs[] = {
>  	&dev_attr_rc6_enable.attr,
>  	&dev_attr_rc6_residency_ms.attr,
>  	&dev_attr_rc6p_residency_ms.attr,
>  	&dev_attr_rc6pp_residency_ms.attr,
> +	&dev_attr_gt_energy_uJ.attr,
>  	NULL
>  };
>  
>  static struct attribute_group rc6_attr_group = {
>  	.name = power_group_name,
> -	.attrs =  rc6_attrs
> +	.attrs =  power_attrs
>  };
>  #endif
>  
> -- 
> 1.7.12
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 7/7] drm/i915: expose energy counter on SNB and IVB

[Ben Widawsky]

On Fri, 7 Sep 2012 12:03:37 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Thu, Sep 06, 2012 at 01:54:10PM -0700, Ben Widawsky wrote:
> > From: Jesse Barnes <jbarnes@virtuousgeek.org>
> > 
> > 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 sysfs
> > to make it easy to do power comparisons with different configurations.
> > 
> > If the platform supports it, the file will show up under the
> > drm/card0/power subdirectory of the PCI device in sysfs as gt_energy_uJ.
> > The value in the file is a running total of energy (in microjoules)
> > consumed by the graphics device.
> > 
> > v2: move to sysfs (Ben, Daniel)
> >     expose a simple value (Chris)
> >     drop unrelated hunk (Ben)
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > 
> > v3: by Ben
> > Tied it into existing rc6  sysfs entries and named that a more generic
> > "power attrs." Fixed rebase conflicts.
> > 
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h   |  2 ++
> >  drivers/gpu/drm/i915/i915_sysfs.c | 25 +++++++++++++++++++++++--
> >  2 files changed, 25 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index d0b60f2..0e34f5f 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1242,6 +1242,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
> > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> > index e3a31ae..6bbc9af 100644
> > --- a/drivers/gpu/drm/i915/i915_sysfs.c
> > +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> > @@ -76,22 +76,43 @@ show_rc6pp_ms(struct device *kdev, struct device_attribute *attr, char *buf)
> >  	return snprintf(buf, PAGE_SIZE, "%u", rc6pp_residency);
> >  }
> >  
> > +#define MSR_IA32_PACKAGE_POWER_SKU_UNIT		0x00000606
> > +
> > +static ssize_t
> > +show_gt_energy_uJ(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > +	struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
> > +	struct drm_i915_private *dev_priv = dminor->dev->dev_private;
> > +	u64 ppsu;
> > +	u32 val, units;
> > +
> > +	rdmsrl(MSR_IA32_PACKAGE_POWER_SKU_UNIT, ppsu);
> > +
> > +	ppsu = (ppsu & 0x1f00) >> 8;
> > +	units = 1000000 / (1 << ppsu); /* convert to uJ */
> > +	val = I915_READ(SECP_NRG_STTS);
> > +
> > +	return snprintf(buf, PAGE_SIZE, "%u", val * units);
> 
> Besides that this could overflow it also introduces rounding errors that
> accumulate. I guess we need a 64 mul+div here, like in the residency
> counters.
> -Daniel
> 

Jesse, I'm not fixing this one for you since I screwed up the residency
math enough times before.

> > +}
> > +
> >  static DEVICE_ATTR(rc6_enable, S_IRUGO, show_rc6_mask, NULL);
> >  static DEVICE_ATTR(rc6_residency_ms, S_IRUGO, show_rc6_ms, NULL);
> >  static DEVICE_ATTR(rc6p_residency_ms, S_IRUGO, show_rc6p_ms, NULL);
> >  static DEVICE_ATTR(rc6pp_residency_ms, S_IRUGO, show_rc6pp_ms, NULL);
> > +static DEVICE_ATTR(gt_energy_uJ, S_IRUGO, show_gt_energy_uJ, NULL);
> >  
> > -static struct attribute *rc6_attrs[] = {
> > +static struct attribute *power_attrs[] = {
> >  	&dev_attr_rc6_enable.attr,
> >  	&dev_attr_rc6_residency_ms.attr,
> >  	&dev_attr_rc6p_residency_ms.attr,
> >  	&dev_attr_rc6pp_residency_ms.attr,
> > +	&dev_attr_gt_energy_uJ.attr,
> >  	NULL
> >  };
> >  
> >  static struct attribute_group rc6_attr_group = {
> >  	.name = power_group_name,
> > -	.attrs =  rc6_attrs
> > +	.attrs =  power_attrs
> >  };
> >  #endif
> >  
> > -- 
> > 1.7.12
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 



-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 7/7] drm/i915: expose energy counter on SNB and IVB

[Chris Wilson]

On Fri, Sep 07, 2012 at 10:51:44AM -0700, Ben Widawsky wrote:
> On Fri, 7 Sep 2012 12:03:37 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Thu, Sep 06, 2012 at 01:54:10PM -0700, Ben Widawsky wrote:
> > > From: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > 
> > > 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 sysfs
> > > to make it easy to do power comparisons with different configurations.
> > > 
> > > If the platform supports it, the file will show up under the
> > > drm/card0/power subdirectory of the PCI device in sysfs as gt_energy_uJ.
> > > The value in the file is a running total of energy (in microjoules)
> > > consumed by the graphics device.
> > > 
> > > v2: move to sysfs (Ben, Daniel)
> > >     expose a simple value (Chris)
> > >     drop unrelated hunk (Ben)
> > > 
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > 
> > > v3: by Ben
> > > Tied it into existing rc6  sysfs entries and named that a more generic
> > > "power attrs." Fixed rebase conflicts.
> > > 
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

A reminder that we never finished this patch.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 7/7] drm/i915: expose energy counter on SNB and IVB

[Ben Widawsky]

On Wed, Apr 10, 2013 at 03:27:05PM +0100, Chris Wilson wrote:
> On Fri, Sep 07, 2012 at 10:51:44AM -0700, Ben Widawsky wrote:
> > On Fri, 7 Sep 2012 12:03:37 +0200
> > Daniel Vetter <daniel@ffwll.ch> wrote:
> > 
> > > On Thu, Sep 06, 2012 at 01:54:10PM -0700, Ben Widawsky wrote:
> > > > From: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > > 
> > > > 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 sysfs
> > > > to make it easy to do power comparisons with different configurations.
> > > > 
> > > > If the platform supports it, the file will show up under the
> > > > drm/card0/power subdirectory of the PCI device in sysfs as gt_energy_uJ.
> > > > The value in the file is a running total of energy (in microjoules)
> > > > consumed by the graphics device.
> > > > 
> > > > v2: move to sysfs (Ben, Daniel)
> > > >     expose a simple value (Chris)
> > > >     drop unrelated hunk (Ben)
> > > > 
> > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > > 
> > > > v3: by Ben
> > > > Tied it into existing rc6  sysfs entries and named that a more generic
> > > > "power attrs." Fixed rebase conflicts.
> > > > 
> > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> 
> A reminder that we never finished this patch.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

This will be supported in the RAPL driver, so I think we don't need it.
The latest version of that can be found:
https://lkml.org/lkml/2013/4/9/812

-- 
Ben Widawsky, Intel Open Source Technology Center