| Message ID | 1364244952-25996-5-git-send-email-rodrigo.vivi@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 25, 2013 at 05:55:52PM -0300, Rodrigo Vivi wrote: > Power management, in special RC6 enabling, differs across platforms. > This patch just split out enabling function for HSW. > This is an attempt to make pm code more clean without multiple IS_HASWELL > inside enable_rps function. This actually tends to get worse with upcoming > platforms. > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> I think the split starts to make sense, but ... > --- > drivers/gpu/drm/i915/intel_pm.c | 211 +++++++++++++++++++++++++++------------- > 1 file changed, 146 insertions(+), 65 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index f6a7366..814846d 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2566,13 +2566,9 @@ static void gen6_enable_rps(struct drm_device *dev) > /* disable the counters and set deterministic thresholds */ > I915_WRITE(GEN6_RC_CONTROL, 0); > > - if (IS_HASWELL(dev)) > - I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16); > - else { > - I915_WRITE(GEN6_RC1_WAKE_RATE_LIMIT, 1000 << 16); > - I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16 | 30); > - I915_WRITE(GEN6_RC6pp_WAKE_RATE_LIMIT, 30); > - } Adding an if block and then again killing it looks strange. -Daniel > + I915_WRITE(GEN6_RC1_WAKE_RATE_LIMIT, 1000 << 16); > + I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16 | 30); > + I915_WRITE(GEN6_RC6pp_WAKE_RATE_LIMIT, 30); > I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); > I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25); > > @@ -2580,27 +2576,19 @@ static void gen6_enable_rps(struct drm_device *dev) > I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10); > > I915_WRITE(GEN6_RC_SLEEP, 0); > - if (!IS_HASWELL(dev)) > - I915_WRITE(GEN6_RC1e_THRESHOLD, 1000); > + I915_WRITE(GEN6_RC1e_THRESHOLD, 1000); > I915_WRITE(GEN6_RC6_THRESHOLD, 50000); > - if (!IS_HASWELL(dev)) { > - I915_WRITE(GEN6_RC6p_THRESHOLD, 150000); > - I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */ > - } > + I915_WRITE(GEN6_RC6p_THRESHOLD, 150000); > + I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */ > > /* Check if we are enabling RC6 */ > rc6_mode = intel_enable_rc6(dev_priv->dev); > if (rc6_mode & INTEL_RC6_ENABLE) > rc6_mask |= GEN6_RC_CTL_RC6_ENABLE; > - > - /* We don't use those on Haswell */ > - if (!IS_HASWELL(dev)) { > - if (rc6_mode & INTEL_RC6p_ENABLE) > - rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE; > - > - if (rc6_mode & INTEL_RC6pp_ENABLE) > - rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE; > - } > + if (rc6_mode & INTEL_RC6p_ENABLE) > + rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE; > + if (rc6_mode & INTEL_RC6pp_ENABLE) > + rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE; > > DRM_INFO("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n", > (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off", > @@ -2612,19 +2600,12 @@ static void gen6_enable_rps(struct drm_device *dev) > GEN6_RC_CTL_EI_MODE(1) | > GEN6_RC_CTL_HW_ENABLE); > > - if (IS_HASWELL(dev)) { > - I915_WRITE(GEN6_RPNSWREQ, > - HSW_FREQUENCY(10)); > - I915_WRITE(GEN6_RC_VIDEO_FREQ, > - HSW_FREQUENCY(12)); > - } else { > - I915_WRITE(GEN6_RPNSWREQ, > - GEN6_FREQUENCY(10) | > - GEN6_OFFSET(0) | > - GEN6_AGGRESSIVE_TURBO); > - I915_WRITE(GEN6_RC_VIDEO_FREQ, > - GEN6_FREQUENCY(12)); > - } > + I915_WRITE(GEN6_RPNSWREQ, > + GEN6_FREQUENCY(10) | > + GEN6_OFFSET(0) | > + GEN6_AGGRESSIVE_TURBO); > + I915_WRITE(GEN6_RC_VIDEO_FREQ, > + GEN6_FREQUENCY(12)); > > I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 1000000); > I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, > @@ -2643,22 +2624,20 @@ static void gen6_enable_rps(struct drm_device *dev) > GEN6_RP_MEDIA_IS_GFX | > GEN6_RP_ENABLE | > GEN6_RP_UP_BUSY_AVG | > - (IS_HASWELL(dev) ? GEN7_RP_DOWN_IDLE_AVG : GEN6_RP_DOWN_IDLE_CONT)); > - > - 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"); > + 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; > } > + } else { > + DRM_DEBUG_DRIVER("Failed to set the min frequency\n"); > } > > gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8); > @@ -2672,25 +2651,124 @@ static void gen6_enable_rps(struct drm_device *dev) > /* enable all PM interrupts */ > I915_WRITE(GEN6_PMINTRMSK, 0); > > - 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"); > - } > + 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); > } > > +static void hsw_enable_rps(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_ring_buffer *ring; > + u32 rp_state_cap; > + u32 gt_perf_status; > + u32 rc6_mask = 0; > + u32 gtfifodbg; > + int rc6_mode; > + int i; > + > + WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); > + > + /* Here begins a magic sequence of register writes to enable > + * auto-downclocking. > + * > + * Perhaps there might be some value in exposing these to > + * userspace... > + */ > + I915_WRITE(GEN6_RC_STATE, 0); > + > + /* Clear the DBG now so we don't confuse earlier errors */ > + if ((gtfifodbg = I915_READ(GTFIFODBG))) { > + DRM_ERROR("GT fifo had a previous error %x\n", gtfifodbg); > + I915_WRITE(GTFIFODBG, gtfifodbg); > + } > + > + gen6_gt_force_wake_get(dev_priv); > + > + rp_state_cap = I915_READ(GEN6_RP_STATE_CAP); > + gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS); > + > + /* In units of 100MHz */ > + dev_priv->rps.max_delay = rp_state_cap & 0xff; > + dev_priv->rps.min_delay = (rp_state_cap & 0xff0000) >> 16; > + dev_priv->rps.cur_delay = 0; > + > + /* disable the counters and set deterministic thresholds */ > + I915_WRITE(GEN6_RC_CONTROL, 0); > + > + I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16); > + I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); > + I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25); > + > + for_each_ring(ring, dev_priv, i) > + I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10); > + > + I915_WRITE(GEN6_RC_SLEEP, 0); > + I915_WRITE(GEN6_RC6_THRESHOLD, 50000); > + > + /* Check if we are enabling RC6 */ > + rc6_mode = intel_enable_rc6(dev_priv->dev); > + if (rc6_mode & INTEL_RC6_ENABLE) > + rc6_mask |= GEN6_RC_CTL_RC6_ENABLE; > + > + DRM_INFO("Enabling RC6 states: RC6 %s\n", > + (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off"); > + > + I915_WRITE(GEN6_RC_CONTROL, > + rc6_mask | > + GEN6_RC_CTL_EI_MODE(1) | > + GEN6_RC_CTL_HW_ENABLE); > + > + I915_WRITE(GEN6_RPNSWREQ, > + HSW_FREQUENCY(10)); > + I915_WRITE(GEN6_RC_VIDEO_FREQ, > + HSW_FREQUENCY(12)); > + > + I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 1000000); > + I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, > + dev_priv->rps.max_delay << 24 | > + dev_priv->rps.min_delay << 16); > + > + I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400); > + I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 245000); > + I915_WRITE(GEN6_RP_UP_EI, 66000); > + I915_WRITE(GEN6_RP_DOWN_EI, 350000); > + > + I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10); > + I915_WRITE(GEN6_RP_CONTROL, > + GEN6_RP_MEDIA_TURBO | > + GEN6_RP_MEDIA_HW_NORMAL_MODE | > + GEN6_RP_MEDIA_IS_GFX | > + GEN6_RP_ENABLE | > + GEN6_RP_UP_BUSY_AVG | > + GEN7_RP_DOWN_IDLE_AVG); > + > + gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8); > + > + /* requires MSI enabled */ > + I915_WRITE(GEN6_PMIER, GEN6_PM_DEFERRED_EVENTS); > + spin_lock_irq(&dev_priv->rps.lock); > + WARN_ON(dev_priv->rps.pm_iir != 0); > + I915_WRITE(GEN6_PMIMR, 0); > + spin_unlock_irq(&dev_priv->rps.lock); > + /* enable all PM interrupts */ > + I915_WRITE(GEN6_PMINTRMSK, 0); > + > + gen6_gt_force_wake_put(dev_priv); > +} > + > static void gen6_update_ring_freq(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -3480,7 +3558,10 @@ static void intel_gen6_powersave_work(struct work_struct *work) > struct drm_device *dev = dev_priv->dev; > > mutex_lock(&dev_priv->rps.hw_lock); > - gen6_enable_rps(dev); > + if (IS_HASWELL(dev)) > + hsw_enable_rps(dev); > + else > + gen6_enable_rps(dev); > gen6_update_ring_freq(dev); > mutex_unlock(&dev_priv->rps.hw_lock); > } > -- > 1.8.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
ops, when reworking to let the split as last patch I missed this one... I'll resend it soon On Tue, Mar 26, 2013 at 5:05 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Mar 25, 2013 at 05:55:52PM -0300, Rodrigo Vivi wrote: > > Power management, in special RC6 enabling, differs across platforms. > > This patch just split out enabling function for HSW. > > This is an attempt to make pm code more clean without multiple IS_HASWELL > > inside enable_rps function. This actually tends to get worse with > upcoming > > platforms. > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> > > I think the split starts to make sense, but ... > > > --- > > drivers/gpu/drm/i915/intel_pm.c | 211 > +++++++++++++++++++++++++++------------- > > 1 file changed, 146 insertions(+), 65 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > > index f6a7366..814846d 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -2566,13 +2566,9 @@ static void gen6_enable_rps(struct drm_device > *dev) > > /* disable the counters and set deterministic thresholds */ > > I915_WRITE(GEN6_RC_CONTROL, 0); > > > > - if (IS_HASWELL(dev)) > > - I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16); > > - else { > > - I915_WRITE(GEN6_RC1_WAKE_RATE_LIMIT, 1000 << 16); > > - I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16 | 30); > > - I915_WRITE(GEN6_RC6pp_WAKE_RATE_LIMIT, 30); > > - } > > Adding an if block and then again killing it looks strange. > -Daniel > > > + I915_WRITE(GEN6_RC1_WAKE_RATE_LIMIT, 1000 << 16); > > + I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16 | 30); > > + I915_WRITE(GEN6_RC6pp_WAKE_RATE_LIMIT, 30); > > I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); > > I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25); > > > > @@ -2580,27 +2576,19 @@ static void gen6_enable_rps(struct drm_device > *dev) > > I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10); > > > > I915_WRITE(GEN6_RC_SLEEP, 0); > > - if (!IS_HASWELL(dev)) > > - I915_WRITE(GEN6_RC1e_THRESHOLD, 1000); > > + I915_WRITE(GEN6_RC1e_THRESHOLD, 1000); > > I915_WRITE(GEN6_RC6_THRESHOLD, 50000); > > - if (!IS_HASWELL(dev)) { > > - I915_WRITE(GEN6_RC6p_THRESHOLD, 150000); > > - I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */ > > - } > > + I915_WRITE(GEN6_RC6p_THRESHOLD, 150000); > > + I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */ > > > > /* Check if we are enabling RC6 */ > > rc6_mode = intel_enable_rc6(dev_priv->dev); > > if (rc6_mode & INTEL_RC6_ENABLE) > > rc6_mask |= GEN6_RC_CTL_RC6_ENABLE; > > - > > - /* We don't use those on Haswell */ > > - if (!IS_HASWELL(dev)) { > > - if (rc6_mode & INTEL_RC6p_ENABLE) > > - rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE; > > - > > - if (rc6_mode & INTEL_RC6pp_ENABLE) > > - rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE; > > - } > > + if (rc6_mode & INTEL_RC6p_ENABLE) > > + rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE; > > + if (rc6_mode & INTEL_RC6pp_ENABLE) > > + rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE; > > > > DRM_INFO("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n", > > (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off", > > @@ -2612,19 +2600,12 @@ static void gen6_enable_rps(struct drm_device > *dev) > > GEN6_RC_CTL_EI_MODE(1) | > > GEN6_RC_CTL_HW_ENABLE); > > > > - if (IS_HASWELL(dev)) { > > - I915_WRITE(GEN6_RPNSWREQ, > > - HSW_FREQUENCY(10)); > > - I915_WRITE(GEN6_RC_VIDEO_FREQ, > > - HSW_FREQUENCY(12)); > > - } else { > > - I915_WRITE(GEN6_RPNSWREQ, > > - GEN6_FREQUENCY(10) | > > - GEN6_OFFSET(0) | > > - GEN6_AGGRESSIVE_TURBO); > > - I915_WRITE(GEN6_RC_VIDEO_FREQ, > > - GEN6_FREQUENCY(12)); > > - } > > + I915_WRITE(GEN6_RPNSWREQ, > > + GEN6_FREQUENCY(10) | > > + GEN6_OFFSET(0) | > > + GEN6_AGGRESSIVE_TURBO); > > + I915_WRITE(GEN6_RC_VIDEO_FREQ, > > + GEN6_FREQUENCY(12)); > > > > I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 1000000); > > I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, > > @@ -2643,22 +2624,20 @@ static void gen6_enable_rps(struct drm_device > *dev) > > GEN6_RP_MEDIA_IS_GFX | > > GEN6_RP_ENABLE | > > GEN6_RP_UP_BUSY_AVG | > > - (IS_HASWELL(dev) ? GEN7_RP_DOWN_IDLE_AVG : > GEN6_RP_DOWN_IDLE_CONT)); > > - > > - 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"); > > + 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; > > } > > + } else { > > + DRM_DEBUG_DRIVER("Failed to set the min frequency\n"); > > } > > > > gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8); > > @@ -2672,25 +2651,124 @@ static void gen6_enable_rps(struct drm_device > *dev) > > /* enable all PM interrupts */ > > I915_WRITE(GEN6_PMINTRMSK, 0); > > > > - 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"); > > - } > > + 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); > > } > > > > +static void hsw_enable_rps(struct drm_device *dev) > > +{ > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + struct intel_ring_buffer *ring; > > + u32 rp_state_cap; > > + u32 gt_perf_status; > > + u32 rc6_mask = 0; > > + u32 gtfifodbg; > > + int rc6_mode; > > + int i; > > + > > + WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); > > + > > + /* Here begins a magic sequence of register writes to enable > > + * auto-downclocking. > > + * > > + * Perhaps there might be some value in exposing these to > > + * userspace... > > + */ > > + I915_WRITE(GEN6_RC_STATE, 0); > > + > > + /* Clear the DBG now so we don't confuse earlier errors */ > > + if ((gtfifodbg = I915_READ(GTFIFODBG))) { > > + DRM_ERROR("GT fifo had a previous error %x\n", gtfifodbg); > > + I915_WRITE(GTFIFODBG, gtfifodbg); > > + } > > + > > + gen6_gt_force_wake_get(dev_priv); > > + > > + rp_state_cap = I915_READ(GEN6_RP_STATE_CAP); > > + gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS); > > + > > + /* In units of 100MHz */ > > + dev_priv->rps.max_delay = rp_state_cap & 0xff; > > + dev_priv->rps.min_delay = (rp_state_cap & 0xff0000) >> 16; > > + dev_priv->rps.cur_delay = 0; > > + > > + /* disable the counters and set deterministic thresholds */ > > + I915_WRITE(GEN6_RC_CONTROL, 0); > > + > > + I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16); > > + I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); > > + I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25); > > + > > + for_each_ring(ring, dev_priv, i) > > + I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10); > > + > > + I915_WRITE(GEN6_RC_SLEEP, 0); > > + I915_WRITE(GEN6_RC6_THRESHOLD, 50000); > > + > > + /* Check if we are enabling RC6 */ > > + rc6_mode = intel_enable_rc6(dev_priv->dev); > > + if (rc6_mode & INTEL_RC6_ENABLE) > > + rc6_mask |= GEN6_RC_CTL_RC6_ENABLE; > > + > > + DRM_INFO("Enabling RC6 states: RC6 %s\n", > > + (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : > "off"); > > + > > + I915_WRITE(GEN6_RC_CONTROL, > > + rc6_mask | > > + GEN6_RC_CTL_EI_MODE(1) | > > + GEN6_RC_CTL_HW_ENABLE); > > + > > + I915_WRITE(GEN6_RPNSWREQ, > > + HSW_FREQUENCY(10)); > > + I915_WRITE(GEN6_RC_VIDEO_FREQ, > > + HSW_FREQUENCY(12)); > > + > > + I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 1000000); > > + I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, > > + dev_priv->rps.max_delay << 24 | > > + dev_priv->rps.min_delay << 16); > > + > > + I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400); > > + I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 245000); > > + I915_WRITE(GEN6_RP_UP_EI, 66000); > > + I915_WRITE(GEN6_RP_DOWN_EI, 350000); > > + > > + I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10); > > + I915_WRITE(GEN6_RP_CONTROL, > > + GEN6_RP_MEDIA_TURBO | > > + GEN6_RP_MEDIA_HW_NORMAL_MODE | > > + GEN6_RP_MEDIA_IS_GFX | > > + GEN6_RP_ENABLE | > > + GEN6_RP_UP_BUSY_AVG | > > + GEN7_RP_DOWN_IDLE_AVG); > > + > > + gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8); > > + > > + /* requires MSI enabled */ > > + I915_WRITE(GEN6_PMIER, GEN6_PM_DEFERRED_EVENTS); > > + spin_lock_irq(&dev_priv->rps.lock); > > + WARN_ON(dev_priv->rps.pm_iir != 0); > > + I915_WRITE(GEN6_PMIMR, 0); > > + spin_unlock_irq(&dev_priv->rps.lock); > > + /* enable all PM interrupts */ > > + I915_WRITE(GEN6_PMINTRMSK, 0); > > + > > + gen6_gt_force_wake_put(dev_priv); > > +} > > + > > static void gen6_update_ring_freq(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > @@ -3480,7 +3558,10 @@ static void intel_gen6_powersave_work(struct > work_struct *work) > > struct drm_device *dev = dev_priv->dev; > > > > mutex_lock(&dev_priv->rps.hw_lock); > > - gen6_enable_rps(dev); > > + if (IS_HASWELL(dev)) > > + hsw_enable_rps(dev); > > + else > > + gen6_enable_rps(dev); > > gen6_update_ring_freq(dev); > > mutex_unlock(&dev_priv->rps.hw_lock); > > } > > -- > > 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 >
Hi Daniel, I just checked the code and this patch looks right for me. it doesn't add any if block... just remove them. What am I missing? Thanks, Rodrigo. On Tue, Mar 26, 2013 at 10:54 AM, Rodrigo Vivi <rodrigo.vivi@gmail.com>wrote: > ops, when reworking to let the split as last patch I missed this one... > I'll resend it soon > > > On Tue, Mar 26, 2013 at 5:05 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > >> On Mon, Mar 25, 2013 at 05:55:52PM -0300, Rodrigo Vivi wrote: >> > Power management, in special RC6 enabling, differs across platforms. >> > This patch just split out enabling function for HSW. >> > This is an attempt to make pm code more clean without multiple >> IS_HASWELL >> > inside enable_rps function. This actually tends to get worse with >> upcoming >> > platforms. >> > >> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> >> >> I think the split starts to make sense, but ... >> >> > --- >> > drivers/gpu/drm/i915/intel_pm.c | 211 >> +++++++++++++++++++++++++++------------- >> > 1 file changed, 146 insertions(+), 65 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_pm.c >> b/drivers/gpu/drm/i915/intel_pm.c >> > index f6a7366..814846d 100644 >> > --- a/drivers/gpu/drm/i915/intel_pm.c >> > +++ b/drivers/gpu/drm/i915/intel_pm.c >> > @@ -2566,13 +2566,9 @@ static void gen6_enable_rps(struct drm_device >> *dev) >> > /* disable the counters and set deterministic thresholds */ >> > I915_WRITE(GEN6_RC_CONTROL, 0); >> > >> > - if (IS_HASWELL(dev)) >> > - I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16); >> > - else { >> > - I915_WRITE(GEN6_RC1_WAKE_RATE_LIMIT, 1000 << 16); >> > - I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16 | 30); >> > - I915_WRITE(GEN6_RC6pp_WAKE_RATE_LIMIT, 30); >> > - } >> >> Adding an if block and then again killing it looks strange. >> -Daniel >> >> > + I915_WRITE(GEN6_RC1_WAKE_RATE_LIMIT, 1000 << 16); >> > + I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16 | 30); >> > + I915_WRITE(GEN6_RC6pp_WAKE_RATE_LIMIT, 30); >> > I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); >> > I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25); >> > >> > @@ -2580,27 +2576,19 @@ static void gen6_enable_rps(struct drm_device >> *dev) >> > I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10); >> > >> > I915_WRITE(GEN6_RC_SLEEP, 0); >> > - if (!IS_HASWELL(dev)) >> > - I915_WRITE(GEN6_RC1e_THRESHOLD, 1000); >> > + I915_WRITE(GEN6_RC1e_THRESHOLD, 1000); >> > I915_WRITE(GEN6_RC6_THRESHOLD, 50000); >> > - if (!IS_HASWELL(dev)) { >> > - I915_WRITE(GEN6_RC6p_THRESHOLD, 150000); >> > - I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */ >> > - } >> > + I915_WRITE(GEN6_RC6p_THRESHOLD, 150000); >> > + I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */ >> > >> > /* Check if we are enabling RC6 */ >> > rc6_mode = intel_enable_rc6(dev_priv->dev); >> > if (rc6_mode & INTEL_RC6_ENABLE) >> > rc6_mask |= GEN6_RC_CTL_RC6_ENABLE; >> > - >> > - /* We don't use those on Haswell */ >> > - if (!IS_HASWELL(dev)) { >> > - if (rc6_mode & INTEL_RC6p_ENABLE) >> > - rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE; >> > - >> > - if (rc6_mode & INTEL_RC6pp_ENABLE) >> > - rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE; >> > - } >> > + if (rc6_mode & INTEL_RC6p_ENABLE) >> > + rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE; >> > + if (rc6_mode & INTEL_RC6pp_ENABLE) >> > + rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE; >> > >> > DRM_INFO("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n", >> > (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : >> "off", >> > @@ -2612,19 +2600,12 @@ static void gen6_enable_rps(struct drm_device >> *dev) >> > GEN6_RC_CTL_EI_MODE(1) | >> > GEN6_RC_CTL_HW_ENABLE); >> > >> > - if (IS_HASWELL(dev)) { >> > - I915_WRITE(GEN6_RPNSWREQ, >> > - HSW_FREQUENCY(10)); >> > - I915_WRITE(GEN6_RC_VIDEO_FREQ, >> > - HSW_FREQUENCY(12)); >> > - } else { >> > - I915_WRITE(GEN6_RPNSWREQ, >> > - GEN6_FREQUENCY(10) | >> > - GEN6_OFFSET(0) | >> > - GEN6_AGGRESSIVE_TURBO); >> > - I915_WRITE(GEN6_RC_VIDEO_FREQ, >> > - GEN6_FREQUENCY(12)); >> > - } >> > + I915_WRITE(GEN6_RPNSWREQ, >> > + GEN6_FREQUENCY(10) | >> > + GEN6_OFFSET(0) | >> > + GEN6_AGGRESSIVE_TURBO); >> > + I915_WRITE(GEN6_RC_VIDEO_FREQ, >> > + GEN6_FREQUENCY(12)); >> > >> > I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 1000000); >> > I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, >> > @@ -2643,22 +2624,20 @@ static void gen6_enable_rps(struct drm_device >> *dev) >> > GEN6_RP_MEDIA_IS_GFX | >> > GEN6_RP_ENABLE | >> > GEN6_RP_UP_BUSY_AVG | >> > - (IS_HASWELL(dev) ? GEN7_RP_DOWN_IDLE_AVG : >> GEN6_RP_DOWN_IDLE_CONT)); >> > - >> > - 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"); >> > + 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; >> > } >> > + } else { >> > + DRM_DEBUG_DRIVER("Failed to set the min frequency\n"); >> > } >> > >> > gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8); >> > @@ -2672,25 +2651,124 @@ static void gen6_enable_rps(struct drm_device >> *dev) >> > /* enable all PM interrupts */ >> > I915_WRITE(GEN6_PMINTRMSK, 0); >> > >> > - 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"); >> > - } >> > + 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); >> > } >> > >> > +static void hsw_enable_rps(struct drm_device *dev) >> > +{ >> > + struct drm_i915_private *dev_priv = dev->dev_private; >> > + struct intel_ring_buffer *ring; >> > + u32 rp_state_cap; >> > + u32 gt_perf_status; >> > + u32 rc6_mask = 0; >> > + u32 gtfifodbg; >> > + int rc6_mode; >> > + int i; >> > + >> > + WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); >> > + >> > + /* Here begins a magic sequence of register writes to enable >> > + * auto-downclocking. >> > + * >> > + * Perhaps there might be some value in exposing these to >> > + * userspace... >> > + */ >> > + I915_WRITE(GEN6_RC_STATE, 0); >> > + >> > + /* Clear the DBG now so we don't confuse earlier errors */ >> > + if ((gtfifodbg = I915_READ(GTFIFODBG))) { >> > + DRM_ERROR("GT fifo had a previous error %x\n", gtfifodbg); >> > + I915_WRITE(GTFIFODBG, gtfifodbg); >> > + } >> > + >> > + gen6_gt_force_wake_get(dev_priv); >> > + >> > + rp_state_cap = I915_READ(GEN6_RP_STATE_CAP); >> > + gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS); >> > + >> > + /* In units of 100MHz */ >> > + dev_priv->rps.max_delay = rp_state_cap & 0xff; >> > + dev_priv->rps.min_delay = (rp_state_cap & 0xff0000) >> 16; >> > + dev_priv->rps.cur_delay = 0; >> > + >> > + /* disable the counters and set deterministic thresholds */ >> > + I915_WRITE(GEN6_RC_CONTROL, 0); >> > + >> > + I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16); >> > + I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); >> > + I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25); >> > + >> > + for_each_ring(ring, dev_priv, i) >> > + I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10); >> > + >> > + I915_WRITE(GEN6_RC_SLEEP, 0); >> > + I915_WRITE(GEN6_RC6_THRESHOLD, 50000); >> > + >> > + /* Check if we are enabling RC6 */ >> > + rc6_mode = intel_enable_rc6(dev_priv->dev); >> > + if (rc6_mode & INTEL_RC6_ENABLE) >> > + rc6_mask |= GEN6_RC_CTL_RC6_ENABLE; >> > + >> > + DRM_INFO("Enabling RC6 states: RC6 %s\n", >> > + (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : >> "off"); >> > + >> > + I915_WRITE(GEN6_RC_CONTROL, >> > + rc6_mask | >> > + GEN6_RC_CTL_EI_MODE(1) | >> > + GEN6_RC_CTL_HW_ENABLE); >> > + >> > + I915_WRITE(GEN6_RPNSWREQ, >> > + HSW_FREQUENCY(10)); >> > + I915_WRITE(GEN6_RC_VIDEO_FREQ, >> > + HSW_FREQUENCY(12)); >> > + >> > + I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 1000000); >> > + I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, >> > + dev_priv->rps.max_delay << 24 | >> > + dev_priv->rps.min_delay << 16); >> > + >> > + I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400); >> > + I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 245000); >> > + I915_WRITE(GEN6_RP_UP_EI, 66000); >> > + I915_WRITE(GEN6_RP_DOWN_EI, 350000); >> > + >> > + I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10); >> > + I915_WRITE(GEN6_RP_CONTROL, >> > + GEN6_RP_MEDIA_TURBO | >> > + GEN6_RP_MEDIA_HW_NORMAL_MODE | >> > + GEN6_RP_MEDIA_IS_GFX | >> > + GEN6_RP_ENABLE | >> > + GEN6_RP_UP_BUSY_AVG | >> > + GEN7_RP_DOWN_IDLE_AVG); >> > + >> > + gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8); >> > + >> > + /* requires MSI enabled */ >> > + I915_WRITE(GEN6_PMIER, GEN6_PM_DEFERRED_EVENTS); >> > + spin_lock_irq(&dev_priv->rps.lock); >> > + WARN_ON(dev_priv->rps.pm_iir != 0); >> > + I915_WRITE(GEN6_PMIMR, 0); >> > + spin_unlock_irq(&dev_priv->rps.lock); >> > + /* enable all PM interrupts */ >> > + I915_WRITE(GEN6_PMINTRMSK, 0); >> > + >> > + gen6_gt_force_wake_put(dev_priv); >> > +} >> > + >> > static void gen6_update_ring_freq(struct drm_device *dev) >> > { >> > struct drm_i915_private *dev_priv = dev->dev_private; >> > @@ -3480,7 +3558,10 @@ static void intel_gen6_powersave_work(struct >> work_struct *work) >> > struct drm_device *dev = dev_priv->dev; >> > >> > mutex_lock(&dev_priv->rps.hw_lock); >> > - gen6_enable_rps(dev); >> > + if (IS_HASWELL(dev)) >> > + hsw_enable_rps(dev); >> > + else >> > + gen6_enable_rps(dev); >> > gen6_update_ring_freq(dev); >> > mutex_unlock(&dev_priv->rps.hw_lock); >> > } >> > -- >> > 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 >> > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br > >
On Tue, Mar 26, 2013 at 5:25 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote: > I just checked the code and this patch looks right for me. > it doesn't add any if block... just remove them. > What am I missing? You've added it right in the previous patch ;-) Which means if someone tries to understand the history of a given piece of code with git blame, they now have to jump through these 2 patches which change nothing and are right following each another. But in the usual recursive git blame mode you don't see that (or at least I don't check for that by default), so you end up reading both patches to make sure you still see where the code is moving around. So if you want to split (and I agree that it starts to make sense), pls split first, then apply hsw changes to the hsw rps code only. -Daniel
ah... got your point... I just split later because Ben wanted the frequency patch as the first one so I decided to let split at last patch to be really optional... so, you suggestion is to revert the order of this two latest patches or the 3? I guess frequency one was already queued right? On Tue, Mar 26, 2013 at 1:30 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Tue, Mar 26, 2013 at 5:25 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> > wrote: > > I just checked the code and this patch looks right for me. > > it doesn't add any if block... just remove them. > > What am I missing? > > You've added it right in the previous patch ;-) > > Which means if someone tries to understand the history of a given > piece of code with git blame, they now have to jump through these 2 > patches which change nothing and are right following each another. But > in the usual recursive git blame mode you don't see that (or at least > I don't check for that by default), so you end up reading both patches > to make sure you still see where the code is moving around. > > So if you want to split (and I agree that it starts to make sense), > pls split first, then apply hsw changes to the hsw rps code only. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch >
On Tue, Mar 26, 2013 at 01:32:51PM -0300, Rodrigo Vivi wrote: > ah... got your point... > I just split later because Ben wanted the frequency patch as the first one > so I decided to let split at last patch to be really optional... > so, you suggestion is to revert the order of this two latest patches or the > 3? Yeah, that's the idea. But since I've merged the first one already I get minus points for inconsistency, too :( > I guess frequency one was already queued right? Yeah, frequency one is already queued. That one looked more like a real bugfix to me, since it essentially changes what we're writing into functional registers. Hence why I've picked it right away. Another patch which is still dangling around is Chris' revert of commit 1ee9ae3244c4789f3184c5123f3b2d7e405b3f4c Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Wed Aug 15 10:41:45 2012 +0200 drm/i915: use hsw rps tuning values everywhere on gen6+ With the split-up hsw rps stuff that's imo something we should look into again I think. Chris? -Daniel > > > On Tue, Mar 26, 2013 at 1:30 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > > > On Tue, Mar 26, 2013 at 5:25 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> > > wrote: > > > I just checked the code and this patch looks right for me. > > > it doesn't add any if block... just remove them. > > > What am I missing? > > > > You've added it right in the previous patch ;-) > > > > Which means if someone tries to understand the history of a given > > piece of code with git blame, they now have to jump through these 2 > > patches which change nothing and are right following each another. But > > in the usual recursive git blame mode you don't see that (or at least > > I don't check for that by default), so you end up reading both patches > > to make sure you still see where the code is moving around. > > > > So if you want to split (and I agree that it starts to make sense), > > pls split first, then apply hsw changes to the hsw rps code only. > > -Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br
I'm in favor of this revert. Although I don't have any argument in values, I always guessed that many of rc6 bugs we have on snb came from the gap between the threashold values used and documented for snb. On Tue, Mar 26, 2013 at 4:00 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Tue, Mar 26, 2013 at 01:32:51PM -0300, Rodrigo Vivi wrote: > > ah... got your point... > > I just split later because Ben wanted the frequency patch as the first > one > > so I decided to let split at last patch to be really optional... > > so, you suggestion is to revert the order of this two latest patches or > the > > 3? > > Yeah, that's the idea. But since I've merged the first one already I get > minus points for inconsistency, too :( > > > I guess frequency one was already queued right? > > Yeah, frequency one is already queued. That one looked more like a real > bugfix to me, since it essentially changes what we're writing into > functional registers. Hence why I've picked it right away. > > Another patch which is still dangling around is Chris' revert of > > commit 1ee9ae3244c4789f3184c5123f3b2d7e405b3f4c > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > Date: Wed Aug 15 10:41:45 2012 +0200 > > drm/i915: use hsw rps tuning values everywhere on gen6+ > > With the split-up hsw rps stuff that's imo something we should look into > again I think. Chris? > -Daniel > > > > > > > On Tue, Mar 26, 2013 at 1:30 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > > > > > On Tue, Mar 26, 2013 at 5:25 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> > > > wrote: > > > > I just checked the code and this patch looks right for me. > > > > it doesn't add any if block... just remove them. > > > > What am I missing? > > > > > > You've added it right in the previous patch ;-) > > > > > > Which means if someone tries to understand the history of a given > > > piece of code with git blame, they now have to jump through these 2 > > > patches which change nothing and are right following each another. But > > > in the usual recursive git blame mode you don't see that (or at least > > > I don't check for that by default), so you end up reading both patches > > > to make sure you still see where the code is moving around. > > > > > > So if you want to split (and I agree that it starts to make sense), > > > pls split first, then apply hsw changes to the hsw rps code only. > > > -Daniel > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > > > > > > > > > > -- > > Rodrigo Vivi > > Blog: http://blog.vivi.eng.br > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch >
I forgot to say that I tested patch by patch today on my HSW ULT and just noticed real improvement with the patch you already queued. Where improvements are: at least 0.2W and also less oscillation and more % on RC and package C states. Although I'd like to see the rest of the series applied to have the code as clean, organized and matching documentation as possible. On Tue, Mar 26, 2013 at 4:32 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com>wrote: > I'm in favor of this revert. Although I don't have any argument in values, > I always guessed that many of rc6 bugs we have on snb came from the gap > between the threashold values used and documented for snb. > > > On Tue, Mar 26, 2013 at 4:00 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > >> On Tue, Mar 26, 2013 at 01:32:51PM -0300, Rodrigo Vivi wrote: >> > ah... got your point... >> > I just split later because Ben wanted the frequency patch as the first >> one >> > so I decided to let split at last patch to be really optional... >> > so, you suggestion is to revert the order of this two latest patches or >> the >> > 3? >> >> Yeah, that's the idea. But since I've merged the first one already I get >> minus points for inconsistency, too :( >> >> > I guess frequency one was already queued right? >> >> Yeah, frequency one is already queued. That one looked more like a real >> bugfix to me, since it essentially changes what we're writing into >> functional registers. Hence why I've picked it right away. >> >> Another patch which is still dangling around is Chris' revert of >> >> commit 1ee9ae3244c4789f3184c5123f3b2d7e405b3f4c >> Author: Daniel Vetter <daniel.vetter@ffwll.ch> >> Date: Wed Aug 15 10:41:45 2012 +0200 >> >> drm/i915: use hsw rps tuning values everywhere on gen6+ >> >> With the split-up hsw rps stuff that's imo something we should look into >> again I think. Chris? >> -Daniel >> >> > >> > >> > On Tue, Mar 26, 2013 at 1:30 PM, Daniel Vetter <daniel@ffwll.ch> wrote: >> > >> > > On Tue, Mar 26, 2013 at 5:25 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com >> > >> > > wrote: >> > > > I just checked the code and this patch looks right for me. >> > > > it doesn't add any if block... just remove them. >> > > > What am I missing? >> > > >> > > You've added it right in the previous patch ;-) >> > > >> > > Which means if someone tries to understand the history of a given >> > > piece of code with git blame, they now have to jump through these 2 >> > > patches which change nothing and are right following each another. But >> > > in the usual recursive git blame mode you don't see that (or at least >> > > I don't check for that by default), so you end up reading both patches >> > > to make sure you still see where the code is moving around. >> > > >> > > So if you want to split (and I agree that it starts to make sense), >> > > pls split first, then apply hsw changes to the hsw rps code only. >> > > -Daniel >> > > -- >> > > Daniel Vetter >> > > Software Engineer, Intel Corporation >> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch >> > > >> > >> > >> > >> > -- >> > Rodrigo Vivi >> > Blog: http://blog.vivi.eng.br >> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch >> > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br > >
On Tue, Mar 26, 2013 at 8:37 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote: > I forgot to say that I tested patch by patch today on my HSW ULT and just > noticed real improvement with the patch you already queued. > > Where improvements are: at least 0.2W and also less oscillation and more % > on RC and package C states. Cool! > Although I'd like to see the rest of the series applied to have the code as > clean, organized and matching documentation as possible. Yeah, sounds like a good plan (at least for hsw). > On Tue, Mar 26, 2013 at 4:32 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> > wrote: >> >> I'm in favor of this revert. Although I don't have any argument in values, >> I always guessed that many of rc6 bugs we have on snb came from the gap >> between the threashold values used and documented for snb. The reason I'm still wary is that this helped (according to multiple people) to more reliably enter rc6. It might be that we've simply mixed up other rc6 issues and hangs here (or that it's just as flaky as it seems), but for snb/ivb we need to be careful. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index f6a7366..814846d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2566,13 +2566,9 @@ static void gen6_enable_rps(struct drm_device *dev) /* disable the counters and set deterministic thresholds */ I915_WRITE(GEN6_RC_CONTROL, 0); - if (IS_HASWELL(dev)) - I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16); - else { - I915_WRITE(GEN6_RC1_WAKE_RATE_LIMIT, 1000 << 16); - I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16 | 30); - I915_WRITE(GEN6_RC6pp_WAKE_RATE_LIMIT, 30); - } + I915_WRITE(GEN6_RC1_WAKE_RATE_LIMIT, 1000 << 16); + I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16 | 30); + I915_WRITE(GEN6_RC6pp_WAKE_RATE_LIMIT, 30); I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25); @@ -2580,27 +2576,19 @@ static void gen6_enable_rps(struct drm_device *dev) I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10); I915_WRITE(GEN6_RC_SLEEP, 0); - if (!IS_HASWELL(dev)) - I915_WRITE(GEN6_RC1e_THRESHOLD, 1000); + I915_WRITE(GEN6_RC1e_THRESHOLD, 1000); I915_WRITE(GEN6_RC6_THRESHOLD, 50000); - if (!IS_HASWELL(dev)) { - I915_WRITE(GEN6_RC6p_THRESHOLD, 150000); - I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */ - } + I915_WRITE(GEN6_RC6p_THRESHOLD, 150000); + I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */ /* Check if we are enabling RC6 */ rc6_mode = intel_enable_rc6(dev_priv->dev); if (rc6_mode & INTEL_RC6_ENABLE) rc6_mask |= GEN6_RC_CTL_RC6_ENABLE; - - /* We don't use those on Haswell */ - if (!IS_HASWELL(dev)) { - if (rc6_mode & INTEL_RC6p_ENABLE) - rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE; - - if (rc6_mode & INTEL_RC6pp_ENABLE) - rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE; - } + if (rc6_mode & INTEL_RC6p_ENABLE) + rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE; + if (rc6_mode & INTEL_RC6pp_ENABLE) + rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE; DRM_INFO("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n", (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off", @@ -2612,19 +2600,12 @@ static void gen6_enable_rps(struct drm_device *dev) GEN6_RC_CTL_EI_MODE(1) | GEN6_RC_CTL_HW_ENABLE); - if (IS_HASWELL(dev)) { - I915_WRITE(GEN6_RPNSWREQ, - HSW_FREQUENCY(10)); - I915_WRITE(GEN6_RC_VIDEO_FREQ, - HSW_FREQUENCY(12)); - } else { - I915_WRITE(GEN6_RPNSWREQ, - GEN6_FREQUENCY(10) | - GEN6_OFFSET(0) | - GEN6_AGGRESSIVE_TURBO); - I915_WRITE(GEN6_RC_VIDEO_FREQ, - GEN6_FREQUENCY(12)); - } + I915_WRITE(GEN6_RPNSWREQ, + GEN6_FREQUENCY(10) | + GEN6_OFFSET(0) | + GEN6_AGGRESSIVE_TURBO); + I915_WRITE(GEN6_RC_VIDEO_FREQ, + GEN6_FREQUENCY(12)); I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 1000000); I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, @@ -2643,22 +2624,20 @@ static void gen6_enable_rps(struct drm_device *dev) GEN6_RP_MEDIA_IS_GFX | GEN6_RP_ENABLE | GEN6_RP_UP_BUSY_AVG | - (IS_HASWELL(dev) ? GEN7_RP_DOWN_IDLE_AVG : GEN6_RP_DOWN_IDLE_CONT)); - - 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"); + 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; } + } else { + DRM_DEBUG_DRIVER("Failed to set the min frequency\n"); } gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8); @@ -2672,25 +2651,124 @@ static void gen6_enable_rps(struct drm_device *dev) /* enable all PM interrupts */ I915_WRITE(GEN6_PMINTRMSK, 0); - 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"); - } + 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); } +static void hsw_enable_rps(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_ring_buffer *ring; + u32 rp_state_cap; + u32 gt_perf_status; + u32 rc6_mask = 0; + u32 gtfifodbg; + int rc6_mode; + int i; + + WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); + + /* Here begins a magic sequence of register writes to enable + * auto-downclocking. + * + * Perhaps there might be some value in exposing these to + * userspace... + */ + I915_WRITE(GEN6_RC_STATE, 0); + + /* Clear the DBG now so we don't confuse earlier errors */ + if ((gtfifodbg = I915_READ(GTFIFODBG))) { + DRM_ERROR("GT fifo had a previous error %x\n", gtfifodbg); + I915_WRITE(GTFIFODBG, gtfifodbg); + } + + gen6_gt_force_wake_get(dev_priv); + + rp_state_cap = I915_READ(GEN6_RP_STATE_CAP); + gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS); + + /* In units of 100MHz */ + dev_priv->rps.max_delay = rp_state_cap & 0xff; + dev_priv->rps.min_delay = (rp_state_cap & 0xff0000) >> 16; + dev_priv->rps.cur_delay = 0; + + /* disable the counters and set deterministic thresholds */ + I915_WRITE(GEN6_RC_CONTROL, 0); + + I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16); + I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); + I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25); + + for_each_ring(ring, dev_priv, i) + I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10); + + I915_WRITE(GEN6_RC_SLEEP, 0); + I915_WRITE(GEN6_RC6_THRESHOLD, 50000); + + /* Check if we are enabling RC6 */ + rc6_mode = intel_enable_rc6(dev_priv->dev); + if (rc6_mode & INTEL_RC6_ENABLE) + rc6_mask |= GEN6_RC_CTL_RC6_ENABLE; + + DRM_INFO("Enabling RC6 states: RC6 %s\n", + (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off"); + + I915_WRITE(GEN6_RC_CONTROL, + rc6_mask | + GEN6_RC_CTL_EI_MODE(1) | + GEN6_RC_CTL_HW_ENABLE); + + I915_WRITE(GEN6_RPNSWREQ, + HSW_FREQUENCY(10)); + I915_WRITE(GEN6_RC_VIDEO_FREQ, + HSW_FREQUENCY(12)); + + I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 1000000); + I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, + dev_priv->rps.max_delay << 24 | + dev_priv->rps.min_delay << 16); + + I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400); + I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 245000); + I915_WRITE(GEN6_RP_UP_EI, 66000); + I915_WRITE(GEN6_RP_DOWN_EI, 350000); + + I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10); + I915_WRITE(GEN6_RP_CONTROL, + GEN6_RP_MEDIA_TURBO | + GEN6_RP_MEDIA_HW_NORMAL_MODE | + GEN6_RP_MEDIA_IS_GFX | + GEN6_RP_ENABLE | + GEN6_RP_UP_BUSY_AVG | + GEN7_RP_DOWN_IDLE_AVG); + + gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8); + + /* requires MSI enabled */ + I915_WRITE(GEN6_PMIER, GEN6_PM_DEFERRED_EVENTS); + spin_lock_irq(&dev_priv->rps.lock); + WARN_ON(dev_priv->rps.pm_iir != 0); + I915_WRITE(GEN6_PMIMR, 0); + spin_unlock_irq(&dev_priv->rps.lock); + /* enable all PM interrupts */ + I915_WRITE(GEN6_PMINTRMSK, 0); + + gen6_gt_force_wake_put(dev_priv); +} + static void gen6_update_ring_freq(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -3480,7 +3558,10 @@ static void intel_gen6_powersave_work(struct work_struct *work) struct drm_device *dev = dev_priv->dev; mutex_lock(&dev_priv->rps.hw_lock); - gen6_enable_rps(dev); + if (IS_HASWELL(dev)) + hsw_enable_rps(dev); + else + gen6_enable_rps(dev); gen6_update_ring_freq(dev); mutex_unlock(&dev_priv->rps.hw_lock); }
Power management, in special RC6 enabling, differs across platforms. This patch just split out enabling function for HSW. This is an attempt to make pm code more clean without multiple IS_HASWELL inside enable_rps function. This actually tends to get worse with upcoming platforms. Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> --- drivers/gpu/drm/i915/intel_pm.c | 211 +++++++++++++++++++++++++++------------- 1 file changed, 146 insertions(+), 65 deletions(-)