drm/i915: Expand documentation for gen12 DP pre-enable sequence
diff mbox series

Message ID 20191107174527.11165-1-matthew.d.roper@intel.com
State New
Headers show
Series
  • drm/i915: Expand documentation for gen12 DP pre-enable sequence
Related show

Commit Message

Matt Roper Nov. 7, 2019, 5:45 p.m. UTC
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@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 92 ++++++++++++++++++------
 1 file changed, 72 insertions(+), 20 deletions(-)

Comments

Lucas De Marchi Nov. 7, 2019, 8:05 p.m. UTC | #1
On Thu, Nov 07, 2019 at 09:45:27AM -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@intel.com>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

>---
> 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.
> 	 */
> 	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);
> }
>-- 
>2.21.0
>
José Roberto de Souza Nov. 7, 2019, 8:16 p.m. UTC | #2
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@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@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

Other than that:

Reviewed-by: José Roberto de Souza <jose.souza@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 Nov. 7, 2019, 10:33 p.m. UTC | #3
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@intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@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@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);
> >  }

Patch
diff mbox series

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.
 	 */
 	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);
 }