Re: [PATCH] drm/i915: More cautious with pch fifo underruns

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

On Wed, Nov 26, 2014 at 01:37:07PM -0200, Paulo Zanoni wrote:
> 2014-11-24 14:02 GMT-02:00 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > Apparently PCH fifo underruns are tricky, we have plenty reports that
> > we see the occasional underrun (especially at boot-up).
> >
> > So for a change let's see what happens when we don't re-enable pch
> > fifo underrun reporting when the pipe is disabled.
> 
> Does that mean you don't really know if this patch is going to fix something?
> 
> I see what this patch does, but I don't really see what is its
> benefit, besides "we'll get less bug reports". Is there any reason why
> the underruns are expected to happen at this time?

IIRC the conclusion I came to when looking at my IVB was that the underruns
were expected when fdi was disabled but the pch transcoder was enabled,
or maybe it was between pipe disable and pch transcoder disable. Either
or both would make sense to me.

But we already have the underrun reporting disabled around those parts,
so my memory might be failing me here. Also my IVB doesn't seem to
trigger these anymore. Not sure what exactly has changed.

Or maybe I used to get them in the crtc_enable path. We do enable the
underrun reporting already before fdi or pch transcoder are enabled.
Although we do the enable in order, so if my memory about the conditions
is correct we shouldn't get any there either.

> 
> Please explain a little more.
> 
> > This means that the
> > kernel can't catch pch fifo underruns when they happen (except when
> > all pipes are on on the pch). But we'll still catch underruns when
> > disabling the pipe again.
> 
> Are you sure the sentences above are correct?
> 
> 
> > So not a terrible reduction in test
> > coverage.
> 
> Yeah, I agree, but please provide a nice reason for it :)
> 
> >
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=85898
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=86233
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=86478
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 320bf4c78c8c..a4106049e158 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4501,7 +4501,6 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
> >                 ironlake_fdi_disable(crtc);
> >
> >                 ironlake_disable_pch_transcoder(dev_priv, pipe);
> > -               intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
> >
> >                 if (HAS_PCH_CPT(dev)) {
> >                         /* disable TRANS_DP_CTL */
> > @@ -4572,8 +4571,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> >
> >         if (intel_crtc->config.has_pch_encoder) {
> >                 lpt_disable_pch_transcoder(dev_priv);
> > -               intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > -                                                     true);
> >                 intel_ddi_fdi_disable(crtc);
> >         }
> >
> > --
> > 2.1.1
> >
> > _______________________________________________
> > 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

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