drm/i915: Don't rmw PIPESTAT enable bits

i830 seems to occasionally forget the PIPESTAT enable bits when
we read the register. These aren't the only registers on i830 that
have problems with RMW, as reading the double buffered plane
registers returns the latched value rather than the last written
value. So something similar is perhaps going on with PIPESTAT.

This corruption results on vblank interrupts occasionally turning off
on their own, which leads to vblank timeouts and generally a stuck
display subsystem.

So let's not RMW the pipestat enable bits, and instead use the cached
copy we have around.

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170914151731.5034-1-ville.syrjala@linux.intel.com
Reviewed-by: Imre Deak <imre.deak@intel.com>

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0f26aa57a9224841ef492b30f7da9044881b9b97..44f5569ed9951525f1bb81d015eda52c5032e829 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3313,6 +3313,8 @@ static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
        return dev_priv->vgpu.active;
 }
 
+u32 i915_pipestat_enable_mask(struct drm_i915_private *dev_priv,
+                             enum pipe pipe);
 void
 i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
                     u32 status_mask);

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index af82bd721dbc379654edcb817071c75f58140dc6..6a07ef38fb479b2779b16362874de898e04c194c 100644 (file)
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -567,62 +567,16 @@ void ibx_display_interrupt_update(struct drm_i915_private *dev_priv,
        POSTING_READ(SDEIMR);
 }
 
-static void
-__i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
-                      u32 enable_mask, u32 status_mask)
+u32 i915_pipestat_enable_mask(struct drm_i915_private *dev_priv,
+                             enum pipe pipe)
 {
-       i915_reg_t reg = PIPESTAT(pipe);
-       u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
-
-       lockdep_assert_held(&dev_priv->irq_lock);
-       WARN_ON(!intel_irqs_enabled(dev_priv));
-
-       if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
-                     status_mask & ~PIPESTAT_INT_STATUS_MASK,
-                     "pipe %c: enable_mask=0x%x, status_mask=0x%x\n",
-                     pipe_name(pipe), enable_mask, status_mask))
-               return;
-
-       if ((pipestat & enable_mask) == enable_mask)
-               return;
-
-       dev_priv->pipestat_irq_mask[pipe] |= status_mask;
-
-       /* Enable the interrupt, clear any pending status */
-       pipestat |= enable_mask | status_mask;
-       I915_WRITE(reg, pipestat);
-       POSTING_READ(reg);
-}
-
-static void
-__i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
-                       u32 enable_mask, u32 status_mask)
-{
-       i915_reg_t reg = PIPESTAT(pipe);
-       u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK;
+       u32 status_mask = dev_priv->pipestat_irq_mask[pipe];
+       u32 enable_mask = status_mask << 16;
 
        lockdep_assert_held(&dev_priv->irq_lock);
-       WARN_ON(!intel_irqs_enabled(dev_priv));
-
-       if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
-                     status_mask & ~PIPESTAT_INT_STATUS_MASK,
-                     "pipe %c: enable_mask=0x%x, status_mask=0x%x\n",
-                     pipe_name(pipe), enable_mask, status_mask))
-               return;
-
-       if ((pipestat & enable_mask) == 0)
-               return;
-
-       dev_priv->pipestat_irq_mask[pipe] &= ~status_mask;
-
-       pipestat &= ~enable_mask;
-       I915_WRITE(reg, pipestat);
-       POSTING_READ(reg);
-}
 
-static u32 vlv_get_pipestat_enable_mask(struct drm_device *dev, u32 status_mask)
-{
-       u32 enable_mask = status_mask << 16;
+       if (INTEL_GEN(dev_priv) < 5)
+               goto out;
 
        /*
         * On pipe A we don't support the PSR interrupt yet,
@@ -645,35 +599,59 @@ static u32 vlv_get_pipestat_enable_mask(struct drm_device *dev, u32 status_mask)
        if (status_mask & SPRITE1_FLIP_DONE_INT_STATUS_VLV)
                enable_mask |= SPRITE1_FLIP_DONE_INT_EN_VLV;
 
+out:
+       WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK ||
+                 status_mask & ~PIPESTAT_INT_STATUS_MASK,
+                 "pipe %c: enable_mask=0x%x, status_mask=0x%x\n",
+                 pipe_name(pipe), enable_mask, status_mask);
+
        return enable_mask;
 }
 
-void
-i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
-                    u32 status_mask)
+void i915_enable_pipestat(struct drm_i915_private *dev_priv,
+                         enum pipe pipe, u32 status_mask)
 {
+       i915_reg_t reg = PIPESTAT(pipe);
        u32 enable_mask;
 
-       if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-               enable_mask = vlv_get_pipestat_enable_mask(&dev_priv->drm,
-                                                          status_mask);
-       else
-               enable_mask = status_mask << 16;
-       __i915_enable_pipestat(dev_priv, pipe, enable_mask, status_mask);
+       WARN_ONCE(status_mask & ~PIPESTAT_INT_STATUS_MASK,
+                 "pipe %c: status_mask=0x%x\n",
+                 pipe_name(pipe), status_mask);
+
+       lockdep_assert_held(&dev_priv->irq_lock);
+       WARN_ON(!intel_irqs_enabled(dev_priv));
+
+       if ((dev_priv->pipestat_irq_mask[pipe] & status_mask) == status_mask)
+               return;
+
+       dev_priv->pipestat_irq_mask[pipe] |= status_mask;
+       enable_mask = i915_pipestat_enable_mask(dev_priv, pipe);
+
+       I915_WRITE(reg, enable_mask | status_mask);
+       POSTING_READ(reg);
 }
 
-void
-i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
-                     u32 status_mask)
+void i915_disable_pipestat(struct drm_i915_private *dev_priv,
+                          enum pipe pipe, u32 status_mask)
 {
+       i915_reg_t reg = PIPESTAT(pipe);
        u32 enable_mask;
 
-       if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-               enable_mask = vlv_get_pipestat_enable_mask(&dev_priv->drm,
-                                                          status_mask);
-       else
-               enable_mask = status_mask << 16;
-       __i915_disable_pipestat(dev_priv, pipe, enable_mask, status_mask);
+       WARN_ONCE(status_mask & ~PIPESTAT_INT_STATUS_MASK,
+                 "pipe %c: status_mask=0x%x\n",
+                 pipe_name(pipe), status_mask);
+
+       lockdep_assert_held(&dev_priv->irq_lock);
+       WARN_ON(!intel_irqs_enabled(dev_priv));
+
+       if ((dev_priv->pipestat_irq_mask[pipe] & status_mask) == 0)
+               return;
+
+       dev_priv->pipestat_irq_mask[pipe] &= ~status_mask;
+       enable_mask = i915_pipestat_enable_mask(dev_priv, pipe);
+
+       I915_WRITE(reg, enable_mask | status_mask);
+       POSTING_READ(reg);
 }
 
 /**
@@ -1775,7 +1753,7 @@ static void i9xx_pipestat_irq_ack(struct drm_i915_private *dev_priv,
 
        for_each_pipe(dev_priv, pipe) {
                i915_reg_t reg;
-               u32 mask, iir_bit = 0;
+               u32 status_mask, enable_mask, iir_bit = 0;
 
                /*
                 * PIPESTAT bits get signalled even when the interrupt is
@@ -1786,7 +1764,7 @@ static void i9xx_pipestat_irq_ack(struct drm_i915_private *dev_priv,
                 */
 
                /* fifo underruns are filterered in the underrun handler. */
-               mask = PIPE_FIFO_UNDERRUN_STATUS;
+               status_mask = PIPE_FIFO_UNDERRUN_STATUS;
 
                switch (pipe) {
                case PIPE_A:
@@ -1800,21 +1778,20 @@ static void i9xx_pipestat_irq_ack(struct drm_i915_private *dev_priv,
                        break;
                }
                if (iir & iir_bit)
-                       mask |= dev_priv->pipestat_irq_mask[pipe];
+                       status_mask |= dev_priv->pipestat_irq_mask[pipe];
 
-               if (!mask)
+               if (!status_mask)
                        continue;
 
                reg = PIPESTAT(pipe);
-               mask |= PIPESTAT_INT_ENABLE_MASK;
-               pipe_stats[pipe] = I915_READ(reg) & mask;
+               pipe_stats[pipe] = I915_READ(reg) & status_mask;
+               enable_mask = i915_pipestat_enable_mask(dev_priv, pipe);
 
                /*
                 * Clear the PIPE*STAT regs before the IIR
                 */
-               if (pipe_stats[pipe] & (PIPE_FIFO_UNDERRUN_STATUS |
-                                       PIPESTAT_INT_STATUS_MASK))
-                       I915_WRITE(reg, pipe_stats[pipe]);
+               if (pipe_stats[pipe])
+                       I915_WRITE(reg, enable_mask | pipe_stats[pipe]);
        }
        spin_unlock(&dev_priv->irq_lock);
 }

diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
index 04689600e337ccd8d27b3eec244925e5715000e2..77c123cc88179e7f0ac3ef40af6d75fd34f59583 100644 (file)
--- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
+++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
@@ -88,14 +88,15 @@ static void i9xx_check_fifo_underruns(struct intel_crtc *crtc)
 {
        struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
        i915_reg_t reg = PIPESTAT(crtc->pipe);
-       u32 pipestat = I915_READ(reg) & 0xffff0000;
+       u32 enable_mask;
 
        lockdep_assert_held(&dev_priv->irq_lock);
 
-       if ((pipestat & PIPE_FIFO_UNDERRUN_STATUS) == 0)
+       if ((I915_READ(reg) & PIPE_FIFO_UNDERRUN_STATUS) == 0)
                return;
 
-       I915_WRITE(reg, pipestat | PIPE_FIFO_UNDERRUN_STATUS);
+       enable_mask = i915_pipestat_enable_mask(dev_priv, crtc->pipe);
+       I915_WRITE(reg, enable_mask | PIPE_FIFO_UNDERRUN_STATUS);
        POSTING_READ(reg);
 
        trace_intel_cpu_fifo_underrun(dev_priv, crtc->pipe);
@@ -108,15 +109,16 @@ static void i9xx_set_fifo_underrun_reporting(struct drm_device *dev,
 {
        struct drm_i915_private *dev_priv = to_i915(dev);
        i915_reg_t reg = PIPESTAT(pipe);
-       u32 pipestat = I915_READ(reg) & 0xffff0000;
 
        lockdep_assert_held(&dev_priv->irq_lock);
 
        if (enable) {
-               I915_WRITE(reg, pipestat | PIPE_FIFO_UNDERRUN_STATUS);
+               u32 enable_mask = i915_pipestat_enable_mask(dev_priv, pipe);
+
+               I915_WRITE(reg, enable_mask | PIPE_FIFO_UNDERRUN_STATUS);
                POSTING_READ(reg);
        } else {
-               if (old && pipestat & PIPE_FIFO_UNDERRUN_STATUS)
+               if (old && I915_READ(reg) & PIPE_FIFO_UNDERRUN_STATUS)
                        DRM_ERROR("pipe %c underrun\n", pipe_name(pipe));
        }
 }