| Message ID | 1367516890-3469-2-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 02, 2013 at 10:48:08AM -0700, Jesse Barnes wrote: > Both the docs and the existing code were wrong. So fix both and use a > switch statement like we do elsewhere to make things simple & clear. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/intel_pm.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 0f4b46e..556b989 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2902,7 +2902,18 @@ static void valleyview_enable_rps(struct drm_device *dev) > GEN7_RC_CTL_TO_MODE); > > valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &val); > - dev_priv->mem_freq = 800 + (266 * (val >> 6) & 3); > + switch ((val >> 6) & 3) { > + case 0: > + case 1: > + dev_priv->mem_freq = 800; > + break; > + case 2: > + dev_priv->mem_freq = 1066; > + break; > + case 3: > + dev_priv->mem_freq = 1333; > + break; > + } > DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq); > > DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & 0x10 ? "yes" : "no"); The code does what the author wants it to, but I don't have the doc that says this: Reviewed-by: Ben Widawsky <ben@bwidawsk.net> Since I've set up a precedent of providing what I think are better options: freq_lut[] = {800, 800, 1066, 1333}; freq_lut[(val >> 6) & 3]; [really ugly one-liner redacted]
On Fri, May 03, 2013 at 10:52:11AM -0700, Ben Widawsky wrote: > On Thu, May 02, 2013 at 10:48:08AM -0700, Jesse Barnes wrote: > > Both the docs and the existing code were wrong. So fix both and use a > > switch statement like we do elsewhere to make things simple & clear. > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > --- > > drivers/gpu/drm/i915/intel_pm.c | 13 ++++++++++++- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 0f4b46e..556b989 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -2902,7 +2902,18 @@ static void valleyview_enable_rps(struct drm_device *dev) > > GEN7_RC_CTL_TO_MODE); > > > > valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &val); > > - dev_priv->mem_freq = 800 + (266 * (val >> 6) & 3); > > + switch ((val >> 6) & 3) { > > + case 0: > > + case 1: > > + dev_priv->mem_freq = 800; > > + break; > > + case 2: > > + dev_priv->mem_freq = 1066; > > + break; > > + case 3: > > + dev_priv->mem_freq = 1333; > > + break; > > + } > > DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq); > > > > DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & 0x10 ? "yes" : "no"); > > The code does what the author wants it to, but I don't have the doc that > says this: > Reviewed-by: Ben Widawsky <ben@bwidawsk.net> First two patches of this series are merged to dinq, thanks. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 0f4b46e..556b989 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2902,7 +2902,18 @@ static void valleyview_enable_rps(struct drm_device *dev) GEN7_RC_CTL_TO_MODE); valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &val); - dev_priv->mem_freq = 800 + (266 * (val >> 6) & 3); + switch ((val >> 6) & 3) { + case 0: + case 1: + dev_priv->mem_freq = 800; + break; + case 2: + dev_priv->mem_freq = 1066; + break; + case 3: + dev_priv->mem_freq = 1333; + break; + } DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq); DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & 0x10 ? "yes" : "no");
Both the docs and the existing code were wrong. So fix both and use a switch statement like we do elsewhere to make things simple & clear. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/intel_pm.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)