Re: [PATCH 2/5] drm/i915: Fix up fifo underrun tracking, take N

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

On Fri, May 23, 2014 at 10:21:24AM +0200, Daniel Vetter wrote:
> On Fri, May 23, 2014 at 10:11 AM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >> For enabled->enabled I think that can happen in crtc_enable - we
> >> unconditionally enable underrun reporting againg to clear out old fail (or
> >> firmware setups). But we don't always disable it when disabling the crtc
> >> since some platforms/ports don't underrun when disabled.
> >
> > Hmm. Actually since we don't necessarily notice the underruns with the
> > shared error interrupt, maybe we need to also throw an explicit underrun
> > check at the end of crtc_disable. That would mean we'd catch the underrun
> > at the very latest there, and crtc_enable can then clear the bit without
> > worrying about losing valid underrun reports.
> >
> > So, I think this patch looks OK. But we will need to keep this issue in
> > mind if we add the underrun report re-enable timer, or something like it.
> > Since my quick hack for that just blindly walks the crtcs and
> > (re)enables the underrun reporting for everything.
> 
> Hm yeah, we might want a fifo underrun check like on gmch platforms.
> Otoh if we disable the shared error interrupt things are already
> rather bad, and the problem on the first pipe should have accurate
> irq-based reporting. The 2nd pipe is hidden, but should show up as
> soon as the first issue is addressed.
> 
> So I don't think we really should worry about this. Maybe more
> intersting would be to check the _other_ pipes when we re-enable fifo
> underruns. At least some of the reports we've seen suggest that
> modesets on the pch influence each another ...

But we enable underrun reporting before doing the modeset, so at that
time the modeset can't have caused underruns on any pipe. So adding the
explicit check to the end .crtc_enable() (as I did for gmch) should
catch those nicely.

-- 
Ville Syrjälä
Intel OTC