Re: [PATCH 08/14] drm/i915: Disable FIFO underrun reporting around IBX transcoder B workaround

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

On Fri, Oct 30, 2015 at 12:11:45PM +0200, Jani Nikula wrote:
> On Thu, 29 Oct 2015, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Doing the IBX transcoder B workaround causes underruns on
> > pipe/transcoder A. Just hide them by disabling underrun reporting for
> > pipe A around the workaround.
> >
> > It might be possible to avoid the underruns by moving the workaround
> > to be applied only when enabling pipe A. But I was too lazy to try it
> > right now, and the current method has been proven to work, so didn't
> > want to change it too hastily.
> 
> Is it possible this enables underrun reporting on pipe A even if it
> wasn't enabled before?

Yes, it's possible. It would mean that pipe A is currently enabled, and
has already suffered an underrun (which is why the underrun reporting
got disabled). But I think that's OK. We would really want the underrun
reporting to rearm itself after a small delay anyway, but currently that
doesn't happen. I had a hacky patch for that at some point, but it would
probably need more work, and we should first fix up all the known bugs 
that can cause underruns ie. watermark code.

> 
> BR,
> Jani.
> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c   | 11 +++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h  |  9 +++++++++
> >  drivers/gpu/drm/i915/intel_hdmi.c | 11 +++++++++++
> >  drivers/gpu/drm/i915/intel_sdvo.c | 11 +++++++++++
> >  4 files changed, 42 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 8287df4..4a0fb63 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3957,6 +3957,13 @@ intel_dp_link_down(struct intel_dp *intel_dp)
> >  	 * matching HDMI port to be enabled on transcoder A.
> >  	 */
> >  	if (HAS_PCH_IBX(dev) && crtc->pipe == PIPE_B && port != PORT_A) {
> > +		/*
> > +		 * We get CPU/PCH FIFO underruns on the other pipe when
> > +		 * doing the workaround. Sweep them under the rug.
> > +		 */
> > +		intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > +
> >  		/* always enable with pattern 1 (as per spec) */
> >  		DP &= ~(DP_PIPEB_SELECT | DP_LINK_TRAIN_MASK);
> >  		DP |= DP_PORT_EN | DP_LINK_TRAIN_PAT_1;
> > @@ -3966,6 +3973,10 @@ intel_dp_link_down(struct intel_dp *intel_dp)
> >  		DP &= ~DP_PORT_EN;
> >  		I915_WRITE(intel_dp->output_reg, DP);
> >  		POSTING_READ(intel_dp->output_reg);
> > +
> > +		intel_wait_for_vblank_if_active(dev_priv->dev, PIPE_A);
> > +		intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> >  	}
> >  
> >  	msleep(intel_dp->panel_power_down_delay);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 72cc272..35f1457 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1073,6 +1073,15 @@ intel_wait_for_vblank(struct drm_device *dev, int pipe)
> >  {
> >  	drm_wait_one_vblank(dev, pipe);
> >  }
> > +static inline void
> > +intel_wait_for_vblank_if_active(struct drm_device *dev, int pipe)
> > +{
> > +	const struct intel_crtc *crtc =
> > +		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> > +
> > +	if (crtc->active)
> > +		intel_wait_for_vblank(dev, pipe);
> > +}
> >  int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp);
> >  void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
> >  			 struct intel_digital_port *dport,
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 013bd7d..bccbe70 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1108,6 +1108,13 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
> >  	 * matching DP port to be enabled on transcoder A.
> >  	 */
> >  	if (HAS_PCH_IBX(dev) && crtc->pipe == PIPE_B) {
> > +		/*
> > +		 * We get CPU/PCH FIFO underruns on the other pipe when
> > +		 * doing the workaround. Sweep them under the rug.
> > +		 */
> > +		intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > +
> >  		temp &= ~SDVO_PIPE_B_SELECT;
> >  		temp |= SDVO_ENABLE;
> >  		/*
> > @@ -1122,6 +1129,10 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
> >  		temp &= ~SDVO_ENABLE;
> >  		I915_WRITE(intel_hdmi->hdmi_reg, temp);
> >  		POSTING_READ(intel_hdmi->hdmi_reg);
> > +
> > +		intel_wait_for_vblank_if_active(dev_priv->dev, PIPE_A);
> > +		intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> >  	}
> >  
> >  	intel_hdmi->set_infoframes(&encoder->base, false, NULL);
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > index c42b636..267e6cb 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -1464,12 +1464,23 @@ static void intel_disable_sdvo(struct intel_encoder *encoder)
> >  	 * matching DP port to be enabled on transcoder A.
> >  	 */
> >  	if (HAS_PCH_IBX(dev_priv) && crtc->pipe == PIPE_B) {
> > +		/*
> > +		 * We get CPU/PCH FIFO underruns on the other pipe when
> > +		 * doing the workaround. Sweep them under the rug.
> > +		 */
> > +		intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > +
> >  		temp &= ~SDVO_PIPE_B_SELECT;
> >  		temp |= SDVO_ENABLE;
> >  		intel_sdvo_write_sdvox(intel_sdvo, temp);
> >  
> >  		temp &= ~SDVO_ENABLE;
> >  		intel_sdvo_write_sdvox(intel_sdvo, temp);
> > +
> > +		intel_wait_for_vblank_if_active(dev_priv->dev, PIPE_A);
> > +		intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> > +		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> >  	}
> >  }
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

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