No need to check LINK_STATuS_UPDATED bit before return 6 bytes of link status during link training. This patch also fix phy compliance test link rate conversion error.
Signed-off-by: Kuogee Hsieh khsieh@codeaurora.org --- drivers/gpu/drm/msm/dp/dp_ctrl.c | 20 ++++++-------------- drivers/gpu/drm/msm/dp/dp_link.c | 24 +++++++++++------------- 2 files changed, 17 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 6bdaec778c4c..76e891c91c6e 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1061,23 +1061,15 @@ static bool dp_ctrl_train_pattern_set(struct dp_ctrl_private *ctrl, static int dp_ctrl_read_link_status(struct dp_ctrl_private *ctrl, u8 *link_status) { - int len = 0; - u32 const offset = DP_LANE_ALIGN_STATUS_UPDATED - DP_LANE0_1_STATUS; - u32 link_status_read_max_retries = 100; - - while (--link_status_read_max_retries) { - len = drm_dp_dpcd_read_link_status(ctrl->aux, - link_status); - if (len != DP_LINK_STATUS_SIZE) { - DRM_ERROR("DP link status read failed, err: %d\n", len); - return len; - } + int ret = 0, len;
- if (!(link_status[offset] & DP_LINK_STATUS_UPDATED)) - return 0; + len = drm_dp_dpcd_read_link_status(ctrl->aux, link_status); + if (len != DP_LINK_STATUS_SIZE) { + DRM_ERROR("DP link status read failed, err: %d\n", len); + ret = len; }
- return -ETIMEDOUT; + return ret; }
static int dp_ctrl_link_train_1(struct dp_ctrl_private *ctrl, diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c index c811da515fb3..58d65daae3b3 100644 --- a/drivers/gpu/drm/msm/dp/dp_link.c +++ b/drivers/gpu/drm/msm/dp/dp_link.c @@ -773,7 +773,8 @@ static int dp_link_process_link_training_request(struct dp_link_private *link) link->request.test_lane_count);
link->dp_link.link_params.num_lanes = link->request.test_lane_count; - link->dp_link.link_params.rate = link->request.test_link_rate; + link->dp_link.link_params.rate = + drm_dp_bw_code_to_link_rate(link->request.test_link_rate);
return 0; } @@ -939,20 +940,17 @@ static u8 get_link_status(const u8 link_status[DP_LINK_STATUS_SIZE], int r) */ static int dp_link_process_link_status_update(struct dp_link_private *link) { - if (!(get_link_status(link->link_status, - DP_LANE_ALIGN_STATUS_UPDATED) & - DP_LINK_STATUS_UPDATED) || - (drm_dp_clock_recovery_ok(link->link_status, - link->dp_link.link_params.num_lanes) && - drm_dp_channel_eq_ok(link->link_status, - link->dp_link.link_params.num_lanes))) - return -EINVAL; + bool channel_eq_done = drm_dp_channel_eq_ok(link->link_status, + link->dp_link.link_params.num_lanes); + + bool clock_recovery_done = drm_dp_clock_recovery_ok(link->link_status, + link->dp_link.link_params.num_lanes);
DRM_DEBUG_DP("channel_eq_done = %d, clock_recovery_done = %d\n", - drm_dp_clock_recovery_ok(link->link_status, - link->dp_link.link_params.num_lanes), - drm_dp_clock_recovery_ok(link->link_status, - link->dp_link.link_params.num_lanes)); + channel_eq_done, clock_recovery_done); + + if (channel_eq_done && clock_recovery_done) + return -EINVAL;
return 0; }
base-commit: 0855cb4b31953b8c539e57b970da8146bcd4405a
Quoting Kuogee Hsieh (2020-10-20 09:59:59)
No need to check LINK_STATuS_UPDATED bit before
LINK_STATUS_UPDATED?
return 6 bytes of link status during link training.
Why?
This patch also fix phy compliance test link rate conversion error.
How?
Signed-off-by: Kuogee Hsieh khsieh@codeaurora.org
Any Fixes: tag?
drivers/gpu/drm/msm/dp/dp_ctrl.c | 20 ++++++-------------- drivers/gpu/drm/msm/dp/dp_link.c | 24 +++++++++++------------- 2 files changed, 17 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 6bdaec778c4c..76e891c91c6e 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1061,23 +1061,15 @@ static bool dp_ctrl_train_pattern_set(struct dp_ctrl_private *ctrl, static int dp_ctrl_read_link_status(struct dp_ctrl_private *ctrl, u8 *link_status) {
int len = 0;u32 const offset = DP_LANE_ALIGN_STATUS_UPDATED - DP_LANE0_1_STATUS;u32 link_status_read_max_retries = 100;while (--link_status_read_max_retries) {len = drm_dp_dpcd_read_link_status(ctrl->aux,link_status);if (len != DP_LINK_STATUS_SIZE) {DRM_ERROR("DP link status read failed, err: %d\n", len);return len;}
int ret = 0, len;
if (!(link_status[offset] & DP_LINK_STATUS_UPDATED))return 0;
len = drm_dp_dpcd_read_link_status(ctrl->aux, link_status);if (len != DP_LINK_STATUS_SIZE) {DRM_ERROR("DP link status read failed, err: %d\n", len);ret = len;
Could this be positive if the len is greater than 0 but not DP_LINK_STATUS_SIZE? Maybe the check should be len < 0? We certainly don't want to return some smaller size from this function, right?
}
return -ETIMEDOUT;
return ret;}
static int dp_ctrl_link_train_1(struct dp_ctrl_private *ctrl, diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c index c811da515fb3..58d65daae3b3 100644 --- a/drivers/gpu/drm/msm/dp/dp_link.c +++ b/drivers/gpu/drm/msm/dp/dp_link.c @@ -773,7 +773,8 @@ static int dp_link_process_link_training_request(struct dp_link_private *link) link->request.test_lane_count);
link->dp_link.link_params.num_lanes = link->request.test_lane_count;
link->dp_link.link_params.rate = link->request.test_link_rate;
link->dp_link.link_params.rate =drm_dp_bw_code_to_link_rate(link->request.test_link_rate);
Why are we storing bw_code in test_link_rate? This looks very confusing.
return 0;}
On 2020-10-20 15:15, Stephen Boyd wrote:
Quoting Kuogee Hsieh (2020-10-20 09:59:59)
No need to check LINK_STATuS_UPDATED bit before
LINK_STATUS_UPDATED?
return 6 bytes of link status during link training.
Why?
This patch also fix phy compliance test link rate conversion error.
How?
Signed-off-by: Kuogee Hsieh khsieh@codeaurora.org
Any Fixes: tag?
drivers/gpu/drm/msm/dp/dp_ctrl.c | 20 ++++++-------------- drivers/gpu/drm/msm/dp/dp_link.c | 24 +++++++++++------------- 2 files changed, 17 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 6bdaec778c4c..76e891c91c6e 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1061,23 +1061,15 @@ static bool dp_ctrl_train_pattern_set(struct dp_ctrl_private *ctrl, static int dp_ctrl_read_link_status(struct dp_ctrl_private *ctrl, u8 *link_status) {
int len = 0;u32 const offset = DP_LANE_ALIGN_STATUS_UPDATED -DP_LANE0_1_STATUS;
u32 link_status_read_max_retries = 100;while (--link_status_read_max_retries) {len = drm_dp_dpcd_read_link_status(ctrl->aux,link_status);if (len != DP_LINK_STATUS_SIZE) {DRM_ERROR("DP link status read failed, err:%d\n", len);
return len;}
int ret = 0, len;
if (!(link_status[offset] & DP_LINK_STATUS_UPDATED))return 0;
len = drm_dp_dpcd_read_link_status(ctrl->aux, link_status);if (len != DP_LINK_STATUS_SIZE) {DRM_ERROR("DP link status read failed, err: %d\n",len);
ret = len;Could this be positive if the len is greater than 0 but not DP_LINK_STATUS_SIZE? Maybe the check should be len < 0? We certainly don't want to return some smaller size from this function, right?
no, it should be exactly the byte number requested to read. otherwise, it should be failed and will re read at next run.
}
return -ETIMEDOUT;
return ret;}
static int dp_ctrl_link_train_1(struct dp_ctrl_private *ctrl, diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c index c811da515fb3..58d65daae3b3 100644 --- a/drivers/gpu/drm/msm/dp/dp_link.c +++ b/drivers/gpu/drm/msm/dp/dp_link.c @@ -773,7 +773,8 @@ static int dp_link_process_link_training_request(struct dp_link_private *link) link->request.test_lane_count);
link->dp_link.link_params.num_lanes =link->request.test_lane_count;
link->dp_link.link_params.rate = link->request.test_link_rate;
link->dp_link.link_params.rate =drm_dp_bw_code_to_link_rate(link->request.test_link_rate);
Why are we storing bw_code in test_link_rate? This looks very confusing.
Test_link_rate contains link rate from dpcd read. it need to be convert to real rate by timing 2.7Mb before start phy compliance test.
return 0;}
dri-devel@lists.freedesktop.org
-
khsieh＠codeaurora.org -
Kuogee Hsieh
-
Stephen Boyd