2014/1/17 <ville.syrjala@xxxxxxxxxxxxxxx>:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Currently we print all pipe underruns on GMCH platforms. Hook up the
> same logic we use on PCH platforms where we disable the underrun
> reporting after the first underrun.
>
> Underruns don't actually generate interrupts themselves on GMCH
> platforms, we just can detect them whenever we service other
> interrupts. So we don't have any enable bits to worry about. We just
> need to remember to clear the underrun status when enabling underrun
> reporting.
>
> Note that the underrun handling needs to be moved to the non-locked
> pipe_stats[] loop in the interrupt handlers to avoid having to rework
> the locking in intel_set_cpu_fifo_underrun_reporting().
>
It all looks sane and according to the Gen 4 spec I have, but I can't
check stuff like "where's the best place to call the funcs at crtc
enable/disable?", so I'll just trust you here.
You would have got extra points if you defined a nice macro instead of
continuing to use the hardcoded 0x8000ffff and 0x7fff0000 values :)
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 51 +++++++++++++++++++++++-------------
> drivers/gpu/drm/i915/intel_display.c | 3 +++
> 2 files changed, 36 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index b9b3bde..e5cba0b 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -232,6 +232,18 @@ static bool cpt_can_enable_serr_int(struct drm_device *dev)
> return true;
> }
>
> +static void i9xx_clear_fifo_underrun(struct drm_device *dev, enum pipe pipe)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u32 reg = PIPESTAT(pipe);
> + u32 pipestat = I915_READ(reg) & 0x7fff0000;
> +
> + assert_spin_locked(&dev_priv->irq_lock);
> +
> + I915_WRITE(reg, pipestat | PIPE_FIFO_UNDERRUN_STATUS);
> + POSTING_READ(reg);
> +}
> +
> static void ironlake_set_fifo_underrun_reporting(struct drm_device *dev,
> enum pipe pipe, bool enable)
> {
> @@ -393,7 +405,9 @@ bool intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
>
> intel_crtc->cpu_fifo_underrun_disabled = !enable;
>
> - if (IS_GEN5(dev) || IS_GEN6(dev))
> + if (enable && (INTEL_INFO(dev)->gen < 5 || IS_VALLEYVIEW(dev)))
> + i9xx_clear_fifo_underrun(dev, pipe);
> + else if (IS_GEN5(dev) || IS_GEN6(dev))
> ironlake_set_fifo_underrun_reporting(dev, pipe, enable);
> else if (IS_GEN7(dev))
> ivybridge_set_fifo_underrun_reporting(dev, pipe, enable);
> @@ -1439,12 +1453,8 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
> /*
> * Clear the PIPE*STAT regs before the IIR
> */
> - if (pipe_stats[pipe] & 0x8000ffff) {
> - if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS)
> - DRM_DEBUG_DRIVER("pipe %c underrun\n",
> - pipe_name(pipe));
> + if (pipe_stats[pipe] & 0x8000ffff)
> I915_WRITE(reg, pipe_stats[pipe]);
> - }
> }
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>
> @@ -1459,6 +1469,10 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>
> if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
> i9xx_pipe_crc_irq_handler(dev, pipe);
> +
> + if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS &&
> + intel_set_cpu_fifo_underrun_reporting(dev, pipe, false))
> + DRM_DEBUG_DRIVER("pipe %c underrun\n", pipe_name(pipe));
> }
>
> /* Consume port. Then clear IIR or we'll miss events */
> @@ -3188,12 +3202,8 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
> /*
> * Clear the PIPE*STAT regs before the IIR
> */
> - if (pipe_stats[pipe] & 0x8000ffff) {
> - if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS)
> - DRM_DEBUG_DRIVER("pipe %c underrun\n",
> - pipe_name(pipe));
> + if (pipe_stats[pipe] & 0x8000ffff)
> I915_WRITE(reg, pipe_stats[pipe]);
> - }
> }
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>
> @@ -3216,6 +3226,10 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
>
> if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
> i9xx_pipe_crc_irq_handler(dev, pipe);
> +
> + if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS &&
> + intel_set_cpu_fifo_underrun_reporting(dev, pipe, false))
> + DRM_DEBUG_DRIVER("pipe %c underrun\n", pipe_name(pipe));
> }
>
> iir = new_iir;
> @@ -3369,9 +3383,6 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
>
> /* Clear the PIPE*STAT regs before the IIR */
> if (pipe_stats[pipe] & 0x8000ffff) {
> - if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS)
> - DRM_DEBUG_DRIVER("pipe %c underrun\n",
> - pipe_name(pipe));
> I915_WRITE(reg, pipe_stats[pipe]);
> irq_received = true;
> }
> @@ -3416,6 +3427,10 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
>
> if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
> i9xx_pipe_crc_irq_handler(dev, pipe);
> +
> + if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS &&
> + intel_set_cpu_fifo_underrun_reporting(dev, pipe, false))
> + DRM_DEBUG_DRIVER("pipe %c underrun\n", pipe_name(pipe));
> }
>
> if (blc_event || (iir & I915_ASLE_INTERRUPT))
> @@ -3610,9 +3625,6 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
> * Clear the PIPE*STAT regs before the IIR
> */
> if (pipe_stats[pipe] & 0x8000ffff) {
> - if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS)
> - DRM_DEBUG_DRIVER("pipe %c underrun\n",
> - pipe_name(pipe));
> I915_WRITE(reg, pipe_stats[pipe]);
> irq_received = true;
> }
> @@ -3663,8 +3675,11 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
>
> if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
> i9xx_pipe_crc_irq_handler(dev, pipe);
> - }
>
> + if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS &&
> + intel_set_cpu_fifo_underrun_reporting(dev, pipe, false))
> + DRM_DEBUG_DRIVER("pipe %c underrun\n", pipe_name(pipe));
> + }
>
> if (blc_event || (iir & I915_ASLE_INTERRUPT))
> intel_opregion_asle_intr(dev);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dde98020..6369bd7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4146,6 +4146,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>
> intel_update_watermarks(crtc);
> intel_enable_pipe(dev_priv, pipe, false, is_dsi);
> + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> intel_enable_primary_plane(dev_priv, plane, pipe);
> intel_enable_planes(crtc);
> intel_crtc_update_cursor(crtc, true);
> @@ -4184,6 +4185,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>
> intel_update_watermarks(crtc);
> intel_enable_pipe(dev_priv, pipe, false, false);
> + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> intel_enable_primary_plane(dev_priv, plane, pipe);
> intel_enable_planes(crtc);
> /* The fixup needs to happen before cursor is enabled */
> @@ -4242,6 +4244,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
> intel_disable_planes(crtc);
> intel_disable_primary_plane(dev_priv, plane, pipe);
>
> + intel_set_cpu_fifo_underrun_reporting(dev, pipe, false);
> intel_disable_pipe(dev_priv, pipe);
>
> i9xx_pfit_disable(intel_crtc);
> --
> 1.8.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
