[Intel-gfx] [PATCH 1/4] drm/i915/display/hsw+: Do not program the same vswing entry twice

[Ville Syrjälä]

On Fri, May 29, 2020 at 08:52:38PM +0000, Souza, Jose wrote:
> On Fri, 2020-05-29 at 09:51 +0300, Ville Syrjälä wrote:
> > On Thu, May 28, 2020 at 01:03:53PM -0700, José Roberto de Souza wrote:
> > > It will be programed right before the link training, so no need to do
> > > it twice.
> > > It will not strictly follow BSpec sequences but most of this sequences
> > > are not matching anyways.
> > > 
> > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_ddi.c | 19 ++++---------------
> > >  1 file changed, 4 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index aa22465bb56e..c100efc6a2c4 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -3115,7 +3115,6 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
> > >  	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> > >  	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> > >  	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
> > > -	int level = intel_ddi_dp_level(intel_dp);
> > >  	enum transcoder transcoder = crtc_state->cpu_transcoder;
> > >  
> > >  	intel_dp_set_link_params(intel_dp, crtc_state->port_clock,
> > > @@ -3190,9 +3189,10 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
> > >  	 * down this function.
> > >  	 */
> > >  
> > > -	/* 7.e Configure voltage swing and related IO settings */
> > > -	tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock, level,
> > > -				encoder->type);
> > > +	/*
> > > +	 * 7.e Configure voltage swing and related IO settings
> > > +	 * It will be done in intel_dp_start_link_train(), no need to do twice
> > > +	 */
> > 
> > Hmm. Do we still set it up before turning on the port?
> 
> No.
> 
> intel_dp_start_link_train()
> 	intel_dp_link_training_clock_recovery()
> 		intel_dp->prepare_link_retrain(intel_dp)/intel_ddi_prepare_link_retrain();/* Port is enabled here in training mode */
> 
> 		....
> 
> 		intel_dp_reset_link_train()
> 			intel_dp_set_signal_levels() /* Vswing table is set here */
> Guess is safer keep programming it twice then?

Probably.

> 
> 
> > 
> > >  
> > >  	/*
> > >  	 * 7.f Combo PHY: Configure PORT_CL_DW10 Static Power Down to power up
> > > @@ -3257,7 +3257,6 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
> > >  	enum phy phy = intel_port_to_phy(dev_priv, port);
> > >  	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> > >  	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
> > > -	int level = intel_ddi_dp_level(intel_dp);
> > >  
> > >  	if (INTEL_GEN(dev_priv) < 11)
> > >  		drm_WARN_ON(&dev_priv->drm,
> > > @@ -3279,16 +3278,6 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
> > >  
> > >  	icl_program_mg_dp_mode(dig_port, crtc_state);
> > >  
> > > -	if (INTEL_GEN(dev_priv) >= 11)
> > > -		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
> > > -					level, encoder->type);
> > > -	else if (IS_CANNONLAKE(dev_priv))
> > > -		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
> > > -	else if (IS_GEN9_LP(dev_priv))
> > > -		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
> > > -	else
> > > -		intel_prepare_dp_ddi_buffers(encoder, crtc_state);
> > 
> > This last one definitely has to stay IIRC. HSW/BDW/SKL buf trans
> > stuff works quite bit differently than the BXT+ style more manual
> > programming.
> > 
> > > -
> > >  	if (intel_phy_is_combo(dev_priv, phy)) {
> > >  		bool lane_reversal =
> > >  			dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
> > > -- 
> > > 2.26.2
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel