On Wed, Nov 06, 2013 at 01:56:26PM -0200, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>
> The RPS register writing routines use the current value of min/max to
> set certain limits and interrupt gating. If we set those afterwards, we
> risk setting up the hw incorrectly and losing power management events,
> and worse, trigger some internal assertions.
>
> Reorder the calling sequences to be correct, and remove the then
> unrequired clamping from inside set_rps(). And for a bonus, fix the bug
> of calling gen6_set_rps() from Valleyview.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Reviewer: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> CC: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> drivers/gpu/drm/i915/i915_sysfs.c | 16 ++++++++--------
> drivers/gpu/drm/i915/intel_pm.c | 19 +++++--------------
> 3 files changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 7008aac..5b28602 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2758,7 +2758,7 @@ i915_max_freq_set(void *data, u64 val)
> if (IS_VALLEYVIEW(dev)) {
> val = vlv_freq_opcode(dev_priv->mem_freq, val);
> dev_priv->rps.max_delay = val;
> - gen6_set_rps(dev, val);
> + valleyview_set_rps(dev, val);
> } else {
> do_div(val, GT_FREQUENCY_MULTIPLIER);
> dev_priv->rps.max_delay = val;
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index cef38fd..46291c4 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -342,15 +342,15 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
> DRM_DEBUG("User requested overclocking to %d\n",
> val * GT_FREQUENCY_MULTIPLIER);
>
> + dev_priv->rps.max_delay = val;
> +
> if (dev_priv->rps.cur_delay > val) {
> - if (IS_VALLEYVIEW(dev_priv->dev))
> - valleyview_set_rps(dev_priv->dev, val);
> + if (IS_VALLEYVIEW(dev))
> + valleyview_set_rps(dev, val);
> else
> - gen6_set_rps(dev_priv->dev, val);
> + gen6_set_rps(dev, val);
> }
>
> - dev_priv->rps.max_delay = val;
> -
> mutex_unlock(&dev_priv->rps.hw_lock);
>
> return count;
> @@ -411,15 +411,15 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
> return -EINVAL;
> }
>
> + dev_priv->rps.min_delay = val;
> +
> if (dev_priv->rps.cur_delay < val) {
> if (IS_VALLEYVIEW(dev))
> valleyview_set_rps(dev, val);
> else
> - gen6_set_rps(dev_priv->dev, val);
> + gen6_set_rps(dev, val);
> }
>
> - dev_priv->rps.min_delay = val;
> -
> mutex_unlock(&dev_priv->rps.hw_lock);
>
> return count;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 09ac9e7..830865e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3414,26 +3414,19 @@ static void ironlake_disable_drps(struct drm_device *dev)
> * ourselves, instead of doing a rmw cycle (which might result in us clearing
> * all limits and the gpu stuck at whatever frequency it is at atm).
> */
> -static u32 gen6_rps_limits(struct drm_i915_private *dev_priv, u8 *val)
> +static u32 gen6_rps_limits(struct drm_i915_private *dev_priv, u8 val)
> {
> u32 limits;
>
> - limits = 0;
> -
> - if (*val >= dev_priv->rps.max_delay)
> - *val = dev_priv->rps.max_delay;
> - limits |= dev_priv->rps.max_delay << 24;
> -
> /* Only set the down limit when we've reached the lowest level to avoid
> * getting more interrupts, otherwise leave this clear. This prevents a
> * race in the hw when coming out of rc6: There's a tiny window where
> * the hw runs at the minimal clock before selecting the desired
> * frequency, if the down threshold expires in that window we will not
> * receive a down interrupt. */
> - if (*val <= dev_priv->rps.min_delay) {
> - *val = dev_priv->rps.min_delay;
> + limits = dev_priv->rps.max_delay << 24;
> + if (val <= dev_priv->rps.min_delay)
> limits |= dev_priv->rps.min_delay << 16;
> - }
>
> return limits;
> }
> @@ -3533,7 +3526,6 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
> void gen6_set_rps(struct drm_device *dev, u8 val)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 limits = gen6_rps_limits(dev_priv, &val);
>
> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
> WARN_ON(val > dev_priv->rps.max_delay);
> @@ -3556,7 +3548,8 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
> /* Make sure we continue to get interrupts
> * until we hit the minimum or maximum frequencies.
> */
> - I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, limits);
> + I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
> + gen6_rps_limits(dev_priv, val));
>
> POSTING_READ(GEN6_RPNSWREQ);
>
> @@ -3620,8 +3613,6 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> - gen6_rps_limits(dev_priv, &val);
> -
> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
> WARN_ON(val > dev_priv->rps.max_delay);
> WARN_ON(val < dev_priv->rps.min_delay);
> --
> 1.8.3.1
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
