[4/4] drm/i915: Use the real cpu max frequency for ring scaling

Commit Message

Rodrigo Vivi Oct. 7, 2013, 8:15 p.m. UTC

From: Ben Widawsky <[e-mail]>

The policy's max frequency is not equal to the CPU's max frequency. The
ring frequency is derived from the CPU frequency, and not the policy
frequency.

One example of how this may differ through sysfs. If the sysfs max
frequency is modified, that will be used for the max ring frequency
calculation.
(/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq). As far as I
know, no current governor uses anything but max as the default, but in
theory, they could. Similarly distributions might set policy as part of
their init process.

It's ideal to use the real frequency because when we're currently scaled
up on the GPU. In this case we likely want to race to idle, and using a
less than max ring frequency is non-optimal for this situation.

AFAIK, this patch should have no impact on a majority of people.

This behavior hasn't been changed since it was first introduced:
commit 23b2f8bb92feb83127679c53633def32d3108e70
Author: Jesse Barnes <[e-mail]>
Date:   Tue Jun 28 13:04:16 2011 -0700

    drm/i915: load a ring frequency scaling table v3

CC: Jesse Barnes <[e-mail]>
Signed-off-by: Ben Widawsky <[e-mail]>
Reviewed-by: Chris Wilson <[e-mail]>
Signed-off-by: Rodrigo Vivi <[e-mail]>
---
 drivers/gpu/drm/i915/intel_pm.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

Comments

Daniel Vetter Oct. 8, 2013, 6:14 a.m. UTC | #1

On Mon, Oct 07, 2013 at 05:15:48PM -0300, Rodrigo Vivi wrote:
> From: Ben Widawsky <[e-mail]>
> 
> The policy's max frequency is not equal to the CPU's max frequency. The
> ring frequency is derived from the CPU frequency, and not the policy
> frequency.
> 
> One example of how this may differ through sysfs. If the sysfs max
> frequency is modified, that will be used for the max ring frequency
> calculation.
> (/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq). As far as I
> know, no current governor uses anything but max as the default, but in
> theory, they could. Similarly distributions might set policy as part of
> their init process.
> 
> It's ideal to use the real frequency because when we're currently scaled
> up on the GPU. In this case we likely want to race to idle, and using a
> less than max ring frequency is non-optimal for this situation.
> 
> AFAIK, this patch should have no impact on a majority of people.
> 
> This behavior hasn't been changed since it was first introduced:
> commit 23b2f8bb92feb83127679c53633def32d3108e70
> Author: Jesse Barnes <[e-mail]>
> Date:   Tue Jun 28 13:04:16 2011 -0700
> 
>     drm/i915: load a ring frequency scaling table v3
> 
> CC: Jesse Barnes <[e-mail]>
> Signed-off-by: Ben Widawsky <[e-mail]>
> Reviewed-by: Chris Wilson <[e-mail]>
> Signed-off-by: Rodrigo Vivi <[e-mail]>

Queued for -next, thanks for the patch.
-Daniel

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0b4de57..4ec4846 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3740,16 +3740,21 @@  void gen6_update_ring_freq(struct drm_device *dev)
 	unsigned int gpu_freq;
 	unsigned int max_ia_freq, min_ring_freq;
 	int scaling_factor = 180;
+	struct cpufreq_policy *policy;
 
 	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
 
-	max_ia_freq = cpufreq_quick_get_max(0);
-	/*
-	 * Default to measured freq if none found, PCU will ensure we don't go
-	 * over
-	 */
-	if (!max_ia_freq)
+	policy = cpufreq_cpu_get(0);
+	if (policy) {
+		max_ia_freq = policy->cpuinfo.max_freq;
+		cpufreq_cpu_put(policy);
+	} else {
+		/*
+		 * Default to measured freq if none found, PCU will ensure we
+		 * don't go over
+		 */
 		max_ia_freq = tsc_khz;
+	}
 
 	/* Convert from kHz to MHz */
 	max_ia_freq /= 1000;