[Intel-gfx] [PATCH 03/24] drm/i915: assert_spin_locked for pipestat interrupt enable/disable

[Paulo Zanoni]

2013/6/12 Daniel Vetter <daniel.vetter at ffwll.ch>:
> Just to keep the paranoia equal also sprinkle locking asserts over the
> pipestat interrupt enable/disable functions.
>
> Again this results in false positives in the interrupt setup. Add
> bogo-locking for these and a big comment explaining why it's there and
> that it's indeed unnecessary.
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 969da20..c0b6c85 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -329,6 +329,8 @@ i915_enable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask)
>         u32 reg = PIPESTAT(pipe);
>         u32 pipestat = I915_READ(reg) & 0x7fff0000;
>
> +       assert_spin_locked(&dev_priv->irq_lock);
> +
>         if ((pipestat & mask) == mask)
>                 return;
>
> @@ -344,6 +346,8 @@ i915_disable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask)
>         u32 reg = PIPESTAT(pipe);
>         u32 pipestat = I915_READ(reg) & 0x7fff0000;
>
> +       assert_spin_locked(&dev_priv->irq_lock);
> +
>         if ((pipestat & mask) == 0)
>                 return;
>
> @@ -2681,7 +2685,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
>         ibx_irq_postinstall(dev);
>
>         if (IS_IRONLAKE_M(dev)) {
> -               /* Clear & enable PCU event interrupts
> +               /* Enable PCU event interrupts

Oh, here's the "updated comment" I was looking for in the previous
patch! Looks like it missed the train.


>                  *
>                  * spinlocking not required here for correctness since interrupt
>                  * setup is guaranteed to run in single-threaded context. But we
> @@ -2758,6 +2762,7 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
>         u32 gt_irqs;
>         u32 enable_mask;
>         u32 pipestat_enable = PLANE_FLIP_DONE_INT_EN_VLV;
> +       unsigned long irqflags;
>
>         enable_mask = I915_DISPLAY_PORT_INTERRUPT;
>         enable_mask |= I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> @@ -2783,9 +2788,13 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
>         I915_WRITE(PIPESTAT(1), 0xffff);
>         POSTING_READ(VLV_IER);
>
> +       /* Interrup 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);

I guess the same question about irqsave from patch 1 applies here and below.


>         i915_enable_pipestat(dev_priv, 0, pipestat_enable);
>         i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_EVENT_ENABLE);
>         i915_enable_pipestat(dev_priv, 1, pipestat_enable);
> +       spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>
>         I915_WRITE(VLV_IIR, 0xffffffff);
>         I915_WRITE(VLV_IIR, 0xffffffff);
> @@ -3267,6 +3276,7 @@ static int i965_irq_postinstall(struct drm_device *dev)
>         drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
>         u32 enable_mask;
>         u32 error_mask;
> +       unsigned long irqflags;
>
>         /* Unmask the interrupts that we always want on. */
>         dev_priv->irq_mask = ~(I915_ASLE_INTERRUPT |
> @@ -3285,7 +3295,11 @@ static int i965_irq_postinstall(struct drm_device *dev)
>         if (IS_G4X(dev))
>                 enable_mask |= I915_BSD_USER_INTERRUPT;
>
> +       /* Interrup 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_GMBUS_EVENT_ENABLE);
> +       spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>
>         /*
>          * Enable some error detection, note the instruction error mask
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni