2014/1/17 <ville.syrj...@linux.intel.com>: > From: Ville Syrjälä <ville.syrj...@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.zan...@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrj...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx