diff mbox

xdrm/i915: Respect HW RC6 states availability

Message ID 1351116315-1822-1-git-send-email-rodrigo.vivi@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rodrigo Vivi Oct. 24, 2012, 10:05 p.m. UTC
If Hardware doesn't allow RC6p or RC6pp we shall avoid end users turning them on, falling back to the only RC6 deepness available.

v2: fixed "if" comparison pointed by Chris Wilson

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Ben Widawsky Oct. 25, 2012, 5:31 a.m. UTC | #1
On Wed, 24 Oct 2012 20:05:15 -0200
Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:

> If Hardware doesn't allow RC6p or RC6pp we shall avoid end users
> turning them on, falling back to the only RC6 deepness available.
> 
> v2: fixed "if" comparison pointed by Chris Wilson
> 

For future reference, putting "v2" in the title is often helpful in
situations like this.

> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c index 50f5809..b6362d0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2370,7 +2370,7 @@ static void gen6_disable_rps(struct drm_device
> *dev) int intel_enable_rc6(const struct drm_device *dev)
>  {
>  	/* Respect the kernel parameter if it is set */
> -	if (i915_enable_rc6 >= 0)
> +	if (i915_enable_rc6 == 0)
>  		return i915_enable_rc6;
>  
>  	if (INTEL_INFO(dev)->gen == 5) {
> @@ -2394,6 +2394,9 @@ int intel_enable_rc6(const struct drm_device
> *dev) return INTEL_RC6_ENABLE;
>  	}
>  
> +	if (i915_enable_rc6 > 0)
> +		return i915_enable_rc6;
> +
>  	DRM_DEBUG_DRIVER("RC6 and deep RC6 enabled\n");
>  	return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
>  }

I like the direction this patch is going but I think it does limit
people with knowledge might want to do (for instance, I ran with rc6
on ILK just fine for quite a while).

How about we use add_taint instead, and a printk with KERN_WARNING for
platforms which have known issues?
Chris Wilson Oct. 26, 2012, 10:17 a.m. UTC | #2
On Wed, 24 Oct 2012 22:31:01 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> I like the direction this patch is going but I think it does limit
> people with knowledge might want to do (for instance, I ran with rc6
> on ILK just fine for quite a while).
> 
> How about we use add_taint instead, and a printk with KERN_WARNING for
> platforms which have known issues?

Good suggestion. I will definitely try to remember to apply this in
future. :)

I don't see anything wrong with allowing people to shoot themselves in
the foot (as they often have the habit of hitting the wrong one), just so
long as we are clear that it is not the default for a very good reason.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 50f5809..b6362d0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2370,7 +2370,7 @@  static void gen6_disable_rps(struct drm_device *dev)
 int intel_enable_rc6(const struct drm_device *dev)
 {
 	/* Respect the kernel parameter if it is set */
-	if (i915_enable_rc6 >= 0)
+	if (i915_enable_rc6 == 0)
 		return i915_enable_rc6;
 
 	if (INTEL_INFO(dev)->gen == 5) {
@@ -2394,6 +2394,9 @@  int intel_enable_rc6(const struct drm_device *dev)
 		return INTEL_RC6_ENABLE;
 	}
 
+	if (i915_enable_rc6 > 0)
+		return i915_enable_rc6;
+
 	DRM_DEBUG_DRIVER("RC6 and deep RC6 enabled\n");
 	return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
 }