[PATCH v2 1/3] drm/i915: add rc6 residency times to debugfs

[PATCH v2 1/3] drm/i915: add rc6 residency times to debugfs

[Ben Widawsky]

RC6 residency should be in intervals of 1.28us, and the counter wraps.
Here is an example using awk to get the various RC6 and RC6+ residency
times in seconds, since boot.

cat /sys/kernel/debug/dri/0/i915_drpc_info  | grep residency | awk -F':' -F' '  '{print $5 * 1.28 / 1000000}'

This is primarily for debug, and QA/application developers using the
sysfs interface looking for more insight.

Untested on IVB.

v2: move comment to the correct place
commeit message changes

CC: Ouping Zhang <ouping.zhang@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by (v1): Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-off-by (v1): Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   11 +++++++++++
 drivers/gpu/drm/i915/i915_reg.h     |    5 +++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 66c90d4..4257151 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1133,6 +1133,17 @@ static int gen6_drpc_info(struct seq_file *m)
 
 	seq_printf(m, "Core Power Down: %s\n",
 		   yesno(gt_core_status & GEN6_CORE_CPD_STATE_MASK));
+
+	/* Not exactly sure what this is */
+	seq_printf(m, "RC6 \"Locked to RPn\" residency since boot: %d\n",
+		   I915_READ(GEN6_GT_GFX_RC6_LOCKED));
+	seq_printf(m, "RC6 residency since boot: %d\n",
+		   I915_READ(GEN6_GT_GFX_RC6));
+	seq_printf(m, "RC6+ residency since boot: %d\n",
+		   I915_READ(GEN6_GT_GFX_RC6p));
+	seq_printf(m, "RC6++ residency since boot: %d\n",
+		   I915_READ(GEN6_GT_GFX_RC6pp));
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f3609f2..b1c3d35 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3799,6 +3799,11 @@
 						 GEN6_PM_RP_DOWN_THRESHOLD | \
 						 GEN6_PM_RP_DOWN_TIMEOUT)
 
+#define GEN6_GT_GFX_RC6_LOCKED			0x138104
+#define GEN6_GT_GFX_RC6				0x138108
+#define GEN6_GT_GFX_RC6p			0x13810C
+#define GEN6_GT_GFX_RC6pp			0x138110
+
 #define GEN6_PCODE_MAILBOX			0x138124
 #define   GEN6_PCODE_READY			(1<<31)
 #define   GEN6_READ_OC_PARAMS			0xc
-- 
1.7.9.4

[PATCH 2/3] drm/i915: extract intel_enable_rc6()

[Ben Widawsky]

This function tells us the state of RC6. In the future perhaps it will
even give something like a mask back telling which RC6 types are
enabled.

In order to return sane values to userspace for an upcoming RC6 sysfs
interface (since register behavior seems to be somewhat random), we will
query this value, and therefore it needs to be available to the rest of
the driver.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |    2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bbbc1a4..22ab4db 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1414,6 +1414,7 @@ extern void ironlake_enable_rc6(struct drm_device *dev);
 extern void gen6_set_rps(struct drm_device *dev, u8 val);
 extern void intel_detect_pch(struct drm_device *dev);
 extern int intel_trans_dp_port_sel(struct drm_crtc *crtc);
+extern int intel_enable_rc6(const struct drm_device *dev);
 
 extern void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv);
 extern void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ab62c96..efbf709 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8235,7 +8235,7 @@ void intel_init_emon(struct drm_device *dev)
 	dev_priv->corr = (lcfuse & LCFUSE_HIV_MASK);
 }
 
-static int intel_enable_rc6(struct drm_device *dev)
+int intel_enable_rc6(const struct drm_device *dev)
 {
 	/*
 	 * Respect the kernel parameter if it is set
-- 
1.7.9.4

[PATCH v2 3/3] drm/i915: rc6 in sysfs

[Ben Widawsky]

Merge rc6 information into the power group for our device. Until now the
i915 driver has not had any sysfs entries (aside from the connector
stuff enabled by drm core). Since it seems like we're likely to have
more in the future I created a new file for sysfs stubs, as well as the
rc6 sysfs functions which don't really belong elsewhere (perhaps
i915_suspend, but most of the stuff is in intel_display,c).

displays #ms GPU has been in rc6 since boot:
cat /sys/class/drm/card0/power/rc6

displays #ms GPU has been in deep rc6 since boot:
cat /sys/class/drm/card0/power/rc6p

displays #ms GPU has been in deepest rc6 since boot:
cat /sys/class/drm/card0/power/rc6pp

Important note: I've seen on SNB that even when RC6 is *not* enabled the
rc6 register seems to have a random value in it. I cannot explain a
reason for this. Those writing tools that utilize this value need to be
careful and probably want to scrutinize the value very carefully.

Please see intel-gpu-tools patches for sample code.

Untested on IVB.

v2: use calc_residency units to milliseconds for the other RC6 types

CC: Arjan van de Ven <arjan@linux.intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/Makefile     |    1 +
 drivers/gpu/drm/i915/i915_dma.c   |    4 ++
 drivers/gpu/drm/i915/i915_drv.h   |    4 ++
 drivers/gpu/drm/i915/i915_sysfs.c |  100 +++++++++++++++++++++++++++++++++++++
 4 files changed, 109 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/i915_sysfs.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index ce7fc77..f801330 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -12,6 +12,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \
 	  i915_gem_execbuffer.o \
 	  i915_gem_gtt.o \
 	  i915_gem_tiling.o \
+	  i915_sysfs.o \
 	  i915_trace_points.o \
 	  intel_display.o \
 	  intel_crt.o \
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index fdff009..64dfbb8 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2113,6 +2113,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 		}
 	}
 
+	i915_setup_sysfs(dev);
+
 	/* Must be done after probing outputs */
 	intel_opregion_init(dev);
 	acpi_video_register();
@@ -2164,6 +2166,8 @@ int i915_driver_unload(struct drm_device *dev)
 	i915_mch_dev = NULL;
 	spin_unlock(&mchdev_lock);
 
+	i915_teardown_sysfs(dev);
+
 	if (dev_priv->mm.inactive_shrinker.shrink)
 		unregister_shrinker(&dev_priv->mm.inactive_shrinker);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 22ab4db..e267774 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1365,6 +1365,10 @@ extern int i915_restore_state(struct drm_device *dev);
 extern int i915_save_state(struct drm_device *dev);
 extern int i915_restore_state(struct drm_device *dev);
 
+/* i915_sysfs.c */
+void i915_setup_sysfs(struct drm_device *dev_priv);
+void i915_teardown_sysfs(struct drm_device *dev_priv);
+
 /* intel_i2c.c */
 extern int intel_setup_gmbus(struct drm_device *dev);
 extern void intel_teardown_gmbus(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
new file mode 100644
index 0000000..125d0cc
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -0,0 +1,100 @@
+/*
+ * Copyright © 2012 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Ben Widawsky <ben@bwidawsk.net>
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/stat.h>
+#include <linux/sysfs.h>
+#include "i915_drv.h"
+
+static u32 calc_residency(struct drm_device *dev, const u32 reg)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u64 raw_time; /* 32b value may overflow during fixed point math */
+	u32 residency;
+
+	if (!intel_enable_rc6(dev))
+		return 0;
+
+	raw_time = I915_READ(reg) * 128ULL;
+	residency = DIV_ROUND_CLOSEST(raw_time, 1000) / 100;
+	return residency;
+}
+
+static ssize_t
+show_rc6_ms(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct drm_minor *dminor = container_of(dev, 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)
+{
+	struct drm_minor *dminor = container_of(dev, 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)
+{
+	struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
+	u32 rc6pp_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6pp);
+	return snprintf(buf, PAGE_SIZE, "%u", rc6pp_residency);
+}
+
+static DEVICE_ATTR(rc6, S_IRUGO, show_rc6_ms, NULL);
+static DEVICE_ATTR(rc6p, S_IRUGO, show_rc6p_ms, NULL);
+static DEVICE_ATTR(rc6pp, S_IRUGO, show_rc6pp_ms, NULL);
+
+static struct attribute *rc6_attrs[] = {
+	&dev_attr_rc6.attr,
+	&dev_attr_rc6p.attr,
+	&dev_attr_rc6pp.attr,
+	NULL
+};
+
+static struct attribute_group rc6_attr_group = {
+	.name = power_group_name,
+	.attrs =  rc6_attrs
+};
+
+void i915_setup_sysfs(struct drm_device *dev)
+{
+	int ret;
+
+	ret = sysfs_merge_group(&dev->primary->kdev.kobj, &rc6_attr_group);
+	if (ret)
+		DRM_ERROR("sysfs setup failed\n");
+}
+
+void i915_teardown_sysfs(struct drm_device *dev)
+{
+	sysfs_remove_group(&dev->primary->kdev.kobj, &rc6_attr_group);
+}
-- 
1.7.9.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/3] build: make sure we have asprintf

[Ben Widawsky]

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 configure.ac |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure.ac b/configure.ac
index f778a9a..567f10d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -32,6 +32,7 @@ AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_USE_SYSTEM_EXTENSIONS
 AC_SYS_LARGEFILE
+AC_GNU_SOURCE
 
 AM_INIT_AUTOMAKE([foreign dist-bzip2])
 AM_PATH_PYTHON([3],, [:])
@@ -43,6 +44,7 @@ AC_CHECK_MEMBERS([struct sysinfo.totalram],[],[],[AC_INCLUDES_DEFAULT
 #include <sys/sysinfo.h>
 ])
 AC_CHECK_FUNCS([swapctl])
+AC_CHECK_FUNCS([asprintf])
 
 # Initialize libtool
 AC_DISABLE_STATIC
-- 
1.7.9.4

[PATCH 2/3] drm/i915: extract card getting

[Ben Widawsky]

I didn't test this very thoroughly...

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 lib/drmtest.c |  122 +++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 83 insertions(+), 39 deletions(-)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index f9b7a6f..063b5c5 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -26,6 +26,8 @@
  *
  */
 
+#define _GNU_SOURCE
+#include <stdio.h>
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <sys/ioctl.h>
@@ -112,70 +114,112 @@ void gem_quiescent_gpu(int fd)
 	gem_sync(fd, handle);
 }
 
-/** Open the first DRM device we can find, searching up to 16 device nodes */
-int drm_open_any(void)
+static bool is_master(int fd)
 {
-	char name[20];
+	drm_client_t client;
+	int ret;
+
+	/* Check that we're the only opener and authed. */
+	client.idx = 0;
+	ret = ioctl(fd, DRM_IOCTL_GET_CLIENT, &client);
+	assert (ret == 0);
+	if (!client.auth) {
+		return 0;
+	}
+	client.idx = 1;
+	ret = ioctl(fd, DRM_IOCTL_GET_CLIENT, &client);
+	if (ret != -1 || errno != EINVAL) {
+		return 0;
+	}
+	return 1;
+}
+
+/**
+ * drm_get_card() - get an intel card number for use in /dev or /sys
+ *
+ * @master: -1 not a master, 0 don't care, 1 is the master
+ *
+ * returns -1 on error
+ */
+int drm_get_card(int master)
+{
+	char *name;
 	int i, fd;
 
 	for (i = 0; i < 16; i++) {
-		sprintf(name, "/dev/dri/card%d", i);
+		int ret;
+
+		ret = asprintf(&name, "/dev/dri/card%u", i);
+		if (ret == -1)
+			return -1;
 		fd = open(name, O_RDWR);
+		free(name);
+
 		if (fd == -1)
 			continue;
 
-		if (is_intel(fd)) {
+		if (is_intel(fd) && master == 0) {
 			gem_quiescent_gpu(fd);
-			return fd;
+			break;
+		}
+
+		if (master == 1 && is_master(fd)) {
+			close(fd);
+			break;
+		}
+
+		if (master == -1 && !is_master(fd)) {
+			close(fd);
+			break;
 		}
 
 		close(fd);
 	}
-	fprintf(stderr, "failed to open any drm device. retry as root?\n");
-	abort();
+
+	return i;
 }
 
+/** Open the first DRM device we can find, searching up to 16 device nodes */
+int drm_open_any(void)
+{
+	char *name;
+	int ret, fd;
+
+	ret = asprintf(&name, "/dev/dri/card%d", drm_get_card(0));
+	if (ret == -1)
+		return -1;
+
+	fd = open(name, O_RDWR);
+	free(name);
+
+	if (fd == -1)
+		fprintf(stderr, "failed to open any drm device. retry as root?\n");
+
+	assert(is_intel(fd));
+
+	return fd;
+}
 
 /**
  * Open the first DRM device we can find where we end up being the master.
  */
 int drm_open_any_master(void)
 {
-	char name[20];
-	int i, fd;
+	char *name;
+	int ret, fd;
 
-	for (i = 0; i < 16; i++) {
-		drm_client_t client;
-		int ret;
+	ret = asprintf(&name, "/dev/dri/card%d", drm_get_card(1));
+	if (ret == -1)
+		return -1;
 
-		sprintf(name, "/dev/dri/card%d", i);
-		fd = open(name, O_RDWR);
-		if (fd == -1)
-			continue;
+	fd = open(name, O_RDWR);
+	free(name);
+	if (fd == -1)
+		fprintf(stderr, "Couldn't find an un-controlled DRM device\n");
 
-		if (!is_intel(fd)) {
-			close(fd);
-			continue;
-		}
+	assert(is_intel(fd));
 
-		/* Check that we're the only opener and authed. */
-		client.idx = 0;
-		ret = ioctl(fd, DRM_IOCTL_GET_CLIENT, &client);
-		assert (ret == 0);
-		if (!client.auth) {
-			close(fd);
-			continue;
-		}
-		client.idx = 1;
-		ret = ioctl(fd, DRM_IOCTL_GET_CLIENT, &client);
-		if (ret != -1 || errno != EINVAL) {
-			close(fd);
-			continue;
-		}
-		return fd;
-	}
-	fprintf(stderr, "Couldn't find an un-controlled DRM device\n");
-	abort();
+	return fd;
 }
 
 void gem_set_tiling(int fd, uint32_t handle, int tiling, int stride)
-- 
1.7.9.4

[PATCH v2 3/3] tests: rc6 residency test

[Ben Widawsky]

This is meant to test the sysfs entry for showing rc6 residency in
milliseconds. Remember, sysfs is a permanent interface.

v2: use new get_card interface to try "all" devices
check rc6p and rc6pp in addition to rc6

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 lib/drmtest.h         |    1 +
 tests/Makefile.am     |    1 +
 tests/rc6_residency.c |   91 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+)
 create mode 100644 tests/rc6_residency.c

diff --git a/lib/drmtest.h b/lib/drmtest.h
index 96fbf1a..42f238c 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -35,6 +35,7 @@
 #include "xf86drm.h"
 #include "intel_batchbuffer.h"
 
+int drm_get_card(int master);
 int drm_open_any(void);
 int drm_open_any_master(void);
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6544ec7..a8eed88 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -56,6 +56,7 @@ TESTS_progs = \
 	drm_vma_limiter_cpu \
 	drm_vma_limiter_gtt \
 	drm_vma_limiter_cached \
+	rc6_residency \
 	$(NULL)
 
 # IMPORTANT: The ZZ_ tests need to be run last!
diff --git a/tests/rc6_residency.c b/tests/rc6_residency.c
new file mode 100644
index 0000000..5c2e442
--- /dev/null
+++ b/tests/rc6_residency.c
@@ -0,0 +1,91 @@
+/*
+ * Copyright © 2012 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Ben Widawsky <ben@bwidawsk.net>
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include "drmtest.h"
+
+static unsigned int readit(const char *path)
+{
+	unsigned int ret;
+
+	FILE *file;
+	file = fopen(path, "r");
+	if (file == NULL) {
+		fprintf(stderr, "Couldn't open %s (%d)\n", path, errno);
+		abort();
+	}
+	fscanf(file, "%u", &ret);
+	fclose(file);
+
+	return ret;
+}
+
+int main(int argc, char *argv[])
+{
+	const int device = drm_get_card(0);
+	char *path, *pathp, *pathpp;
+	int fd, ret;
+	unsigned int value1, value1p, value1pp, value2, value2p, value2pp;
+	int diff;
+
+	/* Use drm_open_any to verify device existence */
+	fd = drm_open_any();
+	close(fd);
+
+	ret = asprintf(&path, "/sys/class/drm/card%d/power/rc6", device);
+	assert(ret != -1);
+	ret = asprintf(&pathp, "/sys/class/drm/card%d/power/rc6p", device);
+	assert(ret != -1);
+	ret = asprintf(&pathpp, "/sys/class/drm/card%d/power/rc6pp", device);
+	assert(ret != -1);
+
+	value1 = readit(path);
+	value1p = readit(pathp);
+	value1pp = readit(pathpp);
+	// Sleep for 3 seconds and compare
+	sleep(3);
+	value2 = readit(path);
+	value2p = readit(pathp);
+	value2pp = readit(pathpp);
+
+	diff = (value2pp - value1pp) +
+		(value2p - value1p) +
+		(value2 - value1);
+
+	/* Plenty of fudge */
+	if (diff > 3900 || diff < 2100) {
+		printf("%d\n", diff);
+		exit(EXIT_FAILURE);
+	}
+
+	free(path);
+	exit(EXIT_SUCCESS);
+}
-- 
1.7.9.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: extract intel_enable_rc6()

[Daniel Vetter]

On Sun, Mar 25, 2012 at 05:33:28PM -0700, Ben Widawsky wrote:
> This function tells us the state of RC6. In the future perhaps it will
> even give something like a mask back telling which RC6 types are
> enabled.

This comment somehow sounds like you expect to use this when Eugeni's
patches are merged. But actually your patch doesn't apply without Eugeni's
patches, because they change the return type from bool to int.

So can you be slightly less fuzzy in your comments, please?

/rant
-Daniel
> 
> In order to return sane values to userspace for an upcoming RC6 sysfs
> interface (since register behavior seems to be somewhat random), we will
> query this value, and therefore it needs to be available to the rest of
> the driver.
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |    2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bbbc1a4..22ab4db 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1414,6 +1414,7 @@ extern void ironlake_enable_rc6(struct drm_device *dev);
>  extern void gen6_set_rps(struct drm_device *dev, u8 val);
>  extern void intel_detect_pch(struct drm_device *dev);
>  extern int intel_trans_dp_port_sel(struct drm_crtc *crtc);
> +extern int intel_enable_rc6(const struct drm_device *dev);
>  
>  extern void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv);
>  extern void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ab62c96..efbf709 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8235,7 +8235,7 @@ void intel_init_emon(struct drm_device *dev)
>  	dev_priv->corr = (lcfuse & LCFUSE_HIV_MASK);
>  }
>  
> -static int intel_enable_rc6(struct drm_device *dev)
> +int intel_enable_rc6(const struct drm_device *dev)
>  {
>  	/*
>  	 * Respect the kernel parameter if it is set
> -- 
> 1.7.9.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 2/3] drm/i915: extract card getting

[Daniel Vetter]

On Sun, Mar 25, 2012 at 05:33:31PM -0700, Ben Widawsky wrote:
> I didn't test this very thoroughly...
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

Tested on my ivb, where i915 is drm device 1 (instead of the usual 0).
Seems to work.
> ---
>  lib/drmtest.c |  122 +++++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 83 insertions(+), 39 deletions(-)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index f9b7a6f..063b5c5 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -26,6 +26,8 @@
>   *
>   */
>  
> +#define _GNU_SOURCE
> +#include <stdio.h>
>  #include <fcntl.h>
>  #include <sys/stat.h>
>  #include <sys/ioctl.h>
> @@ -112,70 +114,112 @@ void gem_quiescent_gpu(int fd)
>  	gem_sync(fd, handle);
>  }
>  
> -/** Open the first DRM device we can find, searching up to 16 device nodes */
> -int drm_open_any(void)
> +static bool is_master(int fd)
>  {
> -	char name[20];
> +	drm_client_t client;
> +	int ret;
> +
> +	/* Check that we're the only opener and authed. */
> +	client.idx = 0;
> +	ret = ioctl(fd, DRM_IOCTL_GET_CLIENT, &client);
> +	assert (ret == 0);
> +	if (!client.auth) {
> +		return 0;
> +	}
> +	client.idx = 1;
> +	ret = ioctl(fd, DRM_IOCTL_GET_CLIENT, &client);
> +	if (ret != -1 || errno != EINVAL) {
> +		return 0;
> +	}
> +	return 1;
> +}
> +
> +/**
> + * drm_get_card() - get an intel card number for use in /dev or /sys
> + *
> + * @master: -1 not a master, 0 don't care, 1 is the master
> + *
> + * returns -1 on error
> + */
> +int drm_get_card(int master)
> +{
> +	char *name;
>  	int i, fd;
>  
>  	for (i = 0; i < 16; i++) {
> -		sprintf(name, "/dev/dri/card%d", i);
> +		int ret;
> +
> +		ret = asprintf(&name, "/dev/dri/card%u", i);
> +		if (ret == -1)
> +			return -1;
>  		fd = open(name, O_RDWR);
> +		free(name);
> +
>  		if (fd == -1)
>  			continue;
>  
> -		if (is_intel(fd)) {
> +		if (is_intel(fd) && master == 0) {
>  			gem_quiescent_gpu(fd);
> -			return fd;
> +			break;
> +		}
> +
> +		if (master == 1 && is_master(fd)) {
> +			close(fd);
> +			break;
> +		}
> +
> +		if (master == -1 && !is_master(fd)) {
> +			close(fd);
> +			break;
>  		}
>  
>  		close(fd);
>  	}
> -	fprintf(stderr, "failed to open any drm device. retry as root?\n");
> -	abort();
> +
> +	return i;
>  }
>  
> +/** Open the first DRM device we can find, searching up to 16 device nodes */
> +int drm_open_any(void)
> +{
> +	char *name;
> +	int ret, fd;
> +
> +	ret = asprintf(&name, "/dev/dri/card%d", drm_get_card(0));
> +	if (ret == -1)
> +		return -1;
> +
> +	fd = open(name, O_RDWR);
> +	free(name);
> +
> +	if (fd == -1)
> +		fprintf(stderr, "failed to open any drm device. retry as root?\n");
> +
> +	assert(is_intel(fd));
> +
> +	return fd;
> +}
>  
>  /**
>   * Open the first DRM device we can find where we end up being the master.
>   */
>  int drm_open_any_master(void)
>  {
> -	char name[20];
> -	int i, fd;
> +	char *name;
> +	int ret, fd;
>  
> -	for (i = 0; i < 16; i++) {
> -		drm_client_t client;
> -		int ret;
> +	ret = asprintf(&name, "/dev/dri/card%d", drm_get_card(1));
> +	if (ret == -1)
> +		return -1;
>  
> -		sprintf(name, "/dev/dri/card%d", i);
> -		fd = open(name, O_RDWR);
> -		if (fd == -1)
> -			continue;
> +	fd = open(name, O_RDWR);
> +	free(name);
> +	if (fd == -1)
> +		fprintf(stderr, "Couldn't find an un-controlled DRM device\n");
>  
> -		if (!is_intel(fd)) {
> -			close(fd);
> -			continue;
> -		}
> +	assert(is_intel(fd));
>  
> -		/* Check that we're the only opener and authed. */
> -		client.idx = 0;
> -		ret = ioctl(fd, DRM_IOCTL_GET_CLIENT, &client);
> -		assert (ret == 0);
> -		if (!client.auth) {
> -			close(fd);
> -			continue;
> -		}
> -		client.idx = 1;
> -		ret = ioctl(fd, DRM_IOCTL_GET_CLIENT, &client);
> -		if (ret != -1 || errno != EINVAL) {
> -			close(fd);
> -			continue;
> -		}
> -		return fd;
> -	}
> -	fprintf(stderr, "Couldn't find an un-controlled DRM device\n");
> -	abort();
> +	return fd;
>  }
>  
>  void gem_set_tiling(int fd, uint32_t handle, int tiling, int stride)
> -- 
> 1.7.9.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH v2 1/3] drm/i915: add rc6 residency times to debugfs

[Daniel Vetter]

On Sun, Mar 25, 2012 at 05:33:27PM -0700, Ben Widawsky wrote:
> RC6 residency should be in intervals of 1.28us, and the counter wraps.
> Here is an example using awk to get the various RC6 and RC6+ residency
> times in seconds, since boot.
> 
> cat /sys/kernel/debug/dri/0/i915_drpc_info  | grep residency | awk -F':' -F' '  '{print $5 * 1.28 / 1000000}'
> 
> This is primarily for debug, and QA/application developers using the
> sysfs interface looking for more insight.
> 
> Untested on IVB.

Tested on my ivb, seems to yield sane numbers for the default
configuration (i.e. enable rc+rc6p, disable rc6pp).
-Daniel

> 
> v2: move comment to the correct place
> commeit message changes
> 
> CC: Ouping Zhang <ouping.zhang@intel.com>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> Reviewed-by (v1): Eugeni Dodonov <eugeni.dodonov@intel.com>
> Signed-off-by (v1): Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |   11 +++++++++++
>  drivers/gpu/drm/i915/i915_reg.h     |    5 +++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 66c90d4..4257151 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1133,6 +1133,17 @@ static int gen6_drpc_info(struct seq_file *m)
>  
>  	seq_printf(m, "Core Power Down: %s\n",
>  		   yesno(gt_core_status & GEN6_CORE_CPD_STATE_MASK));
> +
> +	/* Not exactly sure what this is */
> +	seq_printf(m, "RC6 \"Locked to RPn\" residency since boot: %d\n",
> +		   I915_READ(GEN6_GT_GFX_RC6_LOCKED));
> +	seq_printf(m, "RC6 residency since boot: %d\n",
> +		   I915_READ(GEN6_GT_GFX_RC6));
> +	seq_printf(m, "RC6+ residency since boot: %d\n",
> +		   I915_READ(GEN6_GT_GFX_RC6p));
> +	seq_printf(m, "RC6++ residency since boot: %d\n",
> +		   I915_READ(GEN6_GT_GFX_RC6pp));
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f3609f2..b1c3d35 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3799,6 +3799,11 @@
>  						 GEN6_PM_RP_DOWN_THRESHOLD | \
>  						 GEN6_PM_RP_DOWN_TIMEOUT)
>  
> +#define GEN6_GT_GFX_RC6_LOCKED			0x138104
> +#define GEN6_GT_GFX_RC6				0x138108
> +#define GEN6_GT_GFX_RC6p			0x13810C
> +#define GEN6_GT_GFX_RC6pp			0x138110
> +
>  #define GEN6_PCODE_MAILBOX			0x138124
>  #define   GEN6_PCODE_READY			(1<<31)
>  #define   GEN6_READ_OC_PARAMS			0xc
> -- 
> 1.7.9.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH v2 3/3] tests: rc6 residency test

[Daniel Vetter]

On Sun, Mar 25, 2012 at 05:33:32PM -0700, Ben Widawsky wrote:
> This is meant to test the sysfs entry for showing rc6 residency in
> milliseconds. Remember, sysfs is a permanent interface.
> 
> v2: use new get_card interface to try "all" devices
> check rc6p and rc6pp in addition to rc6
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

You're still missing the drm_quiescent_gpu(fd) call before doing the
residency measurements. Which makes this test fail reliably in the i-g-t
testsuite because the gpu is still busy from previous tests. Please run
new tests also in the context of the full testuite to catch such issues.

If this leaking of gpu business annoys you to much, you could create a
drm_close_dev(fd) which calls drm_quiescent_gpu and replace the final
close on the dev with this in all tests.

Also, to keep things in line with other testes, can you name this
sysfs_rc6_residency?
-Daniel

> ---
>  lib/drmtest.h         |    1 +
>  tests/Makefile.am     |    1 +
>  tests/rc6_residency.c |   91 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 93 insertions(+)
>  create mode 100644 tests/rc6_residency.c
> 
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 96fbf1a..42f238c 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -35,6 +35,7 @@
>  #include "xf86drm.h"
>  #include "intel_batchbuffer.h"
>  
> +int drm_get_card(int master);
>  int drm_open_any(void);
>  int drm_open_any_master(void);
>  
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 6544ec7..a8eed88 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -56,6 +56,7 @@ TESTS_progs = \
>  	drm_vma_limiter_cpu \
>  	drm_vma_limiter_gtt \
>  	drm_vma_limiter_cached \
> +	rc6_residency \
>  	$(NULL)
>  
>  # IMPORTANT: The ZZ_ tests need to be run last!
> diff --git a/tests/rc6_residency.c b/tests/rc6_residency.c
> new file mode 100644
> index 0000000..5c2e442
> --- /dev/null
> +++ b/tests/rc6_residency.c
> @@ -0,0 +1,91 @@
> +/*
> + * Copyright © 2012 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *    Ben Widawsky <ben@bwidawsk.net>
> + *
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include "drmtest.h"
> +
> +static unsigned int readit(const char *path)
> +{
> +	unsigned int ret;
> +
> +	FILE *file;
> +	file = fopen(path, "r");
> +	if (file == NULL) {
> +		fprintf(stderr, "Couldn't open %s (%d)\n", path, errno);
> +		abort();
> +	}
> +	fscanf(file, "%u", &ret);
> +	fclose(file);
> +
> +	return ret;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	const int device = drm_get_card(0);
> +	char *path, *pathp, *pathpp;
> +	int fd, ret;
> +	unsigned int value1, value1p, value1pp, value2, value2p, value2pp;
> +	int diff;
> +
> +	/* Use drm_open_any to verify device existence */
> +	fd = drm_open_any();
> +	close(fd);
> +
> +	ret = asprintf(&path, "/sys/class/drm/card%d/power/rc6", device);
> +	assert(ret != -1);
> +	ret = asprintf(&pathp, "/sys/class/drm/card%d/power/rc6p", device);
> +	assert(ret != -1);
> +	ret = asprintf(&pathpp, "/sys/class/drm/card%d/power/rc6pp", device);
> +	assert(ret != -1);
> +
> +	value1 = readit(path);
> +	value1p = readit(pathp);
> +	value1pp = readit(pathpp);
> +	// Sleep for 3 seconds and compare
> +	sleep(3);
> +	value2 = readit(path);
> +	value2p = readit(pathp);
> +	value2pp = readit(pathpp);
> +
> +	diff = (value2pp - value1pp) +
> +		(value2p - value1p) +
> +		(value2 - value1);
> +
> +	/* Plenty of fudge */
> +	if (diff > 3900 || diff < 2100) {
> +		printf("%d\n", diff);
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	free(path);
> +	exit(EXIT_SUCCESS);
> +}
> -- 
> 1.7.9.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH v2 3/3] tests: rc6 residency test

[Daniel Vetter]

On Mon, Mar 26, 2012 at 11:37:13AM +0200, Daniel Vetter wrote:
> On Sun, Mar 25, 2012 at 05:33:32PM -0700, Ben Widawsky wrote:
> > This is meant to test the sysfs entry for showing rc6 residency in
> > milliseconds. Remember, sysfs is a permanent interface.
> > 
> > v2: use new get_card interface to try "all" devices
> > check rc6p and rc6pp in addition to rc6
> > 
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> 
> You're still missing the drm_quiescent_gpu(fd) call before doing the
> residency measurements. Which makes this test fail reliably in the i-g-t
> testsuite because the gpu is still busy from previous tests. Please run
> new tests also in the context of the full testuite to catch such issues.

Actually the quiescent seems to be there, so I don't know what's going on.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH v2 1/3] drm/i915: add rc6 residency times to debugfs

[Eugeni Dodonov]

[-- Attachment #1.1: Type: text/plain, Size: 1247 bytes --]

On Mon, Mar 26, 2012 at 06:34, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Sun, Mar 25, 2012 at 05:33:27PM -0700, Ben Widawsky wrote:
> > RC6 residency should be in intervals of 1.28us, and the counter wraps.
> > Here is an example using awk to get the various RC6 and RC6+ residency
> > times in seconds, since boot.
> >
> > cat /sys/kernel/debug/dri/0/i915_drpc_info  | grep residency | awk -F':'
> -F' '  '{print $5 * 1.28 / 1000000}'
> >
> > This is primarily for debug, and QA/application developers using the
> > sysfs interface looking for more insight.
> >
> > Untested on IVB.
>
> Tested on my ivb, seems to yield sane numbers for the default
> configuration (i.e. enable rc+rc6p, disable rc6pp).
>

Yeah, it should work on IVB except for some strange behavior sometimes (I
noticed that on IVB we sometimes get into RC6 even if we pass the
i915_enable_rc6=0 parameter). I don't know if it is Bios playing some
tricks with us or something changed in the mailbox communication, but no
harm came from this so far. I'll see what is going on there.

But for the patch itself:
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@gmail.com>

Nothing like new and shiny powertop power-saving options :).

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 1782 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/3] drm/i915: add rc6 residency times to debugfs

[Ben Widawsky]

On Mon, 26 Mar 2012 09:33:51 -0300
Eugeni Dodonov <eugeni@dodonov.net> wrote:

> On Mon, Mar 26, 2012 at 06:34, Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Sun, Mar 25, 2012 at 05:33:27PM -0700, Ben Widawsky wrote:
> > > RC6 residency should be in intervals of 1.28us, and the counter
> > > wraps. Here is an example using awk to get the various RC6 and
> > > RC6+ residency times in seconds, since boot.
> > >
> > > cat /sys/kernel/debug/dri/0/i915_drpc_info  | grep residency |
> > > awk -F':'
> > -F' '  '{print $5 * 1.28 / 1000000}'
> > >
> > > This is primarily for debug, and QA/application developers using
> > > the sysfs interface looking for more insight.
> > >
> > > Untested on IVB.
> >
> > Tested on my ivb, seems to yield sane numbers for the default
> > configuration (i.e. enable rc+rc6p, disable rc6pp).
> >
> 
> Yeah, it should work on IVB except for some strange behavior
> sometimes (I noticed that on IVB we sometimes get into RC6 even if we
> pass the i915_enable_rc6=0 parameter). I don't know if it is Bios
> playing some tricks with us or something changed in the mailbox
> communication, but no harm came from this so far. I'll see what is
> going on there.
> 
> But for the patch itself:
> Reviewed-by: Eugeni Dodonov <eugeni.dodonov@gmail.com>
> 
> Nothing like new and shiny powertop power-saving options :).
> 


Well as I said... at least on SNB it seems the RC6 value is never 0.
But I suspect this isn't because we go into RC6, but rather just a
uninitialized register value. Probably would be nice to follow up with
the designers on this one.

Re: [PATCH v2 3/3] tests: rc6 residency test

[Ben Widawsky]

On Mon, 26 Mar 2012 11:40:55 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Mon, Mar 26, 2012 at 11:37:13AM +0200, Daniel Vetter wrote:
> > On Sun, Mar 25, 2012 at 05:33:32PM -0700, Ben Widawsky wrote:
> > > This is meant to test the sysfs entry for showing rc6 residency in
> > > milliseconds. Remember, sysfs is a permanent interface.
> > > 
> > > v2: use new get_card interface to try "all" devices
> > > check rc6p and rc6pp in addition to rc6
> > > 
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > 
> > You're still missing the drm_quiescent_gpu(fd) call before doing the
> > residency measurements. Which makes this test fail reliably in the
> > i-g-t testsuite because the gpu is still busy from previous tests.
> > Please run new tests also in the context of the full testuite to
> > catch such issues.
> 
> Actually the quiescent seems to be there, so I don't know what's
> going on. -Daniel

I ran this on IVB, both individual and with make test (Ken's machine),
and it passed.

Ben

Re: [PATCH v2 1/3] drm/i915: add rc6 residency times to debugfs

[Ben Widawsky]

On Mon, 26 Mar 2012 09:33:51 -0300
Eugeni Dodonov <eugeni@dodonov.net> wrote:

> On Mon, Mar 26, 2012 at 06:34, Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Sun, Mar 25, 2012 at 05:33:27PM -0700, Ben Widawsky wrote:
> > > RC6 residency should be in intervals of 1.28us, and the counter
> > > wraps. Here is an example using awk to get the various RC6 and
> > > RC6+ residency times in seconds, since boot.
> > >
> > > cat /sys/kernel/debug/dri/0/i915_drpc_info  | grep residency |
> > > awk -F':'
> > -F' '  '{print $5 * 1.28 / 1000000}'
> > >
> > > This is primarily for debug, and QA/application developers using
> > > the sysfs interface looking for more insight.
> > >
> > > Untested on IVB.
> >
> > Tested on my ivb, seems to yield sane numbers for the default
> > configuration (i.e. enable rc+rc6p, disable rc6pp).
> >
> 
> Yeah, it should work on IVB except for some strange behavior
> sometimes (I noticed that on IVB we sometimes get into RC6 even if we
> pass the i915_enable_rc6=0 parameter). I don't know if it is Bios
> playing some tricks with us or something changed in the mailbox
> communication, but no harm came from this so far. I'll see what is
> going on there.
> 
> But for the patch itself:
> Reviewed-by: Eugeni Dodonov <eugeni.dodonov@gmail.com>
> 
> Nothing like new and shiny powertop power-saving options :).
> 

I'd like to resubmit this patch with either %u or %x instead of the %d.
Can I still keep your r-b?

Re: [PATCH v2 1/3] drm/i915: add rc6 residency times to debugfs

[Eugeni Dodonov]

[-- Attachment #1.1: Type: text/plain, Size: 340 bytes --]

On Mon, Mar 26, 2012 at 19:34, Ben Widawsky <ben@bwidawsk.net> wrote:

> I'd like to resubmit this patch with either %u or %x instead of the %d.
> Can I still keep your r-b?
>

I'd vote for %u - it makes more sense as we are counting real time here.
But sure, you can keep my R-b for this.

-- 
Eugeni Dodonov
 <http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 668 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/3] tests: rc6 residency test

[Daniel Vetter]

On Mon, Mar 26, 2012 at 02:54:35PM -0700, Ben Widawsky wrote:
> On Mon, 26 Mar 2012 11:40:55 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Mon, Mar 26, 2012 at 11:37:13AM +0200, Daniel Vetter wrote:
> > > On Sun, Mar 25, 2012 at 05:33:32PM -0700, Ben Widawsky wrote:
> > > > This is meant to test the sysfs entry for showing rc6 residency in
> > > > milliseconds. Remember, sysfs is a permanent interface.
> > > > 
> > > > v2: use new get_card interface to try "all" devices
> > > > check rc6p and rc6pp in addition to rc6
> > > > 
> > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > > 
> > > You're still missing the drm_quiescent_gpu(fd) call before doing the
> > > residency measurements. Which makes this test fail reliably in the
> > > i-g-t testsuite because the gpu is still busy from previous tests.
> > > Please run new tests also in the context of the full testuite to
> > > catch such issues.
> > 
> > Actually the quiescent seems to be there, so I don't know what's
> > going on. -Daniel
> 
> I ran this on IVB, both individual and with make test (Ken's machine),
> and it passed.

Ok, I'll digg into this and try to find out why it fails on my ivb.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH v2 3/3] tests: rc6 residency test

[Daniel Vetter]

On Sun, Mar 25, 2012 at 05:33:32PM -0700, Ben Widawsky wrote:
> +int main(int argc, char *argv[])
> +{
> +	const int device = drm_get_card(0);
> +	char *path, *pathp, *pathpp;
> +	int fd, ret;
> +	unsigned int value1, value1p, value1pp, value2, value2p, value2pp;
> +	int diff;
> +
> +	/* Use drm_open_any to verify device existence */
> +	fd = drm_open_any();
> +	close(fd);
> +
> +	ret = asprintf(&path, "/sys/class/drm/card%d/power/rc6", device);
> +	assert(ret != -1);
> +	ret = asprintf(&pathp, "/sys/class/drm/card%d/power/rc6p", device);
> +	assert(ret != -1);
> +	ret = asprintf(&pathpp, "/sys/class/drm/card%d/power/rc6pp", device);
> +	assert(ret != -1);
> +
> +	value1 = readit(path);
> +	value1p = readit(pathp);
> +	value1pp = readit(pathpp);
> +	// Sleep for 3 seconds and compare
> +	sleep(3);
> +	value2 = readit(path);
> +	value2p = readit(pathp);
> +	value2pp = readit(pathpp);
> +
> +	diff = (value2pp - value1pp) +
> +		(value2p - value1p) +
> +		(value2 - value1);
> +
> +	/* Plenty of fudge */
> +	if (diff > 3900 || diff < 2100) {
> +		printf("%d\n", diff);

While I bitch around: A slightly more informative error message could be
usueful ;-)

Cheers, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48