Re: [PATCH 11/16] drm/i915: Enable CRC interrupts on pre-gen5/vlv

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Oct 21, 2013 at 01:49:24PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 16, 2013 at 10:55:56PM +0200, Daniel Vetter wrote:
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 21 ++++++++++++++++++++-
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index b31e7ca..5c3baa0 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2574,7 +2574,8 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
> >  {
> >  	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> >  	u32 enable_mask;
> > -	u32 pipestat_enable = PLANE_FLIP_DONE_INT_EN_VLV;
> > +	u32 pipestat_enable = PLANE_FLIP_DONE_INT_EN_VLV |
> > +		PIPE_CRC_DONE_ENABLE;
> >  	unsigned long irqflags;
> >  
> >  	enable_mask = I915_DISPLAY_PORT_INTERRUPT;
> > @@ -2697,6 +2698,7 @@ static void i8xx_irq_preinstall(struct drm_device * dev)
> >  static int i8xx_irq_postinstall(struct drm_device *dev)
> >  {
> >  	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> > +	unsigned long irqflags;
> >  
> >  	I915_WRITE16(EMR,
> >  		     ~(I915_ERROR_PAGE_TABLE | I915_ERROR_MEMORY_REFRESH));
> > @@ -2717,6 +2719,13 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
> >  		     I915_USER_INTERRUPT);
> >  	POSTING_READ16(IER);
> >  
> > +	/* Interrupt setup is already guaranteed to be single-threaded, this is
> > +	 * just to make the assert_spin_locked check happy. */
> > +	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> > +	i915_enable_pipestat(dev_priv, 0, PIPE_CRC_DONE_ENABLE);
> > +	i915_enable_pipestat(dev_priv, 1, PIPE_CRC_DONE_ENABLE);
> 
> Could use PIPE_A/B instead of raw numbers. Maybe a separate patch
> to fix it all up since we're already using raw numbers in some
> other places in i915_irq.c.

Yeah, this is just for consistency. I'll do a follow-up patch to sprinkle
nice enums over i915_irq.c. There's also 1-2 places that would benefit
from a for_each_pipe loop.
-Daniel

> 
> > +	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> > +
> >  	return 0;
> >  }
> >  
> > @@ -2857,6 +2866,7 @@ static int i915_irq_postinstall(struct drm_device *dev)
> >  {
> >  	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> >  	u32 enable_mask;
> > +	unsigned long irqflags;
> >  
> >  	I915_WRITE(EMR, ~(I915_ERROR_PAGE_TABLE | I915_ERROR_MEMORY_REFRESH));
> >  
> > @@ -2892,6 +2902,13 @@ static int i915_irq_postinstall(struct drm_device *dev)
> >  
> >  	i915_enable_asle_pipestat(dev);
> >  
> > +	/* Interrupt setup is already guaranteed to be single-threaded, this is
> > +	 * just to make the assert_spin_locked check happy. */
> > +	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> > +	i915_enable_pipestat(dev_priv, 0, PIPE_CRC_DONE_ENABLE);
> > +	i915_enable_pipestat(dev_priv, 1, PIPE_CRC_DONE_ENABLE);
> > +	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> > +
> >  	return 0;
> >  }
> >  
> > @@ -3105,6 +3122,8 @@ static int i965_irq_postinstall(struct drm_device *dev)
> >  	 * just to make the assert_spin_locked check happy. */
> >  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> >  	i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_EVENT_ENABLE);
> > +	i915_enable_pipestat(dev_priv, 0, PIPE_CRC_DONE_ENABLE);
> > +	i915_enable_pipestat(dev_priv, 1, PIPE_CRC_DONE_ENABLE);
> >  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> >  
> >  	/*
> > -- 
> > 1.8.4.rc3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch