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

Jesse Barnes
2015-10-29T19:35:59Z

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.

>
> 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>