[PATCH] xdrm/i915: Respect HW RC6 states availability

[PATCH] xdrm/i915: Respect HW RC6 states availability

[Rodrigo Vivi]

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(-)

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);
 }
-- 
1.7.11.7

[PATCH] drm/i915: Respect HW RC6 states availability

[Rodrigo Vivi]

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(-)

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);
 }
-- 
1.7.11.7

Re: [PATCH] drm/i915: Respect HW RC6 states availability

[Rodrigo Vivi]

please ignore the other one. Stupid me (again) confused emacs with vim
and messed title up.
also I don't know why git send-email didn't like my in-reply-to

On Wed, Oct 24, 2012 at 8:07 PM, 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
>
> 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);
>  }
> --
> 1.7.11.7
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH] xdrm/i915: Respect HW RC6 states availability

[Ben Widawsky]

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?

Re: [PATCH] xdrm/i915: Respect HW RC6 states availability

[Chris Wilson]

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

-- 
Chris Wilson, Intel Open Source Technology Centre