On ti, 2016-04-12 at 12:05 +0300, Ville Syrjälä wrote:
> On Mon, Apr 11, 2016 at 07:29:13PM +0300, Imre Deak wrote:
> > 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.
>
> OTOH we can't mask PIPESTAT bits, so even if we clear them here, it's
> very likely some of the bits will be set again by the time we
> actually
> enable an interrupt.
>
> In any case, I think I'll be posting a patch to clean up the PIPESTAT
> clearing/disabling acrosss the board. It's a bit of a mess right now,
> with each platform doing things slightly differently.
Ok, you could add something about the above to the commit message. My
R-b applies in any case.
> > 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
