On ma, 2016-04-11 at 16:56 +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > The vlv/chv display irq setup was a bit of mess after I ran out of steam > when working on it last. Fix it up so that we just have a _reset() and > _postinstall() hooks for the display irqs, and use those consistently. > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_irq.c | 102 ++++++++++------------------------------ > 1 file changed, 24 insertions(+), 78 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 1d21ebfffd4d..a1239fedc086 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -3306,13 +3306,15 @@ static void vlv_display_irq_reset(struct drm_i915_private *dev_priv) > { > enum pipe pipe; > > - i915_hotplug_interrupt_update(dev_priv, 0xFFFFFFFF, 0); > + i915_hotplug_interrupt_update_locked(dev_priv, 0xffffffff, 0); > I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); > > for_each_pipe(dev_priv, pipe) > I915_WRITE(PIPESTAT(pipe), 0xffff); Since vlv_display_irq_reset() will be used in place of valleyview_display_irqs_uninstall()/i915_disable_pipestat() we'll leave now stale bits in pipestat_irq_mask[pipe]. It's not a problem since display_irqs_enabled effectively masks these bits, but for consistency I'd clear them. The same goes for clearing PIPE_FIFO_UNDERRUN_STATUS. With that: Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx> > > GEN5_IRQ_RESET(VLV_); > + > + dev_priv->irq_mask = ~0; > } > > static void valleyview_irq_preinstall(struct drm_device *dev) > @@ -3323,7 +3325,9 @@ static void valleyview_irq_preinstall(struct drm_device *dev) > > I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK); > > + spin_lock_irq(&dev_priv->irq_lock); > vlv_display_irq_reset(dev_priv); > + spin_unlock_irq(&dev_priv->irq_lock); > } > > static void gen8_gt_irq_reset(struct drm_i915_private *dev_priv) > @@ -3398,7 +3402,9 @@ static void cherryview_irq_preinstall(struct drm_device *dev) > > I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK_CHV); > > + spin_lock_irq(&dev_priv->irq_lock); > vlv_display_irq_reset(dev_priv); > + spin_unlock_irq(&dev_priv->irq_lock); > } > > static u32 intel_hpd_enabled_irqs(struct drm_device *dev, > @@ -3645,7 +3651,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev) > return 0; > } > > -static void valleyview_display_irqs_install(struct drm_i915_private *dev_priv) > +static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv) > { > u32 pipestat_mask; > u32 iir_mask; > @@ -3679,40 +3685,6 @@ static void valleyview_display_irqs_install(struct drm_i915_private *dev_priv) > POSTING_READ(VLV_IMR); > } > > -static void valleyview_display_irqs_uninstall(struct drm_i915_private *dev_priv) > -{ > - u32 pipestat_mask; > - u32 iir_mask; > - enum pipe pipe; > - > - iir_mask = I915_DISPLAY_PORT_INTERRUPT | > - I915_DISPLAY_PIPE_A_EVENT_INTERRUPT | > - I915_DISPLAY_PIPE_B_EVENT_INTERRUPT; > - if (IS_CHERRYVIEW(dev_priv)) > - iir_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT; > - > - dev_priv->irq_mask |= iir_mask; > - I915_WRITE(VLV_IMR, dev_priv->irq_mask); > - I915_WRITE(VLV_IER, ~dev_priv->irq_mask); > - I915_WRITE(VLV_IIR, iir_mask); > - I915_WRITE(VLV_IIR, iir_mask); > - POSTING_READ(VLV_IIR); > - > - pipestat_mask = PLANE_FLIP_DONE_INT_STATUS_VLV | > - PIPE_CRC_DONE_INTERRUPT_STATUS; > - > - i915_disable_pipestat(dev_priv, PIPE_A, PIPE_GMBUS_INTERRUPT_STATUS); > - for_each_pipe(dev_priv, pipe) > - i915_disable_pipestat(dev_priv, pipe, pipestat_mask); > - > - pipestat_mask = PIPESTAT_INT_STATUS_MASK | > - PIPE_FIFO_UNDERRUN_STATUS; > - > - for_each_pipe(dev_priv, pipe) > - I915_WRITE(PIPESTAT(pipe), pipestat_mask); > - POSTING_READ(PIPESTAT(PIPE_A)); > -} > - > void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv) > { > assert_spin_locked(&dev_priv->irq_lock); > @@ -3723,7 +3695,7 @@ void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv) > dev_priv->display_irqs_enabled = true; > > if (intel_irqs_enabled(dev_priv)) > - valleyview_display_irqs_install(dev_priv); > + vlv_display_irq_postinstall(dev_priv); > } > > void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv) > @@ -3736,36 +3708,14 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv) > dev_priv->display_irqs_enabled = false; > > if (intel_irqs_enabled(dev_priv)) > - valleyview_display_irqs_uninstall(dev_priv); > + vlv_display_irq_reset(dev_priv); > } > > -static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv) > -{ > - dev_priv->irq_mask = ~0; > - > - i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0); > - POSTING_READ(PORT_HOTPLUG_EN); > - > - I915_WRITE(VLV_IIR, 0xffffffff); > - I915_WRITE(VLV_IIR, 0xffffffff); > - I915_WRITE(VLV_IER, ~dev_priv->irq_mask); > - I915_WRITE(VLV_IMR, dev_priv->irq_mask); > - POSTING_READ(VLV_IMR); > - > - /* Interrupt setup is already guaranteed to be single-threaded, this is > - * just to make the assert_spin_locked check happy. */ > - spin_lock_irq(&dev_priv->irq_lock); > - if (dev_priv->display_irqs_enabled) > - valleyview_display_irqs_install(dev_priv); > - spin_unlock_irq(&dev_priv->irq_lock); > -} > > static int valleyview_irq_postinstall(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > - vlv_display_irq_postinstall(dev_priv); > - > gen5_gt_irq_postinstall(dev); > > /* ack & enable invalid PTE error interrupts */ > @@ -3774,6 +3724,10 @@ static int valleyview_irq_postinstall(struct drm_device *dev) > I915_WRITE(DPINVGTT, DPINVGTT_EN_MASK); > #endif > > + spin_lock_irq(&dev_priv->irq_lock); > + vlv_display_irq_postinstall(dev_priv); > + spin_unlock_irq(&dev_priv->irq_lock); > + > I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE); > > return 0; > @@ -3874,10 +3828,12 @@ static int cherryview_irq_postinstall(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > - vlv_display_irq_postinstall(dev_priv); > - > gen8_gt_irq_postinstall(dev_priv); > > + spin_lock_irq(&dev_priv->irq_lock); > + vlv_display_irq_postinstall(dev_priv); > + spin_unlock_irq(&dev_priv->irq_lock); > + > I915_WRITE(GEN8_MASTER_IRQ, MASTER_INTERRUPT_ENABLE); > POSTING_READ(GEN8_MASTER_IRQ); > > @@ -3894,20 +3850,6 @@ static void gen8_irq_uninstall(struct drm_device *dev) > gen8_irq_reset(dev); > } > > -static void vlv_display_irq_uninstall(struct drm_i915_private *dev_priv) > -{ > - /* Interrupt setup is already guaranteed to be single-threaded, this is > - * just to make the assert_spin_locked check happy. */ > - spin_lock_irq(&dev_priv->irq_lock); > - if (dev_priv->display_irqs_enabled) > - valleyview_display_irqs_uninstall(dev_priv); > - spin_unlock_irq(&dev_priv->irq_lock); > - > - vlv_display_irq_reset(dev_priv); > - > - dev_priv->irq_mask = ~0; > -} > - > static void valleyview_irq_uninstall(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -3921,7 +3863,9 @@ static void valleyview_irq_uninstall(struct drm_device *dev) > > I915_WRITE(HWSTAM, 0xffffffff); > > - vlv_display_irq_uninstall(dev_priv); > + spin_lock_irq(&dev_priv->irq_lock); > + vlv_display_irq_reset(dev_priv); > + spin_unlock_irq(&dev_priv->irq_lock); > } > > static void cherryview_irq_uninstall(struct drm_device *dev) > @@ -3938,7 +3882,9 @@ static void cherryview_irq_uninstall(struct drm_device *dev) > > GEN5_IRQ_RESET(GEN8_PCU_); > > - vlv_display_irq_uninstall(dev_priv); > + spin_lock_irq(&dev_priv->irq_lock); > + vlv_display_irq_reset(dev_priv); > + spin_unlock_irq(&dev_priv->irq_lock); > } > > static void ironlake_irq_uninstall(struct drm_device *dev) _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx