diff mbox series

[v3] drm/i915/display: Don't disable DDI/Transcoder when setting phy test pattern

Message ID 20221117061506.296392-1-khaled.almahallawy@intel.com (mailing list archive)
State New, archived
Headers show
Series [v3] drm/i915/display: Don't disable DDI/Transcoder when setting phy test pattern | expand

Commit Message

Almahallawy, Khaled Nov. 17, 2022, 6:15 a.m. UTC 
Bspecs has updated recently to remove the restriction to disable
DDI/Transcoder before setting PHY test pattern. This update is to
address PHY compliance test failures observed on a port with LTTPR.
The issue is that when Transc. is disabled, the main link signals fed
to LTTPR will be dropped invalidating link training, which will affect
the quality of the phy test pattern when the transcoder is enabled again.

v2: Update commit message (Clint)
v3: Add missing Signed-off in v2

Bspec: 50482
Cc: Imre Deak <imre.deak@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
CC: Jani Nikula <jani.nikula@intel.com>
Tested-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
Reviewed-by: Clint Taylor <clinton.a.taylor@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 59 -------------------------
 1 file changed, 59 deletions(-)

Comments

Jani Nikula Nov. 18, 2022, 11:49 a.m. UTC | #1 
On Wed, 16 Nov 2022, Khaled Almahallawy <khaled.almahallawy@intel.com> wrote:
> Bspecs has updated recently to remove the restriction to disable
> DDI/Transcoder before setting PHY test pattern. This update is to
> address PHY compliance test failures observed on a port with LTTPR.
> The issue is that when Transc. is disabled, the main link signals fed
> to LTTPR will be dropped invalidating link training, which will affect
> the quality of the phy test pattern when the transcoder is enabled again.
>
> v2: Update commit message (Clint)
> v3: Add missing Signed-off in v2

Bspec 7555 for pre-gen12 still says "TRANS_CONF and TRANS_DDI_FUNC_CTL
must be disabled prior to enabling the test pattern."

All I have against that is your word [1], and it's not even recorded in
the commit message.

BR,
Jani.


[1] https://lore.kernel.org/r/8f0e921300db2b6a36497773d4508892e1ea5a43.camel@intel.com

>
> Bspec: 50482
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> CC: Jani Nikula <jani.nikula@intel.com>
> Tested-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
> Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
> Reviewed-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 59 -------------------------
>  1 file changed, 59 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 914161d7d122..16cf961b4d1a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3679,61 +3679,6 @@ static void intel_dp_phy_pattern_update(struct intel_dp *intel_dp,
>  	}
>  }
>  
> -static void
> -intel_dp_autotest_phy_ddi_disable(struct intel_dp *intel_dp,
> -				  const struct intel_crtc_state *crtc_state)
> -{
> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_device *dev = dig_port->base.base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
> -	enum pipe pipe = crtc->pipe;
> -	u32 trans_ddi_func_ctl_value, trans_conf_value, dp_tp_ctl_value;
> -
> -	trans_ddi_func_ctl_value = intel_de_read(dev_priv,
> -						 TRANS_DDI_FUNC_CTL(pipe));
> -	trans_conf_value = intel_de_read(dev_priv, PIPECONF(pipe));
> -	dp_tp_ctl_value = intel_de_read(dev_priv, TGL_DP_TP_CTL(pipe));
> -
> -	trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE |
> -				      TGL_TRANS_DDI_PORT_MASK);
> -	trans_conf_value &= ~PIPECONF_ENABLE;
> -	dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE;
> -
> -	intel_de_write(dev_priv, PIPECONF(pipe), trans_conf_value);
> -	intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(pipe),
> -		       trans_ddi_func_ctl_value);
> -	intel_de_write(dev_priv, TGL_DP_TP_CTL(pipe), dp_tp_ctl_value);
> -}
> -
> -static void
> -intel_dp_autotest_phy_ddi_enable(struct intel_dp *intel_dp,
> -				 const struct intel_crtc_state *crtc_state)
> -{
> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_device *dev = dig_port->base.base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	enum port port = dig_port->base.port;
> -	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
> -	enum pipe pipe = crtc->pipe;
> -	u32 trans_ddi_func_ctl_value, trans_conf_value, dp_tp_ctl_value;
> -
> -	trans_ddi_func_ctl_value = intel_de_read(dev_priv,
> -						 TRANS_DDI_FUNC_CTL(pipe));
> -	trans_conf_value = intel_de_read(dev_priv, PIPECONF(pipe));
> -	dp_tp_ctl_value = intel_de_read(dev_priv, TGL_DP_TP_CTL(pipe));
> -
> -	trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE |
> -				    TGL_TRANS_DDI_SELECT_PORT(port);
> -	trans_conf_value |= PIPECONF_ENABLE;
> -	dp_tp_ctl_value |= DP_TP_CTL_ENABLE;
> -
> -	intel_de_write(dev_priv, PIPECONF(pipe), trans_conf_value);
> -	intel_de_write(dev_priv, TGL_DP_TP_CTL(pipe), dp_tp_ctl_value);
> -	intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(pipe),
> -		       trans_ddi_func_ctl_value);
> -}
> -
>  static void intel_dp_process_phy_request(struct intel_dp *intel_dp,
>  					 const struct intel_crtc_state *crtc_state)
>  {
> @@ -3752,14 +3697,10 @@ static void intel_dp_process_phy_request(struct intel_dp *intel_dp,
>  	intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX,
>  				  link_status);
>  
> -	intel_dp_autotest_phy_ddi_disable(intel_dp, crtc_state);
> -
>  	intel_dp_set_signal_levels(intel_dp, crtc_state, DP_PHY_DPRX);
>  
>  	intel_dp_phy_pattern_update(intel_dp, crtc_state);
>  
> -	intel_dp_autotest_phy_ddi_enable(intel_dp, crtc_state);
> -
>  	drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET,
>  			  intel_dp->train_set, crtc_state->lane_count);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 914161d7d122..16cf961b4d1a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -3679,61 +3679,6 @@  static void intel_dp_phy_pattern_update(struct intel_dp *intel_dp,
 	}
 }
 
-static void
-intel_dp_autotest_phy_ddi_disable(struct intel_dp *intel_dp,
-				  const struct intel_crtc_state *crtc_state)
-{
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-	struct drm_device *dev = dig_port->base.base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
-	enum pipe pipe = crtc->pipe;
-	u32 trans_ddi_func_ctl_value, trans_conf_value, dp_tp_ctl_value;
-
-	trans_ddi_func_ctl_value = intel_de_read(dev_priv,
-						 TRANS_DDI_FUNC_CTL(pipe));
-	trans_conf_value = intel_de_read(dev_priv, PIPECONF(pipe));
-	dp_tp_ctl_value = intel_de_read(dev_priv, TGL_DP_TP_CTL(pipe));
-
-	trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE |
-				      TGL_TRANS_DDI_PORT_MASK);
-	trans_conf_value &= ~PIPECONF_ENABLE;
-	dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE;
-
-	intel_de_write(dev_priv, PIPECONF(pipe), trans_conf_value);
-	intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(pipe),
-		       trans_ddi_func_ctl_value);
-	intel_de_write(dev_priv, TGL_DP_TP_CTL(pipe), dp_tp_ctl_value);
-}
-
-static void
-intel_dp_autotest_phy_ddi_enable(struct intel_dp *intel_dp,
-				 const struct intel_crtc_state *crtc_state)
-{
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-	struct drm_device *dev = dig_port->base.base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum port port = dig_port->base.port;
-	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
-	enum pipe pipe = crtc->pipe;
-	u32 trans_ddi_func_ctl_value, trans_conf_value, dp_tp_ctl_value;
-
-	trans_ddi_func_ctl_value = intel_de_read(dev_priv,
-						 TRANS_DDI_FUNC_CTL(pipe));
-	trans_conf_value = intel_de_read(dev_priv, PIPECONF(pipe));
-	dp_tp_ctl_value = intel_de_read(dev_priv, TGL_DP_TP_CTL(pipe));
-
-	trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE |
-				    TGL_TRANS_DDI_SELECT_PORT(port);
-	trans_conf_value |= PIPECONF_ENABLE;
-	dp_tp_ctl_value |= DP_TP_CTL_ENABLE;
-
-	intel_de_write(dev_priv, PIPECONF(pipe), trans_conf_value);
-	intel_de_write(dev_priv, TGL_DP_TP_CTL(pipe), dp_tp_ctl_value);
-	intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(pipe),
-		       trans_ddi_func_ctl_value);
-}
-
 static void intel_dp_process_phy_request(struct intel_dp *intel_dp,
 					 const struct intel_crtc_state *crtc_state)
 {
@@ -3752,14 +3697,10 @@  static void intel_dp_process_phy_request(struct intel_dp *intel_dp,
 	intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX,
 				  link_status);
 
-	intel_dp_autotest_phy_ddi_disable(intel_dp, crtc_state);
-
 	intel_dp_set_signal_levels(intel_dp, crtc_state, DP_PHY_DPRX);
 
 	intel_dp_phy_pattern_update(intel_dp, crtc_state);
 
-	intel_dp_autotest_phy_ddi_enable(intel_dp, crtc_state);
-
 	drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET,
 			  intel_dp->train_set, crtc_state->lane_count);