From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 37/40] drm/i915: Fix eDP link training when switching pipes Date: Tue, 29 Jul 2014 20:06:57 +0200 Message-ID: <20140729180657.GX4747@phenom.ffwll.local> References: <1403910271-24984-1-git-send-email-ville.syrjala@linux.intel.com> <1403910271-24984-38-git-send-email-ville.syrjala@linux.intel.com> <20140630145212.32279203@jbarnes-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-wg0-f51.google.com (mail-wg0-f51.google.com [74.125.82.51]) by gabe.freedesktop.org (Postfix) with ESMTP id 4315389A14 for ; Tue, 29 Jul 2014 11:06:49 -0700 (PDT) Received: by mail-wg0-f51.google.com with SMTP id b13so27568wgh.10 for ; Tue, 29 Jul 2014 11:06:48 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140630145212.32279203@jbarnes-desktop> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Jun 30, 2014 at 02:52:12PM -0700, Jesse Barnes wrote: > On Sat, 28 Jun 2014 02:04:28 +0300 > ville.syrjala@linux.intel.com wrote: > = > > From: Ville Syrj=E4l=E4 > > = > > When switching from one pipe to another, the power sequencer of the new > > pipe seems to need a bit of kicking to lock into the port. Even the vdd > > force bit doesn't work before the power sequencer has been sufficiently > > kicked, so this must be done even before any AUX transactions. > > = > > This sequence has been found to do the trick: > > 1) enable port with idle pattern > > 2) enable the power sequencer > > 3) proceed with link training > > = > > Signed-off-by: Ville Syrj=E4l=E4 > > --- > > drivers/gpu/drm/i915/intel_dp.c | 34 ++++++++++++++++++++++++++++++++-- > > 1 file changed, 32 insertions(+), 2 deletions(-) > > = > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/int= el_dp.c > > index 65ab54c..07b0320 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -2010,6 +2010,37 @@ static void chv_post_disable_dp(struct intel_enc= oder *encoder) > > mutex_unlock(&dev_priv->dpio_lock); > > } > > = > > +static void intel_edp_init_train(struct intel_dp *intel_dp) > > +{ > > + struct intel_digital_port *intel_dig_port =3D dp_to_dig_port(intel_dp= ); > > + struct drm_device *dev =3D intel_dig_port->base.base.dev; > > + struct drm_i915_private *dev_priv =3D dev->dev_private; > > + > > + if (!is_edp(intel_dp)) > > + return; This changes the order of events as observed by the sink, so I really wonder why this is edp specific? We do have bug reports about external DP monitors not waking up from the sink_dpms call properly ... -Daniel > > + > > + /* > > + * Need to enable the port with idle pattern to allow the power > > + * sequencer to lock into the port. Otherwise the power sequencer > > + * (including vdd force bit!) doesn't work on this port. > > + */ > > + if (IS_VALLEYVIEW(dev)) { > > + intel_dp->DP |=3D DP_PORT_EN; > > + > > + if (IS_CHERRYVIEW(dev)) > > + intel_dp->DP &=3D ~DP_LINK_TRAIN_MASK_CHV; > > + else > > + intel_dp->DP &=3D ~DP_LINK_TRAIN_MASK; > > + intel_dp->DP |=3D DP_LINK_TRAIN_PAT_IDLE; > > + > > + I915_WRITE(intel_dp->output_reg, intel_dp->DP); > > + POSTING_READ(intel_dp->output_reg); > > + } > > + > > + intel_edp_panel_on(intel_dp); > > + edp_panel_vdd_off(intel_dp, true); > > +} > > + > > static void intel_enable_dp(struct intel_encoder *encoder) > > { > > struct intel_dp *intel_dp =3D enc_to_intel_dp(&encoder->base); > > @@ -2021,10 +2052,9 @@ static void intel_enable_dp(struct intel_encoder= *encoder) > > return; > > = > > intel_edp_panel_vdd_on(intel_dp); > > + intel_edp_init_train(intel_dp); > > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > intel_dp_start_link_train(intel_dp); > > - intel_edp_panel_on(intel_dp); > > - edp_panel_vdd_off(intel_dp, true); > > intel_dp_complete_link_train(intel_dp); > > intel_dp_stop_link_train(intel_dp); > > } > = > Yeah I think this matches the doc too. I never pushed this change > because I could never find anything that it actually fixed. > = > I guess you have something now though! > = > Reviewed-by: Jesse Barnes > = > -- = > Jesse Barnes, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch