[PATCH] drm/i915: initialize ring frequency scaling table on SNB

[PATCH] drm/i915: initialize ring frequency scaling table on SNB

[Jesse Barnes]

The ring frequency scaling table tells the PCU to treat certain GPU
frequencies as if they were a given CPU frequency for purposes of
scaling the ring frequency.  Normally the PCU will scale the ring
frequency based on the CPU P-state, but with the table present, it will
also take the GPU frequency into account.  The scaling_factor used in
this patch may not be ideal, but is enough to increase performance in
nexuiz on a 1366x768 panel by about 20%.

The main downside of keeping the ring frequency high while the CPU is
at a low frequency (or asleep altogether) is increased power
consumption.  But then if you're keeping your GPU busy, you probably
want the extra performance.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2f967af..15ee639 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3433,7 +3433,8 @@
 #define GEN6_PCODE_MAILBOX			0x138124
 #define   GEN6_PCODE_READY			(1<<31)
 #define   GEN6_READ_OC_PARAMS			0xc
-#define   GEN6_PCODE_WRITE_MIN_FREQ_TABLE	0x9
+#define   GEN6_PCODE_WRITE_MIN_FREQ_TABLE	0x8
 #define GEN6_PCODE_DATA				0x138128
+#define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
 
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 60a94d2..03f0fac 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -870,8 +870,10 @@ int i915_restore_state(struct drm_device *dev)
 		intel_init_emon(dev);
 	}
 
-	if (IS_GEN6(dev))
+	if (IS_GEN6(dev)) {
 		gen6_enable_rps(dev_priv);
+		gen6_update_ring_freq(dev_priv);
+	}
 
 	/* Cache mode state */
 	I915_WRITE (CACHE_MODE_0, dev_priv->saveCACHE_MODE_0 | 0xffff0000);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 86a3ec1..05c28eb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7159,6 +7159,46 @@ void gen6_enable_rps(struct drm_i915_private *dev_priv)
 	mutex_unlock(&dev_priv->dev->struct_mutex);
 }
 
+void gen6_update_ring_freq(struct drm_i915_private *dev_priv)
+{
+	int min_freq = 15, max_freq = 32;
+	int gpu_freq, ia_freq;
+	int scaling_factor = 180;
+
+	mutex_lock(&dev_priv->dev->struct_mutex);
+	gen6_gt_force_wake_get(dev_priv);
+
+	/*
+	 * For each potential GPU frequency, load a ring frequency we'd like
+	 * to use for memory access.  We do this by specifying the IA frequency
+	 * the PCU should use as a reference to determine the ring frequency.
+	 */
+	for (gpu_freq = dev_priv->min_delay; gpu_freq <= dev_priv->max_delay;
+	     gpu_freq++) {
+		if (gpu_freq < min_freq) /* Use 800MHz min IA reference freq */
+			ia_freq = 800;
+		else if (gpu_freq > max_freq) /* Clamp to 3GHz IA ref. freq */
+			ia_freq = 3000;
+		else
+			ia_freq = (gpu_freq * scaling_factor) / 2;
+		ia_freq = DIV_ROUND_UP(ia_freq, 100);
+
+		I915_WRITE(GEN6_PCODE_DATA,
+			   (ia_freq << GEN6_PCODE_FREQ_IA_RATIO_SHIFT) |
+			   gpu_freq);
+		I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY |
+			   GEN6_PCODE_WRITE_MIN_FREQ_TABLE);
+		if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) &
+			      GEN6_PCODE_READY) == 0, 10)) {
+			DRM_ERROR("pcode write of freq table timed out\n");
+			continue;
+		}
+	}
+
+	gen6_gt_force_wake_put(dev_priv);
+	mutex_unlock(&dev_priv->dev->struct_mutex);
+}
+
 static void ironlake_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7777,8 +7817,10 @@ void intel_modeset_init(struct drm_device *dev)
 		intel_init_emon(dev);
 	}
 
-	if (IS_GEN6(dev))
+	if (IS_GEN6(dev)) {
 		gen6_enable_rps(dev_priv);
+		gen6_update_ring_freq(dev_priv);
+	}
 
 	INIT_WORK(&dev_priv->idle_work, intel_idle_update);
 	setup_timer(&dev_priv->idle_timer, intel_gpu_idle_timer,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9ffa61e..8ac3bd8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -317,6 +317,7 @@ extern void intel_enable_clock_gating(struct drm_device *dev);
 extern void ironlake_enable_drps(struct drm_device *dev);
 extern void ironlake_disable_drps(struct drm_device *dev);
 extern void gen6_enable_rps(struct drm_i915_private *dev_priv);
+extern void gen6_update_ring_freq(struct drm_i915_private *dev_priv);
 extern void gen6_disable_rps(struct drm_device *dev);
 extern void intel_init_emon(struct drm_device *dev);

Re: [PATCH] drm/i915: initialize ring frequency scaling table on SNB

[Chris Wilson]

On Tue, 21 Jun 2011 15:24:24 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> The ring frequency scaling table tells the PCU to treat certain GPU
> frequencies as if they were a given CPU frequency for purposes of
> scaling the ring frequency.  Normally the PCU will scale the ring
> frequency based on the CPU P-state, but with the table present, it will
> also take the GPU frequency into account.  The scaling_factor used in
> this patch may not be ideal, but is enough to increase performance in
> nexuiz on a 1366x768 panel by about 20%.

Am I right in thinking that the improvement offered by these new defaults
is dependent upon the previous values programmed by the BIOS? On my
desktop SNB, I only see a marginal improvement (<~1%) for games and
similarly small differences for cairo-gl/xlib.

Plus they also revealed my confusion over dev_priv->min_delay is actually
min_freq on SNB. Evil. :-p
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: initialize ring frequency scaling table on SNB

[Jesse Barnes]

Chris Wilson <chris@chris-wilson.co.uk> wrote:

>On Tue, 21 Jun 2011 15:24:24 -0700, Jesse Barnes
><jbarnes@virtuousgeek.org> wrote:
>> The ring frequency scaling table tells the PCU to treat certain GPU
>> frequencies as if they were a given CPU frequency for purposes of
>> scaling the ring frequency.  Normally the PCU will scale the ring
>> frequency based on the CPU P-state, but with the table present, it
>will
>> also take the GPU frequency into account.  The scaling_factor used in
>> this patch may not be ideal, but is enough to increase performance in
>> nexuiz on a 1366x768 panel by about 20%.
>
>Am I right in thinking that the improvement offered by these new
>defaults
>is dependent upon the previous values programmed by the BIOS? On my
>desktop SNB, I only see a marginal improvement (<~1%) for games and
>similarly small differences for cairo-gl/xlib.
>
>Plus they also revealed my confusion over dev_priv->min_delay is
>actually
>min_freq on SNB. Evil. :-p
>-Chris
>
>-- 
>Chris Wilson, Intel Open Source Technology Centre

Yes, though on my machines the BIOS programs these values to 0, indicating no minimum ring freq.  The perf increase will be proportional to the bandwidth requirements not met by the existing frequency.  So for your tests, it's possible that the CPU is being kept busy enough to kee the ring freq up anyway (iow this patch is redundant for those loads) or that your workloads don't have high memory bw requirements to begin with, so the higher ring freq isn't a benefit.
-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] drm/i915: initialize ring frequency scaling table on SNB

[Eric Anholt]

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

On Tue, 21 Jun 2011 15:24:24 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> The ring frequency scaling table tells the PCU to treat certain GPU
> frequencies as if they were a given CPU frequency for purposes of
> scaling the ring frequency.  Normally the PCU will scale the ring
> frequency based on the CPU P-state, but with the table present, it will
> also take the GPU frequency into account.  The scaling_factor used in
> this patch may not be ideal, but is enough to increase performance in
> nexuiz on a 1366x768 panel by about 20%.
> 
> The main downside of keeping the ring frequency high while the CPU is
> at a low frequency (or asleep altogether) is increased power
> consumption.  But then if you're keeping your GPU busy, you probably
> want the extra performance.

The intent of the patch sounds good, but it doesn't seem to be doing
anything here (graphs below).  If I run a "while true; do; done" loop to
keep a CPU busy during measurement, though, OA gets a 15.9% +/- 0.6%
win, and nexuiz around 20%.  There are no complaints in dmesg.

x /home/anholt/nexuiz-before
+ /home/anholt/nexuiz-after
+-------------------------------------------------------------------------------+
| x       +                +    x                                x             +|
||_|_______________________M____MA_____A_________________________|_________|    |
+-------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   3     59.217461     59.775295     59.479322     59.490693    0.27909057
+   3     59.287311     59.893383     59.440451     59.540382    0.31515189
No difference proven at 95.0% confidence

x /home/anholt/oa-before
+ /home/anholt/oa-after
+-------------------------------------------------------------------------------+
|x  x                           +  +    +               x      *          +    x|
|    |________________________|_________MA_______A______M__________|________|   |
+-------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   5         184.2         187.2         186.3        185.72     1.3809417
+   5         185.4           187         185.7        186.04    0.71624018
No difference proven at 95.0% confidence

x /home/anholt/taiji-before
+ /home/anholt/taiji-after
+-------------------------------------------------------------------------------+
|x      x                  +     xx       +    x          + ++     +           x|
|    |____________________________A___|______________A______M_|_____|           |
+-------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   6       132.486       139.931       135.608       135.596     2.6966051
+   6       134.974       138.832       138.111     137.41183      1.434609
No difference proven at 95.0% confidence

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 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] drm/i915: initialize ring frequency scaling table on SNB

[Jesse Barnes]

On Wed, 22 Jun 2011 11:42:04 -0700
Eric Anholt <eric@anholt.net> wrote:

> On Tue, 21 Jun 2011 15:24:24 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > The ring frequency scaling table tells the PCU to treat certain GPU
> > frequencies as if they were a given CPU frequency for purposes of
> > scaling the ring frequency.  Normally the PCU will scale the ring
> > frequency based on the CPU P-state, but with the table present, it will
> > also take the GPU frequency into account.  The scaling_factor used in
> > this patch may not be ideal, but is enough to increase performance in
> > nexuiz on a 1366x768 panel by about 20%.
> > 
> > The main downside of keeping the ring frequency high while the CPU is
> > at a low frequency (or asleep altogether) is increased power
> > consumption.  But then if you're keeping your GPU busy, you probably
> > want the extra performance.
> 
> The intent of the patch sounds good, but it doesn't seem to be doing
> anything here (graphs below).  If I run a "while true; do; done" loop to
> keep a CPU busy during measurement, though, OA gets a 15.9% +/- 0.6%
> win, and nexuiz around 20%.  There are no complaints in dmesg.
> 
> x /home/anholt/nexuiz-before
> + /home/anholt/nexuiz-after
> +-------------------------------------------------------------------------------+
> | x       +                +    x                                x             +|
> ||_|_______________________M____MA_____A_________________________|_________|    |
> +-------------------------------------------------------------------------------+
>     N           Min           Max        Median           Avg        Stddev
> x   3     59.217461     59.775295     59.479322     59.490693    0.27909057
> +   3     59.287311     59.893383     59.440451     59.540382    0.31515189
> No difference proven at 95.0% confidence
> 
> x /home/anholt/oa-before
> + /home/anholt/oa-after
> +-------------------------------------------------------------------------------+
> |x  x                           +  +    +               x      *          +    x|
> |    |________________________|_________MA_______A______M__________|________|   |
> +-------------------------------------------------------------------------------+
>     N           Min           Max        Median           Avg        Stddev
> x   5         184.2         187.2         186.3        185.72     1.3809417
> +   5         185.4           187         185.7        186.04    0.71624018
> No difference proven at 95.0% confidence
> 
> x /home/anholt/taiji-before
> + /home/anholt/taiji-after
> +-------------------------------------------------------------------------------+
> |x      x                  +     xx       +    x          + ++     +           x|
> |    |____________________________A___|______________A______M_|_____|           |
> +-------------------------------------------------------------------------------+
>     N           Min           Max        Median           Avg        Stddev
> x   6       132.486       139.931       135.608       135.596     2.6966051
> +   6       134.974       138.832       138.111     137.41183      1.434609
> No difference proven at 95.0% confidence

Re: [PATCH] drm/i915: initialize ring frequency scaling table on SNB

[Jesse Barnes]

On Wed, 22 Jun 2011 11:42:04 -0700
Eric Anholt <eric@anholt.net> wrote:

> On Tue, 21 Jun 2011 15:24:24 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > The ring frequency scaling table tells the PCU to treat certain GPU
> > frequencies as if they were a given CPU frequency for purposes of
> > scaling the ring frequency.  Normally the PCU will scale the ring
> > frequency based on the CPU P-state, but with the table present, it will
> > also take the GPU frequency into account.  The scaling_factor used in
> > this patch may not be ideal, but is enough to increase performance in
> > nexuiz on a 1366x768 panel by about 20%.
> > 
> > The main downside of keeping the ring frequency high while the CPU is
> > at a low frequency (or asleep altogether) is increased power
> > consumption.  But then if you're keeping your GPU busy, you probably
> > want the extra performance.
> 
> The intent of the patch sounds good, but it doesn't seem to be doing
> anything here (graphs below).  If I run a "while true; do; done" loop to
> keep a CPU busy during measurement, though, OA gets a 15.9% +/- 0.6%
> win, and nexuiz around 20%.  There are no complaints in dmesg.

Ok I'll checck it out; I admit I tested it then made a few changes then
sent it out, so I must have broken something...

Jesse

Re: [PATCH] drm/i915: initialize ring frequency scaling table on SNB

[Jesse Barnes]

I tested again and things were working as expected.  This patch adds a
debugfs file for dumping the ring freq table, can you guys test it out
(try disabling the call to update the table, dump it, make sure it's 0,
then re-enable and make sure it's programmed correctly).

-- 
Jesse Barnes, Intel Open Source Technology Center

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 4d46441..79394cd 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1123,6 +1123,48 @@ static int i915_emon_status(struct seq_file *m, void *unused)
 	return 0;
 }
 
+static int i915_ring_freq_table(struct seq_file *m, void *unused)
+{
+	struct drm_info_node *node = (struct drm_info_node *) m->private;
+	struct drm_device *dev = node->minor->dev;
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	int ret;
+	int gpu_freq, ia_freq;
+
+	if (!IS_GEN6(dev)) {
+		seq_printf(m, "unsupported on this chipset\n");
+		return 0;
+	}
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	gen6_gt_force_wake_get(dev_priv);
+
+	seq_printf(m, "GPU freq\tEffective CPU freq\n");
+
+	for (gpu_freq = dev_priv->min_delay; gpu_freq <= dev_priv->max_delay;
+	     gpu_freq++) {
+		I915_WRITE(GEN6_PCODE_DATA, gpu_freq);
+		I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY |
+			   GEN6_PCODE_READ_MIN_FREQ_TABLE);
+		if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) &
+			      GEN6_PCODE_READY) == 0, 10)) {
+			DRM_ERROR("pcode write of freq table timed out\n");
+			continue;
+		}
+		ia_freq = I915_READ(GEN6_PCODE_DATA);
+		seq_printf(m, "%d\t\t%d\n", gpu_freq * 50, ia_freq * 100);
+	}
+
+	gen6_gt_force_wake_put(dev_priv);
+
+	mutex_unlock(&dev->struct_mutex);
+
+	return 0;
+}
+
 static int i915_gfxec(struct seq_file *m, void *unused)
 {
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
@@ -1426,6 +1468,7 @@ static struct drm_info_list i915_debugfs_list[] = {
 	{"i915_inttoext_table", i915_inttoext_table, 0},
 	{"i915_drpc_info", i915_drpc_info, 0},
 	{"i915_emon_status", i915_emon_status, 0},
+	{"i915_ring_freq_table", i915_ring_freq_table, 0},
 	{"i915_gfxec", i915_gfxec, 0},
 	{"i915_fbc_status", i915_fbc_status, 0},
 	{"i915_sr_status", i915_sr_status, 0},
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2f967af..757e024 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3433,7 +3433,9 @@
 #define GEN6_PCODE_MAILBOX			0x138124
 #define   GEN6_PCODE_READY			(1<<31)
 #define   GEN6_READ_OC_PARAMS			0xc
-#define   GEN6_PCODE_WRITE_MIN_FREQ_TABLE	0x9
+#define   GEN6_PCODE_WRITE_MIN_FREQ_TABLE	0x8
+#define   GEN6_PCODE_READ_MIN_FREQ_TABLE	0x9
 #define GEN6_PCODE_DATA				0x138128
+#define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
 
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 60a94d2..03f0fac 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -870,8 +870,10 @@ int i915_restore_state(struct drm_device *dev)
 		intel_init_emon(dev);
 	}
 
-	if (IS_GEN6(dev))
+	if (IS_GEN6(dev)) {
 		gen6_enable_rps(dev_priv);
+		gen6_update_ring_freq(dev_priv);
+	}
 
 	/* Cache mode state */
 	I915_WRITE (CACHE_MODE_0, dev_priv->saveCACHE_MODE_0 | 0xffff0000);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 86a3ec1..05c28eb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7159,6 +7159,46 @@ void gen6_enable_rps(struct drm_i915_private *dev_priv)
 	mutex_unlock(&dev_priv->dev->struct_mutex);
 }
 
+void gen6_update_ring_freq(struct drm_i915_private *dev_priv)
+{
+	int min_freq = 15, max_freq = 32;
+	int gpu_freq, ia_freq;
+	int scaling_factor = 180;
+
+	mutex_lock(&dev_priv->dev->struct_mutex);
+	gen6_gt_force_wake_get(dev_priv);
+
+	/*
+	 * For each potential GPU frequency, load a ring frequency we'd like
+	 * to use for memory access.  We do this by specifying the IA frequency
+	 * the PCU should use as a reference to determine the ring frequency.
+	 */
+	for (gpu_freq = dev_priv->min_delay; gpu_freq <= dev_priv->max_delay;
+	     gpu_freq++) {
+		if (gpu_freq < min_freq) /* Use 800MHz min IA reference freq */
+			ia_freq = 800;
+		else if (gpu_freq > max_freq) /* Clamp to 3GHz IA ref. freq */
+			ia_freq = 3000;
+		else
+			ia_freq = (gpu_freq * scaling_factor) / 2;
+		ia_freq = DIV_ROUND_UP(ia_freq, 100);
+
+		I915_WRITE(GEN6_PCODE_DATA,
+			   (ia_freq << GEN6_PCODE_FREQ_IA_RATIO_SHIFT) |
+			   gpu_freq);
+		I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY |
+			   GEN6_PCODE_WRITE_MIN_FREQ_TABLE);
+		if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) &
+			      GEN6_PCODE_READY) == 0, 10)) {
+			DRM_ERROR("pcode write of freq table timed out\n");
+			continue;
+		}
+	}
+
+	gen6_gt_force_wake_put(dev_priv);
+	mutex_unlock(&dev_priv->dev->struct_mutex);
+}
+
 static void ironlake_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7777,8 +7817,10 @@ void intel_modeset_init(struct drm_device *dev)
 		intel_init_emon(dev);
 	}
 
-	if (IS_GEN6(dev))
+	if (IS_GEN6(dev)) {
 		gen6_enable_rps(dev_priv);
+		gen6_update_ring_freq(dev_priv);
+	}
 
 	INIT_WORK(&dev_priv->idle_work, intel_idle_update);
 	setup_timer(&dev_priv->idle_timer, intel_gpu_idle_timer,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9ffa61e..8ac3bd8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -317,6 +317,7 @@ extern void intel_enable_clock_gating(struct drm_device *dev);
 extern void ironlake_enable_drps(struct drm_device *dev);
 extern void ironlake_disable_drps(struct drm_device *dev);
 extern void gen6_enable_rps(struct drm_i915_private *dev_priv);
+extern void gen6_update_ring_freq(struct drm_i915_private *dev_priv);
 extern void gen6_disable_rps(struct drm_device *dev);
 extern void intel_init_emon(struct drm_device *dev);

Re: [PATCH] drm/i915: initialize ring frequency scaling table on SNB

[Chris Wilson]

On Thu, 23 Jun 2011 10:11:26 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> I tested again and things were working as expected.  This patch adds a
> debugfs file for dumping the ring freq table, can you guys test it out
> (try disabling the call to update the table, dump it, make sure it's 0,
> then re-enable and make sure it's programmed correctly).

According to the debugfs, your defaults should be effective:

sandybridge:~$ cat /sys/kernel/debug/dri/1/i915_ring_freq_table 
GPU freq	Effective CPU freq
850		0 -> 1600
900		0 -> 1700
950		0 -> 1800
1000		0 -> 1800
1050		0 -> 1900
1100		0 -> 2000

I'll give the tyres another kick and see if I can spot any differences.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: initialize ring frequency scaling table on SNB

[Jesse Barnes]

On Thu, 23 Jun 2011 21:08:02 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Thu, 23 Jun 2011 10:11:26 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > I tested again and things were working as expected.  This patch adds a
> > debugfs file for dumping the ring freq table, can you guys test it out
> > (try disabling the call to update the table, dump it, make sure it's 0,
> > then re-enable and make sure it's programmed correctly).
> 
> According to the debugfs, your defaults should be effective:
> 
> sandybridge:~$ cat /sys/kernel/debug/dri/1/i915_ring_freq_table 
> GPU freq	Effective CPU freq
> 850		0 -> 1600
> 900		0 -> 1700
> 950		0 -> 1800
> 1000		0 -> 1800
> 1050		0 -> 1900
> 1100		0 -> 2000
> 
> I'll give the tyres another kick and see if I can spot any differences.

Eric was seeing an increase, but not as large as expected or hoped
for.  He saw a greater increase in performance than this patch provides
by simply keeping the CPU busy during his benchmark.  That could be due
to additional ring frequency when the CPU is in a turbo mode.  You may
be able to approach that by making the effective CPU frequency much
higher (maybe going up to 4GHz).

-- 
Jesse Barnes, Intel Open Source Technology Center