On Mon, Nov 30, 2015 at 09:25:03AM +0100, Daniel Vetter wrote:
> On Thu, Nov 26, 2015 at 10:55:53PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > LPT/WPT only have transcoder A, so we shouldn't look at FIFO underruns
> > for transocoder B/C. And more importnatnly we should not consider
> > the state of underrun reporting for transcoders B/C when checking
> > whether we can enable the south error interrupt.
> >
> > The whole thing is a bit of mess since we store the underrun reporting
> > state for transcoder A under intel_crtc for pipe A, irrespective of
> > which pipe may actually be driving the transcoder. But I figured trying
> > to change that would result in more churn.
> >
> > Caveat: Still untested
> >
> > v2: Use HAS_PCH_LPT_H instead of HAS_DDI
> > Use cpt_check_pch_fifo_underruns() on LPT-H/WPT-H too
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> I've battled hsw lpt fifo underrun issues last week and never seen a fifo
> underrun on pipe B/C. Have you seen them anywhere really?
No, I've not seen it. We mark underrun detection as disabled for active
pipes in intel_sanitize_crtc(), and we never turn it back on except for
transcoder A, so I'm thinking we'll never actually enable the south
error interrup on LPT if a non-pch port is enabled on boot.
Note that so far I didn't check if that's actually the case, but based
on the code I can't come to any other conclusion.
>
> If not I think we can skip this patch here, since I think I tracked it all
> down.
> -Daniel
>
> > ---
> > drivers/gpu/drm/i915/intel_fifo_underrun.c | 27 ++++++++++++++++++++++-----
> > 1 file changed, 22 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > index bda526660e20..3d3acc8b8367 100644
> > --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > @@ -48,6 +48,14 @@
> > * The code also supports underrun detection on the PCH transcoder.
> > */
> >
> > +static bool cpt_transcoder_exists(struct drm_i915_private *dev_priv,
> > + enum transcoder pch_transcoder)
> > +{
> > + /* LPT-H/WPT-H have only transcoder A */
> > + return HAS_PCH_CPT(dev_priv) ||
> > + (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == TRANSCODER_A);
> > +}
> > +
> > static bool ivb_can_enable_err_int(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -69,13 +77,16 @@ static bool ivb_can_enable_err_int(struct drm_device *dev)
> > static bool cpt_can_enable_serr_int(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > - enum pipe pipe;
> > - struct intel_crtc *crtc;
> > + enum transcoder pch_transcoder;
> >
> > assert_spin_locked(&dev_priv->irq_lock);
> >
> > - for_each_pipe(dev_priv, pipe) {
> > - crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> > + for_each_pipe(dev_priv, pch_transcoder) {
> > + struct intel_crtc *crtc =
> > + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pch_transcoder]);
> > +
> > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder))
> > + continue;
> >
> > if (crtc->pch_fifo_underrun_disabled)
> > return false;
> > @@ -206,6 +217,9 @@ static void cpt_check_pch_fifo_underruns(struct intel_crtc *crtc)
> >
> > assert_spin_locked(&dev_priv->irq_lock);
> >
> > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder))
> > + return;
> > +
> > if ((serr_int & SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)) == 0)
> > return;
> >
> > @@ -222,6 +236,9 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev,
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder))
> > + return;
> > +
> > if (enable) {
> > I915_WRITE(SERR_INT,
> > SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder));
> > @@ -436,7 +453,7 @@ void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv)
> > if (crtc->pch_fifo_underrun_disabled)
> > continue;
> >
> > - if (HAS_PCH_CPT(dev_priv))
> > + if (HAS_PCH_CPT(dev_priv) || HAS_PCH_LPT_H(dev_priv))
> > cpt_check_pch_fifo_underruns(crtc);
> > }
> >
> > --
> > 2.4.10
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
