| Message ID | 20201030232310.11100-1-khsieh@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/msm/dp: skip checking LINK_STATUS_UPDATED bit | expand |
Quoting Kuogee Hsieh (2020-10-30 16:23:10) > Some dongle will not clear LINK_STATUS_UPDATED bit after > DPCD read which cause link training failed. This patch $ git grep 'this patch' -- Documentation/process/submitting-patches.rst > just read 6 bytes of DPCD link status from sink and return > without checking LINK_STATUS_UPDATED bit. > Link rate read back from sink need to be convert into > really rate by timing 2.7Mb. This last sentence doesn't make sense to me, sorry. What is being said? > For example 0x0A is equivalent > to 2.7Gb. This patch also convert link rate correctly to fix > phy compliance test link rate error. > > Chanegs in V2: > -- revise commit text > > Fixes: fd4a29bed29b (drm/msm/dp: DisplayPort PHY compliance tests fixup) > Shouldn't be any space here between SoB and Fixes tag. > 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 904698dfc7f7..844ba756a2c6 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; So if this returns the integer 2 it's OK? Shouldn't it return some error value? > } > > - 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 49d7fad36fc4..64a002d100c7 100644 > --- a/drivers/gpu/drm/msm/dp/dp_link.c > +++ b/drivers/gpu/drm/msm/dp/dp_link.c > @@ -943,20 +944,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: 03a9adc88c206b3857ce95f4f4d3b185d429fa31 What is this commit?
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 904698dfc7f7..844ba756a2c6 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 49d7fad36fc4..64a002d100c7 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; } @@ -943,20 +944,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; }
Some dongle will not clear LINK_STATUS_UPDATED bit after DPCD read which cause link training failed. This patch just read 6 bytes of DPCD link status from sink and return without checking LINK_STATUS_UPDATED bit. Link rate read back from sink need to be convert into really rate by timing 2.7Mb. For example 0x0A is equivalent to 2.7Gb. This patch also convert link rate correctly to fix phy compliance test link rate error. Chanegs in V2: -- revise commit text Fixes: fd4a29bed29b (drm/msm/dp: DisplayPort PHY compliance tests fixup) 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(-) base-commit: 03a9adc88c206b3857ce95f4f4d3b185d429fa31