Re: [PATCH 32/33] drm/amd/display: fix link training regression for 1 or 2 lane

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Wenjing, dear Rodrigo,


On 24.10.21 15:31, Rodrigo Siqueira wrote:
From: Wenjing Liu <wenjing.liu@xxxxxxx>

[why]
We have a regression that cause maximize lane settings to use
uninitialized data from unused lanes.

Which commit caused the regression? Please amend the commit message.

This will cause link training to fail for 1 or 2 lanes because the lane
adjust is populated incorrectly sometimes.

On what card did you test this, and how can it be reproduced?

Please describe the fix/implemantation in the commit message.

Reviewed-by: Eric Yang <eric.yang2@xxxxxxx>
Acked-by: Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx>
Signed-off-by: Wenjing Liu <wenjing.liu@xxxxxxx>
---
  .../gpu/drm/amd/display/dc/core/dc_link_dp.c  | 35 +++++++++++++++++--
  1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 653279ab96f4..f6ba7c734f54 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -108,6 +108,9 @@ static struct dc_link_settings get_common_supported_link_settings(
  		struct dc_link_settings link_setting_b);
  static void maximize_lane_settings(const struct link_training_settings *lt_settings,
  		struct dc_lane_settings lane_settings[LANE_COUNT_DP_MAX]);
+static void override_lane_settings(const struct link_training_settings *lt_settings,
+		struct dc_lane_settings lane_settings[LANE_COUNT_DP_MAX]);
+
  static uint32_t get_cr_training_aux_rd_interval(struct dc_link *link,
  		const struct dc_link_settings *link_settings)
  {
@@ -734,15 +737,13 @@ void dp_decide_lane_settings(
  		}
  #endif
  	}
-
-	/* we find the maximum of the requested settings across all lanes*/
-	/* and set this maximum for all lanes*/
  	dp_hw_to_dpcd_lane_settings(lt_settings, hw_lane_settings, dpcd_lane_settings);
if (lt_settings->disallow_per_lane_settings) {
  		/* we find the maximum of the requested settings across all lanes*/
  		/* and set this maximum for all lanes*/
  		maximize_lane_settings(lt_settings, hw_lane_settings);
+		override_lane_settings(lt_settings, hw_lane_settings);
if (lt_settings->always_match_dpcd_with_hw_lane_settings)
  			dp_hw_to_dpcd_lane_settings(lt_settings, hw_lane_settings, dpcd_lane_settings);
@@ -833,6 +834,34 @@ static void maximize_lane_settings(const struct link_training_settings *lt_setti
  	}
  }
+static void override_lane_settings(const struct link_training_settings *lt_settings,
+		struct dc_lane_settings lane_settings[LANE_COUNT_DP_MAX])
+{
+	uint32_t lane;
+
+	if (lt_settings->voltage_swing == NULL &&
+			lt_settings->pre_emphasis == NULL &&
+#if defined(CONFIG_DRM_AMD_DC_DP2_0)
+			lt_settings->ffe_preset == NULL &&
+#endif
+			lt_settings->post_cursor2 == NULL)
+
+		return;
+
+	for (lane = 1; lane < LANE_COUNT_DP_MAX; lane++) {
+		if (lt_settings->voltage_swing)
+			lane_settings[lane].VOLTAGE_SWING = *lt_settings->voltage_swing;
+		if (lt_settings->pre_emphasis)
+			lane_settings[lane].PRE_EMPHASIS = *lt_settings->pre_emphasis;
+		if (lt_settings->post_cursor2)
+			lane_settings[lane].POST_CURSOR2 = *lt_settings->post_cursor2;
+#if defined(CONFIG_DRM_AMD_DC_DP2_0)
+		if (lt_settings->ffe_preset)
+			lane_settings[lane].FFE_PRESET = *lt_settings->ffe_preset;
+#endif

Normally these checks should be done in C and not the preprocessor. `if CONFIG(DRM_AMD_DC_DP2_0)` or similar should work.

+	}
+}
+
  enum dc_status dp_get_lane_status_and_lane_adjust(
  	struct dc_link *link,
  	const struct link_training_settings *link_training_setting,



Kind regards,

Paul



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux