On 16 May 2014 17:40, <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Gen2 reports FIFO underruns whenever no planes are enabled on the pipe. > So in order to avoid false positives we must enable the FIFO underrun > reporting only when at least one plane is enabled on the pipe. For > now just move the underrun reporting enable/disable points to the > other side of the plane enable/disable point. That doesn't cover cases > when we turn off all the planes for the pipe but leave the pipe running > on purpose, but it's better than the current situation. > > On gen4+ we can actually move the underrun reporting enable/disable to > the opposite ends of the crtc enable/disable hooks. I suppose in theory > we could leave the underrun reporting enabled all the time, except on > VLV where PIPESTAT stops working when the display power well is down. > If we ever get around to unifying the PIPESTAT irq handling for all > gmch platforms, we should still follow the VLV route for other platforms. > It would also micro-optimize the irq handler a bit since we could then > skip the PIPESTAT reads for all disabled pipes. > > Gen3 is still a mystery, but for now I'm going to assume it behaves > like gen4+. > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> This doesn't apply to drm-intel-nightly, but looks fine in principle: Reviewed-by: Thomas Wood <thomas.wood@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_display.c | 30 +++++++++++++++++++++++++++--- > 1 file changed, 27 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 1b5164c..7f61047 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4514,6 +4514,8 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) > > intel_crtc->active = true; > > + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); > + > for_each_encoder_on_crtc(dev, crtc, encoder) > if (encoder->pre_pll_enable) > encoder->pre_pll_enable(encoder); > @@ -4537,7 +4539,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) > > intel_update_watermarks(crtc); > intel_enable_pipe(intel_crtc); > - intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); > > for_each_encoder_on_crtc(dev, crtc, encoder) > encoder->enable(encoder); > @@ -4564,6 +4565,9 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) > > intel_crtc->active = true; > > + if (!IS_GEN2(dev)) > + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); > + > for_each_encoder_on_crtc(dev, crtc, encoder) > if (encoder->pre_enable) > encoder->pre_enable(encoder); > @@ -4576,13 +4580,22 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) > > intel_update_watermarks(crtc); > intel_enable_pipe(intel_crtc); > - intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); > > for_each_encoder_on_crtc(dev, crtc, encoder) > encoder->enable(encoder); > > intel_crtc_enable_planes(crtc); > > + /* > + * Gen2 reports pipe underruns whenever all planes are disabled. > + * So don't enable underrun reporting before at least some planes > + * are enabled. > + * FIXME: Need to fix the logic to work when we turn off all planes > + * but leave the pipe running. > + */ > + if (IS_GEN2(dev)) > + intel_set_cpu_fifo_underrun_reporting(dev, pipe, true); > + > drm_vblank_on(dev, pipe); > > /* Underruns don't raise interrupts, so check manually. */ > @@ -4615,12 +4628,20 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) > if (!intel_crtc->active) > return; > > + /* > + * Gen2 reports pipe underruns whenever all planes are disabled. > + * So diasble underrun reporting before all the planes get disabled. > + * FIXME: Need to fix the logic to work when we turn off all planes > + * but leave the pipe running. > + */ > + if (IS_GEN2(dev)) > + intel_set_cpu_fifo_underrun_reporting(dev, pipe, false); > + > intel_crtc_disable_planes(crtc); > > for_each_encoder_on_crtc(dev, crtc, encoder) > encoder->disable(encoder); > > - intel_set_cpu_fifo_underrun_reporting(dev, pipe, false); > intel_disable_pipe(dev_priv, pipe); > > i9xx_pfit_disable(intel_crtc); > @@ -4638,6 +4659,9 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) > i9xx_disable_pll(dev_priv, pipe); > } > > + if (!IS_GEN2(dev)) > + intel_set_cpu_fifo_underrun_reporting(dev, pipe, false); > + > intel_crtc->active = false; > intel_update_watermarks(crtc); > > -- > 1.8.5.5 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
