On Tue, 24 Apr 2012 22:59:51 +0100
Chris Wilson <chris at chris-wilson.co.uk> wrote:
> Since there is only one remaining user of I915_INTERRUPT_ENABLE_FIX,
> expand it at the callsite. Quoting Jesse Barnes:
>
> "I'd really like to get rid of these defines at the top of i915_irq.c.
> Some are unused and the others just make you check for the right bits
> everytime your read the code."
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 38 ++++++++++++++++++--------------------
> 1 file changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 1d320e0..a1150b7 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -39,24 +39,6 @@
>
> #define MAX_NOPID ((u32)~0)
>
> -/**
> - * Interrupts that are always left unmasked.
> - *
> - * Since pipe events are edge-triggered from the PIPESTAT register to IIR,
> - * we leave them always unmasked in IMR and then control enabling them through
> - * PIPESTAT alone.
> - */
> -#define I915_INTERRUPT_ENABLE_FIX \
> - (I915_ASLE_INTERRUPT | \
> - I915_DISPLAY_PIPE_A_EVENT_INTERRUPT | \
> - I915_DISPLAY_PIPE_B_EVENT_INTERRUPT | \
> - I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT | \
> - I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT | \
> - I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
> -
> -/** Interrupts that we mask and unmask at runtime. */
> -#define I915_INTERRUPT_ENABLE_VAR (I915_USER_INTERRUPT | I915_BSD_USER_INTERRUPT)
> -
> #define I915_PIPE_VBLANK_STATUS (PIPE_START_VBLANK_INTERRUPT_STATUS |\
> PIPE_VBLANK_INTERRUPT_STATUS)
>
> @@ -2549,13 +2531,29 @@ static void i965_irq_preinstall(struct drm_device * dev)
> 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 = I915_INTERRUPT_ENABLE_FIX | I915_INTERRUPT_ENABLE_VAR;
> + u32 enable_mask;
> u32 error_mask;
>
> dev_priv->vblank_pipe = DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B;
>
> /* Unmask the interrupts that we always want on. */
> - dev_priv->irq_mask = ~I915_INTERRUPT_ENABLE_FIX;
> + dev_priv->irq_mask = ~(I915_ASLE_INTERRUPT |
> + I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> + I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
> + I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
> + I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT |
> + I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT);
> +
> + enable_mask = (I915_ASLE_INTERRUPT |
> + I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> + I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
> + I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
> + I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT |
> + I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT |
> + I915_USER_INTERRUPT);
You could set the enable mask to ~irq_mask | I915_USER_INTERRUPT I
think just to save a little duplication? No biggie though.
Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>
Also this one is a little sneaky because it silently fixes the
BSD_USER_INTERRUPT bug on pre-Cantiga. But I suppose that's fine...
--
Jesse Barnes, Intel Open Source Technology Center
