2013/6/12 Daniel Vetter <daniel.vetter at ffwll.ch>: > It's racy: There's no guarantee that we won't walk this code (due to a > pch fifo underrun interrupt) while someone is changing the pointers > around. > > The only reason we do this is to use the righ crtc for the pch fifo > underrun accounting. But we never expose this to userspace, so > essentially no one really cares if we use the "wrong" crtc. > > So let's just rip it out. > > With this patch fifo underrun code will always use crtc A for tracking > underruns on the (only) pch transcoder on LPT. Please add a big comment in the source code explaining the hack and its reason. > > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch> > --- > drivers/gpu/drm/i915/i915_irq.c | 33 +++++++-------------------------- > 1 file changed, 7 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index bb26555..b98ea4e 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -193,13 +193,13 @@ static void ibx_display_interrupt_update(struct drm_i915_private *dev_priv, > POSTING_READ(SDEIMR); > } > > -static void ibx_set_fifo_underrun_reporting(struct intel_crtc *crtc, > +static void ibx_set_fifo_underrun_reporting(struct drm_device *dev, > + enum transcoder pch_transcoder, > bool enable) > { > - struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - uint32_t bit = (crtc->pipe == PIPE_A) ? SDE_TRANSA_FIFO_UNDER : > - SDE_TRANSB_FIFO_UNDER; > + uint32_t bit = (pch_transcoder == TRANSCODER_A) ? > + SDE_TRANSA_FIFO_UNDER : SDE_TRANSB_FIFO_UNDER; > > ibx_display_interrupt_update(dev_priv, bit, > enable ? bit : 0); > @@ -292,30 +292,11 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev, > bool enable) > { > struct drm_i915_private *dev_priv = dev->dev_private; > - enum pipe p; > - struct drm_crtc *crtc; > - struct intel_crtc *intel_crtc; > + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pch_transcoder]; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > unsigned long flags; > bool ret; > > - if (HAS_PCH_LPT(dev)) { > - crtc = NULL; > - for_each_pipe(p) { > - struct drm_crtc *c = dev_priv->pipe_to_crtc_mapping[p]; > - if (intel_pipe_has_type(c, INTEL_OUTPUT_ANALOG)) { > - crtc = c; > - break; > - } > - } > - if (!crtc) { > - DRM_ERROR("PCH FIFO underrun, but no CRTC using the PCH found\n"); > - return false; > - } > - } else { > - crtc = dev_priv->pipe_to_crtc_mapping[pch_transcoder]; > - } > - intel_crtc = to_intel_crtc(crtc); > - > spin_lock_irqsave(&dev_priv->irq_lock, flags); > > ret = !intel_crtc->pch_fifo_underrun_disabled; > @@ -326,7 +307,7 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev, > intel_crtc->pch_fifo_underrun_disabled = !enable; > > if (HAS_PCH_IBX(dev)) > - ibx_set_fifo_underrun_reporting(intel_crtc, enable); > + ibx_set_fifo_underrun_reporting(dev, pch_transcoder, enable); > else > cpt_set_fifo_underrun_reporting(dev, pch_transcoder, enable); > > -- > 1.8.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni