| Message ID | 1446146763-31821-6-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
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>
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); 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)