| Message ID | 1344461740-1231-9-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 8 Aug 2012 23:35:40 +0200 Daniel Vetter <daniel.vetter@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@ffwll.ch> Acked-by: Ben Widawsky <ben@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); > }
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); }
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@ffwll.ch> --- drivers/gpu/drm/i915/intel_pm.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-)