On Wed, 8 Aug 2012 23:35:40 +0200
Daniel Vetter <daniel.vetter at 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.vetter at ffwll.ch>
Acked-by: Ben Widawsky <ben at 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
