Re: [PATCH 6/7] drm/i915: Optimize gen8_enable|disable_vblank functions

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

On Thu, Nov 07, 2013 at 11:05:45AM +0100, Daniel Vetter wrote:
> Let's cache the IMR value like on other platforms. This is needed to
> implement the underrun reporting since then we'll have two places that
> change the same register at runtime.

This looks OK, so:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

But I think gen8_de_irq_postinstall() isn't quite right. It'll already
enable and unmask the vblank irqs, even though it should just enable
them, but leave them masked.

> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d2d678f72486..51966feee5d2 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2203,17 +2203,14 @@ static int gen8_enable_vblank(struct drm_device *dev, int pipe)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	unsigned long irqflags;
> -	uint32_t imr;
>  
>  	if (!i915_pipe_enabled(dev, pipe))
>  		return -EINVAL;
>  
>  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> -	imr = I915_READ(GEN8_DE_PIPE_IMR(pipe));
> -	if ((imr & GEN8_PIPE_VBLANK) == 1) {
> -		I915_WRITE(GEN8_DE_PIPE_IMR(pipe), imr & ~GEN8_PIPE_VBLANK);
> -		POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
> -	}
> +	dev_priv->de_irq_mask[pipe] &= ~GEN8_PIPE_VBLANK;
> +	I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
> +	POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>  	return 0;
>  }
> @@ -2270,17 +2267,14 @@ static void gen8_disable_vblank(struct drm_device *dev, int pipe)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	unsigned long irqflags;
> -	uint32_t imr;
>  
>  	if (!i915_pipe_enabled(dev, pipe))
>  		return;
>  
>  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> -	imr = I915_READ(GEN8_DE_PIPE_IMR(pipe));
> -	if ((imr & GEN8_PIPE_VBLANK) == 0) {
> -		I915_WRITE(GEN8_DE_PIPE_IMR(pipe), imr | GEN8_PIPE_VBLANK);
> -		POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
> -	}
> +	dev_priv->de_irq_mask[pipe] |= GEN8_PIPE_VBLANK;
> +	I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
> +	POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
>  	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