On Fri, Oct 17, 2014 at 11:41:13AM -0700, Todd Previte wrote: > Reorder the function calls in chv/vlv_pre_enable_dp() such that link training > is not initiated before the PHYs come up out of reset. Also check the status > of vlv_wait_port_ready() and only attempt to train if the PHYs are actually > running. > > The specification lists the wait for the PHYs as one of the final steps in > enabling the Displayport hardware for use. While the PHYs are in reset, no > communication is possible across the link. Attempting to train the link while > the PHYs are in reset will result in link training failure with one or more > WARN() in the logs. Moving the intel_enable_dp() function after > vlv_wait_port_ready() and only when the PHYs are ready helps ensure reliable > operation of the Displayport link. > > To comply with the specification, the call to enable_port() has been moved of > enable_dp() and placed before the wait functions for the PHYs and prior to > the call to enable_dp(). This is going to conflict with my PPS series. I have a similar patch in there, except it doesn't skip the link training. I'm not sure we should bother doing that since the wait_port_ready() problem should never ever happen as long as we do things correctly, which we should do after my series lands. > > Signed-off-by: Todd Previte <tprevite@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_display.c | 8 ++++++-- > drivers/gpu/drm/i915/intel_dp.c | 16 ++++++++-------- > drivers/gpu/drm/i915/intel_drv.h | 2 +- > 3 files changed, 15 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index c51d950..4b280c1 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1723,7 +1723,7 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe) > mutex_unlock(&dev_priv->dpio_lock); > } > > -void vlv_wait_port_ready(struct drm_i915_private *dev_priv, > +int vlv_wait_port_ready(struct drm_i915_private *dev_priv, > struct intel_digital_port *dport) > { > u32 port_mask; > @@ -1746,9 +1746,13 @@ void vlv_wait_port_ready(struct drm_i915_private *dev_priv, > BUG(); > } > > - if (wait_for((I915_READ(dpll_reg) & port_mask) == 0, 1000)) > + if (wait_for((I915_READ(dpll_reg) & port_mask) == 0, 1000)) { > WARN(1, "timed out waiting for port %c ready: 0x%08x\n", > port_name(dport->port), I915_READ(dpll_reg)); > + return -EIO; > + } > + > + return 0; > } > > static void intel_prepare_shared_dpll(struct intel_crtc *crtc) > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index a8352c4..c1ce738 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -2532,7 +2532,7 @@ static void intel_dp_enable_port(struct intel_dp *intel_dp) > POSTING_READ(intel_dp->output_reg); > } > > -static void intel_enable_dp(struct intel_encoder *encoder) > +static bool intel_enable_dp(struct intel_encoder *encoder) > { > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > struct drm_device *dev = encoder->base.dev; > @@ -2544,7 +2544,6 @@ static void intel_enable_dp(struct intel_encoder *encoder) > if (WARN_ON(dp_reg & DP_PORT_EN)) > return false; > > - intel_dp_enable_port(intel_dp); > intel_edp_panel_vdd_on(intel_dp); > intel_edp_panel_on(intel_dp); > intel_edp_panel_vdd_off(intel_dp, true); > @@ -2576,6 +2575,7 @@ static void g4x_enable_dp(struct intel_encoder *encoder) > { > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > > + intel_dp_enable_port(intel_dp); > intel_enable_dp(encoder); > intel_edp_backlight_on(intel_dp); > } > @@ -2705,9 +2705,9 @@ static void vlv_pre_enable_dp(struct intel_encoder *encoder) > pps_unlock(intel_dp); > } > > - intel_enable_dp(encoder); > - > - vlv_wait_port_ready(dev_priv, dport); > + intel_dp_enable_port(intel_dp); > + if (vlv_wait_port_ready(dev_priv, dport) == 0) > + intel_enable_dp(encoder); > } > > static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder) > @@ -2805,9 +2805,9 @@ static void chv_pre_enable_dp(struct intel_encoder *encoder) > pps_unlock(intel_dp); > } > > - intel_enable_dp(encoder); > - > - vlv_wait_port_ready(dev_priv, dport); > + intel_dp_enable_port(intel_dp); > + if (vlv_wait_port_ready(dev_priv, dport) == 0) > + intel_enable_dp(encoder); > } > > static void chv_dp_pre_pll_enable(struct intel_encoder *encoder) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index dc80444..2ff2c8c 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -876,7 +876,7 @@ intel_wait_for_vblank(struct drm_device *dev, int pipe) > drm_wait_one_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, > +int vlv_wait_port_ready(struct drm_i915_private *dev_priv, > struct intel_digital_port *dport); > bool intel_get_load_detect_pipe(struct drm_connector *connector, > struct drm_display_mode *mode, > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx