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>. > > So no we're not just delaying them since the time when they're block is > now _much_ longer (the entire time when the pch transcoder is off). > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
