* [PATCH 1/4] drm/i915: Only disable execlist preemption for the duration of the request
@ 2017-01-21 9:28 Chris Wilson
2017-01-21 9:28 ` [PATCH 2/4] drm/i915: Skip the execlists CSB scan and rewrite if the ring is untouched Chris Wilson
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Chris Wilson @ 2017-01-21 9:28 UTC (permalink / raw)
To: intel-gfx
We need to prevent resubmission of the context immediately following an
initial resubmit (which does a lite-restore preemption). Currently we do
this by disabling all submission whilst the context is still active, but
we can improve this by limiting the restriction to only until we
receive notification from the context-switch interrupt that the
lite-restore preemption is complete.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_debugfs.c | 18 ++++++++++++------
drivers/gpu/drm/i915/intel_lrc.c | 7 +++----
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
3 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index fa69d72fdcb9..9d7a77ecec3d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3320,15 +3320,21 @@ static int i915_engine_info(struct seq_file *m, void *unused)
rcu_read_lock();
rq = READ_ONCE(engine->execlist_port[0].request);
- if (rq)
- print_request(m, rq, "\t\tELSP[0] ");
- else
+ if (rq) {
+ seq_printf(m, "\t\tELSP[0] count=%d, ",
+ engine->execlist_port[0].count);
+ print_request(m, rq, "rq: ");
+ } else {
seq_printf(m, "\t\tELSP[0] idle\n");
+ }
rq = READ_ONCE(engine->execlist_port[1].request);
- if (rq)
- print_request(m, rq, "\t\tELSP[1] ");
- else
+ if (rq) {
+ seq_printf(m, "\t\tELSP[1] count=%d, ",
+ engine->execlist_port[1].count);
+ print_request(m, rq, "rq: ");
+ } else {
seq_printf(m, "\t\tELSP[1] idle\n");
+ }
rcu_read_unlock();
spin_lock_irq(&engine->timeline->lock);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 432ee495dec2..706b6596ada6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -388,7 +388,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
execlists_context_status_change(port[0].request,
INTEL_CONTEXT_SCHEDULE_IN);
desc[0] = execlists_update_context(port[0].request);
- engine->preempt_wa = port[0].count++; /* bdw only? fixed on skl? */
+ port[0].count++;
if (port[1].request) {
GEM_BUG_ON(port[1].count);
@@ -558,7 +558,8 @@ static bool execlists_elsp_ready(struct intel_engine_cs *engine)
int port;
port = 1; /* wait for a free slot */
- if (engine->disable_lite_restore_wa || engine->preempt_wa)
+ if (engine->disable_lite_restore_wa ||
+ engine->execlist_port[0].count > 1)
port = 0; /* wait for GPU to be idle before continuing */
return !engine->execlist_port[port].request;
@@ -604,8 +605,6 @@ static void intel_lrc_irq_handler(unsigned long data)
i915_gem_request_put(port[0].request);
port[0] = port[1];
memset(&port[1], 0, sizeof(port[1]));
-
- engine->preempt_wa = false;
}
GEM_BUG_ON(port[0].count == 0 &&
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 79c2b8d72322..34cdbb6350a8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -381,7 +381,6 @@ struct intel_engine_cs {
struct rb_node *execlist_first;
unsigned int fw_domains;
bool disable_lite_restore_wa;
- bool preempt_wa;
u32 ctx_desc_template;
/* Contexts are pinned whilst they are active on the GPU. The last
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/4] drm/i915: Skip the execlists CSB scan and rewrite if the ring is untouched
2017-01-21 9:28 [PATCH 1/4] drm/i915: Only disable execlist preemption for the duration of the request Chris Wilson
@ 2017-01-21 9:28 ` Chris Wilson
2017-01-23 11:33 ` Joonas Lahtinen
2017-01-21 9:28 ` [PATCH 3/4] drm/i915: Dequeue execlists on a new request if any port is available Chris Wilson
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2017-01-21 9:28 UTC (permalink / raw)
To: intel-gfx
If the CSB head/tail pointers are unchanged, we can skip the update of
the CSB register afterwards.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_lrc.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 706b6596ada6..9e56e1604e7f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -577,7 +577,7 @@ static void intel_lrc_irq_handler(unsigned long data)
intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
- if (!execlists_elsp_idle(engine)) {
+ if (!execlists_elsp_idle(engine)) do {
u32 __iomem *csb_mmio =
dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine));
u32 __iomem *buf =
@@ -587,9 +587,12 @@ static void intel_lrc_irq_handler(unsigned long data)
csb = readl(csb_mmio);
head = GEN8_CSB_READ_PTR(csb);
tail = GEN8_CSB_WRITE_PTR(csb);
+ if (head == tail)
+ break;
+
if (tail < head)
tail += GEN8_CSB_ENTRIES;
- while (head < tail) {
+ do {
unsigned int idx = ++head % GEN8_CSB_ENTRIES;
unsigned int status = readl(buf + 2 * idx);
@@ -609,12 +612,12 @@ static void intel_lrc_irq_handler(unsigned long data)
GEM_BUG_ON(port[0].count == 0 &&
!(status & GEN8_CTX_STATUS_ACTIVE_IDLE));
- }
+ } while (head < tail);
writel(_MASKED_FIELD(GEN8_CSB_READ_PTR_MASK,
GEN8_CSB_WRITE_PTR(csb) << 8),
csb_mmio);
- }
+ } while (0);
if (execlists_elsp_ready(engine))
execlists_dequeue(engine);
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/4] drm/i915: Dequeue execlists on a new request if any port is available
2017-01-21 9:28 [PATCH 1/4] drm/i915: Only disable execlist preemption for the duration of the request Chris Wilson
2017-01-21 9:28 ` [PATCH 2/4] drm/i915: Skip the execlists CSB scan and rewrite if the ring is untouched Chris Wilson
@ 2017-01-21 9:28 ` Chris Wilson
2017-01-23 10:56 ` Tvrtko Ursulin
2017-01-21 9:28 ` [PATCH 4/4] drm/i915: Emit dma-fence (and execlists submit) first from signaler Chris Wilson
2017-01-21 10:54 ` ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Only disable execlist preemption for the duration of the request Patchwork
3 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2017-01-21 9:28 UTC (permalink / raw)
To: intel-gfx
If the second ELSP port is available, schedule the execlists tasklet to
see if the new request can occupy it.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 9e56e1604e7f..d2cbdc730e14 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -661,7 +661,7 @@ static void execlists_submit_request(struct drm_i915_gem_request *request)
if (insert_request(&request->priotree, &engine->execlist_queue))
engine->execlist_first = &request->priotree.node;
- if (execlists_elsp_idle(engine))
+ if (execlists_elsp_ready(engine))
tasklet_hi_schedule(&engine->irq_tasklet);
spin_unlock_irqrestore(&engine->timeline->lock, flags);
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/4] drm/i915: Emit dma-fence (and execlists submit) first from signaler
2017-01-21 9:28 [PATCH 1/4] drm/i915: Only disable execlist preemption for the duration of the request Chris Wilson
2017-01-21 9:28 ` [PATCH 2/4] drm/i915: Skip the execlists CSB scan and rewrite if the ring is untouched Chris Wilson
2017-01-21 9:28 ` [PATCH 3/4] drm/i915: Dequeue execlists on a new request if any port is available Chris Wilson
@ 2017-01-21 9:28 ` Chris Wilson
2017-01-23 10:58 ` Tvrtko Ursulin
2017-01-23 11:37 ` Joonas Lahtinen
2017-01-21 10:54 ` ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Only disable execlist preemption for the duration of the request Patchwork
3 siblings, 2 replies; 12+ messages in thread
From: Chris Wilson @ 2017-01-21 9:28 UTC (permalink / raw)
To: intel-gfx
When introduced, I thought that reducing client latency from the
signaler was the priority. Since its inception the signaler has become
responsible for keeping the execlists full, via the dma-fence. As this
is very important to minimise overall execution time, signal the
dma-fence first and then signal any waiting clients.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_breadcrumbs.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index fcfa423d08bd..c1f3bd2e053b 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -459,16 +459,16 @@ static int intel_breadcrumbs_signaler(void *arg)
*/
request = READ_ONCE(b->first_signal);
if (signal_complete(request)) {
+ local_bh_disable();
+ dma_fence_signal(&request->fence);
+ local_bh_enable(); /* kick start the tasklets */
+
/* Wake up all other completed waiters and select the
* next bottom-half for the next user interrupt.
*/
intel_engine_remove_wait(engine,
&request->signaling.wait);
- local_bh_disable();
- dma_fence_signal(&request->fence);
- local_bh_enable(); /* kick start the tasklets */
-
/* Find the next oldest signal. Note that as we have
* not been holding the lock, another client may
* have installed an even older signal than the one
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Only disable execlist preemption for the duration of the request
2017-01-21 9:28 [PATCH 1/4] drm/i915: Only disable execlist preemption for the duration of the request Chris Wilson
` (2 preceding siblings ...)
2017-01-21 9:28 ` [PATCH 4/4] drm/i915: Emit dma-fence (and execlists submit) first from signaler Chris Wilson
@ 2017-01-21 10:54 ` Patchwork
2017-01-21 11:02 ` Chris Wilson
3 siblings, 1 reply; 12+ messages in thread
From: Patchwork @ 2017-01-21 10:54 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915: Only disable execlist preemption for the duration of the request
URL : https://patchwork.freedesktop.org/series/18340/
State : warning
== Summary ==
Series 18340v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/18340/revisions/1/mbox/
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6700k)
fi-bdw-5557u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14
fi-bsw-n3050 total:246 pass:207 dwarn:0 dfail:0 fail:0 skip:39
fi-bxt-j4205 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22
fi-bxt-t5700 total:79 pass:66 dwarn:0 dfail:0 fail:0 skip:12
fi-byt-j1900 total:246 pass:219 dwarn:0 dfail:0 fail:0 skip:27
fi-byt-n2820 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31
fi-hsw-4770 total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19
fi-hsw-4770r total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19
fi-ivb-3520m total:246 pass:224 dwarn:0 dfail:0 fail:1 skip:21
fi-ivb-3770 total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21
fi-kbl-7500u total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21
fi-skl-6260u total:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13
fi-skl-6700hq total:246 pass:225 dwarn:1 dfail:0 fail:0 skip:20
fi-skl-6700k total:246 pass:221 dwarn:4 dfail:0 fail:0 skip:21
fi-skl-6770hq total:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13
fi-snb-2520m total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31
fi-snb-2600 total:246 pass:214 dwarn:0 dfail:0 fail:0 skip:32
ef0d1b98a69f8a909d933e54cb8e0dd55d4bcc08 drm-tip: 2017y-01m-21d-09h-20m-23s UTC integration manifest
65306dd drm/i915: Emit dma-fence (and execlists submit) first from signaler
4b38bed drm/i915: Dequeue execlists on a new request if any port is available
ef303cf drm/i915: Skip the execlists CSB scan and rewrite if the ring is untouched
98bb83f drm/i915: Only disable execlist preemption for the duration of the request
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3570/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Only disable execlist preemption for the duration of the request
2017-01-21 10:54 ` ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Only disable execlist preemption for the duration of the request Patchwork
@ 2017-01-21 11:02 ` Chris Wilson
0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2017-01-21 11:02 UTC (permalink / raw)
To: intel-gfx
On Sat, Jan 21, 2017 at 10:54:09AM -0000, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [1/4] drm/i915: Only disable execlist preemption for the duration of the request
> URL : https://patchwork.freedesktop.org/series/18340/
> State : warning
>
> == Summary ==
>
> Series 18340v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/18340/revisions/1/mbox/
>
> Test gem_exec_suspend:
> Subgroup basic-s4-devices:
> pass -> DMESG-WARN (fi-skl-6700hq)
> pass -> DMESG-WARN (fi-skl-6700k)
This is an odd one, after resume skl (and only some skl gt) loses track
temporarily. From our pov, we start from an empty list after resume
(which we assert before we suspend). A reset seems enough to make it
behave again.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] drm/i915: Dequeue execlists on a new request if any port is available
2017-01-21 9:28 ` [PATCH 3/4] drm/i915: Dequeue execlists on a new request if any port is available Chris Wilson
@ 2017-01-23 10:56 ` Tvrtko Ursulin
2017-01-23 11:21 ` Chris Wilson
0 siblings, 1 reply; 12+ messages in thread
From: Tvrtko Ursulin @ 2017-01-23 10:56 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 21/01/2017 09:28, Chris Wilson wrote:
> If the second ELSP port is available, schedule the execlists tasklet to
> see if the new request can occupy it.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 9e56e1604e7f..d2cbdc730e14 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -661,7 +661,7 @@ static void execlists_submit_request(struct drm_i915_gem_request *request)
>
> if (insert_request(&request->priotree, &engine->execlist_queue))
> engine->execlist_first = &request->priotree.node;
> - if (execlists_elsp_idle(engine))
> + if (execlists_elsp_ready(engine))
> tasklet_hi_schedule(&engine->irq_tasklet);
>
> spin_unlock_irqrestore(&engine->timeline->lock, flags);
>
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] drm/i915: Emit dma-fence (and execlists submit) first from signaler
2017-01-21 9:28 ` [PATCH 4/4] drm/i915: Emit dma-fence (and execlists submit) first from signaler Chris Wilson
@ 2017-01-23 10:58 ` Tvrtko Ursulin
2017-01-23 11:37 ` Joonas Lahtinen
1 sibling, 0 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2017-01-23 10:58 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 21/01/2017 09:28, Chris Wilson wrote:
> When introduced, I thought that reducing client latency from the
> signaler was the priority. Since its inception the signaler has become
> responsible for keeping the execlists full, via the dma-fence. As this
> is very important to minimise overall execution time, signal the
> dma-fence first and then signal any waiting clients.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/intel_breadcrumbs.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> index fcfa423d08bd..c1f3bd2e053b 100644
> --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> @@ -459,16 +459,16 @@ static int intel_breadcrumbs_signaler(void *arg)
> */
> request = READ_ONCE(b->first_signal);
> if (signal_complete(request)) {
> + local_bh_disable();
> + dma_fence_signal(&request->fence);
> + local_bh_enable(); /* kick start the tasklets */
> +
> /* Wake up all other completed waiters and select the
> * next bottom-half for the next user interrupt.
> */
> intel_engine_remove_wait(engine,
> &request->signaling.wait);
>
> - local_bh_disable();
> - dma_fence_signal(&request->fence);
> - local_bh_enable(); /* kick start the tasklets */
> -
> /* Find the next oldest signal. Note that as we have
> * not been holding the lock, another client may
> * have installed an even older signal than the one
>
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] drm/i915: Dequeue execlists on a new request if any port is available
2017-01-23 10:56 ` Tvrtko Ursulin
@ 2017-01-23 11:21 ` Chris Wilson
0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2017-01-23 11:21 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Mon, Jan 23, 2017 at 10:56:12AM +0000, Tvrtko Ursulin wrote:
>
> On 21/01/2017 09:28, Chris Wilson wrote:
> >If the second ELSP port is available, schedule the execlists tasklet to
> >see if the new request can occupy it.
> >
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >---
> > drivers/gpu/drm/i915/intel_lrc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >index 9e56e1604e7f..d2cbdc730e14 100644
> >--- a/drivers/gpu/drm/i915/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >@@ -661,7 +661,7 @@ static void execlists_submit_request(struct drm_i915_gem_request *request)
> >
> > if (insert_request(&request->priotree, &engine->execlist_queue))
> > engine->execlist_first = &request->priotree.node;
> >- if (execlists_elsp_idle(engine))
> >+ if (execlists_elsp_ready(engine))
> > tasklet_hi_schedule(&engine->irq_tasklet);
> >
> > spin_unlock_irqrestore(&engine->timeline->lock, flags);
> >
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Note that I need to fix the irq_tasklet not to read CSB prior to the
first interrupt before this patch works in all circumstances.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] drm/i915: Skip the execlists CSB scan and rewrite if the ring is untouched
2017-01-21 9:28 ` [PATCH 2/4] drm/i915: Skip the execlists CSB scan and rewrite if the ring is untouched Chris Wilson
@ 2017-01-23 11:33 ` Joonas Lahtinen
2017-01-23 11:47 ` Chris Wilson
0 siblings, 1 reply; 12+ messages in thread
From: Joonas Lahtinen @ 2017-01-23 11:33 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On la, 2017-01-21 at 09:28 +0000, Chris Wilson wrote:
> If the CSB head/tail pointers are unchanged, we can skip the update of
> the CSB register afterwards.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
<SNIP>
> @@ -577,7 +577,7 @@ static void intel_lrc_irq_handler(unsigned long data)
>
> intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
>
> - if (!execlists_elsp_idle(engine)) {
> + if (!execlists_elsp_idle(engine)) do {
Oh dear, not like this. I hope this was a leftover from debugging.
> @@ -587,9 +587,12 @@ static void intel_lrc_irq_handler(unsigned long data)
> csb = readl(csb_mmio);
> head = GEN8_CSB_READ_PTR(csb);
> tail = GEN8_CSB_WRITE_PTR(csb);
> + if (head == tail)
> + break;
> +
> if (tail < head)
> tail += GEN8_CSB_ENTRIES;
> - while (head < tail) {
> + do {
> unsigned int idx = ++head % GEN8_CSB_ENTRIES;
> unsigned int status = readl(buf + 2 * idx);
Theoretically it should not matter, I wonder if this is covered well
enough in CI that we're confident to skip such writes?
Code is;
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] drm/i915: Emit dma-fence (and execlists submit) first from signaler
2017-01-21 9:28 ` [PATCH 4/4] drm/i915: Emit dma-fence (and execlists submit) first from signaler Chris Wilson
2017-01-23 10:58 ` Tvrtko Ursulin
@ 2017-01-23 11:37 ` Joonas Lahtinen
1 sibling, 0 replies; 12+ messages in thread
From: Joonas Lahtinen @ 2017-01-23 11:37 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On la, 2017-01-21 at 09:28 +0000, Chris Wilson wrote:
> When introduced, I thought that reducing client latency from the
> signaler was the priority. Since its inception the signaler has become
> responsible for keeping the execlists full, via the dma-fence. As this
> is very important to minimise overall execution time, signal the
> dma-fence first and then signal any waiting clients.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Do we have a clear testcase where this can be observed?
Gladly it's well isolated to one patch, so should make bisecting easier
during testing;
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] drm/i915: Skip the execlists CSB scan and rewrite if the ring is untouched
2017-01-23 11:33 ` Joonas Lahtinen
@ 2017-01-23 11:47 ` Chris Wilson
0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2017-01-23 11:47 UTC (permalink / raw)
To: Joonas Lahtinen; +Cc: intel-gfx
On Mon, Jan 23, 2017 at 01:33:32PM +0200, Joonas Lahtinen wrote:
> On la, 2017-01-21 at 09:28 +0000, Chris Wilson wrote:
> > If the CSB head/tail pointers are unchanged, we can skip the update of
> > the CSB register afterwards.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> <SNIP>
>
> > @@ -577,7 +577,7 @@ static void intel_lrc_irq_handler(unsigned long data)
> >
> > intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
> >
> > - if (!execlists_elsp_idle(engine)) {
> > + if (!execlists_elsp_idle(engine)) do {
>
> Oh dear, not like this. I hope this was a leftover from debugging.
>
> > @@ -587,9 +587,12 @@ static void intel_lrc_irq_handler(unsigned long data)
> > csb = readl(csb_mmio);
> > head = GEN8_CSB_READ_PTR(csb);
> > tail = GEN8_CSB_WRITE_PTR(csb);
> > + if (head == tail)
> > + break;
> > +
> > if (tail < head)
> > tail += GEN8_CSB_ENTRIES;
> > - while (head < tail) {
> > + do {
> > unsigned int idx = ++head % GEN8_CSB_ENTRIES;
> > unsigned int status = readl(buf + 2 * idx);
>
> Theoretically it should not matter, I wonder if this is covered well
> enough in CI that we're confident to skip such writes?
We see head == tail reasonably frequently (just the effect of timing
between interrupts and mmio reads). The single register write is not of
huge consequence, it just looked easy to remove by adjusting the
loop. The dominant issue here are the mmio reads of the buffer. The
documentation talks about mapping these registers as cacheable and so
doing a single cacheline read to grab the entire set. That keeps
frustrating me.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-01-23 11:47 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-21 9:28 [PATCH 1/4] drm/i915: Only disable execlist preemption for the duration of the request Chris Wilson
2017-01-21 9:28 ` [PATCH 2/4] drm/i915: Skip the execlists CSB scan and rewrite if the ring is untouched Chris Wilson
2017-01-23 11:33 ` Joonas Lahtinen
2017-01-23 11:47 ` Chris Wilson
2017-01-21 9:28 ` [PATCH 3/4] drm/i915: Dequeue execlists on a new request if any port is available Chris Wilson
2017-01-23 10:56 ` Tvrtko Ursulin
2017-01-23 11:21 ` Chris Wilson
2017-01-21 9:28 ` [PATCH 4/4] drm/i915: Emit dma-fence (and execlists submit) first from signaler Chris Wilson
2017-01-23 10:58 ` Tvrtko Ursulin
2017-01-23 11:37 ` Joonas Lahtinen
2017-01-21 10:54 ` ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Only disable execlist preemption for the duration of the request Patchwork
2017-01-21 11:02 ` Chris Wilson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox