subject:"\[Intel\-gfx\] \[PATCH\] drm\/i915\/cnl\: Get RC6 working."

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

2017-10-24 Thread Rodrigo Vivi

On Tue, Oct 24, 2017 at 12:50:13PM +, David Weinehall wrote:
> 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 
> > Cc: Wayne Boyer 
> > Cc: Joe Konno 
> > Cc: David Weinehall 
> > Signed-off-by: Rodrigo Vivi 
> > Reviewed-by: James Ausmus 
> 
> I've verified that RC6 works with your patch applied.
> Minor comments below, but nothing major. Great work!
> 
> 
> Reviewed-by: David Weinehall 

thanks. merged to dinq.

> 
> > ---
> >  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 believe if IS_SKYLAKE(dev_priv) && *!* NEEDS_WaRsDisableCoarsePowerGating
since by name it seems WaRsDoubleRc6WrlWithCoarsePowerGating
is only needed with coarsepowergating on and the other one is disable
course power gating. right?

> 
> 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.

Since this would change the behaviour of the wa on few SKL skus
I believe it deserves a different patch.

Also on that patch we would need to check if we need to extend this
Wa to other gen9 platforms. I'd say we probably need this on kbl and cfl :/

Thanks,
Rodrigo.

> 
> > +   /*
> > +* 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

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

2017-10-24 Thread David Weinehall

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 
> Cc: Wayne Boyer 
> Cc: Joe Konno 
> Cc: David Weinehall 
> Signed-off-by: Rodrigo Vivi 
> Reviewed-by: James Ausmus 

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


Reviewed-by: David Weinehall 

> ---
>  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

[Intel-gfx] [PATCH] drm/i915/cnl: Get RC6 working.

2017-10-23 Thread Rodrigo Vivi

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 
Cc: Wayne Boyer 
Cc: Joe Konno 
Cc: David Weinehall 
Signed-off-by: Rodrigo Vivi 
Reviewed-by: James Ausmus 
---
 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)) {
+   /*
+* 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

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

2017-10-23 Thread James Ausmus

On Mon, Oct 23, 2017 at 02:20:01PM -0700, Rodrigo Vivi wrote:
> On CNL, individual ware rate limit was added to each engine.
s/ware/wake/

> 
> GT can only go to RC6 if both Render and Media engines are
> idividually qualified. So we need to set their individual
s/idividually/individually/

> wake rate limit.
> 
> +-+---+--+--+
> | |GT RC6 |  Render C6   |   Media C6   |
> +-+---+--+--+
> | Wake rate limit | 0xA09C[31:16] | 0xA09C[15:0] | 0xA0A0[15:0] |
> +-+---+--+--+
> 
> Cc: Nathan Ciobanu 
> Cc: Wayne Boyer 
> Cc: Joe Konno 
> Cc: David Weinehall 
> Signed-off-by: Rodrigo Vivi 

Matches my read of the spec. With the commit message nitpicking fixed :)

Reviewed-by: James Ausmus 

> ---
>  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..b193eda65e81 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 | 54);
> + I915_WRITE(GEN10_MEDIA_WAKE_RATE_LIMIT, 54);
> + } else if (IS_SKYLAKE(dev_priv)) {
> + /*
> +  * 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/cnl: Get RC6 working.

2017-10-23 Thread Rodrigo Vivi

On CNL, individual ware rate limit was added to each engine.

GT can only go to RC6 if both Render and Media engines are
idividually 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] |
+-+---+--+--+

Cc: Nathan Ciobanu 
Cc: Wayne Boyer 
Cc: Joe Konno 
Cc: David Weinehall 
Signed-off-by: Rodrigo Vivi 
---
 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..b193eda65e81 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 | 54);
+   I915_WRITE(GEN10_MEDIA_WAKE_RATE_LIMIT, 54);
+   } else if (IS_SKYLAKE(dev_priv)) {
+   /*
+* 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

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

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

[Intel-gfx] [PATCH] drm/i915/cnl: Get RC6 working.

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

[Intel-gfx] [PATCH] drm/i915/cnl: Get RC6 working.

5 matches

Site Navigation

Mail list logo

Footer information