[Intel-gfx] [PATCH v2 2/3] drm/i915: Split breadcrumbs spinlock into two

[Chris Wilson]

On Fri, Mar 03, 2017 at 07:26:34PM +0200, Mika Kuoppala wrote:
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> > @@ -768,7 +776,7 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine)
> >  	struct intel_breadcrumbs *b = &engine->breadcrumbs;
> >  
> >  	cancel_fake_irq(engine);
> > -	spin_lock_irq(&b->lock);
> > +	spin_lock_irq(&b->irq_lock);
> 
> In here I was thinking that you want both locks help, but
> then can't find a reason why. Perhaps just to ensure that
> the wait tree stays still.

Right, here we are just rearming the irq, so I didn't think taking the
rb_lock helped with clarity. (Just the opposite, this showed nicely the
divison in labour between the locks :)

> >  	if (b->irq_enabled)
> >  		irq_enable(engine);
> > @@ -787,7 +795,7 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine)
> >  	if (b->irq_armed)
> >  		enable_fake_irq(b);
> >  
> > -	spin_unlock_irq(&b->lock);
> > +	spin_unlock_irq(&b->irq_lock);
> >  }
> >  
> >  void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine)
> > @@ -811,7 +819,7 @@ bool intel_breadcrumbs_busy(struct intel_engine_cs *engine)
> >  	struct intel_breadcrumbs *b = &engine->breadcrumbs;
> >  	bool busy = false;
> >  
> > -	spin_lock_irq(&b->lock);
> > +	spin_lock_irq(&b->rb_lock);
> 
> Wrong lock taken and relased in this function?

Here we can simply take the outer rb_lock, as that guarantees the rbtree
won't change and therefore also the first_wait/irq_wait cannot be lost.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre