[CI] drm/i915: Skip execlists_dequeue() early if the list is empty

[CI] drm/i915: Skip execlists_dequeue() early if the list is empty

[Chris Wilson]

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

Re: [CI] drm/i915: Skip execlists_dequeue() early if the list is empty

[Michal Wajdeczko]

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

[PATCH v3] drm/i915: Skip execlists_dequeue() early if the list is empty

[Chris Wilson]

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

✓ Fi.CI.BAT: success for drm/i915: Skip execlists_dequeue() early if the list is empty (rev2)

[Patchwork]

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