On Mon, 01 Dec 2014, Daniel Vetter <daniel@xxxxxxxx> wrote: > On Mon, Dec 01, 2014 at 03:04:50PM -0200, Paulo Zanoni wrote: >> 2014-12-01 14:36 GMT-02:00 Daniel Vetter <daniel@xxxxxxxx>: >> > On Mon, Dec 01, 2014 at 11:41:42AM -0200, Paulo Zanoni wrote: >> >> 2014-11-26 16:17 GMT-02:00 Daniel Vetter <daniel@xxxxxxxx>: >> >> > 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@xxxxxxxx>: >> >> >> > 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? >> >> >> >> >> >> Please explain a little more. >> >> > >> >> > No reason really beyond "less bug reports" and "no reduction in underrun >> >> > reporting abilities when the pipe is actually enabled". Only a reduction >> >> > in how quickly we'll notice an underrun, but since we mostly need cpu fifo >> >> > underruns for debugging wm issues I don't think that has an impact for >> >> > developers either. fifo underruns are useful for debugging some modeset >> >> > issues, but as soon as you do modeset we'll spot the underrun. >> >> > >> >> >> > 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? >> >> > >> >> > We always re-enable underrun reporting in the crtc_enable hooks. That >> >> > still doesn't enable the interrupts (when some other pch pipe is off), but >> >> > it updates the sw tracking. >> >> > >> >> > When we again disable the fifo underrun reporting we do check the status >> >> > bits, so if an underrun happened we won't get the interrupt right away. >> >> > But when you shut down the pipe we'll notice that an interrupt happened. >> >> > >> >> > So yeah, the above claim should be correct. >> >> > >> >> >> > So not a terrible reduction in test >> >> >> > coverage. >> >> >> >> >> >> Yeah, I agree, but please provide a nice reason for it :) >> >> > >> >> > See my reply to this patch, a bug reporter came around and tested this as >> >> > "it works". I really do send out patches without testing them at all for >> >> > bug team work ;-) >> >> >> >> But why does he say it works? Aren't we just delaying the DRM_ERROR message? >> > >> > Before we only disabled pch underruns while we disable the pch. But at the >> > end of the ->crtc_disable hook pch underrun reporting is enabled. >> > >> > With my patch we keep pch underrun reporting disabled until ->crtc_enable. >> > It seems like doing a modeset on the other pipe also gives us underruns on >> > disabled pipes somehow. Or at least that's my (bad) theory. >> >> I guess you convinced me on IRC that this is better than reverting the >> DRM_ERROR to DRM_DEBUG_KMS. >> >> Anyway, the patch does what it says and doesn't seem to add any >> regressions, so Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>. > > Since this dmesg noise is a regression, also Cc: stable@xxxxxxxxxxxxxxx > > Jani, can you pls pick this patch up? Perhaps for the record it would be > best to paste the entire discussion here into the commit log, too. Pushed to drm-intel-fixes, thanks for the patch and review. BR, Jani. > > Thanks, Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
