| Message ID | 20240203-dp-swing-3-v1-1-6545e1706196@linaro.org |
|---|---|
| State | Accepted |
| Commit | 22578178e5dd6d3aa4490879df8b6c2977d980be |
| Headers | show |
| Series | drm/msm/dp: allow voltage swing / pre emphasis of 3 | expand |
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
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.
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;
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(-) --- base-commit: 41d66f96d0f15a0a2ad6fa2208f6bac1a66cbd52 change-id: 20240203-dp-swing-3-b64ffce415d9 Best regards,