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

[PATCH] 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.

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

With the plan to kick the tasklet before it is ready, eliminating a few
of the heavier checks seems sensible.
-Chris

---
 drivers/gpu/drm/i915/intel_lrc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 89f38e7def9f..17342bc939ad 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -402,6 +402,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 	struct rb_node *rb;
 	bool submit = false;
 
+	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: failure for drm/i915: Skip execlists_dequeue() early if the list is empty

[Patchwork]

== Series Details ==

Series: drm/i915: Skip execlists_dequeue() early if the list is empty
URL   : https://patchwork.freedesktop.org/series/21382/
State : failure

== Summary ==

Series 21382v1 drm/i915: Skip execlists_dequeue() early if the list is empty
https://patchwork.freedesktop.org/api/1.0/series/21382/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test gem_exec_reloc:
        Subgroup basic-gtt-noreloc:
                pass       -> INCOMPLETE (fi-byt-j1900)
Test kms_cursor_legacy:
        Subgroup basic-flip-before-cursor-atomic:
                fail       -> PASS       (fi-snb-2520m)

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: 458s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 580s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 528s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 557s
fi-byt-j1900     total:81   pass:71   dwarn:0   dfail:0   fail:0   skip:9   time: 0s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 501s
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: 442s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 440s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 513s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 495s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 483s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 484s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 601s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 482s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 518s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 551s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 418s

c1374a7a4a7fb0093d98ad6aa7071c8671fcdf08 drm-tip: 2017y-03m-16d-14h-28m-12s UTC integration manifest
8dd7763 drm/i915: Skip execlists_dequeue() early if the list is empty

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4202/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Michał Winiarski]

On Thu, Mar 16, 2017 at 02:53:05PM +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.
> 
> 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>
> ---
> 
> With the plan to kick the tasklet before it is ready, eliminating a few
> of the heavier checks seems sensible.
> -Chris

Now that we're also driving GuC submission in a similar way, this patch should
also include similar early read for guc dequeue.

With that:
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

-Michał

> 
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 89f38e7def9f..17342bc939ad 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -402,6 +402,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>  	struct rb_node *rb;
>  	bool submit = false;
>  
> +	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: [PATCH] drm/i915: Skip execlists_dequeue() early if the list is empty

[Tvrtko Ursulin]

On 16/03/2017 14:53, 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.

Which one is the memory barrier, tasklet_hi_schedule? Wouldn't we need 
an explicit one after setting engine->execlist_first in 
execlists_submit_request now?

Regards,

Tvrtko

> 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>
> ---
>
> With the plan to kick the tasklet before it is ready, eliminating a few
> of the heavier checks seems sensible.
> -Chris
>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 89f38e7def9f..17342bc939ad 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -402,6 +402,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>  	struct rb_node *rb;
>  	bool submit = false;
>
> +	if (!READ_ONCE(engine->execlist_first))
> +		return;
> +
>  	last = port->request;
>  	if (last)
>  		/* WaIdleLiteRestore:bdw,skl
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

On Fri, Mar 17, 2017 at 10:30:10AM +0000, Tvrtko Ursulin wrote:
> 
> On 16/03/2017 14:53, 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.
> 
> Which one is the memory barrier, tasklet_hi_schedule? Wouldn't we
> need an explicit one after setting engine->execlist_first in
> execlists_submit_request now?

Yes, the tasklet scheduling incurs a memory barrier, which we do after
setting execlist_first i.e. we know that after doing so there is always
at least one more invocation of the tasklet.
-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

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

[Tvrtko Ursulin]

On 17/03/2017 10:34, Chris Wilson wrote:
> On Fri, Mar 17, 2017 at 10:30:10AM +0000, Tvrtko Ursulin wrote:
>>
>> On 16/03/2017 14:53, 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.
>>
>> Which one is the memory barrier, tasklet_hi_schedule? Wouldn't we
>> need an explicit one after setting engine->execlist_first in
>> execlists_submit_request now?
>
> Yes, the tasklet scheduling incurs a memory barrier, which we do after
> setting execlist_first i.e. we know that after doing so there is always
> at least one more invocation of the tasklet.

Okay yes, test_and_set_bit in tasklet_schedule is a memory barrier.

See that Michal beat me to it, but anyway:

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