On Mon, 2 Jul 2012 11:51:05 -0300
Eugeni Dodonov <eugeni.dodonov at intel.com> wrote:
> Most of the RPS and RC6 enabling functionality is similar to what we had
> on Gen6/Gen7, so we preserve most of the registers.
>
> Note that Haswell only has RC6, so account for that as well. As suggested
> by Daniel Vetter, to reduce the amount of changes in the patch, we still
> write the RC6p/RC6pp thresholds, but those are ignored on Haswell.
>
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov at intel.com>
Reviewed-by: Ben Widawsky <ben at bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 37 +++++++++++++++++++++++++------------
> 2 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f17de3d..9d5bf06 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4156,6 +4156,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 29720d2..a3ee1b1 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2402,20 +2402,24 @@ static void gen6_enable_rps(struct drm_device *dev)
> I915_WRITE(GEN6_RC6p_THRESHOLD, 100000);
> 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;
>
> - if (rc6_mode & INTEL_RC6p_ENABLE)
> - rc6_mask |= GEN6_RC_CTL_RC6p_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_RC6pp_ENABLE)
> + rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE;
> + }
>
> DRM_INFO("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n",
> - (rc6_mode & INTEL_RC6_ENABLE) ? "on" : "off",
> - (rc6_mode & INTEL_RC6p_ENABLE) ? "on" : "off",
> - (rc6_mode & INTEL_RC6pp_ENABLE) ? "on" : "off");
> + (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off",
> + (rc6_mask & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off",
> + (rc6_mask & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off");
Belongs in a separate patch, but meh.
>
> I915_WRITE(GEN6_RC_CONTROL,
> rc6_mask |
> @@ -2433,10 +2437,19 @@ static void gen6_enable_rps(struct drm_device *dev)
> I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
> dev_priv->max_delay << 24 |
> dev_priv->min_delay << 16);
> - I915_WRITE(GEN6_RP_UP_THRESHOLD, 10000);
> - I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 1000000);
> - I915_WRITE(GEN6_RP_UP_EI, 100000);
> - I915_WRITE(GEN6_RP_DOWN_EI, 5000000);
> +
> + if (IS_HASWELL(dev)) {
> + 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);
> + } else {
> + I915_WRITE(GEN6_RP_UP_THRESHOLD, 10000);
> + I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 1000000);
> + I915_WRITE(GEN6_RP_UP_EI, 100000);
> + I915_WRITE(GEN6_RP_DOWN_EI, 5000000);
> + }
> +
I can't really comment much on this other than what I said below. It'd
be nice if the policy didn't change between platforms unless we
know/understand why.
> I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
> I915_WRITE(GEN6_RP_CONTROL,
> GEN6_RP_MEDIA_TURBO |
> @@ -2444,7 +2457,7 @@ static void gen6_enable_rps(struct drm_device *dev)
> GEN6_RP_MEDIA_IS_GFX |
> GEN6_RP_ENABLE |
> GEN6_RP_UP_BUSY_AVG |
> - GEN6_RP_DOWN_IDLE_CONT);
> + (IS_HASWELL(dev)) ? GEN7_RP_DOWN_IDLE_AVG : GEN6_RP_DOWN_IDLE_CONT);
I think this warrants an explanation. I think we would ideally like to
avoid different algorithms for different platforms.
>
> if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
> 500))
--
Ben Widawsky, Intel Open Source Technology Center
