Message ID | 1364244952-25996-4-git-send-email-rodrigo.vivi@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Mon, Mar 25, 2013 at 05:55:51PM -0300, Rodrigo Vivi wrote: > Yet according to pm spec pcode read/write operations aren't necessary for HSW. > > CC: Paulo Zanoni <paulo.r.zanoni@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 50 ++++++++++++++++++++++------------------- > 1 file changed, 27 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index aea5fac..f6a7366 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2645,18 +2645,20 @@ static void gen6_enable_rps(struct drm_device *dev) > GEN6_RP_UP_BUSY_AVG | > (IS_HASWELL(dev) ? GEN7_RP_DOWN_IDLE_AVG : GEN6_RP_DOWN_IDLE_CONT)); > > - ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_MIN_FREQ_TABLE, 0); > - if (!ret && (IS_GEN6(dev) || IS_IVYBRIDGE(dev))) { > - pcu_mbox = 0; > - ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox); > - if (!ret && (pcu_mbox & (1<<31))) { /* OC supported */ > - DRM_DEBUG_DRIVER("overclocking supported, adjusting frequency max from %dMHz to %dMHz\n", > - (dev_priv->rps.max_delay & 0xff) * 50, > - (pcu_mbox & 0xff) * 50); > - dev_priv->rps.max_delay = pcu_mbox & 0xff; > + if (!IS_HASWELL(dev)) { > + ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_MIN_FREQ_TABLE, 0); > + if (!ret && (IS_GEN6(dev) || IS_IVYBRIDGE(dev))) { > + pcu_mbox = 0; > + ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox); > + if (!ret && (pcu_mbox & (1<<31))) { /* OC supported */ > + DRM_DEBUG_DRIVER("overclocking supported, adjusting frequency max from %dMHz to %dMHz\n", > + (dev_priv->rps.max_delay & 0xff) * 50, > + (pcu_mbox & 0xff) * 50); > + dev_priv->rps.max_delay = pcu_mbox & 0xff; > + } > + } else { > + DRM_DEBUG_DRIVER("Failed to set the min frequency\n"); > } > - } else { > - DRM_DEBUG_DRIVER("Failed to set the min frequency\n"); Note that the turbo code only runs on snb/ivb - Ben has promised to supply the hsw turbo support. So imo adding this check isn't required really. > } > > gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8); > @@ -2670,18 +2672,20 @@ static void gen6_enable_rps(struct drm_device *dev) > /* enable all PM interrupts */ > I915_WRITE(GEN6_PMINTRMSK, 0); > > - rc6vids = 0; > - ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids); > - if (IS_GEN6(dev) && ret) { > - DRM_DEBUG_DRIVER("Couldn't check for BIOS workaround\n"); > - } else if (IS_GEN6(dev) && (GEN6_DECODE_RC6_VID(rc6vids & 0xff) < 450)) { > - DRM_DEBUG_DRIVER("You should update your BIOS. Correcting minimum rc6 voltage (%dmV->%dmV)\n", > - GEN6_DECODE_RC6_VID(rc6vids & 0xff), 450); > - rc6vids &= 0xffff00; > - rc6vids |= GEN6_ENCODE_RC6_VID(450); > - ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_RC6VIDS, rc6vids); > - if (ret) > - DRM_ERROR("Couldn't fix incorrect rc6 voltage\n"); > + if (!IS_HASWELL(dev)) { > + rc6vids = 0; > + ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids); > + if (IS_GEN6(dev) && ret) { > + DRM_DEBUG_DRIVER("Couldn't check for BIOS workaround\n"); > + } else if (IS_GEN6(dev) && (GEN6_DECODE_RC6_VID(rc6vids & 0xff) < 450)) { > + DRM_DEBUG_DRIVER("You should update your BIOS. Correcting minimum rc6 voltage (%dmV->%dmV)\n", > + GEN6_DECODE_RC6_VID(rc6vids & 0xff), 450); > + rc6vids &= 0xffff00; > + rc6vids |= GEN6_ENCODE_RC6_VID(450); > + ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_RC6VIDS, rc6vids); > + if (ret) > + DRM_ERROR("Couldn't fix incorrect rc6 voltage\n"); > + } This is a w/a only for snb - it would be much clearer to slap an if (IS_GEN6) check around it (and remove the funny logic with the existing GEN6 checks). Maybe resend just that? -Daniel > } > > gen6_gt_force_wake_put(dev_priv); > -- > 1.8.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Yeah, this kind of overlaps ben's patch, but also remove unecessary sandybridge_pcode_write. On Tue, Mar 26, 2013 at 5:02 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Mar 25, 2013 at 05:55:51PM -0300, Rodrigo Vivi wrote: > > Yet according to pm spec pcode read/write operations aren't necessary > for HSW. > > > > CC: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> > > --- > > drivers/gpu/drm/i915/intel_pm.c | 50 > ++++++++++++++++++++++------------------- > > 1 file changed, 27 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > > index aea5fac..f6a7366 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -2645,18 +2645,20 @@ static void gen6_enable_rps(struct drm_device > *dev) > > GEN6_RP_UP_BUSY_AVG | > > (IS_HASWELL(dev) ? GEN7_RP_DOWN_IDLE_AVG : > GEN6_RP_DOWN_IDLE_CONT)); > > > > - ret = sandybridge_pcode_write(dev_priv, > GEN6_PCODE_WRITE_MIN_FREQ_TABLE, 0); > > - if (!ret && (IS_GEN6(dev) || IS_IVYBRIDGE(dev))) { > > - pcu_mbox = 0; > > - ret = sandybridge_pcode_read(dev_priv, > GEN6_READ_OC_PARAMS, &pcu_mbox); > > - if (!ret && (pcu_mbox & (1<<31))) { /* OC supported */ > > - DRM_DEBUG_DRIVER("overclocking supported, > adjusting frequency max from %dMHz to %dMHz\n", > > - (dev_priv->rps.max_delay & 0xff) > * 50, > > - (pcu_mbox & 0xff) * 50); > > - dev_priv->rps.max_delay = pcu_mbox & 0xff; > > + if (!IS_HASWELL(dev)) { > > + ret = sandybridge_pcode_write(dev_priv, > GEN6_PCODE_WRITE_MIN_FREQ_TABLE, 0); > > + if (!ret && (IS_GEN6(dev) || IS_IVYBRIDGE(dev))) { > > + pcu_mbox = 0; > > + ret = sandybridge_pcode_read(dev_priv, > GEN6_READ_OC_PARAMS, &pcu_mbox); > > + if (!ret && (pcu_mbox & (1<<31))) { /* OC > supported */ > > + DRM_DEBUG_DRIVER("overclocking supported, > adjusting frequency max from %dMHz to %dMHz\n", > > + (dev_priv->rps.max_delay > & 0xff) * 50, > > + (pcu_mbox & 0xff) * 50); > > + dev_priv->rps.max_delay = pcu_mbox & 0xff; > > + } > > + } else { > > + DRM_DEBUG_DRIVER("Failed to set the min > frequency\n"); > > } > > - } else { > > - DRM_DEBUG_DRIVER("Failed to set the min frequency\n"); > > Note that the turbo code only runs on snb/ivb - Ben has promised to supply > the hsw turbo support. So imo adding this check isn't required really. > > > } > > > > gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8); > > @@ -2670,18 +2672,20 @@ static void gen6_enable_rps(struct drm_device > *dev) > > /* enable all PM interrupts */ > > I915_WRITE(GEN6_PMINTRMSK, 0); > > > > - rc6vids = 0; > > - ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, > &rc6vids); > > - if (IS_GEN6(dev) && ret) { > > - DRM_DEBUG_DRIVER("Couldn't check for BIOS workaround\n"); > > - } else if (IS_GEN6(dev) && (GEN6_DECODE_RC6_VID(rc6vids & 0xff) < > 450)) { > > - DRM_DEBUG_DRIVER("You should update your BIOS. Correcting > minimum rc6 voltage (%dmV->%dmV)\n", > > - GEN6_DECODE_RC6_VID(rc6vids & 0xff), 450); > > - rc6vids &= 0xffff00; > > - rc6vids |= GEN6_ENCODE_RC6_VID(450); > > - ret = sandybridge_pcode_write(dev_priv, > GEN6_PCODE_WRITE_RC6VIDS, rc6vids); > > - if (ret) > > - DRM_ERROR("Couldn't fix incorrect rc6 voltage\n"); > > + if (!IS_HASWELL(dev)) { > > + rc6vids = 0; > > + ret = sandybridge_pcode_read(dev_priv, > GEN6_PCODE_READ_RC6VIDS, &rc6vids); > > + if (IS_GEN6(dev) && ret) { > > + DRM_DEBUG_DRIVER("Couldn't check for BIOS > workaround\n"); > > + } else if (IS_GEN6(dev) && (GEN6_DECODE_RC6_VID(rc6vids & > 0xff) < 450)) { > > + DRM_DEBUG_DRIVER("You should update your BIOS. > Correcting minimum rc6 voltage (%dmV->%dmV)\n", > > + GEN6_DECODE_RC6_VID(rc6vids & > 0xff), 450); > > + rc6vids &= 0xffff00; > > + rc6vids |= GEN6_ENCODE_RC6_VID(450); > > + ret = sandybridge_pcode_write(dev_priv, > GEN6_PCODE_WRITE_RC6VIDS, rc6vids); > > + if (ret) > > + DRM_ERROR("Couldn't fix incorrect rc6 > voltage\n"); > > + } > > This is a w/a only for snb - it would be much clearer to slap an if > (IS_GEN6) check around it (and remove the funny logic with the existing > GEN6 checks). Maybe resend just that? > -Daniel > > > } > > > > gen6_gt_force_wake_put(dev_priv); > > -- > > 1.8.1.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch >
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index aea5fac..f6a7366 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2645,18 +2645,20 @@ static void gen6_enable_rps(struct drm_device *dev) GEN6_RP_UP_BUSY_AVG | (IS_HASWELL(dev) ? GEN7_RP_DOWN_IDLE_AVG : GEN6_RP_DOWN_IDLE_CONT)); - ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_MIN_FREQ_TABLE, 0); - if (!ret && (IS_GEN6(dev) || IS_IVYBRIDGE(dev))) { - pcu_mbox = 0; - ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox); - if (!ret && (pcu_mbox & (1<<31))) { /* OC supported */ - DRM_DEBUG_DRIVER("overclocking supported, adjusting frequency max from %dMHz to %dMHz\n", - (dev_priv->rps.max_delay & 0xff) * 50, - (pcu_mbox & 0xff) * 50); - dev_priv->rps.max_delay = pcu_mbox & 0xff; + if (!IS_HASWELL(dev)) { + ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_MIN_FREQ_TABLE, 0); + if (!ret && (IS_GEN6(dev) || IS_IVYBRIDGE(dev))) { + pcu_mbox = 0; + ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox); + if (!ret && (pcu_mbox & (1<<31))) { /* OC supported */ + DRM_DEBUG_DRIVER("overclocking supported, adjusting frequency max from %dMHz to %dMHz\n", + (dev_priv->rps.max_delay & 0xff) * 50, + (pcu_mbox & 0xff) * 50); + dev_priv->rps.max_delay = pcu_mbox & 0xff; + } + } else { + DRM_DEBUG_DRIVER("Failed to set the min frequency\n"); } - } else { - DRM_DEBUG_DRIVER("Failed to set the min frequency\n"); } gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8); @@ -2670,18 +2672,20 @@ static void gen6_enable_rps(struct drm_device *dev) /* enable all PM interrupts */ I915_WRITE(GEN6_PMINTRMSK, 0); - rc6vids = 0; - ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids); - if (IS_GEN6(dev) && ret) { - DRM_DEBUG_DRIVER("Couldn't check for BIOS workaround\n"); - } else if (IS_GEN6(dev) && (GEN6_DECODE_RC6_VID(rc6vids & 0xff) < 450)) { - DRM_DEBUG_DRIVER("You should update your BIOS. Correcting minimum rc6 voltage (%dmV->%dmV)\n", - GEN6_DECODE_RC6_VID(rc6vids & 0xff), 450); - rc6vids &= 0xffff00; - rc6vids |= GEN6_ENCODE_RC6_VID(450); - ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_RC6VIDS, rc6vids); - if (ret) - DRM_ERROR("Couldn't fix incorrect rc6 voltage\n"); + if (!IS_HASWELL(dev)) { + rc6vids = 0; + ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids); + if (IS_GEN6(dev) && ret) { + DRM_DEBUG_DRIVER("Couldn't check for BIOS workaround\n"); + } else if (IS_GEN6(dev) && (GEN6_DECODE_RC6_VID(rc6vids & 0xff) < 450)) { + DRM_DEBUG_DRIVER("You should update your BIOS. Correcting minimum rc6 voltage (%dmV->%dmV)\n", + GEN6_DECODE_RC6_VID(rc6vids & 0xff), 450); + rc6vids &= 0xffff00; + rc6vids |= GEN6_ENCODE_RC6_VID(450); + ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_RC6VIDS, rc6vids); + if (ret) + DRM_ERROR("Couldn't fix incorrect rc6 voltage\n"); + } } gen6_gt_force_wake_put(dev_priv);
Yet according to pm spec pcode read/write operations aren't necessary for HSW. CC: Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> --- drivers/gpu/drm/i915/intel_pm.c | 50 ++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 23 deletions(-)