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 <tprev...@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

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

Reply via email to