[PATCH] drm/msm/dp: allow voltage swing / pre emphasis of 3

[PATCH] drm/msm/dp: allow voltage swing / pre emphasis of 3

[Dmitry Baryshkov]

Both dp_link_adjust_levels() and dp_ctrl_update_vx_px() limit swing and
pre-emphasis to 2, while the real maximum value for the sum of the
voltage swing and pre-emphasis is 3. Fix the DP code to remove this
limitation.

Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c |  6 +++---
 drivers/gpu/drm/msm/dp/dp_link.c | 22 +++++++++++-----------
 drivers/gpu/drm/msm/dp/dp_link.h | 14 +-------------
 3 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 77a8d9366ed7..26241970019f 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1024,14 +1024,14 @@ static int dp_ctrl_update_vx_px(struct dp_ctrl_private *ctrl)
 	if (ret)
 		return ret;
 
-	if (voltage_swing_level >= DP_TRAIN_VOLTAGE_SWING_MAX) {
+	if (voltage_swing_level >= DP_TRAIN_LEVEL_MAX) {
 		drm_dbg_dp(ctrl->drm_dev,
 				"max. voltage swing level reached %d\n",
 				voltage_swing_level);
 		max_level_reached |= DP_TRAIN_MAX_SWING_REACHED;
 	}
 
-	if (pre_emphasis_level >= DP_TRAIN_PRE_EMPHASIS_MAX) {
+	if (pre_emphasis_level >= DP_TRAIN_LEVEL_MAX) {
 		drm_dbg_dp(ctrl->drm_dev,
 				"max. pre-emphasis level reached %d\n",
 				pre_emphasis_level);
@@ -1122,7 +1122,7 @@ static int dp_ctrl_link_train_1(struct dp_ctrl_private *ctrl,
 		}
 
 		if (ctrl->link->phy_params.v_level >=
-			DP_TRAIN_VOLTAGE_SWING_MAX) {
+			DP_TRAIN_LEVEL_MAX) {
 			DRM_ERROR_RATELIMITED("max v_level reached\n");
 			return -EAGAIN;
 		}
diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
index 98427d45e9a7..e7da0571ecff 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -1107,6 +1107,7 @@ int dp_link_get_colorimetry_config(struct dp_link *dp_link)
 int dp_link_adjust_levels(struct dp_link *dp_link, u8 *link_status)
 {
 	int i;
+	u8 max_p_level;
 	int v_max = 0, p_max = 0;
 	struct dp_link_private *link;
 
@@ -1138,30 +1139,29 @@ int dp_link_adjust_levels(struct dp_link *dp_link, u8 *link_status)
 	 * Adjust the voltage swing and pre-emphasis level combination to within
 	 * the allowable range.
 	 */
-	if (dp_link->phy_params.v_level > DP_TRAIN_VOLTAGE_SWING_MAX) {
+	if (dp_link->phy_params.v_level > DP_TRAIN_LEVEL_MAX) {
 		drm_dbg_dp(link->drm_dev,
 			"Requested vSwingLevel=%d, change to %d\n",
 			dp_link->phy_params.v_level,
-			DP_TRAIN_VOLTAGE_SWING_MAX);
-		dp_link->phy_params.v_level = DP_TRAIN_VOLTAGE_SWING_MAX;
+			DP_TRAIN_LEVEL_MAX);
+		dp_link->phy_params.v_level = DP_TRAIN_LEVEL_MAX;
 	}
 
-	if (dp_link->phy_params.p_level > DP_TRAIN_PRE_EMPHASIS_MAX) {
+	if (dp_link->phy_params.p_level > DP_TRAIN_LEVEL_MAX) {
 		drm_dbg_dp(link->drm_dev,
 			"Requested preEmphasisLevel=%d, change to %d\n",
 			dp_link->phy_params.p_level,
-			DP_TRAIN_PRE_EMPHASIS_MAX);
-		dp_link->phy_params.p_level = DP_TRAIN_PRE_EMPHASIS_MAX;
+			DP_TRAIN_LEVEL_MAX);
+		dp_link->phy_params.p_level = DP_TRAIN_LEVEL_MAX;
 	}
 
-	if ((dp_link->phy_params.p_level > DP_TRAIN_PRE_EMPHASIS_LVL_1)
-		&& (dp_link->phy_params.v_level ==
-			DP_TRAIN_VOLTAGE_SWING_LVL_2)) {
+	max_p_level = DP_TRAIN_LEVEL_MAX - dp_link->phy_params.v_level;
+	if (dp_link->phy_params.p_level > max_p_level) {
 		drm_dbg_dp(link->drm_dev,
 			"Requested preEmphasisLevel=%d, change to %d\n",
 			dp_link->phy_params.p_level,
-			DP_TRAIN_PRE_EMPHASIS_LVL_1);
-		dp_link->phy_params.p_level = DP_TRAIN_PRE_EMPHASIS_LVL_1;
+			max_p_level);
+		dp_link->phy_params.p_level = max_p_level;
 	}
 
 	drm_dbg_dp(link->drm_dev, "adjusted: v_level=%d, p_level=%d\n",
diff --git a/drivers/gpu/drm/msm/dp/dp_link.h b/drivers/gpu/drm/msm/dp/dp_link.h
index 9dd4dd926530..79c3a02b8dac 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.h
+++ b/drivers/gpu/drm/msm/dp/dp_link.h
@@ -19,19 +19,7 @@ struct dp_link_info {
 	unsigned long capabilities;
 };
 
-enum dp_link_voltage_level {
-	DP_TRAIN_VOLTAGE_SWING_LVL_0	= 0,
-	DP_TRAIN_VOLTAGE_SWING_LVL_1	= 1,
-	DP_TRAIN_VOLTAGE_SWING_LVL_2	= 2,
-	DP_TRAIN_VOLTAGE_SWING_MAX	= DP_TRAIN_VOLTAGE_SWING_LVL_2,
-};
-
-enum dp_link_preemaphasis_level {
-	DP_TRAIN_PRE_EMPHASIS_LVL_0	= 0,
-	DP_TRAIN_PRE_EMPHASIS_LVL_1	= 1,
-	DP_TRAIN_PRE_EMPHASIS_LVL_2	= 2,
-	DP_TRAIN_PRE_EMPHASIS_MAX	= DP_TRAIN_PRE_EMPHASIS_LVL_2,
-};
+#define DP_TRAIN_LEVEL_MAX	3
 
 struct dp_link_test_video {
 	u32 test_video_pattern;

---
base-commit: 41d66f96d0f15a0a2ad6fa2208f6bac1a66cbd52
change-id: 20240203-dp-swing-3-b64ffce415d9

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Re: [PATCH] drm/msm/dp: allow voltage swing / pre emphasis of 3

[Doug Anderson]

Hi,

On Sat, Feb 3, 2024 at 5:47 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> Both dp_link_adjust_levels() and dp_ctrl_update_vx_px() limit swing and
> pre-emphasis to 2, while the real maximum value for the sum of the
> voltage swing and pre-emphasis is 3. Fix the DP code to remove this
> limitation.
>
> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/msm/dp/dp_ctrl.c |  6 +++---
>  drivers/gpu/drm/msm/dp/dp_link.c | 22 +++++++++++-----------
>  drivers/gpu/drm/msm/dp/dp_link.h | 14 +-------------
>  3 files changed, 15 insertions(+), 27 deletions(-)

What ever happened with this patch? It seemed important so I've been
trying to check back on it, but it seems to still be in limbo. I was
assuming that (maybe?) Abhinav would check things against the hardware
documentation and give it a Reviewed-by and then it would land...

-Doug

Re: [PATCH] drm/msm/dp: allow voltage swing / pre emphasis of 3

[Abhinav Kumar]

Hi Doug

On 3/29/2024 4:07 PM, Doug Anderson wrote:
> Hi,
> 
> On Sat, Feb 3, 2024 at 5:47 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>>
>> Both dp_link_adjust_levels() and dp_ctrl_update_vx_px() limit swing and
>> pre-emphasis to 2, while the real maximum value for the sum of the
>> voltage swing and pre-emphasis is 3. Fix the DP code to remove this
>> limitation.
>>
>> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_ctrl.c |  6 +++---
>>   drivers/gpu/drm/msm/dp/dp_link.c | 22 +++++++++++-----------
>>   drivers/gpu/drm/msm/dp/dp_link.h | 14 +-------------
>>   3 files changed, 15 insertions(+), 27 deletions(-)
> 
> What ever happened with this patch? It seemed important so I've been
> trying to check back on it, but it seems to still be in limbo. I was
> assuming that (maybe?) Abhinav would check things against the hardware
> documentation and give it a Reviewed-by and then it would land...
> 
> -Doug

The issue for which this patch was originally made (DP link training 
issue on x1e80100) was not getting fixed by this patch.

That one turned out as actually a PLL locking issue. So this kind of 
went off the radar as it was not immediately needed to fix anything.

I will wait for Kuogee's response on this patch. He was OOO entire Feb 
so this got missed.

Re: [PATCH] drm/msm/dp: allow voltage swing / pre emphasis of 3

[Kuogee Hsieh]

On 2/3/2024 5:47 AM, Dmitry Baryshkov wrote:
> Both dp_link_adjust_levels() and dp_ctrl_update_vx_px() limit swing and
> pre-emphasis to 2, while the real maximum value for the sum of the
> voltage swing and pre-emphasis is 3. Fix the DP code to remove this
> limitation.
>
> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_ctrl.c |  6 +++---
>   drivers/gpu/drm/msm/dp/dp_link.c | 22 +++++++++++-----------
>   drivers/gpu/drm/msm/dp/dp_link.h | 14 +-------------
>   3 files changed, 15 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 77a8d9366ed7..26241970019f 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1024,14 +1024,14 @@ static int dp_ctrl_update_vx_px(struct dp_ctrl_private *ctrl)
>   	if (ret)
>   		return ret;
>   
> -	if (voltage_swing_level >= DP_TRAIN_VOLTAGE_SWING_MAX) {
> +	if (voltage_swing_level >= DP_TRAIN_LEVEL_MAX) {
>   		drm_dbg_dp(ctrl->drm_dev,
>   				"max. voltage swing level reached %d\n",
>   				voltage_swing_level);
>   		max_level_reached |= DP_TRAIN_MAX_SWING_REACHED;
>   	}
>   
> -	if (pre_emphasis_level >= DP_TRAIN_PRE_EMPHASIS_MAX) {
> +	if (pre_emphasis_level >= DP_TRAIN_LEVEL_MAX) {
>   		drm_dbg_dp(ctrl->drm_dev,
>   				"max. pre-emphasis level reached %d\n",
>   				pre_emphasis_level);
> @@ -1122,7 +1122,7 @@ static int dp_ctrl_link_train_1(struct dp_ctrl_private *ctrl,
>   		}
>   
>   		if (ctrl->link->phy_params.v_level >=
> -			DP_TRAIN_VOLTAGE_SWING_MAX) {
> +			DP_TRAIN_LEVEL_MAX) {
>   			DRM_ERROR_RATELIMITED("max v_level reached\n");
>   			return -EAGAIN;
>   		}
> diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
> index 98427d45e9a7..e7da0571ecff 100644
> --- a/drivers/gpu/drm/msm/dp/dp_link.c
> +++ b/drivers/gpu/drm/msm/dp/dp_link.c
> @@ -1107,6 +1107,7 @@ int dp_link_get_colorimetry_config(struct dp_link *dp_link)
>   int dp_link_adjust_levels(struct dp_link *dp_link, u8 *link_status)
>   {
>   	int i;
> +	u8 max_p_level;
>   	int v_max = 0, p_max = 0;
>   	struct dp_link_private *link;
>   
> @@ -1138,30 +1139,29 @@ int dp_link_adjust_levels(struct dp_link *dp_link, u8 *link_status)
>   	 * Adjust the voltage swing and pre-emphasis level combination to within
>   	 * the allowable range.
>   	 */
> -	if (dp_link->phy_params.v_level > DP_TRAIN_VOLTAGE_SWING_MAX) {
> +	if (dp_link->phy_params.v_level > DP_TRAIN_LEVEL_MAX) {
>   		drm_dbg_dp(link->drm_dev,
>   			"Requested vSwingLevel=%d, change to %d\n",
>   			dp_link->phy_params.v_level,
> -			DP_TRAIN_VOLTAGE_SWING_MAX);
> -		dp_link->phy_params.v_level = DP_TRAIN_VOLTAGE_SWING_MAX;
> +			DP_TRAIN_LEVEL_MAX);
> +		dp_link->phy_params.v_level = DP_TRAIN_LEVEL_MAX;
>   	}
>   
> -	if (dp_link->phy_params.p_level > DP_TRAIN_PRE_EMPHASIS_MAX) {
> +	if (dp_link->phy_params.p_level > DP_TRAIN_LEVEL_MAX) {
>   		drm_dbg_dp(link->drm_dev,
>   			"Requested preEmphasisLevel=%d, change to %d\n",
>   			dp_link->phy_params.p_level,
> -			DP_TRAIN_PRE_EMPHASIS_MAX);
> -		dp_link->phy_params.p_level = DP_TRAIN_PRE_EMPHASIS_MAX;
> +			DP_TRAIN_LEVEL_MAX);
> +		dp_link->phy_params.p_level = DP_TRAIN_LEVEL_MAX;
>   	}
>   
> -	if ((dp_link->phy_params.p_level > DP_TRAIN_PRE_EMPHASIS_LVL_1)
> -		&& (dp_link->phy_params.v_level ==
> -			DP_TRAIN_VOLTAGE_SWING_LVL_2)) {
> +	max_p_level = DP_TRAIN_LEVEL_MAX - dp_link->phy_params.v_level;
> +	if (dp_link->phy_params.p_level > max_p_level) {
>   		drm_dbg_dp(link->drm_dev,
>   			"Requested preEmphasisLevel=%d, change to %d\n",
>   			dp_link->phy_params.p_level,
> -			DP_TRAIN_PRE_EMPHASIS_LVL_1);
> -		dp_link->phy_params.p_level = DP_TRAIN_PRE_EMPHASIS_LVL_1;
> +			max_p_level);
> +		dp_link->phy_params.p_level = max_p_level;
>   	}
>   
>   	drm_dbg_dp(link->drm_dev, "adjusted: v_level=%d, p_level=%d\n",
> diff --git a/drivers/gpu/drm/msm/dp/dp_link.h b/drivers/gpu/drm/msm/dp/dp_link.h
> index 9dd4dd926530..79c3a02b8dac 100644
> --- a/drivers/gpu/drm/msm/dp/dp_link.h
> +++ b/drivers/gpu/drm/msm/dp/dp_link.h
> @@ -19,19 +19,7 @@ struct dp_link_info {
>   	unsigned long capabilities;
>   };
>   
> -enum dp_link_voltage_level {
> -	DP_TRAIN_VOLTAGE_SWING_LVL_0	= 0,
> -	DP_TRAIN_VOLTAGE_SWING_LVL_1	= 1,
> -	DP_TRAIN_VOLTAGE_SWING_LVL_2	= 2,
> -	DP_TRAIN_VOLTAGE_SWING_MAX	= DP_TRAIN_VOLTAGE_SWING_LVL_2,
> -};
> -
> -enum dp_link_preemaphasis_level {
> -	DP_TRAIN_PRE_EMPHASIS_LVL_0	= 0,
> -	DP_TRAIN_PRE_EMPHASIS_LVL_1	= 1,
> -	DP_TRAIN_PRE_EMPHASIS_LVL_2	= 2,
> -	DP_TRAIN_PRE_EMPHASIS_MAX	= DP_TRAIN_PRE_EMPHASIS_LVL_2,
> -};
> +#define DP_TRAIN_LEVEL_MAX	3
>   
>   struct dp_link_test_video {
>   	u32 test_video_pattern;
>
> ---
> base-commit: 41d66f96d0f15a0a2ad6fa2208f6bac1a66cbd52
> change-id: 20240203-dp-swing-3-b64ffce415d9
>
> Best regards,