[Intel-gfx] [PATCH 1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list

Joonas Lahtinen joonas.lahtinen at linux.intel.com
Mon Mar 5 14:25:07 UTC 2018

Quoting Chris Wilson (2018-02-22 11:25:44)
> The goal here is to try and reduce the latency of signaling additional
> requests following the wakeup from interrupt by reducing the list of
> to-be-signaled requests from an rbtree to a sorted linked list. The
> original choice of using an rbtree was to facilitate random insertions
> of request into the signaler while maintaining a sorted list. However,
> if we assume that most new requests are added when they are submitted,
> we see those new requests in execution order making a insertion sort
> fast, and the reduction in overhead of each signaler iteration
> significant.
> Since commit 56299fb7d904 ("drm/i915: Signal first fence from irq handler
> if complete"), we signal most fences directly from notify_ring() in the
> interrupt handler greatly reducing the amount of work that actually
> needs to be done by the signaler kthread. All the thread is then
> required to do is operate as the bottom-half, cleaning up after the
> interrupt handler and preparing the next waiter. This includes signaling
> all later completed fences in a saturated system, but on a mostly idle
> system we only have to rebuild the wait rbtree in time for the next
> interrupt. With this de-emphasis of the signaler's role, we want to
> rejig it's datastructures to reduce the amount of work we require to
> both setup the signal tree and maintain it on every interrupt.
> References: 56299fb7d904 ("drm/i915: Signal first fence from irq handler if complete")
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>

"rb_lock" is now bit of a misleading name. We could generally improve a
lot on documenting which locks apply to which data.

As a pre-existing condition it's bit surprising that intel_breadcrumbs_busy()
kicks the signaler as a side-effect.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>

Regards, Joonas

More information about the Intel-gfx mailing list