* [PATCH 1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list
@ 2018-02-22 9:25 Chris Wilson
2018-02-22 9:25 ` [PATCH 2/2] drm/i915/breadcrumbs: Assert all missed breadcrumbs were signaled Chris Wilson
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Chris Wilson @ 2018-02-22 9:25 UTC (permalink / raw)
To: intel-gfx
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@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_request.h | 2 +-
drivers/gpu/drm/i915/intel_breadcrumbs.c | 261 +++++++++++++------------------
drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +-
3 files changed, 116 insertions(+), 151 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index 74311fc53e2f..7d6eb82eeb91 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -44,8 +44,8 @@ struct intel_wait {
};
struct intel_signal_node {
- struct rb_node node;
struct intel_wait wait;
+ struct list_head link;
};
struct i915_dependency {
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index a83690642aab..be50c2bebdf0 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -336,7 +336,8 @@ static inline void __intel_breadcrumbs_finish(struct intel_breadcrumbs *b,
lockdep_assert_held(&b->rb_lock);
GEM_BUG_ON(b->irq_wait == wait);
- /* This request is completed, so remove it from the tree, mark it as
+ /*
+ * This request is completed, so remove it from the tree, mark it as
* complete, and *then* wake up the associated task. N.B. when the
* task wakes up, it will find the empty rb_node, discern that it
* has already been removed from the tree and skip the serialisation
@@ -347,7 +348,8 @@ static inline void __intel_breadcrumbs_finish(struct intel_breadcrumbs *b,
rb_erase(&wait->node, &b->waiters);
RB_CLEAR_NODE(&wait->node);
- wake_up_process(wait->tsk); /* implicit smp_wmb() */
+ if (wait->tsk->state != TASK_RUNNING)
+ wake_up_process(wait->tsk); /* implicit smp_wmb() */
}
static inline void __intel_breadcrumbs_next(struct intel_engine_cs *engine,
@@ -588,23 +590,6 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine,
spin_unlock_irq(&b->rb_lock);
}
-static bool signal_complete(const struct i915_request *request)
-{
- if (!request)
- return false;
-
- /*
- * Carefully check if the request is complete, giving time for the
- * seqno to be visible or if the GPU hung.
- */
- return __i915_request_irq_complete(request);
-}
-
-static struct i915_request *to_signaler(struct rb_node *rb)
-{
- return rb_entry(rb, struct i915_request, signaling.node);
-}
-
static void signaler_set_rtpriority(void)
{
struct sched_param param = { .sched_priority = 1 };
@@ -612,78 +597,26 @@ static void signaler_set_rtpriority(void)
sched_setscheduler_nocheck(current, SCHED_FIFO, ¶m);
}
-static void __intel_engine_remove_signal(struct intel_engine_cs *engine,
- struct i915_request *request)
-{
- struct intel_breadcrumbs *b = &engine->breadcrumbs;
-
- lockdep_assert_held(&b->rb_lock);
-
- /*
- * 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);
-
- /*
- * 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
- * we just completed - so double check we are still
- * the oldest before picking the next one.
- */
- if (request->signaling.wait.seqno) {
- if (request == rcu_access_pointer(b->first_signal)) {
- struct rb_node *rb = rb_next(&request->signaling.node);
- rcu_assign_pointer(b->first_signal,
- rb ? to_signaler(rb) : NULL);
- }
-
- rb_erase(&request->signaling.node, &b->signals);
- request->signaling.wait.seqno = 0;
- }
-}
-
-static struct i915_request *
-get_first_signal_rcu(struct intel_breadcrumbs *b)
-{
- /*
- * See the big warnings for i915_gem_active_get_rcu() and similarly
- * for dma_fence_get_rcu_safe() that explain the intricacies involved
- * here with defeating CPU/compiler speculation and enforcing
- * the required memory barriers.
- */
- do {
- struct i915_request *request;
-
- request = rcu_dereference(b->first_signal);
- if (request)
- request = i915_request_get_rcu(request);
-
- barrier();
-
- if (!request || request == rcu_access_pointer(b->first_signal))
- return rcu_pointer_handoff(request);
-
- i915_request_put(request);
- } while (1);
-}
-
static int intel_breadcrumbs_signaler(void *arg)
{
struct intel_engine_cs *engine = arg;
struct intel_breadcrumbs *b = &engine->breadcrumbs;
- struct i915_request *request;
+ struct i915_request *rq, *n;
/* Install ourselves with high priority to reduce signalling latency */
signaler_set_rtpriority();
do {
bool do_schedule = true;
+ LIST_HEAD(list);
+ u32 seqno;
set_current_state(TASK_INTERRUPTIBLE);
+ if (list_empty(&b->signals))
+ goto sleep;
- /* We are either woken up by the interrupt bottom-half,
+ /*
+ * We are either woken up by the interrupt bottom-half,
* or by a client adding a new signaller. In both cases,
* the GPU seqno may have advanced beyond our oldest signal.
* If it has, propagate the signal, remove the waiter and
@@ -691,25 +624,45 @@ static int intel_breadcrumbs_signaler(void *arg)
* need to wait for a new interrupt from the GPU or for
* a new client.
*/
- rcu_read_lock();
- request = get_first_signal_rcu(b);
- rcu_read_unlock();
- if (signal_complete(request)) {
- if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
- &request->fence.flags)) {
- local_bh_disable();
- dma_fence_signal(&request->fence);
- GEM_BUG_ON(!i915_request_completed(request));
- local_bh_enable(); /* kick start the tasklets */
- }
+ seqno = intel_engine_get_seqno(engine);
+
+ spin_lock_irq(&b->rb_lock);
+ list_for_each_entry_safe(rq, n, &b->signals, signaling.link) {
+ u32 this = rq->signaling.wait.seqno;
+
+ GEM_BUG_ON(!rq->signaling.wait.seqno);
- if (READ_ONCE(request->signaling.wait.seqno)) {
- spin_lock_irq(&b->rb_lock);
- __intel_engine_remove_signal(engine, request);
- spin_unlock_irq(&b->rb_lock);
+ if (!i915_seqno_passed(seqno, this))
+ break;
+
+ if (likely(this == i915_request_global_seqno(rq))) {
+ __intel_engine_remove_wait(engine,
+ &rq->signaling.wait);
+
+ rq->signaling.wait.seqno = 0;
+ __list_del_entry(&rq->signaling.link);
+
+ if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
+ &rq->fence.flags)) {
+ list_add_tail(&rq->signaling.link,
+ &list);
+ i915_request_get(rq);
+ }
+ }
+ }
+ spin_unlock_irq(&b->rb_lock);
+
+ if (!list_empty(&list)) {
+ local_bh_disable();
+ list_for_each_entry_safe(rq, n, &list, signaling.link) {
+ dma_fence_signal(&rq->fence);
+ GEM_BUG_ON(!i915_request_completed(rq));
+ i915_request_put(rq);
}
+ local_bh_enable(); /* kick start the tasklets */
- /* If the engine is saturated we may be continually
+ /*
+ * If the engine is saturated we may be continually
* processing completed requests. This angers the
* NMI watchdog if we never let anything else
* have access to the CPU. Let's pretend to be nice
@@ -718,9 +671,19 @@ static int intel_breadcrumbs_signaler(void *arg)
*/
do_schedule = need_resched();
}
- i915_request_put(request);
if (unlikely(do_schedule)) {
+ /* Before we sleep, check for a missed seqno */
+ if (current->state & TASK_NORMAL &&
+ !list_empty(&b->signals) &&
+ engine->irq_seqno_barrier &&
+ test_and_clear_bit(ENGINE_IRQ_BREADCRUMB,
+ &engine->irq_posted)) {
+ engine->irq_seqno_barrier(engine);
+ intel_engine_wakeup(engine);
+ }
+
+sleep:
if (kthread_should_park())
kthread_parkme();
@@ -735,13 +698,40 @@ static int intel_breadcrumbs_signaler(void *arg)
return 0;
}
+static void insert_signal(struct intel_breadcrumbs *b,
+ struct i915_request *request,
+ const u32 seqno)
+{
+ struct i915_request *iter;
+
+ lockdep_assert_held(&b->rb_lock);
+
+ /*
+ * A reasonable assumption is that we are called to add signals
+ * in sequence, as the requests are submitted for execution and
+ * assigned a global_seqno. This will be the case for the majority
+ * of internally generated signals (inter-engine signaling).
+ *
+ * Out of order waiters triggering random signaling enabling will
+ * be more problematic, but hopefully rare enough and the list
+ * small enough that the O(N) insertion sort is not an issue.
+ */
+
+ list_for_each_entry_reverse(iter, &b->signals, signaling.link)
+ if (i915_seqno_passed(seqno, iter->signaling.wait.seqno))
+ break;
+
+ list_add(&request->signaling.link, &iter->signaling.link);
+}
+
void intel_engine_enable_signaling(struct i915_request *request, bool wakeup)
{
struct intel_engine_cs *engine = request->engine;
struct intel_breadcrumbs *b = &engine->breadcrumbs;
u32 seqno;
- /* Note that we may be called from an interrupt handler on another
+ /*
+ * Note that we may be called from an interrupt handler on another
* device (e.g. nouveau signaling a fence completion causing us
* to submit a request, and so enable signaling). As such,
* we need to make sure that all other users of b->rb_lock protect
@@ -753,17 +743,16 @@ void intel_engine_enable_signaling(struct i915_request *request, bool wakeup)
lockdep_assert_held(&request->lock);
seqno = i915_request_global_seqno(request);
- if (!seqno)
+ if (!seqno) /* will be enabled later upon execution */
return;
- spin_lock(&b->rb_lock);
-
GEM_BUG_ON(request->signaling.wait.seqno);
request->signaling.wait.tsk = b->signaler;
request->signaling.wait.request = request;
request->signaling.wait.seqno = seqno;
- /* First add ourselves into the list of waiters, but register our
+ /*
+ * Add ourselves into the list of waiters, but registering our
* bottom-half as the signaller thread. As per usual, only the oldest
* waiter (not just signaller) is tasked as the bottom-half waking
* up all completed waiters after the user interrupt.
@@ -771,39 +760,9 @@ void intel_engine_enable_signaling(struct i915_request *request, bool wakeup)
* If we are the oldest waiter, enable the irq (after which we
* must double check that the seqno did not complete).
*/
+ spin_lock(&b->rb_lock);
+ insert_signal(b, request, seqno);
wakeup &= __intel_engine_add_wait(engine, &request->signaling.wait);
-
- if (!__i915_request_completed(request, seqno)) {
- struct rb_node *parent, **p;
- bool first;
-
- /* Now insert ourselves into the retirement ordered list of
- * signals on this engine. We track the oldest seqno as that
- * will be the first signal to complete.
- */
- parent = NULL;
- first = true;
- p = &b->signals.rb_node;
- while (*p) {
- parent = *p;
- if (i915_seqno_passed(seqno,
- to_signaler(parent)->signaling.wait.seqno)) {
- p = &parent->rb_right;
- first = false;
- } else {
- p = &parent->rb_left;
- }
- }
- rb_link_node(&request->signaling.node, parent, p);
- rb_insert_color(&request->signaling.node, &b->signals);
- if (first)
- rcu_assign_pointer(b->first_signal, request);
- } else {
- __intel_engine_remove_wait(engine, &request->signaling.wait);
- request->signaling.wait.seqno = 0;
- wakeup = false;
- }
-
spin_unlock(&b->rb_lock);
if (wakeup)
@@ -812,17 +771,20 @@ void intel_engine_enable_signaling(struct i915_request *request, bool wakeup)
void intel_engine_cancel_signaling(struct i915_request *request)
{
+ struct intel_engine_cs *engine = request->engine;
+ struct intel_breadcrumbs *b = &engine->breadcrumbs;
+
GEM_BUG_ON(!irqs_disabled());
lockdep_assert_held(&request->lock);
- if (READ_ONCE(request->signaling.wait.seqno)) {
- struct intel_engine_cs *engine = request->engine;
- struct intel_breadcrumbs *b = &engine->breadcrumbs;
+ if (!READ_ONCE(request->signaling.wait.seqno))
+ return;
- spin_lock(&b->rb_lock);
- __intel_engine_remove_signal(engine, request);
- spin_unlock(&b->rb_lock);
- }
+ spin_lock(&b->rb_lock);
+ __intel_engine_remove_wait(engine, &request->signaling.wait);
+ if (fetch_and_zero(&request->signaling.wait.seqno))
+ __list_del_entry(&request->signaling.link);
+ spin_unlock(&b->rb_lock);
}
int intel_engine_init_breadcrumbs(struct intel_engine_cs *engine)
@@ -836,6 +798,8 @@ int intel_engine_init_breadcrumbs(struct intel_engine_cs *engine)
timer_setup(&b->fake_irq, intel_breadcrumbs_fake_irq, 0);
timer_setup(&b->hangcheck, intel_breadcrumbs_hangcheck, 0);
+ INIT_LIST_HEAD(&b->signals);
+
/* Spawn a thread to provide a common bottom-half for all signals.
* As this is an asynchronous interface we cannot steal the current
* task for handling the bottom-half to the user interrupt, therefore
@@ -895,8 +859,7 @@ void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine)
/* The engines should be idle and all requests accounted for! */
WARN_ON(READ_ONCE(b->irq_wait));
WARN_ON(!RB_EMPTY_ROOT(&b->waiters));
- WARN_ON(rcu_access_pointer(b->first_signal));
- WARN_ON(!RB_EMPTY_ROOT(&b->signals));
+ WARN_ON(!list_empty(&b->signals));
if (!IS_ERR_OR_NULL(b->signaler))
kthread_stop(b->signaler);
@@ -909,20 +872,22 @@ bool intel_breadcrumbs_busy(struct intel_engine_cs *engine)
struct intel_breadcrumbs *b = &engine->breadcrumbs;
bool busy = false;
- spin_lock_irq(&b->rb_lock);
-
if (b->irq_wait) {
- wake_up_process(b->irq_wait->tsk);
- busy = true;
+ spin_lock_irq(&b->irq_lock);
+
+ if (b->irq_wait) {
+ wake_up_process(b->irq_wait->tsk);
+ busy = true;
+ }
+
+ spin_unlock_irq(&b->irq_lock);
}
- if (rcu_access_pointer(b->first_signal)) {
+ if (!busy && !list_empty(&b->signals)) {
wake_up_process(b->signaler);
busy = true;
}
- spin_unlock_irq(&b->rb_lock);
-
return busy;
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index a9b83bf7e837..f179c135178b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -339,9 +339,9 @@ struct intel_engine_cs {
spinlock_t rb_lock; /* protects the rb and wraps irq_lock */
struct rb_root waiters; /* sorted by retirement, priority */
- struct rb_root signals; /* sorted by retirement */
+ struct list_head signals; /* sorted by retirement */
struct task_struct *signaler; /* used for fence signalling */
- struct i915_request __rcu *first_signal;
+
struct timer_list fake_irq; /* used after a missed interrupt */
struct timer_list hangcheck; /* detect missed interrupts */
--
2.16.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] drm/i915/breadcrumbs: Assert all missed breadcrumbs were signaled
2018-02-22 9:25 [PATCH 1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list Chris Wilson
@ 2018-02-22 9:25 ` Chris Wilson
2018-03-05 14:26 ` Joonas Lahtinen
2018-02-22 10:29 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list Patchwork
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2018-02-22 9:25 UTC (permalink / raw)
To: intel-gfx
When parking the engines and their breadcrumbs, if we have waiters left
then they missed their wakeup. Verify that each waiter's seqno did
complete.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_breadcrumbs.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index be50c2bebdf0..15a7ad998fe8 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -243,6 +243,8 @@ void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine)
spin_unlock(&b->irq_lock);
rbtree_postorder_for_each_entry_safe(wait, n, &b->waiters, node) {
+ GEM_BUG_ON(!i915_seqno_passed(intel_engine_get_seqno(engine),
+ wait->seqno));
RB_CLEAR_NODE(&wait->node);
wake_up_process(wait->tsk);
}
--
2.16.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list
2018-02-22 9:25 [PATCH 1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list Chris Wilson
2018-02-22 9:25 ` [PATCH 2/2] drm/i915/breadcrumbs: Assert all missed breadcrumbs were signaled Chris Wilson
@ 2018-02-22 10:29 ` Patchwork
2018-02-22 13:51 ` ✓ Fi.CI.IGT: " Patchwork
2018-03-05 14:25 ` [PATCH 1/2] " Joonas Lahtinen
3 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-02-22 10:29 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list
URL : https://patchwork.freedesktop.org/series/38755/
State : success
== Summary ==
Series 38755v1 series starting with [1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list
https://patchwork.freedesktop.org/api/1.0/series/38755/revisions/1/mbox/
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:419s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:427s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:372s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:481s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:284s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:483s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:466s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:454s
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:566s
fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:414s
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:286s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:506s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:387s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:414s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:443s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:410s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:452s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:488s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:447s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:491s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:583s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:428s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:499s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:518s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:489s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:480s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:411s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:427s
fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:521s
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:389s
Blacklisted hosts:
fi-cfl-8700k total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:393s
fi-bxt-dsi failed to collect. IGT log at Patchwork_8122/fi-bxt-dsi/run0.log
56c6f2eb05bf491778aee6f4d5851212e0ae9f2d drm-tip: 2018y-02m-22d-09h-36m-00s UTC integration manifest
c9e80255c278 drm/i915/breadcrumbs: Assert all missed breadcrumbs were signaled
82bf8c1b6e10 drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8122/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list
2018-02-22 9:25 [PATCH 1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list Chris Wilson
2018-02-22 9:25 ` [PATCH 2/2] drm/i915/breadcrumbs: Assert all missed breadcrumbs were signaled Chris Wilson
2018-02-22 10:29 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list Patchwork
@ 2018-02-22 13:51 ` Patchwork
2018-03-06 12:23 ` Chris Wilson
2018-03-05 14:25 ` [PATCH 1/2] " Joonas Lahtinen
3 siblings, 1 reply; 8+ messages in thread
From: Patchwork @ 2018-02-22 13:51 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list
URL : https://patchwork.freedesktop.org/series/38755/
State : success
== Summary ==
Test pm_rps:
Subgroup reset:
incomplete -> PASS (shard-apl) fdo#102250
Test kms_flip:
Subgroup 2x-plain-flip-fb-recreate:
fail -> PASS (shard-hsw) fdo#100368
Test kms_plane:
Subgroup plane-panning-bottom-right-suspend-pipe-c-planes:
pass -> INCOMPLETE (shard-hsw) fdo#103375
Test gem_eio:
Subgroup in-flight:
incomplete -> PASS (shard-apl) fdo#104945
Test kms_sysfs_edid_timing:
warn -> PASS (shard-apl) fdo#100047
Test perf:
Subgroup oa-exponents:
fail -> PASS (shard-apl) fdo#102254
Test kms_setmode:
Subgroup basic:
pass -> FAIL (shard-apl) fdo#99912
fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#104945 https://bugs.freedesktop.org/show_bug.cgi?id=104945
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
shard-apl total:3465 pass:1820 dwarn:1 dfail:0 fail:13 skip:1631 time:12439s
shard-hsw total:3455 pass:1760 dwarn:1 dfail:0 fail:3 skip:1689 time:11305s
shard-snb total:3465 pass:1355 dwarn:1 dfail:0 fail:3 skip:2106 time:6584s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8122/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list
2018-02-22 9:25 [PATCH 1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list Chris Wilson
` (2 preceding siblings ...)
2018-02-22 13:51 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-03-05 14:25 ` Joonas Lahtinen
2018-03-06 12:12 ` Chris Wilson
3 siblings, 1 reply; 8+ messages in thread
From: Joonas Lahtinen @ 2018-03-05 14:25 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
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@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@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@linux.intel.com>
Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/i915/breadcrumbs: Assert all missed breadcrumbs were signaled
2018-02-22 9:25 ` [PATCH 2/2] drm/i915/breadcrumbs: Assert all missed breadcrumbs were signaled Chris Wilson
@ 2018-03-05 14:26 ` Joonas Lahtinen
0 siblings, 0 replies; 8+ messages in thread
From: Joonas Lahtinen @ 2018-03-05 14:26 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Quoting Chris Wilson (2018-02-22 11:25:45)
> When parking the engines and their breadcrumbs, if we have waiters left
> then they missed their wakeup. Verify that each waiter's seqno did
> complete.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list
2018-03-05 14:25 ` [PATCH 1/2] " Joonas Lahtinen
@ 2018-03-06 12:12 ` Chris Wilson
0 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2018-03-06 12:12 UTC (permalink / raw)
To: Joonas Lahtinen, intel-gfx
Quoting Joonas Lahtinen (2018-03-05 14:25:07)
> As a pre-existing condition it's bit surprising that intel_breadcrumbs_busy()
> kicks the signaler as a side-effect.
Since we explicitly cancel the signaling on retire now (to prevent the
accumulation of references), we can actually say that we don't need to
check the signaler for busy any more. We still need to spin over kicking
the waiters though... Hmm, though we can just walk the tree and mark
them as complete now.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list
2018-02-22 13:51 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-03-06 12:23 ` Chris Wilson
0 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2018-03-06 12:23 UTC (permalink / raw)
To: Patchwork; +Cc: intel-gfx
Quoting Patchwork (2018-02-22 13:51:19)
> == Series Details ==
>
> Series: series starting with [1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list
> URL : https://patchwork.freedesktop.org/series/38755/
> State : success
>
> == Summary ==
>
> Test pm_rps:
> Subgroup reset:
> incomplete -> PASS (shard-apl) fdo#102250
> Test kms_flip:
> Subgroup 2x-plain-flip-fb-recreate:
> fail -> PASS (shard-hsw) fdo#100368
> Test kms_plane:
> Subgroup plane-panning-bottom-right-suspend-pipe-c-planes:
> pass -> INCOMPLETE (shard-hsw) fdo#103375
> Test gem_eio:
> Subgroup in-flight:
> incomplete -> PASS (shard-apl) fdo#104945
> Test kms_sysfs_edid_timing:
> warn -> PASS (shard-apl) fdo#100047
> Test perf:
> Subgroup oa-exponents:
> fail -> PASS (shard-apl) fdo#102254
> Test kms_setmode:
> Subgroup basic:
> pass -> FAIL (shard-apl) fdo#99912
And applied, thank you for the reviews.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-03-06 12:23 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-22 9:25 [PATCH 1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list Chris Wilson
2018-02-22 9:25 ` [PATCH 2/2] drm/i915/breadcrumbs: Assert all missed breadcrumbs were signaled Chris Wilson
2018-03-05 14:26 ` Joonas Lahtinen
2018-02-22 10:29 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/breadcrumbs: Reduce signaler rbtree to a sorted list Patchwork
2018-02-22 13:51 ` ✓ Fi.CI.IGT: " Patchwork
2018-03-06 12:23 ` Chris Wilson
2018-03-05 14:25 ` [PATCH 1/2] " Joonas Lahtinen
2018-03-06 12:12 ` Chris Wilson
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.