[Intel-gfx] [PATCH 1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB

[Ville Syrjälä]

On Thu, May 24, 2018 at 09:15:37PM +0100, Chris Wilson wrote:
> Quoting Chris Wilson (2018-05-24 21:14:23)
> > Quoting Ville Syrjala (2018-05-24 20:04:05)
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > 
> > > My ILK seems to generate a spurious PCH underrun with most interlaced
> > > HDMI modes. Add a second vblank wait to avoid it.
> > > 
> > > We have seen some spurious PCH underruns still in CI as well, some
> > > of which seem to be progressive DP. The logs also point towards some
> > > spurious underrins with progressive HDMI on SNB. While I don't have
> > > a solid explanation for those let's try to kill all the birds with one
> > > stone and always do the double wait.
> > > 
> > > Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106387
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 11 +++++++++--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 7604fbda0607..b5fa4943372a 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5528,9 +5528,16 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
> > >         if (HAS_PCH_CPT(dev_priv))
> > >                 cpt_verify_modeset(dev, intel_crtc->pipe);
> > >  
> > > -       /* Must wait for vblank to avoid spurious PCH FIFO underruns */
> > > -       if (intel_crtc->config->has_pch_encoder)
> > > +       /*
> > > +        * Must wait for vblank to avoid spurious PCH FIFO underruns.
> > > +        * And a second vblank wait is needed at least on ILK with
> > > +        * some interlaced HDMI modes. Let's do the double wait always
> > > +        * in case there are more corner cases we don't know about.
> > > +        */
> > > +       if (intel_crtc->config->has_pch_encoder) {
> > > +               intel_wait_for_vblank(dev_priv, pipe);
> > >                 intel_wait_for_vblank(dev_priv, pipe);
> > 
> > The only purpose for the double wait here is for delaying the switching
> > on of underrun reporting, right? It doesn't accidentally fix anything
> > else?
> 
> E.g. in a multi-crtc setup, that's a big delay between switching on a
> pair of pipes.

I was testing with one pipe only. So at least it's not meant to do
anything else.

-- 
Ville Syrjälä
Intel