Messages in this thread | | | Date | Fri, 30 Oct 2020 15:06:33 -0700 | From | khsieh@codeauro ... | Subject | Re: [PATCH] drm/msm/dp: skip checking LINK_STATUS_UPDATED bit |
| |
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; >> }
| |