Re: [PATCH 1/3] drm/i915: Try to shut up more ILK underruns

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 01, 2016 at 09:53:17PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> Take a bigger hammer to the underrun suppression on ILK. Instead of
> trying to suppress them at specific points in the modeset sequence just
> silence them across the entire sequence. This gets rid of some underruns
> at least on my ILK. Note that this changes SNB and IVB to follow the
> same approach just to keep the code less convoluted. The difference is
> that on those platforms we won't suppress CPU underruns for port A since
> it doesn't seem to be necessary.
> 
> My ILK has port A eDP and two PCH HDMI ports, so I can't be sure this is
> as effective on other PCH port types. Perhaps we still need some of
> Daniel's extra vblank waits [2]?
> 
> I've still been able to trigger an underrun on the other pipe, but
> fixing that perhaps needs the LP1+ disable trick I implemented here [1]
> which never got merged.
> 
> A few details which hamper stress testing on my ILK are that sometimes
> the PCH transcoder gets messed up and refuses to shut down, and sometimes
> even the panel power sequencer apparently gets stuck on the always on
> position.
> 
> [1] https://lists.freedesktop.org/archives/intel-gfx/2014-March/041317.html
> [2] https://lists.freedesktop.org/archives/intel-gfx/2016-January/086397.html
> 
> v2: Add a note that we also get underruns when enabling PCH ports
> 
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> (v1)

I've not been able to find any additional ILK hardware to test this on but LGTM

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@xxxxxxxxxxxxxxx>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 45 ++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_dp.c      | 12 ----------
>  2 files changed, 23 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e6b5ee51739b..8d2c547b57ee 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4083,12 +4083,6 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
>  	I915_WRITE(FDI_RX_TUSIZE1(pipe),
>  		   I915_READ(PIPE_DATA_M1(pipe)) & TU_SIZE_MASK);
>  
> -	/*
> -	 * Sometimes spurious CPU pipe underruns happen during FDI
> -	 * training, at least with VGA+HDMI cloning. Suppress them.
> -	 */
> -	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
> -
>  	/* For PCH output, training FDI link */
>  	dev_priv->display.fdi_link_train(crtc);
>  
> @@ -4123,8 +4117,6 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
>  
>  	intel_fdi_normal_train(crtc);
>  
> -	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> -
>  	/* For PCH DP, enable TRANS_DP_CTL */
>  	if (HAS_PCH_CPT(dev) && intel_crtc->config->has_dp_encoder) {
>  		const struct drm_display_mode *adjusted_mode =
> @@ -4727,6 +4719,18 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  	if (WARN_ON(intel_crtc->active))
>  		return;
>  
> +	/*
> +	 * Sometimes spurious CPU pipe underruns happen during FDI
> +	 * training, at least with VGA+HDMI cloning. Suppress them.
> +	 *
> +	 * On ILK we get an occasional spurious CPU pipe underruns
> +	 * between eDP port A enable and vdd enable. Also PCH port
> +	 * enable seems to result in the occasional CPU pipe underrun.
> +	 *
> +	 * Spurious PCH underruns also occur during PCH enabling.
> +	 */
> +	if (intel_crtc->config->has_pch_encoder || IS_GEN5(dev_priv))
> +		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
>  	if (intel_crtc->config->has_pch_encoder)
>  		intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
>  
> @@ -4748,8 +4752,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  
>  	intel_crtc->active = true;
>  
> -	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> -
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
>  		if (encoder->pre_enable)
>  			encoder->pre_enable(encoder);
> @@ -4791,6 +4793,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  	/* Must wait for vblank to avoid spurious PCH FIFO underruns */
>  	if (intel_crtc->config->has_pch_encoder)
>  		intel_wait_for_vblank(dev, pipe);
> +	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
>  	intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
>  }
>  
> @@ -4943,8 +4946,15 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>  	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
>  
> -	if (intel_crtc->config->has_pch_encoder)
> +	/*
> +	 * Sometimes spurious CPU pipe underruns happen when the
> +	 * pipe is already disabled, but FDI RX/TX is still enabled.
> +	 * Happens at least with VGA+HDMI cloning. Suppress them.
> +	 */
> +	if (intel_crtc->config->has_pch_encoder) {
> +		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
>  		intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
> +	}
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
>  		encoder->disable(encoder);
> @@ -4952,22 +4962,12 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>  	drm_crtc_vblank_off(crtc);
>  	assert_vblank_disabled(crtc);
>  
> -	/*
> -	 * Sometimes spurious CPU pipe underruns happen when the
> -	 * pipe is already disabled, but FDI RX/TX is still enabled.
> -	 * Happens at least with VGA+HDMI cloning. Suppress them.
> -	 */
> -	if (intel_crtc->config->has_pch_encoder)
> -		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
> -
>  	intel_disable_pipe(intel_crtc);
>  
>  	ironlake_pfit_disable(intel_crtc, false);
>  
> -	if (intel_crtc->config->has_pch_encoder) {
> +	if (intel_crtc->config->has_pch_encoder)
>  		ironlake_fdi_disable(crtc);
> -		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> -	}
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
>  		if (encoder->post_disable)
> @@ -4997,6 +4997,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>  		ironlake_fdi_pll_disable(intel_crtc);
>  	}
>  
> +	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
>  	intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index da0c3d29fda8..95fe01d55bce 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2641,15 +2641,6 @@ static void intel_enable_dp(struct intel_encoder *encoder)
>  	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
>  		vlv_init_panel_power_sequencer(intel_dp);
>  
> -	/*
> -	 * We get an occasional spurious underrun between the port
> -	 * enable and vdd enable, when enabling port A eDP.
> -	 *
> -	 * FIXME: Not sure if this applies to (PCH) port D eDP as well
> -	 */
> -	if (port == PORT_A)
> -		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
> -
>  	intel_dp_enable_port(intel_dp);
>  
>  	if (port == PORT_A && IS_GEN5(dev_priv)) {
> @@ -2667,9 +2658,6 @@ static void intel_enable_dp(struct intel_encoder *encoder)
>  	edp_panel_on(intel_dp);
>  	edp_panel_vdd_off(intel_dp, true);
>  
> -	if (port == PORT_A)
> -		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> -
>  	pps_unlock(intel_dp);
>  
>  	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux