Re: [PATCH 2/9] drm/i915: wrap GTIMR changes

[Paulo Zanoni <przanoni@gmail.com>]

2013/8/14 Rodrigo Vivi <rodrigo.vivi@gmail.com>
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>
> .On Tue, Aug 06, 2013 at 06:57:12PM -0300, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > Just like the functions that touch DEIMR and SDEIMR, but for GTIMR.
> > The new functions contain a POSTING_READ(GTIMR) which was not present
> > at the 2 callers inside i915_irq.c.
> >
> > The implementation is based on ibx_display_interrupt_update.
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c         | 34 +++++++++++++++++++++++++++++----
> >  drivers/gpu/drm/i915/intel_drv.h        |  3 +++
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 22 ++++++---------------
> >  3 files changed, 39 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 6a1c207..a6e98ea 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -104,6 +104,34 @@ ironlake_disable_display_irq(drm_i915_private_t *dev_priv, u32 mask)
> >       }
> >  }
> >
> > +/**
> > + * ilk_update_gt_irq - update GTIMR
> > + * @dev_priv: driver private
> > + * @interrupt_mask: mask of interrupt bits to update
> > + * @enabled_irq_mask: mask of interrupt bits to enable
> > + */
> > +static void ilk_update_gt_irq(struct drm_i915_private *dev_priv,
> > +                           uint32_t interrupt_mask,
> > +                           uint32_t enabled_irq_mask)
> > +{
> > +     assert_spin_locked(&dev_priv->irq_lock);
> > +
> > +     dev_priv->gt_irq_mask &= ~interrupt_mask;
> > +     dev_priv->gt_irq_mask |= (~enabled_irq_mask & interrupt_mask);
>
> my little mind got confused with logic above, but after some minutes I convinced myself this works ;)

And if this contains a bug, then ibx_display_interrupt_update also
contains a bug :)


>
>
> > +     I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
> > +     POSTING_READ(GTIMR);
> > +}
> > +
> > +void ilk_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask)
> > +{
> > +     ilk_update_gt_irq(dev_priv, mask, mask);
> > +}
> > +
> > +void ilk_disable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask)
> > +{
> > +     ilk_update_gt_irq(dev_priv, mask, 0);
> > +}
> > +
> >  static bool ivb_can_enable_err_int(struct drm_device *dev)
> >  {
> >       struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -806,8 +834,7 @@ static void ivybridge_parity_work(struct work_struct *work)
> >       I915_WRITE(GEN7_MISCCPCTL, misccpctl);
> >
> >       spin_lock_irqsave(&dev_priv->irq_lock, flags);
> > -     dev_priv->gt_irq_mask &= ~GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
> > -     I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
> > +     ilk_enable_gt_irq(dev_priv, GT_RENDER_L3_PARITY_ERROR_INTERRUPT);
> >       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> >
> >       mutex_unlock(&dev_priv->dev->struct_mutex);
> > @@ -837,8 +864,7 @@ static void ivybridge_parity_error_irq_handler(struct drm_device *dev)
> >               return;
> >
> >       spin_lock(&dev_priv->irq_lock);
> > -     dev_priv->gt_irq_mask |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
> > -     I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
> > +     ilk_disable_gt_irq(dev_priv, GT_RENDER_L3_PARITY_ERROR_INTERRUPT);
> >       spin_unlock(&dev_priv->irq_lock);
> >
> >       queue_work(dev_priv->wq, &dev_priv->l3_parity.error_work);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 54e389d..82bc78e 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -838,5 +838,8 @@ extern void intel_edp_psr_update(struct drm_device *dev);
> >  extern void hsw_disable_lcpll(struct drm_i915_private *dev_priv,
> >                             bool switch_to_fclk, bool allow_power_down);
> >  extern void hsw_restore_lcpll(struct drm_i915_private *dev_priv);
> > +extern void ilk_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
> > +extern void ilk_disable_gt_irq(struct drm_i915_private *dev_priv,
> > +                            uint32_t mask);
> >
> >  #endif /* __INTEL_DRV_H__ */
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 74d02a7..6eeca1e 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -836,11 +836,8 @@ gen5_ring_get_irq(struct intel_ring_buffer *ring)
> >               return false;
> >
> >       spin_lock_irqsave(&dev_priv->irq_lock, flags);
> > -     if (ring->irq_refcount++ == 0) {
> > -             dev_priv->gt_irq_mask &= ~ring->irq_enable_mask;
> > -             I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
> > -             POSTING_READ(GTIMR);
> > -     }
> > +     if (ring->irq_refcount++ == 0)
> > +             ilk_enable_gt_irq(dev_priv, ring->irq_enable_mask);
> >       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> >
> >       return true;
> > @@ -854,11 +851,8 @@ gen5_ring_put_irq(struct intel_ring_buffer *ring)
> >       unsigned long flags;
> >
> >       spin_lock_irqsave(&dev_priv->irq_lock, flags);
> > -     if (--ring->irq_refcount == 0) {
> > -             dev_priv->gt_irq_mask |= ring->irq_enable_mask;
> > -             I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
> > -             POSTING_READ(GTIMR);
> > -     }
> > +     if (--ring->irq_refcount == 0)
> > +             ilk_disable_gt_irq(dev_priv, ring->irq_enable_mask);
> >       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> >  }
> >
> > @@ -1028,9 +1022,7 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring)
> >                                        GT_RENDER_L3_PARITY_ERROR_INTERRUPT));
> >               else
> >                       I915_WRITE_IMR(ring, ~ring->irq_enable_mask);
> > -             dev_priv->gt_irq_mask &= ~ring->irq_enable_mask;
> > -             I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
> > -             POSTING_READ(GTIMR);
> > +             ilk_enable_gt_irq(dev_priv, ring->irq_enable_mask);
> >       }
> >       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> >
> > @@ -1051,9 +1043,7 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring)
> >                                      ~GT_RENDER_L3_PARITY_ERROR_INTERRUPT);
> >               else
> >                       I915_WRITE_IMR(ring, ~0);
> > -             dev_priv->gt_irq_mask |= ring->irq_enable_mask;
> > -             I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
> > -             POSTING_READ(GTIMR);
> > +             ilk_disable_gt_irq(dev_priv, ring->irq_enable_mask);
> >       }
> >       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> >
> > --
> > 1.8.1.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx




-- 
Paulo Zanoni