From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/3] drm/i915: Change order of operations for VLV/CHV to not train DP link before PHYs are ready Date: Wed, 22 Oct 2014 10:17:07 +0300 Message-ID: <20141022071707.GQ4284@intel.com> References: <87lhoos63a.fsf@intel.com> <1413571273-22919-1-git-send-email-tprevite@gmail.com> <1413571273-22919-2-git-send-email-tprevite@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 4120D6E261 for ; Wed, 22 Oct 2014 00:17:27 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1413571273-22919-2-git-send-email-tprevite@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Todd Previte Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org 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 trai= ning > is not initiated before the PHYs come up out of reset. Also check the sta= tus > of vlv_wait_port_ready() and only attempt to train if the PHYs are actual= ly > 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 w= hile > the PHYs are in reset will result in link training failure with one or mo= re > WARN() in the logs. Moving the intel_enable_dp() function after > vlv_wait_port_ready() and only when the PHYs are ready helps ensure relia= ble > operation of the Displayport link. > = > To comply with the specification, the call to enable_port() has been move= d 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 > --- > 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) =3D=3D 0, 1000)) > + if (wait_for((I915_READ(dpll_reg) & port_mask) =3D=3D 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 *i= ntel_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 =3D enc_to_intel_dp(&encoder->base); > struct drm_device *dev =3D encoder->base.dev; > @@ -2544,7 +2544,6 @@ static void intel_enable_dp(struct intel_encoder *e= ncoder) > 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 *enc= oder) > { > struct intel_dp *intel_dp =3D 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) =3D=3D 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) =3D=3D 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/inte= l_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 pip= e) > 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=E4l=E4 Intel OTC