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

[Daniel Vetter]

On Mon, May 26, 2014 at 10:09 AM, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
> 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 at 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.

Hm yeah, that would indeed be a nice extension of the underrun checks
we have. Not going to volunteer for it though, since my goal here was
to make the pipe crc stuff work more reliably.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch