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

[Paul Menzel]

Dear Wenjing, dear Rodrigo,


On 24.10.21 15:31, Rodrigo Siqueira wrote:
> From: Wenjing Liu <wenjing.liu at amd.com>
> 
> [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 at amd.com>
> Acked-by: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> Signed-off-by: Wenjing Liu <wenjing.liu at amd.com>
> ---
>   .../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