| Message ID | 1413571273-22919-2-git-send-email-tprevite@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@gmail.com> > --- > 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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,
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(). Signed-off-by: Todd Previte <tprevite@gmail.com> --- 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(-)