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@xxxxxxxxx>
> Cc: Wayne Boyer <wayne.boyer@xxxxxxxxx>
> Cc: Joe Konno <joe.konno@xxxxxxxxxxxxxxx>
> Cc: David Weinehall <david.weinehall@xxxxxxxxxxxxxxx>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Reviewed-by: James Ausmus <james.ausmus@xxxxxxxxx>
I've verified that RC6 works with your patch applied.
Minor comments below, but nothing major. Great work!
Reviewed-by: David Weinehall <david.weinehall@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
