[PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Chris Wilson]

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

✓ Fi.CI.BAT: success for drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Patchwork]

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

Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Tvrtko Ursulin]

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

Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Chris Wilson]

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

Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Tvrtko Ursulin]

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

Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Chris Wilson]

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

Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Tvrtko Ursulin]

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

Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Chris Wilson]

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

Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Chris Wilson]

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

Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Chris Wilson]

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

Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Tvrtko Ursulin]

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

[PATCH v2] drm: Add timestamp of last modification to GETCONNECTOR ioctl

[Chris Wilson]

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

Re: [PATCH v2] drm: Add timestamp of last modification to GETCONNECTOR ioctl

[Chris Wilson]

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

[PATCH v2] drm/i915: Convert breadcrumbs spinlock to be irqsafe and bhsafe

[Chris Wilson]

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

Re: [PATCH] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Joonas Lahtinen]

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

[PATCH v3] drm/i915: Convert breadcrumbs spinlock to be irqsafe

[Chris Wilson]

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

✗ Fi.CI.BAT: failure for drm/i915: Convert breadcrumbs spinlock to be irqsafe (rev3)

[Patchwork]

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

✗ Fi.CI.BAT: failure for drm/i915: Convert breadcrumbs spinlock to be irqsafe (rev4)

[Patchwork]

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