Message ID | 20200528200356.36756-1-jose.souza@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/4] drm/i915/display/hsw+: Do not program the same vswing entry twice | expand |
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@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? > > /* > * 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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@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? > > > > > /* > > * 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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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@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@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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 + */ /* * 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); - if (intel_phy_is_combo(dev_priv, phy)) { bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
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@intel.com> --- drivers/gpu/drm/i915/display/intel_ddi.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-)