[PATCH 1/2] drm/i915: Increase render/media power gating hysteresis for gen9+

[PATCH 1/2] drm/i915: Increase render/media power gating hysteresis for gen9+

[Chris Wilson]

On gen9+, after an idle period the HW will disable the entire power well
to conserve power (by preventing current leakage). It takes around a 100
microseconds to bring the power well back online afterwards. With the
current hysteresis value of 25us, we do not have sufficient time to
respond to an interrupt and schedule the next execution before the HW
powers itself down. (At present, we prevent this by grabbing the
forcewake for prolonged periods of time, but that is overkill fixed in
the next patch.) The minimum we want to set the power gating hysteresis
to is the length of time it takes us to service the GPU, which across a
broad spectrum of machines is about 250us.

(Note this also brings guc latency into the same ballpark as execlists.)

Testcase: igt/gem_exec_nop/sequential
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1db79a860b96..6748d3efb537 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6627,8 +6627,8 @@ static void gen9_enable_rc6(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN6_RC_SLEEP, 0);
 
 	/* 2c: Program Coarse Power Gating Policies. */
-	I915_WRITE(GEN9_MEDIA_PG_IDLE_HYSTERESIS, 25);
-	I915_WRITE(GEN9_RENDER_PG_IDLE_HYSTERESIS, 25);
+	I915_WRITE(GEN9_MEDIA_PG_IDLE_HYSTERESIS, 250);
+	I915_WRITE(GEN9_RENDER_PG_IDLE_HYSTERESIS, 250);
 
 	/* 3a: Enable RC6 */
 	I915_WRITE(GEN6_RC6_THRESHOLD, 37500); /* 37.5/125ms per EI */
-- 
2.15.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915/execlists: Skip forcewake for ELSP submission

[Chris Wilson]

Now that we can read the CSB from the HWSP, we may avoid having to
perform mmio reads entirely and so forgo the rigmarole of the forcewake
dance.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lrc.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ff25f209d0a5..7d2df72e68d3 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -778,6 +778,7 @@ static void execlists_submission_tasklet(unsigned long data)
 	struct intel_engine_execlists * const execlists = &engine->execlists;
 	struct execlist_port * const port = execlists->port;
 	struct drm_i915_private *dev_priv = engine->i915;
+	bool fw = false;
 
 	/* We can skip acquiring intel_runtime_pm_get() here as it was taken
 	 * on our behalf by the request (see i915_gem_mark_busy()) and it will
@@ -788,8 +789,6 @@ static void execlists_submission_tasklet(unsigned long data)
 	 */
 	GEM_BUG_ON(!dev_priv->gt.awake);
 
-	intel_uncore_forcewake_get(dev_priv, execlists->fw_domains);
-
 	/* Prefer doing test_and_clear_bit() as a two stage operation to avoid
 	 * imposing the cost of a locked atomic transaction when submitting a
 	 * new request (outside of the context-switch interrupt).
@@ -818,6 +817,12 @@ static void execlists_submission_tasklet(unsigned long data)
 		 */
 		__clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
 		if (unlikely(execlists->csb_head == -1)) { /* following a reset */
+			if (!fw) {
+				intel_uncore_forcewake_get(dev_priv,
+							   execlists->fw_domains);
+				fw = true;
+			}
+
 			head = readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)));
 			tail = GEN8_CSB_WRITE_PTR(head);
 			head = GEN8_CSB_READ_PTR(head);
@@ -943,7 +948,8 @@ static void execlists_submission_tasklet(unsigned long data)
 	if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT))
 		execlists_dequeue(engine);
 
-	intel_uncore_forcewake_put(dev_priv, execlists->fw_domains);
+	if (fw)
+		intel_uncore_forcewake_put(dev_priv, execlists->fw_domains);
 }
 
 static void insert_request(struct intel_engine_cs *engine,
-- 
2.15.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Increase render/media power gating hysteresis for gen9+

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Increase render/media power gating hysteresis for gen9+
URL   : https://patchwork.freedesktop.org/series/36842/
State : success

== Summary ==

Series 36842v1 series starting with [1/2] drm/i915: Increase render/media power gating hysteresis for gen9+
https://patchwork.freedesktop.org/api/1.0/series/36842/revisions/1/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> DMESG-WARN (fi-elk-e7500) fdo#103989 +1
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:433s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:436s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:384s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:515s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:293s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:503s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:500s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:493s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:481s
fi-elk-e7500     total:224  pass:168  dwarn:10  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:305s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:525s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:396s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:408s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:420s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:467s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:422s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:464s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:501s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:461s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:507s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:635s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:440s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:523s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:532s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:497s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:498s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:439s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:543s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:405s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:256  dwarn:0   dfail:0   fail:3   skip:26  time:568s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:485s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:420s

c2ce7a59fe93cae48eec312bb5a80bcd05455f76 drm-tip: 2018y-01m-20d-00h-46m-27s UTC integration manifest
b6a6cf5f48d0 drm/i915/execlists: Skip forcewake for ELSP submission
0bf3b1894f89 drm/i915: Increase render/media power gating hysteresis for gen9+

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7731/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: Increase render/media power gating hysteresis for gen9+

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Increase render/media power gating hysteresis for gen9+
URL   : https://patchwork.freedesktop.org/series/36842/
State : failure

== Summary ==

Warning: bzip CI_DRM_3664/shard-glkb6/results28.json.bz2 wasn't in correct JSON format
Test drv_suspend:
        Subgroup debugfs-reader:
                skip       -> PASS       (shard-hsw)
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                pass       -> FAIL       (shard-snb) fdo#101623
Test kms_flip:
        Subgroup flip-vs-modeset-vs-hang-interruptible:
                pass       -> DMESG-WARN (shard-snb) fdo#104311
        Subgroup 2x-wf_vblank-ts-check:
                fail       -> PASS       (shard-hsw)
Test pm_rc6_residency:
        Subgroup rc6-accuracy:
                skip       -> PASS       (shard-snb)
Test gem_cpu_reloc:
        Subgroup full:
                pass       -> INCOMPLETE (shard-hsw)
Test kms_cursor_crc:
        Subgroup cursor-128x128-suspend:
                pass       -> INCOMPLETE (shard-hsw) fdo#103540
Test gem_eio:
        Subgroup in-flight:
                dmesg-warn -> PASS       (shard-snb) fdo#104058

fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#104311 https://bugs.freedesktop.org/show_bug.cgi?id=104311
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#104058 https://bugs.freedesktop.org/show_bug.cgi?id=104058

shard-apl        total:2780 pass:1716 dwarn:1   dfail:0   fail:23  skip:1040 time:14599s
shard-hsw        total:2712 pass:1680 dwarn:1   dfail:0   fail:11  skip:1017 time:14384s
shard-snb        total:2780 pass:1315 dwarn:2   dfail:0   fail:14  skip:1449 time:8080s
Blacklisted hosts:
shard-kbl        total:2762 pass:1804 dwarn:19  dfail:1   fail:25  skip:912 time:10579s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7731/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Increase render/media power gating hysteresis for gen9+

[Sagar Arun Kamble]

On 1/20/2018 3:01 PM, Chris Wilson wrote:
> On gen9+, after an idle period the HW will disable the entire power well
> to conserve power (by preventing current leakage). It takes around a 100
> microseconds to bring the power well back online afterwards. With the
> current hysteresis value of 25us, we do not have sufficient time to
> respond to an interrupt and schedule the next execution before the HW
> powers itself down. (At present, we prevent this by grabbing the
> forcewake for prolonged periods of time, but that is overkill fixed in
> the next patch.) The minimum we want to set the power gating hysteresis
> to is the length of time it takes us to service the GPU, which across a
> broad spectrum of machines is about 250us.
>
> (Note this also brings guc latency into the same ballpark as execlists.)
>
> Testcase: igt/gem_exec_nop/sequential
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1db79a860b96..6748d3efb537 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6627,8 +6627,8 @@ static void gen9_enable_rc6(struct drm_i915_private *dev_priv)
>   	I915_WRITE(GEN6_RC_SLEEP, 0);
>   
>   	/* 2c: Program Coarse Power Gating Policies. */
> -	I915_WRITE(GEN9_MEDIA_PG_IDLE_HYSTERESIS, 25);
> -	I915_WRITE(GEN9_RENDER_PG_IDLE_HYSTERESIS, 25);
> +	I915_WRITE(GEN9_MEDIA_PG_IDLE_HYSTERESIS, 250);
> +	I915_WRITE(GEN9_RENDER_PG_IDLE_HYSTERESIS, 250);
As per bspec this is in 1.28usec units so it should be 195.
With that:
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>   
>   	/* 3a: Enable RC6 */
>   	I915_WRITE(GEN6_RC6_THRESHOLD, 37500); /* 37.5/125ms per EI */

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Increase render/media power gating hysteresis for gen9+

[Chris Wilson]

Quoting Sagar Arun Kamble (2018-01-21 11:01:23)
> 
> 
> On 1/20/2018 3:01 PM, Chris Wilson wrote:
> > On gen9+, after an idle period the HW will disable the entire power well
> > to conserve power (by preventing current leakage). It takes around a 100
> > microseconds to bring the power well back online afterwards. With the
> > current hysteresis value of 25us, we do not have sufficient time to
> > respond to an interrupt and schedule the next execution before the HW
> > powers itself down. (At present, we prevent this by grabbing the
> > forcewake for prolonged periods of time, but that is overkill fixed in
> > the next patch.) The minimum we want to set the power gating hysteresis
> > to is the length of time it takes us to service the GPU, which across a
> > broad spectrum of machines is about 250us.
> >
> > (Note this also brings guc latency into the same ballpark as execlists.)
> >
> > Testcase: igt/gem_exec_nop/sequential
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> > Cc: Michel Thierry <michel.thierry@intel.com>
> > Cc: Michal Winiarski <michal.winiarski@intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 1db79a860b96..6748d3efb537 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -6627,8 +6627,8 @@ static void gen9_enable_rc6(struct drm_i915_private *dev_priv)
> >       I915_WRITE(GEN6_RC_SLEEP, 0);
> >   
> >       /* 2c: Program Coarse Power Gating Policies. */
> > -     I915_WRITE(GEN9_MEDIA_PG_IDLE_HYSTERESIS, 25);
> > -     I915_WRITE(GEN9_RENDER_PG_IDLE_HYSTERESIS, 25);
> > +     I915_WRITE(GEN9_MEDIA_PG_IDLE_HYSTERESIS, 250);
> > +     I915_WRITE(GEN9_RENDER_PG_IDLE_HYSTERESIS, 250);
> As per bspec this is in 1.28usec units so it should be 195.

Both numbers pulled out of the air, just read us above as approx.us.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Increase render/media power gating hysteresis for gen9+

[Sagar Arun Kamble]

On 1/21/2018 5:55 PM, Chris Wilson wrote:
> Quoting Sagar Arun Kamble (2018-01-21 11:01:23)
>>
>> On 1/20/2018 3:01 PM, Chris Wilson wrote:
>>> On gen9+, after an idle period the HW will disable the entire power well
>>> to conserve power (by preventing current leakage). It takes around a 100
>>> microseconds to bring the power well back online afterwards. With the
>>> current hysteresis value of 25us, we do not have sufficient time to
>>> respond to an interrupt and schedule the next execution before the HW
>>> powers itself down. (At present, we prevent this by grabbing the
>>> forcewake for prolonged periods of time, but that is overkill fixed in
>>> the next patch.) The minimum we want to set the power gating hysteresis
>>> to is the length of time it takes us to service the GPU, which across a
>>> broad spectrum of machines is about 250us.
>>>
>>> (Note this also brings guc latency into the same ballpark as execlists.)
>>>
>>> Testcase: igt/gem_exec_nop/sequential
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>> Cc: Michel Thierry <michel.thierry@intel.com>
>>> Cc: Michal Winiarski <michal.winiarski@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_pm.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index 1db79a860b96..6748d3efb537 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -6627,8 +6627,8 @@ static void gen9_enable_rc6(struct drm_i915_private *dev_priv)
>>>        I915_WRITE(GEN6_RC_SLEEP, 0);
>>>    
>>>        /* 2c: Program Coarse Power Gating Policies. */
>>> -     I915_WRITE(GEN9_MEDIA_PG_IDLE_HYSTERESIS, 25);
>>> -     I915_WRITE(GEN9_RENDER_PG_IDLE_HYSTERESIS, 25);
>>> +     I915_WRITE(GEN9_MEDIA_PG_IDLE_HYSTERESIS, 250);
>>> +     I915_WRITE(GEN9_RENDER_PG_IDLE_HYSTERESIS, 250);
>> As per bspec this is in 1.28usec units so it should be 195.
> Both numbers pulled out of the air, just read us above as approx.us.
ok ... Interestingly spec too says 25us and also say that it is actually 
32us :)
> -Chris

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/execlists: Skip forcewake for ELSP submission

[Tvrtko Ursulin]

On 20/01/2018 09:31, Chris Wilson wrote:
> Now that we can read the CSB from the HWSP, we may avoid having to
> perform mmio reads entirely and so forgo the rigmarole of the forcewake
> dance.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index ff25f209d0a5..7d2df72e68d3 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -778,6 +778,7 @@ static void execlists_submission_tasklet(unsigned long data)
>   	struct intel_engine_execlists * const execlists = &engine->execlists;
>   	struct execlist_port * const port = execlists->port;
>   	struct drm_i915_private *dev_priv = engine->i915;
> +	bool fw = false;
>   
>   	/* We can skip acquiring intel_runtime_pm_get() here as it was taken
>   	 * on our behalf by the request (see i915_gem_mark_busy()) and it will
> @@ -788,8 +789,6 @@ static void execlists_submission_tasklet(unsigned long data)
>   	 */
>   	GEM_BUG_ON(!dev_priv->gt.awake);
>   
> -	intel_uncore_forcewake_get(dev_priv, execlists->fw_domains);
> -
>   	/* Prefer doing test_and_clear_bit() as a two stage operation to avoid
>   	 * imposing the cost of a locked atomic transaction when submitting a
>   	 * new request (outside of the context-switch interrupt).
> @@ -818,6 +817,12 @@ static void execlists_submission_tasklet(unsigned long data)
>   		 */
>   		__clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
>   		if (unlikely(execlists->csb_head == -1)) { /* following a reset */
> +			if (!fw) {
> +				intel_uncore_forcewake_get(dev_priv,
> +							   execlists->fw_domains);
> +				fw = true;
> +			}
> +
>   			head = readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)));
>   			tail = GEN8_CSB_WRITE_PTR(head);
>   			head = GEN8_CSB_READ_PTR(head);

There's a GEM_TRACE which does a readl a line below, outside the if.

Then there's a writel down lower, under "if (head != execlists->csb_head)".

> @@ -943,7 +948,8 @@ static void execlists_submission_tasklet(unsigned long data)
>   	if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT))
>   		execlists_dequeue(engine);
>   
> -	intel_uncore_forcewake_put(dev_priv, execlists->fw_domains);
> +	if (fw)
> +		intel_uncore_forcewake_put(dev_priv, execlists->fw_domains);
>   }
>   
>   static void insert_request(struct intel_engine_cs *engine,
> 

I had a similar patch, which also did some other tweaks, some of which I 
think you recently also sent. So I support it in principle.

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/execlists: Skip forcewake for ELSP submission

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-01-22 09:50:57)
> 
> On 20/01/2018 09:31, Chris Wilson wrote:
> > Now that we can read the CSB from the HWSP, we may avoid having to
> > perform mmio reads entirely and so forgo the rigmarole of the forcewake
> > dance.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/intel_lrc.c | 12 +++++++++---
> >   1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index ff25f209d0a5..7d2df72e68d3 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -778,6 +778,7 @@ static void execlists_submission_tasklet(unsigned long data)
> >       struct intel_engine_execlists * const execlists = &engine->execlists;
> >       struct execlist_port * const port = execlists->port;
> >       struct drm_i915_private *dev_priv = engine->i915;
> > +     bool fw = false;
> >   
> >       /* We can skip acquiring intel_runtime_pm_get() here as it was taken
> >        * on our behalf by the request (see i915_gem_mark_busy()) and it will
> > @@ -788,8 +789,6 @@ static void execlists_submission_tasklet(unsigned long data)
> >        */
> >       GEM_BUG_ON(!dev_priv->gt.awake);
> >   
> > -     intel_uncore_forcewake_get(dev_priv, execlists->fw_domains);
> > -
> >       /* Prefer doing test_and_clear_bit() as a two stage operation to avoid
> >        * imposing the cost of a locked atomic transaction when submitting a
> >        * new request (outside of the context-switch interrupt).
> > @@ -818,6 +817,12 @@ static void execlists_submission_tasklet(unsigned long data)
> >                */
> >               __clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
> >               if (unlikely(execlists->csb_head == -1)) { /* following a reset */
> > +                     if (!fw) {
> > +                             intel_uncore_forcewake_get(dev_priv,
> > +                                                        execlists->fw_domains);
> > +                             fw = true;
> > +                     }
> > +
> >                       head = readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)));
> >                       tail = GEN8_CSB_WRITE_PTR(head);
> >                       head = GEN8_CSB_READ_PTR(head);
> 
> There's a GEM_TRACE which does a readl a line below, outside the if.

Yeah, I didn't care too much about it. Maybe if we include fw into the
trace so we know whether to believe it or not.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915/execlists: Skip forcewake for ELSP submission

[Chris Wilson]

Now that we can read the CSB from the HWSP, we may avoid having to
perform mmio reads entirely and so forgo the rigmarole of the forcewake
dance.

v2: Include forcewake hint for GEM_TRACE readback of mmio. If we don't
hold fw ourselves, the reads may return garbage.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ff25f209d0a5..075e7f56e9ba 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -778,6 +778,7 @@ static void execlists_submission_tasklet(unsigned long data)
 	struct intel_engine_execlists * const execlists = &engine->execlists;
 	struct execlist_port * const port = execlists->port;
 	struct drm_i915_private *dev_priv = engine->i915;
+	bool fw = false;
 
 	/* We can skip acquiring intel_runtime_pm_get() here as it was taken
 	 * on our behalf by the request (see i915_gem_mark_busy()) and it will
@@ -788,8 +789,6 @@ static void execlists_submission_tasklet(unsigned long data)
 	 */
 	GEM_BUG_ON(!dev_priv->gt.awake);
 
-	intel_uncore_forcewake_get(dev_priv, execlists->fw_domains);
-
 	/* Prefer doing test_and_clear_bit() as a two stage operation to avoid
 	 * imposing the cost of a locked atomic transaction when submitting a
 	 * new request (outside of the context-switch interrupt).
@@ -818,6 +817,12 @@ static void execlists_submission_tasklet(unsigned long data)
 		 */
 		__clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
 		if (unlikely(execlists->csb_head == -1)) { /* following a reset */
+			if (!fw) {
+				intel_uncore_forcewake_get(dev_priv,
+							   execlists->fw_domains);
+				fw = true;
+			}
+
 			head = readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)));
 			tail = GEN8_CSB_WRITE_PTR(head);
 			head = GEN8_CSB_READ_PTR(head);
@@ -830,10 +835,10 @@ static void execlists_submission_tasklet(unsigned long data)
 			head = execlists->csb_head;
 			tail = READ_ONCE(buf[write_idx]);
 		}
-		GEM_TRACE("%s cs-irq head=%d [%d], tail=%d [%d]\n",
+		GEM_TRACE("%s cs-irq head=%d [%d%s], tail=%d [%d%s]\n",
 			  engine->name,
-			  head, GEN8_CSB_READ_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))),
-			  tail, GEN8_CSB_WRITE_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))));
+			  head, GEN8_CSB_READ_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))), fw ? "" : "?",
+			  tail, GEN8_CSB_WRITE_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))), fw ? "" : "?");
 
 		while (head != tail) {
 			struct drm_i915_gem_request *rq;
@@ -943,7 +948,8 @@ static void execlists_submission_tasklet(unsigned long data)
 	if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT))
 		execlists_dequeue(engine);
 
-	intel_uncore_forcewake_put(dev_priv, execlists->fw_domains);
+	if (fw)
+		intel_uncore_forcewake_put(dev_priv, execlists->fw_domains);
 }
 
 static void insert_request(struct intel_engine_cs *engine,
-- 
2.15.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Increase render/media power gating hysteresis for gen9+ (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Increase render/media power gating hysteresis for gen9+ (rev2)
URL   : https://patchwork.freedesktop.org/series/36842/
State : success

== Summary ==

Series 36842v2 series starting with [1/2] drm/i915: Increase render/media power gating hysteresis for gen9+
https://patchwork.freedesktop.org/api/1.0/series/36842/revisions/2/mbox/

Test gem_ringfill:
        Subgroup basic-default:
                skip       -> PASS       (fi-bsw-n3050)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:428s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:435s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:382s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:519s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:293s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:503s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:503s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:507s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:481s
fi-elk-e7500     total:224  pass:168  dwarn:10  dfail:0   fail:0   skip:45 
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:527s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:406s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:423s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:474s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:426s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:464s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:506s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:472s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:510s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:642s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:437s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:525s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:535s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:491s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:481s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:423s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:442s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:539s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:409s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:256  dwarn:0   dfail:0   fail:3   skip:26  time:567s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:486s
fi-hsw-4770 failed to connect after reboot

a1f34f9415a41d5a4d219bcbbe5cf39a0b07cc22 drm-tip: 2018y-01m-20d-19h-27m-32s UTC integration manifest
83fbad82bcdf drm/i915/execlists: Skip forcewake for ELSP submission
4b9a27fe28dd drm/i915: Increase render/media power gating hysteresis for gen9+

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7736/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915: Increase render/media power gating hysteresis for gen9+

[Chris Wilson]

On gen9+, after an idle period the HW will disable the entire power well
to conserve power (by preventing current leakage). It takes around a 100
microseconds to bring the power well back online afterwards. With the
current hysteresis value of 25us (really 25 * 1280ns), we do not have
sufficient time to respond to an interrupt and schedule the next execution
before the HW powers itself down. (At present, we prevent this by
grabbing the forcewake for prolonged periods of time, but that overkill
fixed in the next patch.) The minimum we want to set the power gating
hysteresis to is the length of time it takes us to service the GPU, which
across a broad spectrum of machines is about 250us.

(Note this also brings guc latency into the same ballpark as execlists.)

v2: Include some notes on where I plucked the numbers from.

Testcase: igt/gem_exec_nop/sequential
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1db79a860b96..0b92ea1dbd40 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6626,9 +6626,29 @@ static void gen9_enable_rc6(struct drm_i915_private *dev_priv)
 
 	I915_WRITE(GEN6_RC_SLEEP, 0);
 
-	/* 2c: Program Coarse Power Gating Policies. */
-	I915_WRITE(GEN9_MEDIA_PG_IDLE_HYSTERESIS, 25);
-	I915_WRITE(GEN9_RENDER_PG_IDLE_HYSTERESIS, 25);
+	/*
+	 * 2c: Program Coarse Power Gating Policies.
+	 *
+	 * Bspec's guidance is to use 25us (really 25 * 1280ns) here. What we
+	 * use instead is a more conservative estimate for the maximum time
+	 * it takes us to service a CS interrupt and submit a new ELSP - that
+	 * is the time which the GPU is idle waiting for the CPU to select the
+	 * next request to execute. If the idle hysteresis is less than that
+	 * interrupt service latency, the hardware will automatically gate
+	 * the power well and we will then incur the wake up cost on top of
+	 * the service latency. A similar guide from intel_pstate is that we
+	 * do not want the enable hysteresis to less than the wakeup latency.
+	 *
+	 * igt/gem_exec_nop/sequential provides a rough estimate for the
+	 * service latency, and puts it around 10us for Broadwell (and other
+	 * big core) and around 40us for Broxton (and other low power cores).
+	 * [Note that for legacy ringbuffer submission, this is less than 1us!]
+	 * However, the wakeup latency on Broxton is closer to 100us. To be
+	 * conservative, we have to factor in a context switch on top (due
+	 * to ksoftirqd).
+	 */
+	I915_WRITE(GEN9_MEDIA_PG_IDLE_HYSTERESIS, 250);
+	I915_WRITE(GEN9_RENDER_PG_IDLE_HYSTERESIS, 250);
 
 	/* 3a: Enable RC6 */
 	I915_WRITE(GEN6_RC6_THRESHOLD, 37500); /* 37.5/125ms per EI */
-- 
2.15.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [v2] drm/i915: Increase render/media power gating hysteresis for gen9+ (rev3)

[Patchwork]

== Series Details ==

Series: series starting with [v2] drm/i915: Increase render/media power gating hysteresis for gen9+ (rev3)
URL   : https://patchwork.freedesktop.org/series/36842/
State : success

== Summary ==

Series 36842v3 series starting with [v2] drm/i915: Increase render/media power gating hysteresis for gen9+
https://patchwork.freedesktop.org/api/1.0/series/36842/revisions/3/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:428s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:435s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:386s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:520s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:294s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:500s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:505s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:499s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:489s
fi-elk-e7500     total:224  pass:168  dwarn:9   dfail:1   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:309s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:528s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:398s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:407s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:425s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:471s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:422s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:464s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:510s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:462s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:510s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:631s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:441s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:522s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:539s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:492s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:487s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:425s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:442s
fi-snb-2520m     total:3    pass:2    dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:411s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:578s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:487s

8748fd927893780fde7b782d41ed70d0fd1d388e drm-tip: 2018y-01m-22d-12h-23m-58s UTC integration manifest
63181736bea6 drm/i915/execlists: Skip forcewake for ELSP submission
918ce1a71b8f drm/i915: Increase render/media power gating hysteresis for gen9+

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7738/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/execlists: Skip forcewake for ELSP submission

[Tvrtko Ursulin]

On 22/01/2018 10:07, Chris Wilson wrote:
> Now that we can read the CSB from the HWSP, we may avoid having to
> perform mmio reads entirely and so forgo the rigmarole of the forcewake
> dance.
> 
> v2: Include forcewake hint for GEM_TRACE readback of mmio. If we don't
> hold fw ourselves, the reads may return garbage.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index ff25f209d0a5..075e7f56e9ba 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -778,6 +778,7 @@ static void execlists_submission_tasklet(unsigned long data)
>   	struct intel_engine_execlists * const execlists = &engine->execlists;
>   	struct execlist_port * const port = execlists->port;
>   	struct drm_i915_private *dev_priv = engine->i915;
> +	bool fw = false;
>   
>   	/* We can skip acquiring intel_runtime_pm_get() here as it was taken
>   	 * on our behalf by the request (see i915_gem_mark_busy()) and it will
> @@ -788,8 +789,6 @@ static void execlists_submission_tasklet(unsigned long data)
>   	 */
>   	GEM_BUG_ON(!dev_priv->gt.awake);
>   
> -	intel_uncore_forcewake_get(dev_priv, execlists->fw_domains);
> -
>   	/* Prefer doing test_and_clear_bit() as a two stage operation to avoid
>   	 * imposing the cost of a locked atomic transaction when submitting a
>   	 * new request (outside of the context-switch interrupt).
> @@ -818,6 +817,12 @@ static void execlists_submission_tasklet(unsigned long data)
>   		 */
>   		__clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
>   		if (unlikely(execlists->csb_head == -1)) { /* following a reset */
> +			if (!fw) {
> +				intel_uncore_forcewake_get(dev_priv,
> +							   execlists->fw_domains);
> +				fw = true;
> +			}
> +
>   			head = readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)));
>   			tail = GEN8_CSB_WRITE_PTR(head);
>   			head = GEN8_CSB_READ_PTR(head);
> @@ -830,10 +835,10 @@ static void execlists_submission_tasklet(unsigned long data)
>   			head = execlists->csb_head;
>   			tail = READ_ONCE(buf[write_idx]);
>   		}
> -		GEM_TRACE("%s cs-irq head=%d [%d], tail=%d [%d]\n",
> +		GEM_TRACE("%s cs-irq head=%d [%d%s], tail=%d [%d%s]\n",
>   			  engine->name,
> -			  head, GEN8_CSB_READ_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))),
> -			  tail, GEN8_CSB_WRITE_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))));
> +			  head, GEN8_CSB_READ_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))), fw ? "" : "?",
> +			  tail, GEN8_CSB_WRITE_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))), fw ? "" : "?");

Not useful to log the correct value in any case? Or in other words, this 
trace is not useful for debugging potential problems with the HWSP CSB 
copy, while with this change it stops being so?

>   
>   		while (head != tail) {
>   			struct drm_i915_gem_request *rq;
> @@ -943,7 +948,8 @@ static void execlists_submission_tasklet(unsigned long data)

There is a writel a bit above which now has no fw in HWSP mode.

>   	if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT))
>   		execlists_dequeue(engine); > -	intel_uncore_forcewake_put(dev_priv, execlists->fw_domains);
> +	if (fw)
> +		intel_uncore_forcewake_put(dev_priv, execlists->fw_domains);
>   }
>   
>   static void insert_request(struct intel_engine_cs *engine,
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/execlists: Skip forcewake for ELSP submission

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-01-22 16:59:21)
> 
> On 22/01/2018 10:07, Chris Wilson wrote:
> > Now that we can read the CSB from the HWSP, we may avoid having to
> > perform mmio reads entirely and so forgo the rigmarole of the forcewake
> > dance.
> > 
> > v2: Include forcewake hint for GEM_TRACE readback of mmio. If we don't
> > hold fw ourselves, the reads may return garbage.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_lrc.c | 18 ++++++++++++------
> >   1 file changed, 12 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index ff25f209d0a5..075e7f56e9ba 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -778,6 +778,7 @@ static void execlists_submission_tasklet(unsigned long data)
> >       struct intel_engine_execlists * const execlists = &engine->execlists;
> >       struct execlist_port * const port = execlists->port;
> >       struct drm_i915_private *dev_priv = engine->i915;
> > +     bool fw = false;
> >   
> >       /* We can skip acquiring intel_runtime_pm_get() here as it was taken
> >        * on our behalf by the request (see i915_gem_mark_busy()) and it will
> > @@ -788,8 +789,6 @@ static void execlists_submission_tasklet(unsigned long data)
> >        */
> >       GEM_BUG_ON(!dev_priv->gt.awake);
> >   
> > -     intel_uncore_forcewake_get(dev_priv, execlists->fw_domains);
> > -
> >       /* Prefer doing test_and_clear_bit() as a two stage operation to avoid
> >        * imposing the cost of a locked atomic transaction when submitting a
> >        * new request (outside of the context-switch interrupt).
> > @@ -818,6 +817,12 @@ static void execlists_submission_tasklet(unsigned long data)
> >                */
> >               __clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
> >               if (unlikely(execlists->csb_head == -1)) { /* following a reset */
> > +                     if (!fw) {
> > +                             intel_uncore_forcewake_get(dev_priv,
> > +                                                        execlists->fw_domains);
> > +                             fw = true;
> > +                     }
> > +
> >                       head = readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)));
> >                       tail = GEN8_CSB_WRITE_PTR(head);
> >                       head = GEN8_CSB_READ_PTR(head);
> > @@ -830,10 +835,10 @@ static void execlists_submission_tasklet(unsigned long data)
> >                       head = execlists->csb_head;
> >                       tail = READ_ONCE(buf[write_idx]);
> >               }
> > -             GEM_TRACE("%s cs-irq head=%d [%d], tail=%d [%d]\n",
> > +             GEM_TRACE("%s cs-irq head=%d [%d%s], tail=%d [%d%s]\n",
> >                         engine->name,
> > -                       head, GEN8_CSB_READ_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))),
> > -                       tail, GEN8_CSB_WRITE_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))));
> > +                       head, GEN8_CSB_READ_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))), fw ? "" : "?",
> > +                       tail, GEN8_CSB_WRITE_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))), fw ? "" : "?");
> 
> Not useful to log the correct value in any case? Or in other words, this 
> trace is not useful for debugging potential problems with the HWSP CSB 
> copy, while with this change it stops being so?

It also completely invalidates CI's testing as otherwise we will always
be forcewaking on submission. I am not shedding a tear over the loss of
that information on the occasions it is garbage - it too is likely to
cause heisenbugs and mask the very information we are trying to present
(in this case latency between HWSP write and CS interrupt).

> >               while (head != tail) {
> >                       struct drm_i915_gem_request *rq;
> > @@ -943,7 +948,8 @@ static void execlists_submission_tasklet(unsigned long data)
> 
> There is a writel a bit above which now has no fw in HWSP mode.

And it doesn't need one, afaict. There's no imposed wakeup (no forcewake)
and no latency, ergo the HW is pulling the right value immediate.
Otherwise the mostly idle submission of new requests would go entirely
unnoticed. (I've always had a wonder about why they would add RING_TAIL
to the shadow regs but not RING_ELSP.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/execlists: Skip forcewake for ELSP submission

[Tvrtko Ursulin]

On 22/01/2018 17:17, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-01-22 16:59:21)
>>
>> On 22/01/2018 10:07, Chris Wilson wrote:
>>> Now that we can read the CSB from the HWSP, we may avoid having to
>>> perform mmio reads entirely and so forgo the rigmarole of the forcewake
>>> dance.
>>>
>>> v2: Include forcewake hint for GEM_TRACE readback of mmio. If we don't
>>> hold fw ourselves, the reads may return garbage.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_lrc.c | 18 ++++++++++++------
>>>    1 file changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>>> index ff25f209d0a5..075e7f56e9ba 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -778,6 +778,7 @@ static void execlists_submission_tasklet(unsigned long data)
>>>        struct intel_engine_execlists * const execlists = &engine->execlists;
>>>        struct execlist_port * const port = execlists->port;
>>>        struct drm_i915_private *dev_priv = engine->i915;
>>> +     bool fw = false;
>>>    
>>>        /* We can skip acquiring intel_runtime_pm_get() here as it was taken
>>>         * on our behalf by the request (see i915_gem_mark_busy()) and it will
>>> @@ -788,8 +789,6 @@ static void execlists_submission_tasklet(unsigned long data)
>>>         */
>>>        GEM_BUG_ON(!dev_priv->gt.awake);
>>>    
>>> -     intel_uncore_forcewake_get(dev_priv, execlists->fw_domains);
>>> -
>>>        /* Prefer doing test_and_clear_bit() as a two stage operation to avoid
>>>         * imposing the cost of a locked atomic transaction when submitting a
>>>         * new request (outside of the context-switch interrupt).
>>> @@ -818,6 +817,12 @@ static void execlists_submission_tasklet(unsigned long data)
>>>                 */
>>>                __clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
>>>                if (unlikely(execlists->csb_head == -1)) { /* following a reset */
>>> +                     if (!fw) {
>>> +                             intel_uncore_forcewake_get(dev_priv,
>>> +                                                        execlists->fw_domains);
>>> +                             fw = true;
>>> +                     }
>>> +
>>>                        head = readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)));
>>>                        tail = GEN8_CSB_WRITE_PTR(head);
>>>                        head = GEN8_CSB_READ_PTR(head);
>>> @@ -830,10 +835,10 @@ static void execlists_submission_tasklet(unsigned long data)
>>>                        head = execlists->csb_head;
>>>                        tail = READ_ONCE(buf[write_idx]);
>>>                }
>>> -             GEM_TRACE("%s cs-irq head=%d [%d], tail=%d [%d]\n",
>>> +             GEM_TRACE("%s cs-irq head=%d [%d%s], tail=%d [%d%s]\n",
>>>                          engine->name,
>>> -                       head, GEN8_CSB_READ_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))),
>>> -                       tail, GEN8_CSB_WRITE_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))));
>>> +                       head, GEN8_CSB_READ_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))), fw ? "" : "?",
>>> +                       tail, GEN8_CSB_WRITE_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine)))), fw ? "" : "?");
>>
>> Not useful to log the correct value in any case? Or in other words, this
>> trace is not useful for debugging potential problems with the HWSP CSB
>> copy, while with this change it stops being so?
> 
> It also completely invalidates CI's testing as otherwise we will always
> be forcewaking on submission. I am not shedding a tear over the loss of
> that information on the occasions it is garbage - it too is likely to
> cause heisenbugs and mask the very information we are trying to present
> (in this case latency between HWSP write and CS interrupt).

Ookay.

>>>                while (head != tail) {
>>>                        struct drm_i915_gem_request *rq;
>>> @@ -943,7 +948,8 @@ static void execlists_submission_tasklet(unsigned long data)
>>
>> There is a writel a bit above which now has no fw in HWSP mode.
> 
> And it doesn't need one, afaict. There's no imposed wakeup (no forcewake)
> and no latency, ergo the HW is pulling the right value immediate.
> Otherwise the mostly idle submission of new requests would go entirely
> unnoticed. (I've always had a wonder about why they would add RING_TAIL
> to the shadow regs but not RING_ELSP.)

Ha shadowed, ok.. GEM_BUG_ON(intel_reg_forcewake_for_read(...))? But not 
in the tasklet, perhaps somewhere else.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [v2] drm/i915: Increase render/media power gating hysteresis for gen9+ (rev3)

[Patchwork]

== Series Details ==

Series: series starting with [v2] drm/i915: Increase render/media power gating hysteresis for gen9+ (rev3)
URL   : https://patchwork.freedesktop.org/series/36842/
State : success

== Summary ==

Warning: bzip CI_DRM_3667/shard-glkb6/results7.json.bz2 wasn't in correct JSON format
Test kms_cursor_legacy:
        Subgroup cursor-vs-flip-atomic:
                fail       -> PASS       (shard-apl) fdo#103355
        Subgroup flip-vs-cursor-legacy:
                pass       -> FAIL       (shard-apl) fdo#102670
Test perf:
        Subgroup buffer-fill:
                fail       -> PASS       (shard-apl) fdo#103755
        Subgroup oa-exponents:
                fail       -> PASS       (shard-apl) fdo#102254
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
                fail       -> PASS       (shard-snb) fdo#101623 +1

fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#103755 https://bugs.freedesktop.org/show_bug.cgi?id=103755
fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623

shard-apl        total:2780 pass:1716 dwarn:1   dfail:0   fail:23  skip:1040 time:14666s
shard-hsw        total:2780 pass:1724 dwarn:1   dfail:0   fail:12  skip:1042 time:15570s
shard-snb        total:2780 pass:1317 dwarn:1   dfail:0   fail:13  skip:1449 time:8120s
Blacklisted hosts:
shard-kbl        total:2780 pass:1835 dwarn:3   dfail:0   fail:26  skip:916 time:11041s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7738/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.IGT: success for series starting with [v2] drm/i915: Increase render/media power gating hysteresis for gen9+ (rev3)

[Chris Wilson]

Quoting Patchwork (2018-01-22 21:26:48)
> == Series Details ==
> 
> Series: series starting with [v2] drm/i915: Increase render/media power gating hysteresis for gen9+ (rev3)
> URL   : https://patchwork.freedesktop.org/series/36842/
> State : success
> 
> == Summary ==
> 
> Warning: bzip CI_DRM_3667/shard-glkb6/results7.json.bz2 wasn't in correct JSON format
> Test kms_cursor_legacy:
>         Subgroup cursor-vs-flip-atomic:
>                 fail       -> PASS       (shard-apl) fdo#103355
>         Subgroup flip-vs-cursor-legacy:
>                 pass       -> FAIL       (shard-apl) fdo#102670
> Test perf:
>         Subgroup buffer-fill:
>                 fail       -> PASS       (shard-apl) fdo#103755
>         Subgroup oa-exponents:
>                 fail       -> PASS       (shard-apl) fdo#102254
> Test kms_frontbuffer_tracking:
>         Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
>                 fail       -> PASS       (shard-snb) fdo#101623 +1
> 
> fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
> fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
> fdo#103755 https://bugs.freedesktop.org/show_bug.cgi?id=103755
> fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
> fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
> 
> shard-apl        total:2780 pass:1716 dwarn:1   dfail:0   fail:23  skip:1040 time:14666s
> shard-hsw        total:2780 pass:1724 dwarn:1   dfail:0   fail:12  skip:1042 time:15570s
> shard-snb        total:2780 pass:1317 dwarn:1   dfail:0   fail:13  skip:1449 time:8120s
> Blacklisted hosts:
> shard-kbl        total:2780 pass:1835 dwarn:3   dfail:0   fail:26  skip:916 time:11041s
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7738/shards.html

Reran on trybot to confirm the kbl errors were just flukes,
https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_1695/shards.html
and pushed. Given that this is destined for 4.17, that gives us 3 months
to soak it before it gets exposed upstream. Fingers crossed!

Thanks,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx