From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 11/12] drm/i915: Inline I915_INTERRUPT_ENABLE_FIX Date: Wed, 25 Apr 2012 13:55:12 -0700 Message-ID: <20120425135512.6307b991@jbarnes-desktop> References: <1335304792-17636-1-git-send-email-chris@chris-wilson.co.uk> <1335304792-17636-11-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy5-pub.bluehost.com (oproxy5-pub.bluehost.com [67.222.38.55]) by gabe.freedesktop.org (Postfix) with SMTP id B83E29E97F for ; Wed, 25 Apr 2012 13:55:16 -0700 (PDT) In-Reply-To: <1335304792-17636-11-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, 24 Apr 2012 22:59:51 +0100 Chris Wilson 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 > --- > 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 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