Re: [PATCH 05/14] drm/i915: Re-enable PCH FIO underrun reporting after pipe has been disabled

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Oct 29, 2015 at 12:36:34PM -0700, Jesse Barnes wrote:
> On 10/29/2015 12:25 PM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Some hardware (IVB/HSW and CPT/PPT) have a shared error interrupt for
> > all the relevant underrun bits, so in order to keep the error interrupt
> > enabled, we need to have underrun reporting enabled on all PCH
> > transocders. Currently we leave the underrun reporting disabled when
> > the pipe is off, which means we won't get any underrun interrupts
> > when only a subset of the pipes are active.
> > 
> > Fix the problem by re-enabling the underrun reporting after the pipe has
> > been disabled. And to avoid the spurious underruns during pipe enable,
> > disable the underrun reporting before embarking on the pipe enable
> > sequence. So this way we have the error reporting disabled while
> > running through the modeset sequence.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 4fc3d24..c7cd9f7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4857,6 +4857,9 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> >  		return;
> >  
> >  	if (intel_crtc->config->has_pch_encoder)
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
> > +
> > +	if (intel_crtc->config->has_pch_encoder)
> >  		intel_prepare_shared_dpll(intel_crtc);
> 
> I guess these could be combined under the conditional, but no biggie.

I did notice the same thing just before sending the patch, but then I
convinced myself that having all the pch underrun enable/disable calls
as clearly separate steps is perhaps nicer, and so didn't bother
changing it.

> 
> >  
> >  	if (intel_crtc->config->has_dp_encoder)
> > @@ -4939,6 +4942,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> >  	if (WARN_ON(intel_crtc->active))
> >  		return;
> >  
> > +	if (intel_crtc->config->has_pch_encoder)
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > +						      false);
> > +
> >  	if (intel_crtc_to_shared_dpll(intel_crtc))
> >  		intel_enable_shared_dpll(intel_crtc);
> >  
> > @@ -5086,6 +5093,9 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
> >  
> >  		ironlake_fdi_pll_disable(intel_crtc);
> >  	}
> > +
> > +	if (intel_crtc->config->has_pch_encoder)
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
> >  }
> >  
> >  static void haswell_crtc_disable(struct drm_crtc *crtc)
> > @@ -5133,6 +5143,10 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> >  	for_each_encoder_on_crtc(dev, crtc, encoder)
> >  		if (encoder->post_disable)
> >  			encoder->post_disable(encoder);
> > +
> > +	if (intel_crtc->config->has_pch_encoder)
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > +						      true);
> >  }
> >  
> >  static void i9xx_pfit_enable(struct intel_crtc *crtc)
> > 
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx