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? > > So if I adjust the patch to reflect that (and upgrade the relevant bz link > from References: to Bugzilla: plus add the tested by) are you ok with > merging this one?. I still fail to understand how this patch is an improvement to the code base. > > Cheers, Daniel > > >> >> > >> > 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@xxxxxxxxx> >> > --- >> > 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@xxxxxxxxxxxxxxxxxxxxx >> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> >> >> -- >> Paulo Zanoni > > -- > 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
