Re: [PATCH 08/13] drm/i915: Implement MI decode for gen8

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, May 07, 2014 at 09:59:14AM -0700, Ben Widawsky wrote:
> On Wed, Apr 30, 2014 at 02:21:15PM +0300, Ville Syrjälä wrote:
> > On Tue, Apr 29, 2014 at 02:52:35PM -0700, Ben Widawsky wrote:
> > > From: Ben Widawsky <benjamin.widawsky@linux.intel.com>
> > > 
> > > This is needed to implement ipehr_is_semaphore_wait
> > > 
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > > ---
> > >  drivers/gpu/drm/i915/i915_irq.c | 11 +++++------
> > >  1 file changed, 5 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > index 2d76183..bfd21c7 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -2561,12 +2561,9 @@ static bool
> > >  ipehr_is_semaphore_wait(struct drm_device *dev, u32 ipehr)
> > >  {
> > >  	if (INTEL_INFO(dev)->gen >= 8) {
> > > -		/*
> > > -		 * FIXME: gen8 semaphore support - currently we don't emit
> > > -		 * semaphores on bdw anyway, but this needs to be addressed when
> > > -		 * we merge that code.
> > > -		 */
> > > -		return false;
> > > +		/* Broadwell's semaphore wait is 3 dwords. We hope IPEHR is the
> > > +		 * first dword. */
> > > +		return (ipehr >> 23) == 0x1c;
> > >  	} else {
> > >  		ipehr &= ~MI_SEMAPHORE_SYNC_MASK;
> > >  		return ipehr == (MI_SEMAPHORE_MBOX | MI_SEMAPHORE_COMPARE |
> > > @@ -2586,6 +2583,8 @@ semaphore_wait_to_signaller_ring(struct intel_ring_buffer *ring, u32 ipehr)
> > >  		 * FIXME: gen8 semaphore support - currently we don't emit
> > >  		 * semaphores on bdw anyway, but this needs to be addressed when
> > >  		 * we merge that code.
> > > +		 *
> > > +		 * XXX: Gen8 needs more than just IPEHR.
> > >  		 */
> > 
> > I believe something like this should take care of the remaining gap.
> > 
> 
> Thanks for the help. At this point I just want to get the damn thing
> merged, and want to avoid any extra risk. Would you mind resending this
> as a distinct patch (authored by you), after we get the rest merged?

Can do.

> 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 2446e61..cd1069e 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2590,19 +2590,21 @@ ipehr_is_semaphore_wait(struct drm_device *dev, u32 ipehr)
> >  }
> >  
> >  static struct intel_ring_buffer *
> > -semaphore_wait_to_signaller_ring(struct intel_ring_buffer *ring, u32 ipehr)
> > +semaphore_wait_to_signaller_ring(struct intel_ring_buffer *ring,
> > +				 u32 ipehr, u64 offset)
> >  {
> >  	struct drm_i915_private *dev_priv = ring->dev->dev_private;
> >  	struct intel_ring_buffer *signaller;
> >  	int i;
> >  
> >  	if (INTEL_INFO(dev_priv->dev)->gen >= 8) {
> > -		/*
> > -		 * FIXME: gen8 semaphore support - currently we don't emit
> > -		 * semaphores on bdw anyway, but this needs to be addressed when
> > -		 * we merge that code.
> > -		 */
> > -		return NULL;
> > +		for_each_ring(signaller, dev_priv, i) {
> > +			if (ring == signaller)
> > +				continue;
> > +
> > +			if (offset == signaller->semaphore.signal_gtt[ring->id])
> > +				return signaller;
> > +		}
> >  	} else {
> >  		u32 sync_bits = ipehr & MI_SEMAPHORE_SYNC_MASK;
> >  
> > @@ -2627,6 +2629,7 @@ semaphore_waits_for(struct intel_ring_buffer *ring, u32 *seqno)
> >  {
> >  	struct drm_i915_private *dev_priv = ring->dev->dev_private;
> >  	u32 cmd, ipehr, head;
> > +	u64 offset = 0;
> >  	int i;
> >  
> >  	ipehr = I915_READ(RING_IPEHR(ring->mmio_base));
> > @@ -2662,7 +2665,12 @@ semaphore_waits_for(struct intel_ring_buffer *ring, u32 *seqno)
> >  		return NULL;
> >  
> >  	*seqno = ioread32(ring->virtual_start + head + 4) + 1;
> > -	return semaphore_wait_to_signaller_ring(ring, ipehr);
> > +	if (INTEL_INFO(dev_priv->dev)->gen >= 8) {
> > +		offset = ioread32(ring->virtual_start + head + 12);
> > +		offset <<= 32;
> > +		offset |= ioread32(ring->virtual_start + head + 8);
> > +	}
> > +	return semaphore_wait_to_signaller_ring(ring, ipehr, offset);
> >  }
> >  
> >  static int semaphore_passed(struct intel_ring_buffer *ring)
> > -- 
> > 1.8.3.2
> > 
> > 
> > >  		return NULL;
> > >  	} else {
> > > -- 
> > > 1.9.2
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC