[Intel-gfx] [PATCH] drm/i915: Expand documentation for gen12 DP pre-enable sequence

Matt Roper matthew.d.roper at intel.com
Thu Nov 7 22:33:39 UTC 2019


On Thu, Nov 07, 2019 at 12:16:13PM -0800, Souza, Jose wrote:
> On Thu, 2019-11-07 at 09:45 -0800, Matt Roper wrote:
> > Rather than just specifying the bullet numbers from the bspec (e.g.,
> > "4.b") actually include the description of what the bspec wants us to
> > do.  Steps can be renumbered or moved so including the description
> > will
> > help us match the code up to the spec.  Plus if we add support for
> > new
> > platforms, some of the steps may be added/removed so more descriptive
> > comments will be useful for ensuring all of the bspec requirements
> > are
> > met.
> > 
> > Cc: José Roberto de Souza <jose.souza at intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c | 92 ++++++++++++++++++--
> > ----
> >  1 file changed, 72 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 398c6f054a6e..ad39cd608011 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3438,48 +3438,86 @@ static void tgl_ddi_pre_enable_dp(struct
> > intel_encoder *encoder,
> >  	intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(transcoder);
> >  	intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(transcoder);
> >  
> > -	/* 1.a got on intel_atomic_commit_tail() */
> > +	/*
> > +	 * 1. Enable Power Wells
> > +	 *
> > +	 * This was handled at the beginning of
> > intel_atomic_commit_tail(),
> > +	 * before we called down into this function.
> > +	 */
> >  
> > -	/* 2. */
> > +	/* 2. Enable Panel Power if PPS is required */
> >  	intel_edp_panel_on(intel_dp);
> >  
> >  	/*
> > -	 * 1.b, 3. and 4.a is done before tgl_ddi_pre_enable_dp() by:
> > -	 * haswell_crtc_enable()->intel_encoders_pre_pll_enable() and
> > -	 * haswell_crtc_enable()->intel_enable_shared_dpll()
> > +	 * 3. For non-TBT Type-C ports, set FIA lane count
> > +	 * (DFLEXDPSP.DPX4TXLATC)
> > +	 *
> > +	 * This was done before tgl_ddi_pre_enable_dp by
> > +	 * haswell_crtc_enable()->intel_encoders_pre_pll_enable().
> >  	 */
> >  
> > -	/* 4.b */
> > +	/*
> > +	 * 4. Enable the port PLL.
> > +	 *
> > +	 * The PLL enabling itself was already done before this
> > function by
> > +	 * haswell_crtc_enable()->intel_enable_shared_dpll().  We need
> > only
> > +	 * configure the PLL to port mapping here.
> > +	 */
> >  	intel_ddi_clk_select(encoder, crtc_state);
> >  
> > -	/* 5. */
> > +	/* 5. If IO power is controlled through PWR_WELL_CTL, Enable IO
> > Power */
> >  	if (!intel_phy_is_tc(dev_priv, phy) ||
> >  	    dig_port->tc_mode != TC_PORT_TBT_ALT)
> >  		intel_display_power_get(dev_priv,
> >  					dig_port->ddi_io_power_domain);
> >  
> > -	/* 6. */
> > +	/* 6. Program DP_MODE */
> >  	icl_program_mg_dp_mode(dig_port, crtc_state);
> >  
> >  	/*
> > -	 * 7.a - single stream or multi-stream master transcoder:
> > Configure
> > -	 * Transcoder Clock Select. For additional MST streams this
> > will be done
> > -	 * by intel_mst_pre_enable_dp() after programming VC Payload ID
> > through
> > -	 * AUX.
> > +	 * 7. The rest of the below are substeps under the bspec's
> > "Enable and
> > +	 * Train Display Port" step.  Note that steps that are specific
> > to
> > +	 * MST will be handled by intel_mst_pre_enable_dp()
> > before/after it
> > +	 * calls into this function.  Also intel_mst_pre_enable_dp()
> > only calls
> > +	 * us when active_mst_links==0, so any steps designated for
> > "single
> > +	 * stream or multi-stream master transcoder" can just be
> > performed
> > +	 * unconditionally here.
> > +	 */
> > +
> > +	/*
> > +	 * 7a. Configure Transcoder Clock Select to direct the Port
> > clock to the
> > +	 * Transcoder.
> 
> Before push, please fix:
> s/7a/7.a
> s/7b/7.b
> ...
> 
> To keep consistent with the steps above

Made this tweak and pushed to dinq.  Thanks Jose and Lucas for the
reviews.

Technically only BAT results have come back at this point, but since
this commit only touches comments and no code there shouldn't be any
behavioral changes.


Matt

> 
> Other than that:
> 
> Reviewed-by: José Roberto de Souza <jose.souza at intel.com>
> 
> >  	 */
> >  	intel_ddi_enable_pipe_clock(crtc_state);
> >  
> > -	/* 7.b */
> > +	/*
> > +	 * 7b. Configure TRANS_DDI_FUNC_CTL DDI Select, DDI Mode Select
> > & MST
> > +	 * Transport Select
> > +	 */
> >  	intel_ddi_config_transcoder_func(crtc_state);
> >  
> > -	/* 7.d */
> > +	/*
> > +	 * 7c. Configure & enable DP_TP_CTL with link training pattern
> > 1
> > +	 * selected
> > +	 *
> > +	 * This will be handled by the intel_dp_start_link_train()
> > farther
> > +	 * down this function.
> > +	 */
> > +
> > +	/*
> > +	 * 7d. Type C with DP alternate or fixed/legacy/static
> > connection -
> > +	 * Disable PHY clock gating per Type-C DDI Buffer page
> > +	 */
> >  	icl_phy_set_clock_gating(dig_port, false);
> >  
> > -	/* 7.e */
> > +	/* 7e. Configure voltage swing and related IO settings */
> >  	tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock, level,
> >  				encoder->type);
> >  
> > -	/* 7.f */
> > +	/*
> > +	 * 7f. Combo PHY: Configure PORT_CL_DW10 Static Power Down to
> > power up
> > +	 * the used lanes of the DDI.
> > +	 */
> >  	if (intel_phy_is_combo(dev_priv, phy)) {
> >  		bool lane_reversal =
> >  			dig_port->saved_port_bits &
> > DDI_BUF_PORT_REVERSAL;
> > @@ -3489,7 +3527,14 @@ static void tgl_ddi_pre_enable_dp(struct
> > intel_encoder *encoder,
> >  					       lane_reversal);
> >  	}
> >  
> > -	/* 7.g */
> > +	/*
> > +	 * 7g. Configure and enable DDI_BUF_CTL
> > +	 * 7h. Wait for DDI_BUF_CTL DDI Idle Status = 0b (Not Idle),
> > timeout
> > +	 *     after 500 us.
> > +	 *
> > +	 * We only configure what the register value will be
> > here.  Actual
> > +	 * enabling happens during link training farther down.
> > +	 */
> >  	intel_ddi_init_dp_buf_reg(encoder);
> >  
> >  	if (!is_mst)
> > @@ -3502,10 +3547,17 @@ static void tgl_ddi_pre_enable_dp(struct
> > intel_encoder *encoder,
> >  	 * training
> >  	 */
> >  	intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
> > -	/* 7.c, 7.h, 7.i, 7.j */
> > +
> > +	/*
> > +	 * 7i. Follow DisplayPort specification training sequence (see
> > notes for
> > +	 *     failure handling)
> > +	 * 7j. If DisplayPort multi-stream - Set DP_TP_CTL link
> > training to Idle
> > +	 *     Pattern, wait for 5 idle patterns (DP_TP_STATUS
> > Min_Idles_Sent)
> > +	 *     (timeout after 800 us)
> > +	 */
> >  	intel_dp_start_link_train(intel_dp);
> >  
> > -	/* 7.k */
> > +	/* 7k. Set DP_TP_CTL link training to Normal */
> >  	if (!is_trans_port_sync_mode(crtc_state))
> >  		intel_dp_stop_link_train(intel_dp);
> >  
> > @@ -3518,7 +3570,7 @@ static void tgl_ddi_pre_enable_dp(struct
> > intel_encoder *encoder,
> >  	 * so not enabling it for now.
> >  	 */
> >  
> > -	/* 7.l */
> > +	/* 7l. Configure and enable FEC if needed */
> >  	intel_ddi_enable_fec(encoder, crtc_state);
> >  	intel_dsc_enable(encoder, crtc_state);
> >  }

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795


More information about the Intel-gfx mailing list