* [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe @ 2016-10-27 16:10 Chris Wilson 2016-10-27 16:48 ` ✓ Fi.CI.BAT: success for " Patchwork ` (5 more replies) 0 siblings, 6 replies; 18+ messages in thread From: Chris Wilson @ 2016-10-27 16:10 UTC (permalink / raw) To: intel-gfx The breadcrumbs are about to be used from within IRQ context sections, therefore we need to employ the irqsafe spinlock variants. (This is split out of the defer global seqno allocation patch due to realisation that we need a more complete conversion if we want to defer request submission even further.) Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 12 +++++------ drivers/gpu/drm/i915/intel_breadcrumbs.c | 34 +++++++++++++++++++------------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 925aaedd7fd9..8d3cb8f5ff41 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -683,14 +683,14 @@ static void i915_ring_seqno_info(struct seq_file *m, seq_printf(m, "Current sequence (%s): %x\n", engine->name, intel_engine_get_seqno(engine)); - spin_lock(&b->lock); + spin_lock_irq(&b->lock); for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) { struct intel_wait *w = container_of(rb, typeof(*w), node); seq_printf(m, "Waiting (%s): %s [%d] on %x\n", engine->name, w->tsk->comm, w->tsk->pid, w->seqno); } - spin_unlock(&b->lock); + spin_unlock_irq(&b->lock); } static int i915_gem_seqno_info(struct seq_file *m, void *data) @@ -1355,14 +1355,14 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused) yesno(intel_engine_has_waiter(engine)), yesno(test_bit(engine->id, &dev_priv->gpu_error.missed_irq_rings))); - spin_lock(&b->lock); + spin_lock_irq(&b->lock); for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) { struct intel_wait *w = container_of(rb, typeof(*w), node); seq_printf(m, "\t%s [%d] waiting for %x\n", w->tsk->comm, w->tsk->pid, w->seqno); } - spin_unlock(&b->lock); + spin_unlock_irq(&b->lock); seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n", (long long)engine->hangcheck.acthd, @@ -3265,14 +3265,14 @@ static int i915_engine_info(struct seq_file *m, void *unused) I915_READ(RING_PP_DIR_DCLV(engine))); } - spin_lock(&b->lock); + spin_lock_irq(&b->lock); for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) { struct intel_wait *w = container_of(rb, typeof(*w), node); seq_printf(m, "\t%s [%d] waiting for %x\n", w->tsk->comm, w->tsk->pid, w->seqno); } - spin_unlock(&b->lock); + spin_unlock_irq(&b->lock); seq_puts(m, "\n"); } diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index 0d5def0d2dfe..00b25a7cef95 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -83,16 +83,16 @@ static void irq_enable(struct intel_engine_cs *engine) */ engine->breadcrumbs.irq_posted = true; - spin_lock_irq(&engine->i915->irq_lock); + spin_lock(&engine->i915->irq_lock); engine->irq_enable(engine); - spin_unlock_irq(&engine->i915->irq_lock); + spin_unlock(&engine->i915->irq_lock); } static void irq_disable(struct intel_engine_cs *engine) { - spin_lock_irq(&engine->i915->irq_lock); + spin_lock(&engine->i915->irq_lock); engine->irq_disable(engine); - spin_unlock_irq(&engine->i915->irq_lock); + spin_unlock(&engine->i915->irq_lock); engine->breadcrumbs.irq_posted = false; } @@ -291,11 +291,12 @@ bool intel_engine_add_wait(struct intel_engine_cs *engine, struct intel_wait *wait) { struct intel_breadcrumbs *b = &engine->breadcrumbs; + unsigned long flags; bool first; - spin_lock(&b->lock); + spin_lock_irqsave(&b->lock, flags); first = __intel_engine_add_wait(engine, wait); - spin_unlock(&b->lock); + spin_unlock_irqrestore(&b->lock, flags); return first; } @@ -318,6 +319,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine, struct intel_wait *wait) { struct intel_breadcrumbs *b = &engine->breadcrumbs; + unsigned long flags; /* Quick check to see if this waiter was already decoupled from * the tree by the bottom-half to avoid contention on the spinlock @@ -326,7 +328,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine, if (RB_EMPTY_NODE(&wait->node)) return; - spin_lock(&b->lock); + spin_lock_irqsave(&b->lock, flags); if (RB_EMPTY_NODE(&wait->node)) goto out_unlock; @@ -400,7 +402,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine, GEM_BUG_ON(rb_first(&b->waiters) != (b->first_wait ? &b->first_wait->node : NULL)); GEM_BUG_ON(!rcu_access_pointer(b->irq_seqno_bh) ^ RB_EMPTY_ROOT(&b->waiters)); - spin_unlock(&b->lock); + spin_unlock_irqrestore(&b->lock, flags); } static bool signal_complete(struct drm_i915_gem_request *request) @@ -457,6 +459,8 @@ static int intel_breadcrumbs_signaler(void *arg) */ request = READ_ONCE(b->first_signal); if (signal_complete(request)) { + unsigned long flags; + /* Wake up all other completed waiters and select the * next bottom-half for the next user interrupt. */ @@ -473,14 +477,14 @@ static int intel_breadcrumbs_signaler(void *arg) * we just completed - so double check we are still * the oldest before picking the next one. */ - spin_lock(&b->lock); + spin_lock_irqsave(&b->lock, flags); if (request == b->first_signal) { struct rb_node *rb = rb_next(&request->signaling.node); b->first_signal = rb ? to_signaler(rb) : NULL; } rb_erase(&request->signaling.node, &b->signals); - spin_unlock(&b->lock); + spin_unlock_irqrestore(&b->lock, flags); i915_gem_request_put(request); } else { @@ -500,6 +504,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request) struct intel_engine_cs *engine = request->engine; struct intel_breadcrumbs *b = &engine->breadcrumbs; struct rb_node *parent, **p; + unsigned long flags; bool first, wakeup; /* locked by dma_fence_enable_sw_signaling() */ @@ -511,7 +516,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request) request->signaling.wait.seqno = request->global_seqno; i915_gem_request_get(request); - spin_lock(&b->lock); + spin_lock_irqsave(&b->lock, flags); /* First add ourselves into the list of waiters, but register our * bottom-half as the signaller thread. As per usual, only the oldest @@ -545,7 +550,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request) if (first) smp_store_mb(b->first_signal, request); - spin_unlock(&b->lock); + spin_unlock_irqrestore(&b->lock, flags); if (wakeup) wake_up_process(b->signaler); @@ -592,9 +597,10 @@ static void cancel_fake_irq(struct intel_engine_cs *engine) void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) { struct intel_breadcrumbs *b = &engine->breadcrumbs; + unsigned long flags; cancel_fake_irq(engine); - spin_lock(&b->lock); + spin_lock_irqsave(&b->lock, flags); __intel_breadcrumbs_disable_irq(b); if (intel_engine_has_waiter(engine)) { @@ -607,7 +613,7 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) irq_disable(engine); } - spin_unlock(&b->lock); + spin_unlock_irqrestore(&b->lock, flags); } void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine) -- 2.10.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 18+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Convert breadcrumbs spinlock to be irqsafe 2016-10-27 16:10 [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe Chris Wilson @ 2016-10-27 16:48 ` Patchwork 2016-10-28 9:42 ` [PATCH] " Tvrtko Ursulin ` (4 subsequent siblings) 5 siblings, 0 replies; 18+ messages in thread From: Patchwork @ 2016-10-27 16:48 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx == Series Details == Series: drm/i915: Convert breadcrumbs spinlock to be irqsafe URL : https://patchwork.freedesktop.org/series/14488/ State : success == Summary == Series 14488v1 drm/i915: Convert breadcrumbs spinlock to be irqsafe https://patchwork.freedesktop.org/api/1.0/series/14488/revisions/1/mbox/ fi-bdw-5557u total:246 pass:231 dwarn:0 dfail:0 fail:0 skip:15 fi-bsw-n3050 total:246 pass:204 dwarn:0 dfail:0 fail:0 skip:42 fi-bxt-t5700 total:246 pass:216 dwarn:0 dfail:0 fail:0 skip:30 fi-byt-j1900 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-n2820 total:246 pass:211 dwarn:0 dfail:0 fail:0 skip:35 fi-hsw-4770 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22 fi-hsw-4770r total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23 fi-ilk-650 total:246 pass:185 dwarn:0 dfail:0 fail:0 skip:61 fi-ivb-3520m total:246 pass:220 dwarn:0 dfail:0 fail:0 skip:26 fi-ivb-3770 total:246 pass:220 dwarn:0 dfail:0 fail:0 skip:26 fi-kbl-7200u total:246 pass:222 dwarn:0 dfail:0 fail:0 skip:24 fi-skl-6260u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14 fi-skl-6700hq total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23 fi-skl-6700k total:246 pass:222 dwarn:1 dfail:0 fail:0 skip:23 fi-skl-6770hq total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14 fi-snb-2520m total:246 pass:209 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:246 pass:208 dwarn:0 dfail:0 fail:0 skip:38 0fb1abf5eac2230894a7352e36022066f88a9b19 drm-intel-nightly: 2016y-10m-27d-14h-09m-00s UTC integration manifest 725ccf3 drm/i915: Convert breadcrumbs spinlock to be irqsafe == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2842/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe 2016-10-27 16:10 [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe Chris Wilson 2016-10-27 16:48 ` ✓ Fi.CI.BAT: success for " Patchwork @ 2016-10-28 9:42 ` Tvrtko Ursulin 2016-10-28 10:10 ` Chris Wilson 2016-10-28 11:41 ` [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe Joonas Lahtinen ` (3 subsequent siblings) 5 siblings, 1 reply; 18+ messages in thread From: Tvrtko Ursulin @ 2016-10-28 9:42 UTC (permalink / raw) To: Chris Wilson, intel-gfx On 27/10/16 17:10, Chris Wilson wrote: > The breadcrumbs are about to be used from within IRQ context sections, > therefore we need to employ the irqsafe spinlock variants. > > (This is split out of the defer global seqno allocation patch due to > realisation that we need a more complete conversion if we want to defer > request submission even further.) > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com > --- > drivers/gpu/drm/i915/i915_debugfs.c | 12 +++++------ > drivers/gpu/drm/i915/intel_breadcrumbs.c | 34 +++++++++++++++++++------------- > 2 files changed, 26 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 925aaedd7fd9..8d3cb8f5ff41 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -683,14 +683,14 @@ static void i915_ring_seqno_info(struct seq_file *m, > seq_printf(m, "Current sequence (%s): %x\n", > engine->name, intel_engine_get_seqno(engine)); > > - spin_lock(&b->lock); > + spin_lock_irq(&b->lock); > for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) { > struct intel_wait *w = container_of(rb, typeof(*w), node); > > seq_printf(m, "Waiting (%s): %s [%d] on %x\n", > engine->name, w->tsk->comm, w->tsk->pid, w->seqno); > } > - spin_unlock(&b->lock); > + spin_unlock_irq(&b->lock); > } > > static int i915_gem_seqno_info(struct seq_file *m, void *data) > @@ -1355,14 +1355,14 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused) > yesno(intel_engine_has_waiter(engine)), > yesno(test_bit(engine->id, > &dev_priv->gpu_error.missed_irq_rings))); > - spin_lock(&b->lock); > + spin_lock_irq(&b->lock); > for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) { > struct intel_wait *w = container_of(rb, typeof(*w), node); > > seq_printf(m, "\t%s [%d] waiting for %x\n", > w->tsk->comm, w->tsk->pid, w->seqno); > } > - spin_unlock(&b->lock); > + spin_unlock_irq(&b->lock); > > seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n", > (long long)engine->hangcheck.acthd, > @@ -3265,14 +3265,14 @@ static int i915_engine_info(struct seq_file *m, void *unused) > I915_READ(RING_PP_DIR_DCLV(engine))); > } > > - spin_lock(&b->lock); > + spin_lock_irq(&b->lock); > for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) { > struct intel_wait *w = container_of(rb, typeof(*w), node); > > seq_printf(m, "\t%s [%d] waiting for %x\n", > w->tsk->comm, w->tsk->pid, w->seqno); > } > - spin_unlock(&b->lock); > + spin_unlock_irq(&b->lock); > > seq_puts(m, "\n"); > } > diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c > index 0d5def0d2dfe..00b25a7cef95 100644 > --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c > +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c > @@ -83,16 +83,16 @@ static void irq_enable(struct intel_engine_cs *engine) > */ > engine->breadcrumbs.irq_posted = true; > > - spin_lock_irq(&engine->i915->irq_lock); > + spin_lock(&engine->i915->irq_lock); > engine->irq_enable(engine); > - spin_unlock_irq(&engine->i915->irq_lock); > + spin_unlock(&engine->i915->irq_lock); > } > > static void irq_disable(struct intel_engine_cs *engine) > { > - spin_lock_irq(&engine->i915->irq_lock); > + spin_lock(&engine->i915->irq_lock); > engine->irq_disable(engine); > - spin_unlock_irq(&engine->i915->irq_lock); > + spin_unlock(&engine->i915->irq_lock); > > engine->breadcrumbs.irq_posted = false; > } > @@ -291,11 +291,12 @@ bool intel_engine_add_wait(struct intel_engine_cs *engine, > struct intel_wait *wait) > { > struct intel_breadcrumbs *b = &engine->breadcrumbs; > + unsigned long flags; > bool first; > > - spin_lock(&b->lock); > + spin_lock_irqsave(&b->lock, flags); It looks to be called from thread context only so _irq would do it. > first = __intel_engine_add_wait(engine, wait); > - spin_unlock(&b->lock); > + spin_unlock_irqrestore(&b->lock, flags); > > return first; > } > @@ -318,6 +319,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine, > struct intel_wait *wait) > { > struct intel_breadcrumbs *b = &engine->breadcrumbs; > + unsigned long flags; > > /* Quick check to see if this waiter was already decoupled from > * the tree by the bottom-half to avoid contention on the spinlock > @@ -326,7 +328,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine, > if (RB_EMPTY_NODE(&wait->node)) > return; > > - spin_lock(&b->lock); > + spin_lock_irqsave(&b->lock, flags); Same. > > if (RB_EMPTY_NODE(&wait->node)) > goto out_unlock; > @@ -400,7 +402,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine, > GEM_BUG_ON(rb_first(&b->waiters) != > (b->first_wait ? &b->first_wait->node : NULL)); > GEM_BUG_ON(!rcu_access_pointer(b->irq_seqno_bh) ^ RB_EMPTY_ROOT(&b->waiters)); > - spin_unlock(&b->lock); > + spin_unlock_irqrestore(&b->lock, flags); > } > > static bool signal_complete(struct drm_i915_gem_request *request) > @@ -457,6 +459,8 @@ static int intel_breadcrumbs_signaler(void *arg) > */ > request = READ_ONCE(b->first_signal); > if (signal_complete(request)) { > + unsigned long flags; > + > /* Wake up all other completed waiters and select the > * next bottom-half for the next user interrupt. > */ > @@ -473,14 +477,14 @@ static int intel_breadcrumbs_signaler(void *arg) > * we just completed - so double check we are still > * the oldest before picking the next one. > */ > - spin_lock(&b->lock); > + spin_lock_irqsave(&b->lock, flags); This is a thread so _irq. > if (request == b->first_signal) { > struct rb_node *rb = > rb_next(&request->signaling.node); > b->first_signal = rb ? to_signaler(rb) : NULL; > } > rb_erase(&request->signaling.node, &b->signals); > - spin_unlock(&b->lock); > + spin_unlock_irqrestore(&b->lock, flags); > > i915_gem_request_put(request); > } else { > @@ -500,6 +504,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request) > struct intel_engine_cs *engine = request->engine; > struct intel_breadcrumbs *b = &engine->breadcrumbs; > struct rb_node *parent, **p; > + unsigned long flags; > bool first, wakeup; > > /* locked by dma_fence_enable_sw_signaling() */ > @@ -511,7 +516,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request) > request->signaling.wait.seqno = request->global_seqno; > i915_gem_request_get(request); > > - spin_lock(&b->lock); > + spin_lock_irqsave(&b->lock, flags); > > /* First add ourselves into the list of waiters, but register our > * bottom-half as the signaller thread. As per usual, only the oldest > @@ -545,7 +550,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request) > if (first) > smp_store_mb(b->first_signal, request); > > - spin_unlock(&b->lock); > + spin_unlock_irqrestore(&b->lock, flags); > > if (wakeup) > wake_up_process(b->signaler); > @@ -592,9 +597,10 @@ static void cancel_fake_irq(struct intel_engine_cs *engine) > void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) > { > struct intel_breadcrumbs *b = &engine->breadcrumbs; > + unsigned long flags; > > cancel_fake_irq(engine); > - spin_lock(&b->lock); > + spin_lock_irqsave(&b->lock, flags); Called from engine->init only so also the same. > > __intel_breadcrumbs_disable_irq(b); > if (intel_engine_has_waiter(engine)) { > @@ -607,7 +613,7 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) > irq_disable(engine); > } > > - spin_unlock(&b->lock); > + spin_unlock_irqrestore(&b->lock, flags); > } > > void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine) > Assuming I got the above right and you agree to change it: 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe 2016-10-28 9:42 ` [PATCH] " Tvrtko Ursulin @ 2016-10-28 10:10 ` Chris Wilson 2016-10-28 10:27 ` Tvrtko Ursulin 0 siblings, 1 reply; 18+ messages in thread From: Chris Wilson @ 2016-10-28 10:10 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx On Fri, Oct 28, 2016 at 10:42:22AM +0100, Tvrtko Ursulin wrote: > > > On 27/10/16 17:10, Chris Wilson wrote: > >The breadcrumbs are about to be used from within IRQ context sections, > >therefore we need to employ the irqsafe spinlock variants. > > > >(This is split out of the defer global seqno allocation patch due to > >realisation that we need a more complete conversion if we want to defer > >request submission even further.) [snip] > Assuming I got the above right and you agree to change it: You made me go and reduce them to _bh as appropriate anyway!!! -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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe 2016-10-28 10:10 ` Chris Wilson @ 2016-10-28 10:27 ` Tvrtko Ursulin 2016-10-28 10:42 ` Chris Wilson 0 siblings, 1 reply; 18+ messages in thread From: Tvrtko Ursulin @ 2016-10-28 10:27 UTC (permalink / raw) To: Chris Wilson, intel-gfx On 28/10/16 11:10, Chris Wilson wrote: > On Fri, Oct 28, 2016 at 10:42:22AM +0100, Tvrtko Ursulin wrote: >> >> >> On 27/10/16 17:10, Chris Wilson wrote: >>> The breadcrumbs are about to be used from within IRQ context sections, >>> therefore we need to employ the irqsafe spinlock variants. >>> >>> (This is split out of the defer global seqno allocation patch due to >>> realisation that we need a more complete conversion if we want to defer >>> request submission even further.) > > [snip] > >> Assuming I got the above right and you agree to change it: > > You made me go and reduce them to _bh as appropriate anyway!!! Hm, but can't enable signalling be called with irqs disabled when fences are exported? Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe 2016-10-28 10:27 ` Tvrtko Ursulin @ 2016-10-28 10:42 ` Chris Wilson 2016-10-28 10:49 ` Tvrtko Ursulin 0 siblings, 1 reply; 18+ messages in thread From: Chris Wilson @ 2016-10-28 10:42 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx On Fri, Oct 28, 2016 at 11:27:43AM +0100, Tvrtko Ursulin wrote: > > > On 28/10/16 11:10, Chris Wilson wrote: > >On Fri, Oct 28, 2016 at 10:42:22AM +0100, Tvrtko Ursulin wrote: > >> > >> > >>On 27/10/16 17:10, Chris Wilson wrote: > >>>The breadcrumbs are about to be used from within IRQ context sections, > >>>therefore we need to employ the irqsafe spinlock variants. > >>> > >>>(This is split out of the defer global seqno allocation patch due to > >>>realisation that we need a more complete conversion if we want to defer > >>>request submission even further.) > > > >[snip] > > > >>Assuming I got the above right and you agree to change it: > > > >You made me go and reduce them to _bh as appropriate anyway!!! > > Hm, but can't enable signalling be called with irqs disabled when > fences are exported? Yes, but that supercedes the spin_lock_bh, so we can just call spin_lock() in enabling_signaling as we can assert that we will always be called with irqs disabled here (due to spin_lock_irqsafe(fence->lock) in the callpath). -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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe 2016-10-28 10:42 ` Chris Wilson @ 2016-10-28 10:49 ` Tvrtko Ursulin 2016-10-28 11:26 ` Chris Wilson ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Tvrtko Ursulin @ 2016-10-28 10:49 UTC (permalink / raw) To: Chris Wilson, intel-gfx On 28/10/16 11:42, Chris Wilson wrote: > On Fri, Oct 28, 2016 at 11:27:43AM +0100, Tvrtko Ursulin wrote: >> >> >> On 28/10/16 11:10, Chris Wilson wrote: >>> On Fri, Oct 28, 2016 at 10:42:22AM +0100, Tvrtko Ursulin wrote: >>>> >>>> >>>> On 27/10/16 17:10, Chris Wilson wrote: >>>>> The breadcrumbs are about to be used from within IRQ context sections, >>>>> therefore we need to employ the irqsafe spinlock variants. >>>>> >>>>> (This is split out of the defer global seqno allocation patch due to >>>>> realisation that we need a more complete conversion if we want to defer >>>>> request submission even further.) >>> >>> [snip] >>> >>>> Assuming I got the above right and you agree to change it: >>> >>> You made me go and reduce them to _bh as appropriate anyway!!! >> >> Hm, but can't enable signalling be called with irqs disabled when >> fences are exported? > > Yes, but that supercedes the spin_lock_bh, so we can just call > spin_lock() in enabling_signaling as we can assert that we will always > be called with irqs disabled here (due to spin_lock_irqsafe(fence->lock) > in the callpath). But as long as the b->lock is taken in the irqs disabled section somewhere, other callers like signaller thread, debugfs, etc, cannot just take it with _bh. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe 2016-10-28 10:49 ` Tvrtko Ursulin @ 2016-10-28 11:26 ` Chris Wilson 2016-10-28 11:30 ` Chris Wilson 2016-10-28 11:26 ` [PATCH v2] drm: Add timestamp of last modification to GETCONNECTOR ioctl Chris Wilson 2016-10-28 11:31 ` [PATCH v2] drm/i915: Convert breadcrumbs spinlock to be irqsafe and bhsafe Chris Wilson 2 siblings, 1 reply; 18+ messages in thread From: Chris Wilson @ 2016-10-28 11:26 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx On Fri, Oct 28, 2016 at 11:49:34AM +0100, Tvrtko Ursulin wrote: > > > On 28/10/16 11:42, Chris Wilson wrote: > >On Fri, Oct 28, 2016 at 11:27:43AM +0100, Tvrtko Ursulin wrote: > >> > >> > >>On 28/10/16 11:10, Chris Wilson wrote: > >>>On Fri, Oct 28, 2016 at 10:42:22AM +0100, Tvrtko Ursulin wrote: > >>>> > >>>> > >>>>On 27/10/16 17:10, Chris Wilson wrote: > >>>>>The breadcrumbs are about to be used from within IRQ context sections, > >>>>>therefore we need to employ the irqsafe spinlock variants. > >>>>> > >>>>>(This is split out of the defer global seqno allocation patch due to > >>>>>realisation that we need a more complete conversion if we want to defer > >>>>>request submission even further.) > >>> > >>>[snip] > >>> > >>>>Assuming I got the above right and you agree to change it: > >>> > >>>You made me go and reduce them to _bh as appropriate anyway!!! > >> > >>Hm, but can't enable signalling be called with irqs disabled when > >>fences are exported? > > > >Yes, but that supercedes the spin_lock_bh, so we can just call > >spin_lock() in enabling_signaling as we can assert that we will always > >be called with irqs disabled here (due to spin_lock_irqsafe(fence->lock) > >in the callpath). > > But as long as the b->lock is taken in the irqs disabled section > somewhere, other callers like signaller thread, debugfs, etc, cannot > just take it with _bh. Lockdep doesn't complain, if we take spin_lock(b->lock) under irq inside our tasklet (enable_signaling) so long as we use spin_lock_bh() elsewhere. -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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe 2016-10-28 11:26 ` Chris Wilson @ 2016-10-28 11:30 ` Chris Wilson 2016-10-28 11:40 ` Chris Wilson 2016-10-28 11:43 ` Tvrtko Ursulin 0 siblings, 2 replies; 18+ messages in thread From: Chris Wilson @ 2016-10-28 11:30 UTC (permalink / raw) To: Tvrtko Ursulin, intel-gfx On Fri, Oct 28, 2016 at 12:26:09PM +0100, Chris Wilson wrote: > On Fri, Oct 28, 2016 at 11:49:34AM +0100, Tvrtko Ursulin wrote: > > > > > > On 28/10/16 11:42, Chris Wilson wrote: > > >On Fri, Oct 28, 2016 at 11:27:43AM +0100, Tvrtko Ursulin wrote: > > >> > > >> > > >>On 28/10/16 11:10, Chris Wilson wrote: > > >>>On Fri, Oct 28, 2016 at 10:42:22AM +0100, Tvrtko Ursulin wrote: > > >>>> > > >>>> > > >>>>On 27/10/16 17:10, Chris Wilson wrote: > > >>>>>The breadcrumbs are about to be used from within IRQ context sections, > > >>>>>therefore we need to employ the irqsafe spinlock variants. > > >>>>> > > >>>>>(This is split out of the defer global seqno allocation patch due to > > >>>>>realisation that we need a more complete conversion if we want to defer > > >>>>>request submission even further.) > > >>> > > >>>[snip] > > >>> > > >>>>Assuming I got the above right and you agree to change it: > > >>> > > >>>You made me go and reduce them to _bh as appropriate anyway!!! > > >> > > >>Hm, but can't enable signalling be called with irqs disabled when > > >>fences are exported? > > > > > >Yes, but that supercedes the spin_lock_bh, so we can just call > > >spin_lock() in enabling_signaling as we can assert that we will always > > >be called with irqs disabled here (due to spin_lock_irqsafe(fence->lock) > > >in the callpath). > > > > But as long as the b->lock is taken in the irqs disabled section > > somewhere, other callers like signaller thread, debugfs, etc, cannot > > just take it with _bh. > > Lockdep doesn't complain, if we take spin_lock(b->lock) under irq inside our > tasklet (enable_signaling) so long as we use spin_lock_bh() elsewhere. The key part is that we never take the spin_lock in irq context, just I am introducing it into softirq context. Hence why, I think, lockdep is happy. -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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe 2016-10-28 11:30 ` Chris Wilson @ 2016-10-28 11:40 ` Chris Wilson 2016-10-28 11:43 ` Tvrtko Ursulin 1 sibling, 0 replies; 18+ messages in thread From: Chris Wilson @ 2016-10-28 11:40 UTC (permalink / raw) To: Tvrtko Ursulin, intel-gfx On Fri, Oct 28, 2016 at 12:30:55PM +0100, Chris Wilson wrote: > On Fri, Oct 28, 2016 at 12:26:09PM +0100, Chris Wilson wrote: > > On Fri, Oct 28, 2016 at 11:49:34AM +0100, Tvrtko Ursulin wrote: > > > > > > > > > On 28/10/16 11:42, Chris Wilson wrote: > > > >On Fri, Oct 28, 2016 at 11:27:43AM +0100, Tvrtko Ursulin wrote: > > > >> > > > >> > > > >>On 28/10/16 11:10, Chris Wilson wrote: > > > >>>On Fri, Oct 28, 2016 at 10:42:22AM +0100, Tvrtko Ursulin wrote: > > > >>>> > > > >>>> > > > >>>>On 27/10/16 17:10, Chris Wilson wrote: > > > >>>>>The breadcrumbs are about to be used from within IRQ context sections, > > > >>>>>therefore we need to employ the irqsafe spinlock variants. > > > >>>>> > > > >>>>>(This is split out of the defer global seqno allocation patch due to > > > >>>>>realisation that we need a more complete conversion if we want to defer > > > >>>>>request submission even further.) > > > >>> > > > >>>[snip] > > > >>> > > > >>>>Assuming I got the above right and you agree to change it: > > > >>> > > > >>>You made me go and reduce them to _bh as appropriate anyway!!! > > > >> > > > >>Hm, but can't enable signalling be called with irqs disabled when > > > >>fences are exported? > > > > > > > >Yes, but that supercedes the spin_lock_bh, so we can just call > > > >spin_lock() in enabling_signaling as we can assert that we will always > > > >be called with irqs disabled here (due to spin_lock_irqsafe(fence->lock) > > > >in the callpath). > > > > > > But as long as the b->lock is taken in the irqs disabled section > > > somewhere, other callers like signaller thread, debugfs, etc, cannot > > > just take it with _bh. > > > > Lockdep doesn't complain, if we take spin_lock(b->lock) under irq inside our > > tasklet (enable_signaling) so long as we use spin_lock_bh() elsewhere. > > The key part is that we never take the spin_lock in irq context, just I > am introducing it into softirq context. Hence why, I think, lockdep is > happy. However, this doesn't take into account that submit_notify() may be called from interrupt context from nouveau, and that may then use enable_signaling, and so we may take b->lock in irqcontext after all. Ok, this needs a fixes tag. -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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe 2016-10-28 11:30 ` Chris Wilson 2016-10-28 11:40 ` Chris Wilson @ 2016-10-28 11:43 ` Tvrtko Ursulin 1 sibling, 0 replies; 18+ messages in thread From: Tvrtko Ursulin @ 2016-10-28 11:43 UTC (permalink / raw) To: Chris Wilson, intel-gfx On 28/10/16 12:30, Chris Wilson wrote: > On Fri, Oct 28, 2016 at 12:26:09PM +0100, Chris Wilson wrote: >> On Fri, Oct 28, 2016 at 11:49:34AM +0100, Tvrtko Ursulin wrote: >>> >>> >>> On 28/10/16 11:42, Chris Wilson wrote: >>>> On Fri, Oct 28, 2016 at 11:27:43AM +0100, Tvrtko Ursulin wrote: >>>>> >>>>> >>>>> On 28/10/16 11:10, Chris Wilson wrote: >>>>>> On Fri, Oct 28, 2016 at 10:42:22AM +0100, Tvrtko Ursulin wrote: >>>>>>> >>>>>>> >>>>>>> On 27/10/16 17:10, Chris Wilson wrote: >>>>>>>> The breadcrumbs are about to be used from within IRQ context sections, >>>>>>>> therefore we need to employ the irqsafe spinlock variants. >>>>>>>> >>>>>>>> (This is split out of the defer global seqno allocation patch due to >>>>>>>> realisation that we need a more complete conversion if we want to defer >>>>>>>> request submission even further.) >>>>>> >>>>>> [snip] >>>>>> >>>>>>> Assuming I got the above right and you agree to change it: >>>>>> >>>>>> You made me go and reduce them to _bh as appropriate anyway!!! >>>>> >>>>> Hm, but can't enable signalling be called with irqs disabled when >>>>> fences are exported? >>>> >>>> Yes, but that supercedes the spin_lock_bh, so we can just call >>>> spin_lock() in enabling_signaling as we can assert that we will always >>>> be called with irqs disabled here (due to spin_lock_irqsafe(fence->lock) >>>> in the callpath). >>> >>> But as long as the b->lock is taken in the irqs disabled section >>> somewhere, other callers like signaller thread, debugfs, etc, cannot >>> just take it with _bh. >> >> Lockdep doesn't complain, if we take spin_lock(b->lock) under irq inside our >> tasklet (enable_signaling) so long as we use spin_lock_bh() elsewhere. > > The key part is that we never take the spin_lock in irq context, just I > am introducing it into softirq context. Hence why, I think, lockdep is > happy. Hm, but when we export it external fence user could use it from their irq context? We export to userspace, some other driver imports and enables signalling from irq? Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2] drm: Add timestamp of last modification to GETCONNECTOR ioctl 2016-10-28 10:49 ` Tvrtko Ursulin 2016-10-28 11:26 ` Chris Wilson @ 2016-10-28 11:26 ` Chris Wilson 2016-10-28 11:32 ` Chris Wilson 2016-10-28 11:31 ` [PATCH v2] drm/i915: Convert breadcrumbs spinlock to be irqsafe and bhsafe Chris Wilson 2 siblings, 1 reply; 18+ messages in thread From: Chris Wilson @ 2016-10-28 11:26 UTC (permalink / raw) To: intel-gfx After hotplug notification, userspace has to reprobe all connectors to detect any changes. This can be expensive (some outputs may require time consuming load detection, some outputs may simply be slow to respond) and not all outputs need to be checked everytime. The kernel is usually in a very good position to know which outputs need to be reprobed (since it has just responded to the hardware notification) and can convey this information to userspace by tracking the timestamp of the last connector change onto the GETCONNECTOR query. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/drm_connector.c | 4 +++- drivers/gpu/drm/drm_crtc.c | 4 +++- drivers/gpu/drm/drm_probe_helper.c | 6 +++++- drivers/gpu/drm/gma500/mdfld_dsi_dpi.c | 1 + drivers/gpu/drm/gma500/mdfld_dsi_output.c | 1 + drivers/gpu/drm/i915/intel_dp.c | 4 ++++ drivers/gpu/drm/i915/intel_dp_mst.c | 2 ++ drivers/gpu/drm/i915/intel_hotplug.c | 5 ++++- drivers/gpu/drm/i915/intel_lvds.c | 1 + drivers/gpu/drm/nouveau/nouveau_connector.c | 1 + drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 1 + drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 1 + drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 1 + include/drm/drm_connector.h | 8 ++++++++ include/uapi/drm/drm.h | 2 +- include/uapi/drm/drm_mode.h | 26 ++++++++++++++++++++++++++ 16 files changed, 63 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 2db7fb510b6c..cd3d85e94b91 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -227,6 +227,7 @@ int drm_connector_init(struct drm_device *dev, INIT_LIST_HEAD(&connector->modes); connector->edid_blob_ptr = NULL; connector->status = connector_status_unknown; + drm_connector_update_timestamp(connector); drm_connector_get_cmdline_mode(connector); @@ -1010,7 +1011,7 @@ static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode, int drm_mode_getconnector(struct drm_device *dev, void *data, struct drm_file *file_priv) { - struct drm_mode_get_connector *out_resp = data; + struct drm_mode_get_connector_v2 *out_resp = data; struct drm_connector *connector; struct drm_encoder *encoder; struct drm_display_mode *mode; @@ -1058,6 +1059,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, out_resp->mm_height = connector->display_info.height_mm; out_resp->subpixel = connector->display_info.subpixel_order; out_resp->connection = connector->status; + out_resp->timestamp = ktime_to_ns(connector->timestamp); drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); encoder = drm_connector_get_encoder(connector); diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index cfe75773444b..a4fde4d58211 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -948,9 +948,11 @@ void drm_mode_config_reset(struct drm_device *dev) encoder->funcs->reset(encoder); mutex_lock(&dev->mode_config.mutex); - drm_for_each_connector(connector, dev) + drm_for_each_connector(connector, dev) { + drm_connector_update_timestamp(connector); if (connector->funcs->reset) connector->funcs->reset(connector); + } mutex_unlock(&dev->mode_config.mutex); } EXPORT_SYMBOL(drm_mode_config_reset); diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 8790ee35a7cd..4b6882edc3fb 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -249,6 +249,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, * check here, and if anything changed start the hotplug code. */ if (old_status != connector->status) { + drm_connector_update_timestamp(connector); DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n", connector->base.id, connector->name, @@ -438,6 +439,7 @@ static void output_poll_execute(struct work_struct *work) connector->name, old, new); + drm_connector_update_timestamp(connector); changed = true; } } @@ -573,8 +575,10 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) connector->name, drm_get_connector_status_name(old_status), drm_get_connector_status_name(connector->status)); - if (old_status != connector->status) + if (old_status != connector->status) { + drm_connector_update_timestamp(connector); changed = true; + } } mutex_unlock(&dev->mode_config.mutex); diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c b/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c index a05c020602bd..8980d7a08c5d 100644 --- a/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c +++ b/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c @@ -971,6 +971,7 @@ struct mdfld_dsi_encoder *mdfld_dsi_dpi_init(struct drm_device *dev, DRM_INFO("pipe %d power mode 0x%x\n", pipe, data); dsi_connector->status = connector_status_connected; } + drm_connector_update_timestamp(connector); } dpi_output = kzalloc(sizeof(struct mdfld_dsi_dpi_output), GFP_KERNEL); diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_output.c b/drivers/gpu/drm/gma500/mdfld_dsi_output.c index acb3848ef1c9..f288d9ac9b12 100644 --- a/drivers/gpu/drm/gma500/mdfld_dsi_output.c +++ b/drivers/gpu/drm/gma500/mdfld_dsi_output.c @@ -238,6 +238,7 @@ mdfld_dsi_connector_detect(struct drm_connector *connector, bool force) = mdfld_dsi_connector(connector); dsi_connector->status = connector_status_connected; + drm_connector_update_timestamp(connector); return dsi_connector->status; } diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5bbd1a553e56..d3f7955fdb46 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3966,6 +3966,10 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp) DRM_DEBUG_KMS("failed to get ESI - device may have failed\n"); intel_dp->is_mst = false; drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst); + + if (intel_dp->attached_connector) + drm_connector_update_timestamp(&intel_dp->attached_connector->base); + /* send a hotplug event */ drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev, &intel_dp->attached_connector->base); diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 2bd48a987934..d66e576eff83 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -493,6 +493,8 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr) struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct drm_device *dev = intel_dig_port->base.base.dev; + if (intel_dp->attached_connector) + drm_connector_update_timestamp(&intel_dp->attached_connector->base); drm_kms_helper_hotplug_event(dev, &intel_dp->attached_connector->base); } diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c index da0649aff734..f3c2747be183 100644 --- a/drivers/gpu/drm/i915/intel_hotplug.c +++ b/drivers/gpu/drm/i915/intel_hotplug.c @@ -238,6 +238,7 @@ static bool intel_hpd_irq_event(struct drm_device *dev, if (old_status == connector->status) return false; + drm_connector_update_timestamp(connector); DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n", connector->base.id, connector->name, @@ -333,8 +334,10 @@ static void i915_hotplug_work_func(struct work_struct *work) connector->name, intel_encoder->hpd_pin); if (intel_encoder->hot_plug) intel_encoder->hot_plug(intel_encoder); - if (intel_hpd_irq_event(dev, connector)) + if (intel_hpd_irq_event(dev, connector)) { + drm_connector_update_timestamp(connector); changed = true; + } } } mutex_unlock(&mode_config->mutex); diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 199b90c7907a..27f7044300ff 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -545,6 +545,7 @@ static int intel_lid_notify(struct notifier_block *nb, unsigned long val, * the LID nofication event. */ connector->status = connector->funcs->detect(connector, false); + drm_connector_update_timestamp(connector); /* Don't force modeset on machines where it causes a GPU lockup */ if (dmi_check_system(intel_no_modeset_on_lid)) diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c index c1084088f9e4..47ee48489cf3 100644 --- a/drivers/gpu/drm/nouveau/nouveau_connector.c +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c @@ -455,6 +455,7 @@ nouveau_connector_force(struct drm_connector *connector) NV_ERROR(drm, "can't find encoder to force %s on!\n", connector->name); connector->status = connector_status_disconnected; + drm_connector_update_timestamp(connector); return; } diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c index 61a3cce08dfe..4500723cc856 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c @@ -363,6 +363,7 @@ static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit) drm_connector_init(dev, connector, &vmw_legacy_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL); connector->status = vmw_du_connector_detect(connector, true); + drm_connector_update_timestamp(connector); drm_encoder_init(dev, encoder, &vmw_legacy_encoder_funcs, DRM_MODE_ENCODER_VIRTUAL, NULL); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c index 573c7407c32c..acd79e0edf5a 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c @@ -524,6 +524,7 @@ static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit) drm_connector_init(dev, connector, &vmw_sou_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL); connector->status = vmw_du_connector_detect(connector, true); + drm_connector_update_timestamp(connector); drm_encoder_init(dev, encoder, &vmw_screen_object_encoder_funcs, DRM_MODE_ENCODER_VIRTUAL, NULL); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c index 01c4d574fa78..3c2b71e605d6 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c @@ -1117,6 +1117,7 @@ static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit) drm_connector_init(dev, connector, &vmw_stdu_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL); connector->status = vmw_du_connector_detect(connector, false); + drm_connector_update_timestamp(connector); drm_encoder_init(dev, encoder, &vmw_stdu_encoder_funcs, DRM_MODE_ENCODER_VIRTUAL, NULL); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index ac9d7d8e0e43..698d86a6af83 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -25,6 +25,7 @@ #include <linux/list.h> #include <linux/ctype.h> +#include <linux/ktime.h> #include <drm/drm_mode_object.h> #include <uapi/drm/drm_mode.h> @@ -576,6 +577,7 @@ struct drm_connector { struct list_head modes; /* list of modes on this connector */ enum drm_connector_status status; + ktime_t timestamp; /* these are modes added by probing with DDC or the BIOS */ struct list_head probed_modes; @@ -761,6 +763,12 @@ int drm_mode_connector_set_tile_property(struct drm_connector *connector); int drm_mode_connector_update_edid_property(struct drm_connector *connector, const struct edid *edid); +static inline void +drm_connector_update_timestamp(struct drm_connector *connector) +{ + connector->timestamp = ktime_get(); +} + /** * drm_for_each_connector - iterate over all connectors * @connector: the loop cursor diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index b2c52843bc70..5c5941aedd8c 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -787,7 +787,7 @@ extern "C" { #define DRM_IOCTL_MODE_GETGAMMA DRM_IOWR(0xA4, struct drm_mode_crtc_lut) #define DRM_IOCTL_MODE_SETGAMMA DRM_IOWR(0xA5, struct drm_mode_crtc_lut) #define DRM_IOCTL_MODE_GETENCODER DRM_IOWR(0xA6, struct drm_mode_get_encoder) -#define DRM_IOCTL_MODE_GETCONNECTOR DRM_IOWR(0xA7, struct drm_mode_get_connector) +#define DRM_IOCTL_MODE_GETCONNECTOR DRM_IOWR(0xA7, struct drm_mode_get_connector_v2) #define DRM_IOCTL_MODE_ATTACHMODE DRM_IOWR(0xA8, struct drm_mode_mode_cmd) /* deprecated (never worked) */ #define DRM_IOCTL_MODE_DETACHMODE DRM_IOWR(0xA9, struct drm_mode_mode_cmd) /* deprecated (never worked) */ diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 084b50a02dc5..d3ecb546918c 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -286,6 +286,32 @@ struct drm_mode_get_connector { __u32 pad; }; +struct drm_mode_get_connector_v2 { + + __u64 encoders_ptr; + __u64 modes_ptr; + __u64 props_ptr; + __u64 prop_values_ptr; + + __u32 count_modes; + __u32 count_props; + __u32 count_encoders; + + __u32 encoder_id; /**< Current Encoder */ + __u32 connector_id; /**< Id */ + __u32 connector_type; + __u32 connector_type_id; + + __u32 connection; + __u32 mm_width; /**< width in millimeters */ + __u32 mm_height; /**< height in millimeters */ + __u32 subpixel; + + __u32 pad; /* align to u64 */ + + __u64 timestamp; +}; + #define DRM_MODE_PROP_PENDING (1<<0) #define DRM_MODE_PROP_RANGE (1<<1) #define DRM_MODE_PROP_IMMUTABLE (1<<2) -- 2.10.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm: Add timestamp of last modification to GETCONNECTOR ioctl 2016-10-28 11:26 ` [PATCH v2] drm: Add timestamp of last modification to GETCONNECTOR ioctl Chris Wilson @ 2016-10-28 11:32 ` Chris Wilson 0 siblings, 0 replies; 18+ messages in thread From: Chris Wilson @ 2016-10-28 11:32 UTC (permalink / raw) To: intel-gfx On Fri, Oct 28, 2016 at 12:26:24PM +0100, Chris Wilson wrote: > After hotplug notification, userspace has to reprobe all connectors to > detect any changes. This can be expensive (some outputs may require time > consuming load detection, some outputs may simply be slow to respond) > and not all outputs need to be checked everytime. The kernel is usually > in a very good position to know which outputs need to be reprobed (since > it has just responded to the hardware notification) and can convey this > information to userspace by tracking the timestamp of the last connector > change onto the GETCONNECTOR query. Interesting, but not the right patch! :) -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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2] drm/i915: Convert breadcrumbs spinlock to be irqsafe and bhsafe 2016-10-28 10:49 ` Tvrtko Ursulin 2016-10-28 11:26 ` Chris Wilson 2016-10-28 11:26 ` [PATCH v2] drm: Add timestamp of last modification to GETCONNECTOR ioctl Chris Wilson @ 2016-10-28 11:31 ` Chris Wilson 2 siblings, 0 replies; 18+ messages in thread From: Chris Wilson @ 2016-10-28 11:31 UTC (permalink / raw) To: intel-gfx The breadcrumbs are about to be used from within IRQ context sections, and/or from bottom-half tasklets (i.e. intel_lrc_irq_handle), therefore we need to employ the irqsafe spinlock variants. [ 66.388639] ================================= [ 66.388650] [ INFO: inconsistent lock state ] [ 66.388663] 4.9.0-rc2+ #56 Not tainted [ 66.388672] --------------------------------- [ 66.388682] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage. [ 66.388695] swapper/1/0 [HC0[0]:SC1[1]:HE0:SE0] takes: [ 66.388706] (&(&b->lock)->rlock){+.?...} , at: [<ffffffff81401c88>] intel_engine_enable_signaling+0x78/0x150 [ 66.388761] {SOFTIRQ-ON-W} state was registered at: [ 66.388772] [ 66.388783] [<ffffffff810bd842>] __lock_acquire+0x682/0x1870 [ 66.388795] [ 66.388803] [<ffffffff810bedbc>] lock_acquire+0x6c/0xb0 [ 66.388814] [ 66.388824] [<ffffffff8161753a>] _raw_spin_lock+0x2a/0x40 [ 66.388835] [ 66.388845] [<ffffffff81401e41>] intel_engine_reset_breadcrumbs+0x21/0xb0 [ 66.388857] [ 66.388866] [<ffffffff81403ae7>] gen8_init_common_ring+0x67/0x100 [ 66.388878] [ 66.388887] [<ffffffff81403b92>] gen8_init_render_ring+0x12/0x60 [ 66.388903] [ 66.388912] [<ffffffff813f8707>] i915_gem_init_hw+0xf7/0x2a0 [ 66.388927] [ 66.388936] [<ffffffff813f899b>] i915_gem_init+0xbb/0xf0 [ 66.388950] [ 66.388959] [<ffffffff813b4980>] i915_driver_load+0x7e0/0x1330 [ 66.388978] [ 66.388988] [<ffffffff813c09d8>] i915_pci_probe+0x28/0x40 [ 66.389003] [ 66.389013] [<ffffffff812fa0db>] pci_device_probe+0x8b/0xf0 [ 66.389028] [ 66.389037] [<ffffffff8147737e>] driver_probe_device+0x21e/0x430 [ 66.389056] [ 66.389065] [<ffffffff8147766e>] __driver_attach+0xde/0xe0 [ 66.389080] [ 66.389090] [<ffffffff814751ad>] bus_for_each_dev+0x5d/0x90 [ 66.389105] [ 66.389113] [<ffffffff81477799>] driver_attach+0x19/0x20 [ 66.389134] [ 66.389144] [<ffffffff81475ced>] bus_add_driver+0x15d/0x260 [ 66.389159] [ 66.389168] [<ffffffff81477e3b>] driver_register+0x5b/0xd0 [ 66.389183] [ 66.389281] [<ffffffff812fa19b>] __pci_register_driver+0x5b/0x60 [ 66.389301] [ 66.389312] [<ffffffff81aed333>] i915_init+0x3e/0x45 [ 66.389326] [ 66.389336] [<ffffffff81ac2ffa>] do_one_initcall+0x8b/0x118 [ 66.389350] [ 66.389359] [<ffffffff81ac323a>] kernel_init_freeable+0x1b3/0x23b [ 66.389378] [ 66.389387] [<ffffffff8160fc39>] kernel_init+0x9/0x100 [ 66.389402] [ 66.389411] [<ffffffff816180e7>] ret_from_fork+0x27/0x40 [ 66.389426] irq event stamp: 315865 [ 66.389438] hardirqs last enabled at (315864): [<ffffffff816178f1>] _raw_spin_unlock_irqrestore+0x31/0x50 [ 66.389469] hardirqs last disabled at (315865): [<ffffffff816176b3>] _raw_spin_lock_irqsave+0x13/0x50 [ 66.389499] softirqs last enabled at (315818): [<ffffffff8107a04c>] _local_bh_enable+0x1c/0x50 [ 66.389530] softirqs last disabled at (315819): [<ffffffff8107a50e>] irq_exit+0xbe/0xd0 [ 66.389559] [ 66.389559] other info that might help us debug this: [ 66.389580] Possible unsafe locking scenario: [ 66.389580] [ 66.389598] CPU0 [ 66.389609] ---- [ 66.389620] lock(&(&b->lock)->rlock); [ 66.389650] <Interrupt> [ 66.389661] lock(&(&b->lock)->rlock); [ 66.389690] [ 66.389690] *** DEADLOCK *** [ 66.389690] [ 66.389715] 2 locks held by swapper/1/0: [ 66.389728] #0: (&(&tl->lock)->rlock){..-...}, at: [<ffffffff81403e01>] intel_lrc_irq_handler+0x201/0x3c0 [ 66.389785] #1: (&(&req->lock)->rlock/1){..-...}, at: [<ffffffff813fc0af>] __i915_gem_request_submit+0x8f/0x170 [ 66.389854] [ 66.389854] stack backtrace: [ 66.389959] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.9.0-rc2+ #56 [ 66.389976] Hardware name: / , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015 [ 66.389999] ffff88027fd03c58 ffffffff812beae5 ffff88027696e680 ffffffff822afe20 [ 66.390036] ffff88027fd03ca8 ffffffff810bb420 0000000000000001 0000000000000000 [ 66.390070] 0000000000000000 0000000000000006 0000000000000004 ffff88027696ee10 [ 66.390104] Call Trace: [ 66.390117] <IRQ> [ 66.390128] [<ffffffff812beae5>] dump_stack+0x68/0x93 [ 66.390147] [<ffffffff810bb420>] print_usage_bug+0x1d0/0x1e0 [ 66.390164] [<ffffffff810bb8a0>] mark_lock+0x470/0x4f0 [ 66.390181] [<ffffffff810ba9d0>] ? print_shortest_lock_dependencies+0x1b0/0x1b0 [ 66.390203] [<ffffffff810bd75d>] __lock_acquire+0x59d/0x1870 [ 66.390221] [<ffffffff810bedbc>] lock_acquire+0x6c/0xb0 [ 66.390237] [<ffffffff810bedbc>] ? lock_acquire+0x6c/0xb0 [ 66.390255] [<ffffffff81401c88>] ? intel_engine_enable_signaling+0x78/0x150 [ 66.390273] [<ffffffff8161753a>] _raw_spin_lock+0x2a/0x40 [ 66.390291] [<ffffffff81401c88>] ? intel_engine_enable_signaling+0x78/0x150 [ 66.390309] [<ffffffff81401c88>] intel_engine_enable_signaling+0x78/0x150 [ 66.390327] [<ffffffff813fc170>] __i915_gem_request_submit+0x150/0x170 [ 66.390345] [<ffffffff81403e8b>] intel_lrc_irq_handler+0x28b/0x3c0 [ 66.390363] [<ffffffff81079d97>] tasklet_action+0x57/0xc0 [ 66.390380] [<ffffffff8107a249>] __do_softirq+0x119/0x240 [ 66.390396] [<ffffffff8107a50e>] irq_exit+0xbe/0xd0 [ 66.390414] [<ffffffff8101afd5>] do_IRQ+0x65/0x110 [ 66.390431] [<ffffffff81618806>] common_interrupt+0x86/0x86 [ 66.390446] <EOI> [ 66.390457] [<ffffffff814ec6d1>] ? cpuidle_enter_state+0x151/0x200 [ 66.390480] [<ffffffff814ec7a2>] cpuidle_enter+0x12/0x20 [ 66.390498] [<ffffffff810b639e>] call_cpuidle+0x1e/0x40 [ 66.390516] [<ffffffff810b65ae>] cpu_startup_entry+0x10e/0x1f0 [ 66.390534] [<ffffffff81036133>] start_secondary+0x103/0x130 (This is split out of the defer global seqno allocation patch due to realisation that we need a more complete conversion if we want to defer request submission even further.) v2: lockdep was warning about mixed SOFTIRQ contexts not HARDIRQ contexts so we only need to use spin_lock_bh and not disable interrupts. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_debugfs.c | 12 ++++++------ drivers/gpu/drm/i915/intel_breadcrumbs.c | 30 +++++++++++++++++------------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 925aaedd7fd9..448d47c3d3c5 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -683,14 +683,14 @@ static void i915_ring_seqno_info(struct seq_file *m, seq_printf(m, "Current sequence (%s): %x\n", engine->name, intel_engine_get_seqno(engine)); - spin_lock(&b->lock); + spin_lock_bh(&b->lock); for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) { struct intel_wait *w = container_of(rb, typeof(*w), node); seq_printf(m, "Waiting (%s): %s [%d] on %x\n", engine->name, w->tsk->comm, w->tsk->pid, w->seqno); } - spin_unlock(&b->lock); + spin_unlock_bh(&b->lock); } static int i915_gem_seqno_info(struct seq_file *m, void *data) @@ -1355,14 +1355,14 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused) yesno(intel_engine_has_waiter(engine)), yesno(test_bit(engine->id, &dev_priv->gpu_error.missed_irq_rings))); - spin_lock(&b->lock); + spin_lock_bh(&b->lock); for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) { struct intel_wait *w = container_of(rb, typeof(*w), node); seq_printf(m, "\t%s [%d] waiting for %x\n", w->tsk->comm, w->tsk->pid, w->seqno); } - spin_unlock(&b->lock); + spin_unlock_bh(&b->lock); seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n", (long long)engine->hangcheck.acthd, @@ -3265,14 +3265,14 @@ static int i915_engine_info(struct seq_file *m, void *unused) I915_READ(RING_PP_DIR_DCLV(engine))); } - spin_lock(&b->lock); + spin_lock_bh(&b->lock); for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) { struct intel_wait *w = container_of(rb, typeof(*w), node); seq_printf(m, "\t%s [%d] waiting for %x\n", w->tsk->comm, w->tsk->pid, w->seqno); } - spin_unlock(&b->lock); + spin_unlock_bh(&b->lock); seq_puts(m, "\n"); } diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index 0d5def0d2dfe..89da68087840 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -77,22 +77,26 @@ static void intel_breadcrumbs_fake_irq(unsigned long data) static void irq_enable(struct intel_engine_cs *engine) { + unsigned long flags; + /* Enabling the IRQ may miss the generation of the interrupt, but * we still need to force the barrier before reading the seqno, * just in case. */ engine->breadcrumbs.irq_posted = true; - spin_lock_irq(&engine->i915->irq_lock); + spin_lock_irqsave(&engine->i915->irq_lock, flags); engine->irq_enable(engine); - spin_unlock_irq(&engine->i915->irq_lock); + spin_unlock_irqrestore(&engine->i915->irq_lock, flags); } static void irq_disable(struct intel_engine_cs *engine) { - spin_lock_irq(&engine->i915->irq_lock); + unsigned long flags; + + spin_lock_irqsave(&engine->i915->irq_lock, flags); engine->irq_disable(engine); - spin_unlock_irq(&engine->i915->irq_lock); + spin_unlock_irqrestore(&engine->i915->irq_lock, flags); engine->breadcrumbs.irq_posted = false; } @@ -293,9 +297,9 @@ bool intel_engine_add_wait(struct intel_engine_cs *engine, struct intel_breadcrumbs *b = &engine->breadcrumbs; bool first; - spin_lock(&b->lock); + spin_lock_bh(&b->lock); first = __intel_engine_add_wait(engine, wait); - spin_unlock(&b->lock); + spin_unlock_bh(&b->lock); return first; } @@ -326,7 +330,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine, if (RB_EMPTY_NODE(&wait->node)) return; - spin_lock(&b->lock); + spin_lock_bh(&b->lock); if (RB_EMPTY_NODE(&wait->node)) goto out_unlock; @@ -400,7 +404,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine, GEM_BUG_ON(rb_first(&b->waiters) != (b->first_wait ? &b->first_wait->node : NULL)); GEM_BUG_ON(!rcu_access_pointer(b->irq_seqno_bh) ^ RB_EMPTY_ROOT(&b->waiters)); - spin_unlock(&b->lock); + spin_unlock_bh(&b->lock); } static bool signal_complete(struct drm_i915_gem_request *request) @@ -473,14 +477,14 @@ static int intel_breadcrumbs_signaler(void *arg) * we just completed - so double check we are still * the oldest before picking the next one. */ - spin_lock(&b->lock); + spin_lock_bh(&b->lock); if (request == b->first_signal) { struct rb_node *rb = rb_next(&request->signaling.node); b->first_signal = rb ? to_signaler(rb) : NULL; } rb_erase(&request->signaling.node, &b->signals); - spin_unlock(&b->lock); + spin_unlock_bh(&b->lock); i915_gem_request_put(request); } else { @@ -502,7 +506,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request) struct rb_node *parent, **p; bool first, wakeup; - /* locked by dma_fence_enable_sw_signaling() */ + /* locked by dma_fence_enable_sw_signaling() (irqsafe fence->lock) */ assert_spin_locked(&request->lock); if (!request->global_seqno) return; @@ -594,7 +598,7 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) struct intel_breadcrumbs *b = &engine->breadcrumbs; cancel_fake_irq(engine); - spin_lock(&b->lock); + spin_lock_bh(&b->lock); __intel_breadcrumbs_disable_irq(b); if (intel_engine_has_waiter(engine)) { @@ -607,7 +611,7 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) irq_disable(engine); } - spin_unlock(&b->lock); + spin_unlock_bh(&b->lock); } void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine) -- 2.10.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe 2016-10-27 16:10 [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe Chris Wilson 2016-10-27 16:48 ` ✓ Fi.CI.BAT: success for " Patchwork 2016-10-28 9:42 ` [PATCH] " Tvrtko Ursulin @ 2016-10-28 11:41 ` Joonas Lahtinen 2016-10-28 12:13 ` [PATCH v3] " Chris Wilson ` (2 subsequent siblings) 5 siblings, 0 replies; 18+ messages in thread From: Joonas Lahtinen @ 2016-10-28 11:41 UTC (permalink / raw) To: Chris Wilson, intel-gfx On to, 2016-10-27 at 17:10 +0100, Chris Wilson wrote: > @@ -473,14 +477,14 @@ static int intel_breadcrumbs_signaler(void *arg) > * we just completed - so double check we are still > * the oldest before picking the next one. > */ > - spin_lock(&b->lock); > + spin_lock_irqsave(&b->lock, flags); With the convention discussed in IRC, this should be spin_lock_irq(). With that changed; Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3] drm/i915: Convert breadcrumbs spinlock to be irqsafe 2016-10-27 16:10 [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe Chris Wilson ` (2 preceding siblings ...) 2016-10-28 11:41 ` [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe Joonas Lahtinen @ 2016-10-28 12:13 ` Chris Wilson 2016-10-28 12:16 ` ✗ Fi.CI.BAT: failure for drm/i915: Convert breadcrumbs spinlock to be irqsafe (rev3) Patchwork 2016-10-28 13:15 ` ✗ Fi.CI.BAT: failure for drm/i915: Convert breadcrumbs spinlock to be irqsafe (rev4) Patchwork 5 siblings, 0 replies; 18+ messages in thread From: Chris Wilson @ 2016-10-28 12:13 UTC (permalink / raw) To: intel-gfx The breadcrumbs are about to be used from within IRQ context sections (e.g. nouveau signals a fence from an interrupt handler causing us to submit a new request) and/or from bottom-half tasklets (i.e. intel_lrc_irq_handler), therefore we need to employ the irqsafe spinlock variants. For example, deferring the request submission to the intel_lrc_irq_handler generates this trace: [ 66.388639] ================================= [ 66.388650] [ INFO: inconsistent lock state ] [ 66.388663] 4.9.0-rc2+ #56 Not tainted [ 66.388672] --------------------------------- [ 66.388682] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage. [ 66.388695] swapper/1/0 [HC0[0]:SC1[1]:HE0:SE0] takes: [ 66.388706] (&(&b->lock)->rlock){+.?...} , at: [<ffffffff81401c88>] intel_engine_enable_signaling+0x78/0x150 [ 66.388761] {SOFTIRQ-ON-W} state was registered at: [ 66.388772] [ 66.388783] [<ffffffff810bd842>] __lock_acquire+0x682/0x1870 [ 66.388795] [ 66.388803] [<ffffffff810bedbc>] lock_acquire+0x6c/0xb0 [ 66.388814] [ 66.388824] [<ffffffff8161753a>] _raw_spin_lock+0x2a/0x40 [ 66.388835] [ 66.388845] [<ffffffff81401e41>] intel_engine_reset_breadcrumbs+0x21/0xb0 [ 66.388857] [ 66.388866] [<ffffffff81403ae7>] gen8_init_common_ring+0x67/0x100 [ 66.388878] [ 66.388887] [<ffffffff81403b92>] gen8_init_render_ring+0x12/0x60 [ 66.388903] [ 66.388912] [<ffffffff813f8707>] i915_gem_init_hw+0xf7/0x2a0 [ 66.388927] [ 66.388936] [<ffffffff813f899b>] i915_gem_init+0xbb/0xf0 [ 66.388950] [ 66.388959] [<ffffffff813b4980>] i915_driver_load+0x7e0/0x1330 [ 66.388978] [ 66.388988] [<ffffffff813c09d8>] i915_pci_probe+0x28/0x40 [ 66.389003] [ 66.389013] [<ffffffff812fa0db>] pci_device_probe+0x8b/0xf0 [ 66.389028] [ 66.389037] [<ffffffff8147737e>] driver_probe_device+0x21e/0x430 [ 66.389056] [ 66.389065] [<ffffffff8147766e>] __driver_attach+0xde/0xe0 [ 66.389080] [ 66.389090] [<ffffffff814751ad>] bus_for_each_dev+0x5d/0x90 [ 66.389105] [ 66.389113] [<ffffffff81477799>] driver_attach+0x19/0x20 [ 66.389134] [ 66.389144] [<ffffffff81475ced>] bus_add_driver+0x15d/0x260 [ 66.389159] [ 66.389168] [<ffffffff81477e3b>] driver_register+0x5b/0xd0 [ 66.389183] [ 66.389281] [<ffffffff812fa19b>] __pci_register_driver+0x5b/0x60 [ 66.389301] [ 66.389312] [<ffffffff81aed333>] i915_init+0x3e/0x45 [ 66.389326] [ 66.389336] [<ffffffff81ac2ffa>] do_one_initcall+0x8b/0x118 [ 66.389350] [ 66.389359] [<ffffffff81ac323a>] kernel_init_freeable+0x1b3/0x23b [ 66.389378] [ 66.389387] [<ffffffff8160fc39>] kernel_init+0x9/0x100 [ 66.389402] [ 66.389411] [<ffffffff816180e7>] ret_from_fork+0x27/0x40 [ 66.389426] irq event stamp: 315865 [ 66.389438] hardirqs last enabled at (315864): [<ffffffff816178f1>] _raw_spin_unlock_irqrestore+0x31/0x50 [ 66.389469] hardirqs last disabled at (315865): [<ffffffff816176b3>] _raw_spin_lock_irqsave+0x13/0x50 [ 66.389499] softirqs last enabled at (315818): [<ffffffff8107a04c>] _local_bh_enable+0x1c/0x50 [ 66.389530] softirqs last disabled at (315819): [<ffffffff8107a50e>] irq_exit+0xbe/0xd0 [ 66.389559] [ 66.389559] other info that might help us debug this: [ 66.389580] Possible unsafe locking scenario: [ 66.389580] [ 66.389598] CPU0 [ 66.389609] ---- [ 66.389620] lock(&(&b->lock)->rlock); [ 66.389650] <Interrupt> [ 66.389661] lock(&(&b->lock)->rlock); [ 66.389690] [ 66.389690] *** DEADLOCK *** [ 66.389690] [ 66.389715] 2 locks held by swapper/1/0: [ 66.389728] #0: (&(&tl->lock)->rlock){..-...}, at: [<ffffffff81403e01>] intel_lrc_irq_handler+0x201/0x3c0 [ 66.389785] #1: (&(&req->lock)->rlock/1){..-...}, at: [<ffffffff813fc0af>] __i915_gem_request_submit+0x8f/0x170 [ 66.389854] [ 66.389854] stack backtrace: [ 66.389959] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.9.0-rc2+ #56 [ 66.389976] Hardware name: / , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015 [ 66.389999] ffff88027fd03c58 ffffffff812beae5 ffff88027696e680 ffffffff822afe20 [ 66.390036] ffff88027fd03ca8 ffffffff810bb420 0000000000000001 0000000000000000 [ 66.390070] 0000000000000000 0000000000000006 0000000000000004 ffff88027696ee10 [ 66.390104] Call Trace: [ 66.390117] <IRQ> [ 66.390128] [<ffffffff812beae5>] dump_stack+0x68/0x93 [ 66.390147] [<ffffffff810bb420>] print_usage_bug+0x1d0/0x1e0 [ 66.390164] [<ffffffff810bb8a0>] mark_lock+0x470/0x4f0 [ 66.390181] [<ffffffff810ba9d0>] ? print_shortest_lock_dependencies+0x1b0/0x1b0 [ 66.390203] [<ffffffff810bd75d>] __lock_acquire+0x59d/0x1870 [ 66.390221] [<ffffffff810bedbc>] lock_acquire+0x6c/0xb0 [ 66.390237] [<ffffffff810bedbc>] ? lock_acquire+0x6c/0xb0 [ 66.390255] [<ffffffff81401c88>] ? intel_engine_enable_signaling+0x78/0x150 [ 66.390273] [<ffffffff8161753a>] _raw_spin_lock+0x2a/0x40 [ 66.390291] [<ffffffff81401c88>] ? intel_engine_enable_signaling+0x78/0x150 [ 66.390309] [<ffffffff81401c88>] intel_engine_enable_signaling+0x78/0x150 [ 66.390327] [<ffffffff813fc170>] __i915_gem_request_submit+0x150/0x170 [ 66.390345] [<ffffffff81403e8b>] intel_lrc_irq_handler+0x28b/0x3c0 [ 66.390363] [<ffffffff81079d97>] tasklet_action+0x57/0xc0 [ 66.390380] [<ffffffff8107a249>] __do_softirq+0x119/0x240 [ 66.390396] [<ffffffff8107a50e>] irq_exit+0xbe/0xd0 [ 66.390414] [<ffffffff8101afd5>] do_IRQ+0x65/0x110 [ 66.390431] [<ffffffff81618806>] common_interrupt+0x86/0x86 [ 66.390446] <EOI> [ 66.390457] [<ffffffff814ec6d1>] ? cpuidle_enter_state+0x151/0x200 [ 66.390480] [<ffffffff814ec7a2>] cpuidle_enter+0x12/0x20 [ 66.390498] [<ffffffff810b639e>] call_cpuidle+0x1e/0x40 [ 66.390516] [<ffffffff810b65ae>] cpu_startup_entry+0x10e/0x1f0 [ 66.390534] [<ffffffff81036133>] start_secondary+0x103/0x130 (This is split out of the defer global seqno allocation patch due to realisation that we need a more complete conversion if we want to defer request submission even further.) v2: lockdep was warning about mixed SOFTIRQ contexts not HARDIRQ contexts so we only need to use spin_lock_bh and not disable interrupts. v3: We need full irq protection as we may be called from a third party interrupt handler (via fences). Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- This should be v1 with the tweaks both of you commented upon. Please double check you're happy. --- drivers/gpu/drm/i915/i915_debugfs.c | 12 +++++------ drivers/gpu/drm/i915/i915_gpu_error.c | 8 ++++---- drivers/gpu/drm/i915/intel_breadcrumbs.c | 35 ++++++++++++++++++++------------ drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- 4 files changed, 33 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index a60803c4f11a..a2fa445fe409 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -683,14 +683,14 @@ static void i915_ring_seqno_info(struct seq_file *m, seq_printf(m, "Current sequence (%s): %x\n", engine->name, intel_engine_get_seqno(engine)); - spin_lock(&b->lock); + spin_lock_irq(&b->lock); for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) { struct intel_wait *w = container_of(rb, typeof(*w), node); seq_printf(m, "Waiting (%s): %s [%d] on %x\n", engine->name, w->tsk->comm, w->tsk->pid, w->seqno); } - spin_unlock(&b->lock); + spin_unlock_irq(&b->lock); } static int i915_gem_seqno_info(struct seq_file *m, void *data) @@ -1355,14 +1355,14 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused) yesno(intel_engine_has_waiter(engine)), yesno(test_bit(engine->id, &dev_priv->gpu_error.missed_irq_rings))); - spin_lock(&b->lock); + spin_lock_irq(&b->lock); for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) { struct intel_wait *w = container_of(rb, typeof(*w), node); seq_printf(m, "\t%s [%d] waiting for %x\n", w->tsk->comm, w->tsk->pid, w->seqno); } - spin_unlock(&b->lock); + spin_unlock_irq(&b->lock); seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n", (long long)engine->hangcheck.acthd, @@ -3265,14 +3265,14 @@ static int i915_engine_info(struct seq_file *m, void *unused) I915_READ(RING_PP_DIR_DCLV(engine))); } - spin_lock(&b->lock); + spin_lock_irq(&b->lock); for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) { struct intel_wait *w = container_of(rb, typeof(*w), node); seq_printf(m, "\t%s [%d] waiting for %x\n", w->tsk->comm, w->tsk->pid, w->seqno); } - spin_unlock(&b->lock); + spin_unlock_irq(&b->lock); seq_puts(m, "\n"); } diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index ef3698120d92..7ba40487e345 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1043,7 +1043,7 @@ static void error_record_engine_waiters(struct intel_engine_cs *engine, if (RB_EMPTY_ROOT(&b->waiters)) return; - if (!spin_trylock(&b->lock)) { + if (!spin_trylock_irq(&b->lock)) { ee->waiters = ERR_PTR(-EDEADLK); return; } @@ -1051,7 +1051,7 @@ static void error_record_engine_waiters(struct intel_engine_cs *engine, count = 0; for (rb = rb_first(&b->waiters); rb != NULL; rb = rb_next(rb)) count++; - spin_unlock(&b->lock); + spin_unlock_irq(&b->lock); waiter = NULL; if (count) @@ -1061,7 +1061,7 @@ static void error_record_engine_waiters(struct intel_engine_cs *engine, if (!waiter) return; - if (!spin_trylock(&b->lock)) { + if (!spin_trylock_irq(&b->lock)) { kfree(waiter); ee->waiters = ERR_PTR(-EDEADLK); return; @@ -1079,7 +1079,7 @@ static void error_record_engine_waiters(struct intel_engine_cs *engine, if (++ee->num_waiters == count) break; } - spin_unlock(&b->lock); + spin_unlock_irq(&b->lock); } static void error_record_engine_registers(struct drm_i915_error_state *error, diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index 0d5def0d2dfe..c410d3d6465f 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -83,16 +83,18 @@ static void irq_enable(struct intel_engine_cs *engine) */ engine->breadcrumbs.irq_posted = true; - spin_lock_irq(&engine->i915->irq_lock); + /* Caller disables interrupts */ + spin_lock(&engine->i915->irq_lock); engine->irq_enable(engine); - spin_unlock_irq(&engine->i915->irq_lock); + spin_unlock(&engine->i915->irq_lock); } static void irq_disable(struct intel_engine_cs *engine) { - spin_lock_irq(&engine->i915->irq_lock); + /* Caller disables interrupts */ + spin_lock(&engine->i915->irq_lock); engine->irq_disable(engine); - spin_unlock_irq(&engine->i915->irq_lock); + spin_unlock(&engine->i915->irq_lock); engine->breadcrumbs.irq_posted = false; } @@ -293,9 +295,9 @@ bool intel_engine_add_wait(struct intel_engine_cs *engine, struct intel_breadcrumbs *b = &engine->breadcrumbs; bool first; - spin_lock(&b->lock); + spin_lock_irq(&b->lock); first = __intel_engine_add_wait(engine, wait); - spin_unlock(&b->lock); + spin_unlock_irq(&b->lock); return first; } @@ -326,7 +328,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine, if (RB_EMPTY_NODE(&wait->node)) return; - spin_lock(&b->lock); + spin_lock_irq(&b->lock); if (RB_EMPTY_NODE(&wait->node)) goto out_unlock; @@ -400,7 +402,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine, GEM_BUG_ON(rb_first(&b->waiters) != (b->first_wait ? &b->first_wait->node : NULL)); GEM_BUG_ON(!rcu_access_pointer(b->irq_seqno_bh) ^ RB_EMPTY_ROOT(&b->waiters)); - spin_unlock(&b->lock); + spin_unlock_irq(&b->lock); } static bool signal_complete(struct drm_i915_gem_request *request) @@ -473,14 +475,14 @@ static int intel_breadcrumbs_signaler(void *arg) * we just completed - so double check we are still * the oldest before picking the next one. */ - spin_lock(&b->lock); + spin_lock_irq(&b->lock); if (request == b->first_signal) { struct rb_node *rb = rb_next(&request->signaling.node); b->first_signal = rb ? to_signaler(rb) : NULL; } rb_erase(&request->signaling.node, &b->signals); - spin_unlock(&b->lock); + spin_unlock_irq(&b->lock); i915_gem_request_put(request); } else { @@ -502,7 +504,14 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request) struct rb_node *parent, **p; bool first, wakeup; - /* locked by dma_fence_enable_sw_signaling() */ + /* 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->lock protect + * against interrupts, i.e. use spin_lock_irqsave. + */ + + /* locked by dma_fence_enable_sw_signaling() (irqsafe fence->lock) */ assert_spin_locked(&request->lock); if (!request->global_seqno) return; @@ -594,7 +603,7 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) struct intel_breadcrumbs *b = &engine->breadcrumbs; cancel_fake_irq(engine); - spin_lock(&b->lock); + spin_lock_irq(&b->lock); __intel_breadcrumbs_disable_irq(b); if (intel_engine_has_waiter(engine)) { @@ -607,7 +616,7 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) irq_disable(engine); } - spin_unlock(&b->lock); + spin_unlock_irq(&b->lock); } void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 891629caab6c..d16c74ae8f54 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -207,7 +207,7 @@ struct intel_engine_cs { struct task_struct __rcu *irq_seqno_bh; /* bh for interrupts */ bool irq_posted; - spinlock_t lock; /* protects the lists of requests */ + spinlock_t lock; /* protects the lists of requests; irqsafe */ struct rb_root waiters; /* sorted by retirement, priority */ struct rb_root signals; /* sorted by retirement */ struct intel_wait *first_wait; /* oldest waiter by retirement */ -- 2.10.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 18+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Convert breadcrumbs spinlock to be irqsafe (rev3) 2016-10-27 16:10 [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe Chris Wilson ` (3 preceding siblings ...) 2016-10-28 12:13 ` [PATCH v3] " Chris Wilson @ 2016-10-28 12:16 ` Patchwork 2016-10-28 13:15 ` ✗ Fi.CI.BAT: failure for drm/i915: Convert breadcrumbs spinlock to be irqsafe (rev4) Patchwork 5 siblings, 0 replies; 18+ messages in thread From: Patchwork @ 2016-10-28 12:16 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx == Series Details == Series: drm/i915: Convert breadcrumbs spinlock to be irqsafe (rev3) URL : https://patchwork.freedesktop.org/series/14488/ State : failure == Summary == Series 14488v3 drm/i915: Convert breadcrumbs spinlock to be irqsafe https://patchwork.freedesktop.org/api/1.0/series/14488/revisions/3/mbox/ Test gem_exec_nop: Subgroup basic-series: pass -> FAIL (fi-bxt-t5700) Test kms_flip: Subgroup basic-flip-vs-wf_vblank: fail -> SKIP (fi-ivb-3770) fi-bdw-5557u total:239 pass:224 dwarn:0 dfail:0 fail:0 skip:15 fi-bsw-n3050 total:239 pass:199 dwarn:0 dfail:0 fail:0 skip:40 fi-bxt-t5700 total:239 pass:210 dwarn:0 dfail:0 fail:1 skip:28 fi-byt-j1900 total:239 pass:211 dwarn:0 dfail:0 fail:0 skip:28 fi-byt-n2820 total:239 pass:207 dwarn:0 dfail:0 fail:0 skip:32 fi-hsw-4770 total:239 pass:219 dwarn:0 dfail:0 fail:0 skip:20 fi-hsw-4770r total:239 pass:218 dwarn:0 dfail:0 fail:0 skip:21 fi-ilk-650 total:239 pass:185 dwarn:0 dfail:0 fail:0 skip:54 fi-ivb-3520m total:239 pass:216 dwarn:0 dfail:0 fail:0 skip:23 fi-ivb-3770 total:239 pass:216 dwarn:0 dfail:0 fail:0 skip:23 fi-kbl-7200u total:239 pass:217 dwarn:0 dfail:0 fail:0 skip:22 fi-skl-6260u total:239 pass:225 dwarn:0 dfail:0 fail:0 skip:14 fi-skl-6700hq total:239 pass:218 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6700k total:239 pass:217 dwarn:1 dfail:0 fail:0 skip:21 fi-skl-6770hq total:239 pass:225 dwarn:0 dfail:0 fail:0 skip:14 fi-snb-2520m total:239 pass:206 dwarn:0 dfail:0 fail:0 skip:33 fi-snb-2600 total:239 pass:205 dwarn:0 dfail:0 fail:0 skip:34 fdc2de7ae3cf48c2eaee3fb05b63848790725361 drm-intel-nightly: 2016y-10m-28d-10h-48m-07s UTC integration manifest 41145a8 drm/i915: Convert breadcrumbs spinlock to be irqsafe and bhsafe == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2847/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 18+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Convert breadcrumbs spinlock to be irqsafe (rev4) 2016-10-27 16:10 [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe Chris Wilson ` (4 preceding siblings ...) 2016-10-28 12:16 ` ✗ Fi.CI.BAT: failure for drm/i915: Convert breadcrumbs spinlock to be irqsafe (rev3) Patchwork @ 2016-10-28 13:15 ` Patchwork 5 siblings, 0 replies; 18+ messages in thread From: Patchwork @ 2016-10-28 13:15 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx == Series Details == Series: drm/i915: Convert breadcrumbs spinlock to be irqsafe (rev4) URL : https://patchwork.freedesktop.org/series/14488/ State : failure == Summary == Series 14488v4 drm/i915: Convert breadcrumbs spinlock to be irqsafe https://patchwork.freedesktop.org/api/1.0/series/14488/revisions/4/mbox/ Test drv_module_reload_basic: pass -> DMESG-WARN (fi-skl-6770hq) skip -> PASS (fi-skl-6260u) Test gem_exec_suspend: Subgroup basic-s3: pass -> DMESG-WARN (fi-skl-6770hq) Test kms_frontbuffer_tracking: Subgroup basic: pass -> FAIL (fi-skl-6770hq) fi-bdw-5557u total:239 pass:224 dwarn:0 dfail:0 fail:0 skip:15 fi-bsw-n3050 total:239 pass:199 dwarn:0 dfail:0 fail:0 skip:40 fi-byt-j1900 total:239 pass:211 dwarn:0 dfail:0 fail:0 skip:28 fi-byt-n2820 total:239 pass:207 dwarn:0 dfail:0 fail:0 skip:32 fi-hsw-4770 total:239 pass:219 dwarn:0 dfail:0 fail:0 skip:20 fi-hsw-4770r total:239 pass:218 dwarn:0 dfail:0 fail:0 skip:21 fi-ilk-650 total:239 pass:185 dwarn:0 dfail:0 fail:0 skip:54 fi-ivb-3520m total:239 pass:216 dwarn:0 dfail:0 fail:0 skip:23 fi-ivb-3770 total:239 pass:216 dwarn:0 dfail:0 fail:0 skip:23 fi-kbl-7200u total:239 pass:217 dwarn:0 dfail:0 fail:0 skip:22 fi-skl-6260u total:239 pass:225 dwarn:0 dfail:0 fail:0 skip:14 fi-skl-6700hq total:239 pass:218 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6700k total:239 pass:217 dwarn:1 dfail:0 fail:0 skip:21 fi-skl-6770hq total:239 pass:222 dwarn:2 dfail:0 fail:1 skip:14 fi-snb-2520m total:239 pass:206 dwarn:0 dfail:0 fail:0 skip:33 fi-snb-2600 total:239 pass:205 dwarn:0 dfail:0 fail:0 skip:34 c15686c36cae1cac4af0807213a9bb4bf79b8cdb drm-intel-nightly: 2016y-10m-28d-12h-29m-33s UTC integration manifest 880387e drm/i915: Convert breadcrumbs spinlock to be irqsafe == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2848/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-10-28 13:15 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-27 16:10 [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe Chris Wilson 2016-10-27 16:48 ` ✓ Fi.CI.BAT: success for " Patchwork 2016-10-28 9:42 ` [PATCH] " Tvrtko Ursulin 2016-10-28 10:10 ` Chris Wilson 2016-10-28 10:27 ` Tvrtko Ursulin 2016-10-28 10:42 ` Chris Wilson 2016-10-28 10:49 ` Tvrtko Ursulin 2016-10-28 11:26 ` Chris Wilson 2016-10-28 11:30 ` Chris Wilson 2016-10-28 11:40 ` Chris Wilson 2016-10-28 11:43 ` Tvrtko Ursulin 2016-10-28 11:26 ` [PATCH v2] drm: Add timestamp of last modification to GETCONNECTOR ioctl Chris Wilson 2016-10-28 11:32 ` Chris Wilson 2016-10-28 11:31 ` [PATCH v2] drm/i915: Convert breadcrumbs spinlock to be irqsafe and bhsafe Chris Wilson 2016-10-28 11:41 ` [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe Joonas Lahtinen 2016-10-28 12:13 ` [PATCH v3] " Chris Wilson 2016-10-28 12:16 ` ✗ Fi.CI.BAT: failure for drm/i915: Convert breadcrumbs spinlock to be irqsafe (rev3) Patchwork 2016-10-28 13:15 ` ✗ Fi.CI.BAT: failure for drm/i915: Convert breadcrumbs spinlock to be irqsafe (rev4) 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.