Re: [PATCH 03/14] drm/i915: Enable PCH FIFO underruns later on ILK/SNB/IVB

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Oct 29, 2015 at 05:57:57PM -0200, Paulo Zanoni wrote:
> 2015-10-29 17:25 GMT-02:00  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We get spurious PCH FIFO underruns if we enable the reporting too soon
> > after enabling the crtc. Move it to be the last step, after the encoder
> > enable. Additionally we need an extra vblank wait, otherwise we still
> > get the underruns. Presumably the pipe/fdi isn't yet fully up and running
> > otherwise.
> >
> > For symmetry, disable the PCH underrun reporting as the first thing,
> > just before encoder disable, when shutting down the crtc.
> 
> Is there any place that describes where/when a FIFO underrun is
> expected and where/when one is an actual problem that needs to be
> solved? How do we know the underruns avoided by these patch are not a
> signal of real bugs?

Can't be 100% sure since its not documented anywhere. But we've been
getting these since forever now and stuff still works (more or less at
least), so I'm inclined to say we don't have to care about them. Also
in these case we only get PCH FIFO underruns and no CPU pipe
underruns, so I'm tempted to say it's not that serious.

IIRC I once tracked some of these down to having the FDI PLL enabled
with FDI RX/TX disabled. Or something like that, but don't quote me
on that since my memory might be failing here. Obviously that can't
explain it all since I still need the vblank wait to eliminate them.
Anyway, this time I didn't try to narrow it down too much. Instead
my aim was more to reliably eliminate them without permamently disabling
the underrun detection.

In any case, we can't get the bat stuff really working until we get
the results to be stable, and these underruns are one big obstacle
to that.

> 
> The fact that we don't get these 100% of the time suggests that maybe
> they're avoidable somehow.

It might be just about timing and/or the port type. Feel free to dig
into it more if you want, I won't. I think I already wasted enough
time poking at my ILK in an effort to figure out the CPU eDP underruns.

> 
> Perhaps you could update the commit message with the info.
> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 99fb33f..d5cb899 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4874,7 +4874,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> >         intel_crtc->active = true;
> >
> >         intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> > -       intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
> >
> >         for_each_encoder_on_crtc(dev, crtc, encoder)
> >                 if (encoder->pre_enable)
> > @@ -4912,6 +4911,12 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> >
> >         if (HAS_PCH_CPT(dev))
> >                 cpt_verify_modeset(dev, intel_crtc->pipe);
> > +
> > +       if (intel_crtc->config->has_pch_encoder) {
> > +               /* Must wait for vblank to avoid spurious PCH FIFO underruns */
> > +               intel_wait_for_vblank(dev, pipe);
> > +               intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
> > +       }
> >  }
> >
> >  /* IPS only exists on ULT machines and is tied to pipe A. */
> > @@ -5040,15 +5045,15 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
> >         int pipe = intel_crtc->pipe;
> >         u32 reg, temp;
> >
> > +       if (intel_crtc->config->has_pch_encoder)
> > +               intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
> > +
> >         for_each_encoder_on_crtc(dev, crtc, encoder)
> >                 encoder->disable(encoder);
> >
> >         drm_crtc_vblank_off(crtc);
> >         assert_vblank_disabled(crtc);
> >
> > -       if (intel_crtc->config->has_pch_encoder)
> > -               intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
> > -
> >         intel_disable_pipe(intel_crtc);
> >
> >         ironlake_pfit_disable(intel_crtc, false);
> > --
> > 2.4.10
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx