On Wed, 25 Apr 2012 13:55:12 -0700, Jesse Barnes <jbarnes at virtuousgeek.org> wrote: > On Tue, 24 Apr 2012 22:59:51 +0100 > Chris Wilson <chris at chris-wilson.co.uk> wrote: > > + 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. I considered reducing the duplication, but having just done i8xx/i915 where the relationship between irq_mask and enable_mask was a little more complicated, I erred on the side of verbosity. enable_mask = ~dev_priv->irq_mask; enable_mask |= I915_USER_INTERRUPT; if (IS_G4X(dev)) enable_mask |= I915_BSD_USER_INTERRUPT; Works for me as well. -Chris -- Chris Wilson, Intel Open Source Technology Centre