[Intel-gfx] [PATCH 1/3] drm/i915: Limit FIFO underrun reports on GMCH platforms

Paulo Zanoni przanoni at gmail.com
Fri Jan 24 19:37:34 CET 2014


2014/1/17  <ville.syrjala at linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> 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 at intel.com>


> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni



More information about the Intel-gfx mailing list