Re: [PATCH] drm/i915/cnl: Get RC6 working.

[David Weinehall <david.weinehall@linux.intel.com>]

On Mon, Oct 23, 2017 at 03:46:12PM -0700, Rodrigo Vivi wrote:
> On CNL, individual wake rate limit was added to each engine.
> 
> GT can only go to RC6 if both Render and Media engines are
> individually qualified. So we need to set their individual
> wake rate limit.
> 
> +-----------------+---------------+--------------+--------------+
> |                 |    GT RC6     |  Render C6   |   Media C6   |
> +-----------------+---------------+--------------+--------------+
> | Wake rate limit | 0xA09C[31:16] | 0xA09C[15:0] | 0xA0A0[15:0] |
> +-----------------+---------------+--------------+--------------+
> 
> v2: - Tune Render and Media wake rate values according to some extra
>       info I got from HW engineers. Value can be tuned, but for now
>       these are the recommended values.
>     - Fix typos pointed by James.
> 
> Cc: Nathan Ciobanu <nathan.d.ciobanu@intel.com>
> Cc: Wayne Boyer <wayne.boyer@intel.com>
> Cc: Joe Konno <joe.konno@linux.intel.com>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: James Ausmus <james.ausmus@intel.com>

I've verified that RC6 works with your patch applied.
Minor comments below, but nothing major. Great work!


Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 68a58cce6ab1..f138eae82bf0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7905,6 +7905,7 @@ enum {
>  #define GEN6_RC1_WAKE_RATE_LIMIT		_MMIO(0xA098)
>  #define GEN6_RC6_WAKE_RATE_LIMIT		_MMIO(0xA09C)
>  #define GEN6_RC6pp_WAKE_RATE_LIMIT		_MMIO(0xA0A0)
> +#define GEN10_MEDIA_WAKE_RATE_LIMIT		_MMIO(0xA0A0)
>  #define GEN6_RC_EVALUATION_INTERVAL		_MMIO(0xA0A8)
>  #define GEN6_RC_IDLE_HYSTERSIS			_MMIO(0xA0AC)
>  #define GEN6_RC_SLEEP				_MMIO(0xA0B0)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5fdae39b1969..742d5455b201 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6605,12 +6605,19 @@ static void gen9_enable_rc6(struct drm_i915_private *dev_priv)
>  	I915_WRITE(GEN6_RC_CONTROL, 0);
>  
>  	/* 2b: Program RC6 thresholds.*/
> -
> -	/* WaRsDoubleRc6WrlWithCoarsePowerGating: Doubling WRL only when CPG is enabled */
> -	if (IS_SKYLAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 10) {
> +		I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 54 << 16 | 85);
> +		I915_WRITE(GEN10_MEDIA_WAKE_RATE_LIMIT, 150);
> +	} else if (IS_SKYLAKE(dev_priv)) {

How about:

} else if (NEEDS_WaRsDisableCoarsePowerGating(dev_priv)) {

I realise that this isn't code you're introducing, but fixing it at the
same time might make sense. We have a few other cases elsewhere with
where we apply Coarse PG even though (at least according to that
WA-test) we only need it on some Skylakes.

> +		/*
> +		 * WaRsDoubleRc6WrlWithCoarsePowerGating:skl Doubling WRL only
> +		 * when CPG is enabled
> +		 */
>  		I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 108 << 16);
> -	else
> +	} else {
>  		I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 54 << 16);
> +	}
> +
>  	I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); /* 12500 * 1280ns */
>  	I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25); /* 25 * 1280ns */
>  	for_each_engine(engine, dev_priv, id)
> -- 
> 2.13.5
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx