Re: [PATCH 2/3] drm/i915: Refactor gen6_set_rps

[Deepak S <deepak.s@linux.intel.com>]

On Thursday 27 March 2014 01:54 PM, Chris Wilson wrote:
> What used to be a short-circuit now needs to adjust interrupt masking in
> response to user requests for changing the min/max allowed frequencies.
> This is currently done by a special case and early return, but the next
> patch adds another common action to take, so refactor the code to reduce
> duplication.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 34 ++++++++++++++--------------------
>   1 file changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index edf1b29d9856..3ad590924062 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3017,36 +3017,30 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
>   	WARN_ON(val > dev_priv->rps.max_freq_softlimit);
>   	WARN_ON(val < dev_priv->rps.min_freq_softlimit);
>   
> -	if (val == dev_priv->rps.cur_freq) {
> -		/* min/max delay may still have been modified so be sure to
> -		 * write the limits value */
> -		I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
> -			   gen6_rps_limits(dev_priv, val));
> +	/* min/max delay may still have been modified so be sure to
> +	 * write the limits value.
> +	 */
> +	if (val != dev_priv->rps.cur_freq) {
> +		gen6_set_rps_thresholds(dev_priv, val);
>   
> -		return;
> +		if (IS_HASWELL(dev))
> +			I915_WRITE(GEN6_RPNSWREQ,
> +				   HSW_FREQUENCY(val));
> +		else
> +			I915_WRITE(GEN6_RPNSWREQ,
> +				   GEN6_FREQUENCY(val) |
> +				   GEN6_OFFSET(0) |
> +				   GEN6_AGGRESSIVE_TURBO);
>   	}
>   
> -	gen6_set_rps_thresholds(dev_priv, val);
> -
> -	if (IS_HASWELL(dev))
> -		I915_WRITE(GEN6_RPNSWREQ,
> -			   HSW_FREQUENCY(val));
> -	else
> -		I915_WRITE(GEN6_RPNSWREQ,
> -			   GEN6_FREQUENCY(val) |
> -			   GEN6_OFFSET(0) |
> -			   GEN6_AGGRESSIVE_TURBO);
> -
>   	/* Make sure we continue to get interrupts
>   	 * until we hit the minimum or maximum frequencies.
>   	 */
> -	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
> -		   gen6_rps_limits(dev_priv, val));
> +	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, gen6_rps_limits(dev_priv, val));
>   
>   	POSTING_READ(GEN6_RPNSWREQ);
>   
>   	dev_priv->rps.cur_freq = val;
> -
>   	trace_intel_gpu_freq_change(val * 50);
>   }
>   

Reviewed-by:Deepak S <deepak.s@linux.intel.com>