* [CI] drm/i915: Skip execlists_dequeue() early if the list is empty
@ 2017-03-17 11:27 Chris Wilson
2017-03-17 11:47 ` Michal Wajdeczko
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Chris Wilson @ 2017-03-17 11:27 UTC (permalink / raw)
To: intel-gfx
Do an early read of the execlists' queue before we take the spinlock and
start checking. This is safe as the first writer to the execlists queue
will cause the tasklet to be run again after a memory barrier.
v2: Keep guc in sync with execlists queue changes
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++++
drivers/gpu/drm/i915/intel_lrc.c | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index a3636b31ebc3..78718a8e70c3 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -577,6 +577,10 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
struct rb_node *rb;
bool submit = false;
+ /* After execlist_first is updated, a new tasklet must be scheduled */
+ if (!READ_ONCE(engine->execlist_first))
+ return false;
+
spin_lock_irqsave(&engine->timeline->lock, flags);
rb = engine->execlist_first;
while (rb) {
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index becde55b02a3..7adc588d1a19 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -403,6 +403,10 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
struct rb_node *rb;
bool submit = false;
+ /* After execlist_first is updated, a new tasklet must be scheduled */
+ if (!READ_ONCE(engine->execlist_first))
+ return;
+
last = port->request;
if (last)
/* WaIdleLiteRestore:bdw,skl
--
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] 4+ messages in thread
* Re: [CI] drm/i915: Skip execlists_dequeue() early if the list is empty
2017-03-17 11:27 [CI] drm/i915: Skip execlists_dequeue() early if the list is empty Chris Wilson
@ 2017-03-17 11:47 ` Michal Wajdeczko
2017-03-17 12:07 ` [PATCH v3] " Chris Wilson
2017-03-17 12:16 ` ✓ Fi.CI.BAT: success for drm/i915: Skip execlists_dequeue() early if the list is empty (rev2) Patchwork
2 siblings, 0 replies; 4+ messages in thread
From: Michal Wajdeczko @ 2017-03-17 11:47 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Fri, Mar 17, 2017 at 11:27:22AM +0000, Chris Wilson wrote:
> Do an early read of the execlists' queue before we take the spinlock and
> start checking. This is safe as the first writer to the execlists queue
> will cause the tasklet to be run again after a memory barrier.
>
> v2: Keep guc in sync with execlists queue changes
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
> drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++++
> drivers/gpu/drm/i915/intel_lrc.c | 4 ++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index a3636b31ebc3..78718a8e70c3 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -577,6 +577,10 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
> struct rb_node *rb;
> bool submit = false;
>
> + /* After execlist_first is updated, a new tasklet must be scheduled */
Hmm, I think this comment is litlle misleading when placed right to the "if" that do early return.
Either extend it with the info from the commit message, or move down.
-Michal
> + if (!READ_ONCE(engine->execlist_first))
> + return false;
> +
> spin_lock_irqsave(&engine->timeline->lock, flags);
> rb = engine->execlist_first;
> while (rb) {
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index becde55b02a3..7adc588d1a19 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -403,6 +403,10 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
> struct rb_node *rb;
> bool submit = false;
>
> + /* After execlist_first is updated, a new tasklet must be scheduled */
> + if (!READ_ONCE(engine->execlist_first))
> + return;
> +
> last = port->request;
> if (last)
> /* WaIdleLiteRestore:bdw,skl
> --
> 2.11.0
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v3] drm/i915: Skip execlists_dequeue() early if the list is empty
2017-03-17 11:27 [CI] drm/i915: Skip execlists_dequeue() early if the list is empty Chris Wilson
2017-03-17 11:47 ` Michal Wajdeczko
@ 2017-03-17 12:07 ` Chris Wilson
2017-03-17 12:16 ` ✓ Fi.CI.BAT: success for drm/i915: Skip execlists_dequeue() early if the list is empty (rev2) Patchwork
2 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2017-03-17 12:07 UTC (permalink / raw)
To: intel-gfx
Do an early read of the execlists' queue before we take the spinlock and
start checking. This is safe as the first writer to the execlists queue
will cause the tasklet to be run again after a memory barrier.
v2: Keep guc in sync with execlists queue changes
v3: Explain the mb between the tasklet running on one cpu and the
execlist_first update and schedule from a second cpu.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 12 ++++++++++++
drivers/gpu/drm/i915/intel_lrc.c | 12 ++++++++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index a3636b31ebc3..832ac9e45801 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -577,6 +577,18 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
struct rb_node *rb;
bool submit = false;
+ /* After execlist_first is updated, the tasklet will be rescheduled.
+ *
+ * If we are currently running (inside the tasklet) and a third
+ * party queues a request and so updates engine->execlist_first under
+ * the spinlock (which we have elided), it will atomically set the
+ * TASKLET_SCHED flag causing the us to be re-executed and pick up
+ * the change in state (the update to TASKLET_SCHED incurs a memory
+ * barrier making this cross-cpu checking safe).
+ */
+ if (!READ_ONCE(engine->execlist_first))
+ return false;
+
spin_lock_irqsave(&engine->timeline->lock, flags);
rb = engine->execlist_first;
while (rb) {
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index becde55b02a3..77168e673e0a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -403,6 +403,18 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
struct rb_node *rb;
bool submit = false;
+ /* After execlist_first is updated, the tasklet will be rescheduled.
+ *
+ * If we are currently running (inside the tasklet) and a third
+ * party queues a request and so updates engine->execlist_first under
+ * the spinlock (which we have elided), it will atomically set the
+ * TASKLET_SCHED flag causing the us to be re-executed and pick up
+ * the change in state (the update to TASKLET_SCHED incurs a memory
+ * barrier making this cross-cpu checking safe).
+ */
+ if (!READ_ONCE(engine->execlist_first))
+ return;
+
last = port->request;
if (last)
/* WaIdleLiteRestore:bdw,skl
--
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] 4+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Skip execlists_dequeue() early if the list is empty (rev2)
2017-03-17 11:27 [CI] drm/i915: Skip execlists_dequeue() early if the list is empty Chris Wilson
2017-03-17 11:47 ` Michal Wajdeczko
2017-03-17 12:07 ` [PATCH v3] " Chris Wilson
@ 2017-03-17 12:16 ` Patchwork
2 siblings, 0 replies; 4+ messages in thread
From: Patchwork @ 2017-03-17 12:16 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Skip execlists_dequeue() early if the list is empty (rev2)
URL : https://patchwork.freedesktop.org/series/21382/
State : success
== Summary ==
Series 21382v2 drm/i915: Skip execlists_dequeue() early if the list is empty
https://patchwork.freedesktop.org/api/1.0/series/21382/revisions/2/mbox/
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass -> FAIL (fi-snb-2600) fdo#100007
fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 456s
fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 572s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 540s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 537s
fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 506s
fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 506s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 439s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 433s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 435s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 516s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 486s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 487s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 481s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 599s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 478s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 526s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 554s
fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time: 410s
5fef332e008b4531d1d3dfd6ff16308a56fc26ce drm-tip: 2017y-03m-17d-11h-21m-37s UTC integration manifest
42708cc drm/i915: Skip execlists_dequeue() early if the list is empty
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4214/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-17 12:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-17 11:27 [CI] drm/i915: Skip execlists_dequeue() early if the list is empty Chris Wilson
2017-03-17 11:47 ` Michal Wajdeczko
2017-03-17 12:07 ` [PATCH v3] " Chris Wilson
2017-03-17 12:16 ` ✓ Fi.CI.BAT: success for drm/i915: Skip execlists_dequeue() early if the list is empty (rev2) Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.