[Intel-gfx] [PATCH 13/21] drm/i915: add RPS configuration for Haswell
Daniel Vetter
daniel at ffwll.ch
Fri Jun 29 11:56:33 CEST 2012
On Thu, Jun 28, 2012 at 03:55:41PM -0300, Eugeni Dodonov wrote:
> Split Haswell-specific GT algorithms into its own function.
>
> Note that Haswell only has RC6, so account for that as well.
>
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov at intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 160 ++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 155 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2c4be2e..0c53e4a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4131,6 +4131,7 @@
> #define GEN6_RP_UP_IDLE_MIN (0x1<<3)
> #define GEN6_RP_UP_BUSY_AVG (0x2<<3)
> #define GEN6_RP_UP_BUSY_CONT (0x4<<3)
> +#define GEN7_RP_DOWN_IDLE_AVG (0x2<<0)
> #define GEN6_RP_DOWN_IDLE_CONT (0x1<<0)
> #define GEN6_RP_UP_THRESHOLD 0xA02C
> #define GEN6_RP_DOWN_THRESHOLD 0xA030
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0334e42..0733f16 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2301,6 +2301,26 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
> dev_priv->cur_delay = val;
> }
>
> +static void hsw_disable_rps(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + I915_WRITE(GEN6_RC_CONTROL, 0);
> + I915_WRITE(GEN6_RPNSWREQ, 1 << 31);
> + I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
> + I915_WRITE(GEN6_PMIER, 0);
> + /* Complete PM interrupt masking here doesn't race with the rps work
> + * item again unmasking PM interrupts because that is using a different
> + * register (PMIMR) to mask PM interrupts. The only risk is in leaving
> + * stale bits in PMIIR and PMIMR which gen6_enable_rps will clean up. */
> +
> + spin_lock_irq(&dev_priv->rps_lock);
> + dev_priv->pm_iir = 0;
> + spin_unlock_irq(&dev_priv->rps_lock);
> +
> + I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
> +}
The only difference I can see wrt gen6_disable_rps is clearing the
RC_CONTROL reg. And that looks like a bugfix that gen6+ should get, too.
Imo copy&pasting this function hence doesn't make sense.
> +
> static void gen6_disable_rps(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2334,9 +2354,10 @@ int intel_enable_rc6(const struct drm_device *dev)
> if (INTEL_INFO(dev)->gen == 5)
> return 0;
>
> - /* Sorry Haswell, no RC6 for you for now. */
> + /* Haswell does not has RC6p nor RC6pp
> + */
> if (IS_HASWELL(dev))
> - return 0;
> + return INTEL_RC6_ENABLE;
>
> /*
> * Disable rc6 on Sandybridge
> @@ -2349,6 +2370,130 @@ int intel_enable_rc6(const struct drm_device *dev)
> return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
> }
>
> +static void hsw_enable_rps(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_ring_buffer *ring;
> + u32 pcu_mbox, rc6_mask = 0;
> + int rc6_mode;
> + int i;
> +
> + WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> +
> + /* 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);
> +
> + gen6_gt_force_wake_get(dev_priv);
> +
> + /* In units of 100MHz */
> + dev_priv->max_delay = 18;
> + dev_priv->min_delay = 6;
> + dev_priv->cur_delay = 0;
gen6 enable_rps reads these magic values from regs. Shouldn't we do that
for hsw, too?
> +
> + /* disable the counters and set deterministic thresholds */
> + I915_WRITE(GEN6_RC_CONTROL, 0);
> +
> + I915_WRITE(GEN6_RC1_WAKE_RATE_LIMIT, 1000 << 16);
> + 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_RC1e_THRESHOLD, 1000);
> + I915_WRITE(GEN6_RC6_THRESHOLD, 50000);
> +
> + 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 on Haswell: RC6 %s\n",
> + (rc6_mode & INTEL_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,
> + 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,
> + dev_priv->max_delay << 24 |
> + dev_priv->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_MODE |
I figure this should be changed like the gen6 rps code in 89ba829e ...
> + GEN6_RP_MEDIA_IS_GFX |
> + GEN6_RP_ENABLE |
> + GEN6_RP_UP_BUSY_AVG |
> + GEN7_RP_DOWN_IDLE_AVG);
> +
> + if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
> + 500))
> + DRM_ERROR("timeout waiting for pcode mailbox to become idle\n");
> +
> + I915_WRITE(GEN6_PCODE_DATA, 0);
> + I915_WRITE(GEN6_PCODE_MAILBOX,
> + GEN6_PCODE_READY |
> + GEN6_PCODE_WRITE_MIN_FREQ_TABLE);
> + if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
> + 500))
> + DRM_ERROR("timeout waiting for pcode mailbox to finish\n");
> +
> + /* Check for overclock support */
> + if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
> + 500))
> + DRM_ERROR("timeout waiting for pcode mailbox to become idle\n");
> + I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_READ_OC_PARAMS);
> + pcu_mbox = I915_READ(GEN6_PCODE_DATA);
> + if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
> + 500))
> + DRM_ERROR("timeout waiting for pcode mailbox to finish\n");
> + if (pcu_mbox & (1<<31)) { /* OC supported */
> + dev_priv->max_delay = pcu_mbox & 0xff;
> + DRM_DEBUG_DRIVER("overclocking supported, adjusting frequency max to %dMHz\n", pcu_mbox * 50);
> + }
> +
> + /* requires MSI enabled */
> + I915_WRITE(GEN6_PMIER,
> + GEN6_PM_MBOX_EVENT |
> + GEN6_PM_THERMAL_EVENT |
> + GEN6_PM_RP_DOWN_TIMEOUT |
> + GEN6_PM_RP_UP_THRESHOLD |
> + GEN6_PM_RP_DOWN_THRESHOLD |
> + GEN6_PM_RP_UP_EI_EXPIRED |
> + GEN6_PM_RP_DOWN_EI_EXPIRED);
> +
> + spin_lock_irq(&dev_priv->rps_lock);
> + WARN_ON(dev_priv->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);
> +}
Ok, after reviewing this with the gen6 code and hsw power doc I see very
few differences (besides the 2 things that look like mistakes):
- we don't set a few things related to rc6p&rc6pp
- we have slightly different timeout/threshold values.
When copy&pasting code we should balance the risk of introducing
regressions with the risk to not apply bugfixes everywhere they're needed.
Given that you've missed Jesse's fix while hacking on this and that this
seems to work very much like rps on snb/ivb I'd vote for sprinkling a few
ifs into gen6_enable_rps. Maybe set the timing values in local variables
at the beginning of the function.
-Daniel
> +
> static void gen6_enable_rps(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3227,7 +3372,9 @@ void intel_disable_gt_powersave(struct drm_device *dev)
> {
> if (IS_IRONLAKE_M(dev))
> ironlake_disable_drps(dev);
> - if (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev))
> + else if (IS_HASWELL(dev))
> + hsw_disable_rps(dev);
> + else if (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev))
> gen6_disable_rps(dev);
> }
>
> @@ -3237,9 +3384,10 @@ void intel_enable_gt_powersave(struct drm_device *dev)
> ironlake_enable_drps(dev);
> ironlake_enable_rc6(dev);
> intel_init_emon(dev);
> - }
> -
> - if ((IS_GEN6(dev) || IS_GEN7(dev)) && !IS_VALLEYVIEW(dev)) {
> + } else if (IS_HASWELL(dev)) {
> + hsw_enable_rps(dev);
> + gen6_update_ring_freq(dev);
> + } else if ((IS_GEN6(dev) || IS_GEN7(dev)) && !IS_VALLEYVIEW(dev)) {
> gen6_enable_rps(dev);
> gen6_update_ring_freq(dev);
> }
> --
> 1.7.11.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48
More information about the Intel-gfx
mailing list