On Wed,  8 Aug 2012 23:35:40 +0200
Daniel Vetter <daniel.vet...@ffwll.ch> wrote:

> I have the faint hope that the total absence of any locking for the
> rps code wasn't too good an idea and could very well have caused some
> rc6 related regressions.
> 
> Unfortunately we've never managed to reproduce these issues on any of
> our own machines, so the only way to go about this is to enable it and
> see what happens.
> 
> While at it, kill some stale comments and improve the logging.
> 
> Signed-Off-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Acked-by: Ben Widawsky <b...@bwidawsk.net>

After discussing with Daniel on IRC a bit, I've decided it would take
too much time to actually verify the theory. I am acking on the basis
that I also couldn't find the bug report via google, rc6 has worked for
me on ilk forever, and the savings are too big not to try again.

> ---
>  drivers/gpu/drm/i915/intel_pm.c |   25 ++++++++++---------------
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index eff0753..a87c625 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2361,31 +2361,26 @@ 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
> -      */
> +     /* Respect the kernel parameter if it is set */
>       if (i915_enable_rc6 >= 0)
>               return i915_enable_rc6;
>  
> -     /*
> -      * Disable RC6 on Ironlake
> -      */
> -     if (INTEL_INFO(dev)->gen == 5)
> -             return 0;
> +     if (INTEL_INFO(dev)->gen == 5) {
> +             DRM_DEBUG_DRIVER("Ironlake: only RC6 available\n");
> +             return INTEL_RC6_ENABLE;
> +     }
>  
> -     /* On Haswell, only RC6 is available. So let's enable it by default to
> -      * provide better testing and coverage since the beginning.
> -      */
> -     if (IS_HASWELL(dev))
> +     if (IS_HASWELL(dev)) {
> +             DRM_DEBUG_DRIVER("Haswell: only RC6 available\n");
>               return INTEL_RC6_ENABLE;
> +     }
>  
> -     /*
> -      * Disable rc6 on Sandybridge
> -      */
> +     /* snb/ivb have more than one rc6 state. */
>       if (INTEL_INFO(dev)->gen == 6) {
>               DRM_DEBUG_DRIVER("Sandybridge: deep RC6 disabled\n");
>               return INTEL_RC6_ENABLE;
>       }
> +
>       DRM_DEBUG_DRIVER("RC6 and deep RC6 enabled\n");
>       return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
>  }



-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to