[Intel-gfx] [PATCH 3/7] drm/i915/skl: Restructured the gen6_set_rps_thresholds function

[Chris Wilson]

On Fri, Feb 06, 2015 at 08:26:34PM +0530, akash.goel at intel.com wrote:
> From: Akash Goel <akash.goel at intel.com>
> 
> Prior to SKL, the time period programmed in Up/Down EI & Up/Down
> threshold registers was in units of 1.28 micro seconds. But for
> SKL, the units have changed (1.333 micro seconds).
> Have generalized the implementation of gen6_set_rps_thresholds function,
> by removing the hard coding done in it as per 1.28 micro seconds.
> 
> Signed-off-by: Akash Goel <akash.goel at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 70 ++++++++++++++++++++---------------------
>  1 file changed, 34 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 58c8c0e..215b200 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3643,6 +3643,8 @@ static u32 gen6_rps_limits(struct drm_i915_private *dev_priv, u8 val)
>  static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
>  {
>  	int new_power;
> +	u32 threshold_up_pct = 0, threshold_down_pct = 0;

Drop the _pct, unrequired early initialisation, just comment that
up/down are in %.

> +	u32 ei_up = 0, ei_down = 0;
>  
>  	new_power = dev_priv->rps.power;
>  	switch (dev_priv->rps.power) {
> @@ -3675,59 +3677,55 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
>  	switch (new_power) {
>  	case LOW_POWER:
>  		/* Upclock if more than 95% busy over 16ms */
> -		I915_WRITE(GEN6_RP_UP_EI, 12500);
> -		I915_WRITE(GEN6_RP_UP_THRESHOLD, 11800);
> +		ei_up = 16000;
> +		threshold_up_pct = 95; /* x% */

Double comments that this is a %! Really doesn't seem to be required
with the preceeding comment.

> +	I915_WRITE(GEN6_RP_UP_EI,
> +		GT_FREQ_FROM_PERIOD(ei_up, dev_priv->dev));

Just pass dev_priv. It's magic.

> +	I915_WRITE(GEN6_RP_UP_THRESHOLD,
> +		GT_FREQ_FROM_PERIOD((ei_up * threshold_up_pct / 100),

I wonder if it is worth using base 128 instead of 100%.

Otherwise looks good and ties in with using it from vlv. Do you mind
reviewing those patches? They fix a bug in which the manual c0 counting
keeps interrupts alive whilst idle.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre