On Tue 28 Dec 10:51 PST 2021, Kuogee Hsieh wrote: > From: Kuogee Hsieh <khsieh@xxxxxxxxxxxxxx> > > Some DP sinkers prefer to use tps4 instead of tps3 during training #2. > This patch will use tps4 to perform link training #2 if sinker's DPCD > supports it. > > Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx> > --- > drivers/gpu/drm/msm/dp/dp_ctrl.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c > index 39558a2..c7b0657 100644 > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c > @@ -1181,7 +1181,7 @@ static int dp_ctrl_link_train_2(struct dp_ctrl_private *ctrl, > int *training_step) > { > int tries = 0, ret = 0; > - char pattern; > + char pattern, state_ctrl_bit; > int const maximum_retries = 5; > u8 link_status[DP_LINK_STATUS_SIZE]; > > @@ -1189,12 +1189,20 @@ static int dp_ctrl_link_train_2(struct dp_ctrl_private *ctrl, > > *training_step = DP_TRAINING_2; > > - if (drm_dp_tps3_supported(ctrl->panel->dpcd)) > + if (drm_dp_tps4_supported(ctrl->panel->dpcd)) { > + pattern = DP_TRAINING_PATTERN_4; > + state_ctrl_bit = 4; > + } > + else if (drm_dp_tps3_supported(ctrl->panel->dpcd)) { > pattern = DP_TRAINING_PATTERN_3; > - else > + state_ctrl_bit = 3; > + } > + else { > pattern = DP_TRAINING_PATTERN_2; > + state_ctrl_bit = 2; > + } > > - ret = dp_catalog_ctrl_set_pattern(ctrl->catalog, pattern); > + ret = dp_catalog_ctrl_set_pattern(ctrl->catalog, state_ctrl_bit); The patch looks good, but as the state_ctrl_bit is no longer equal to DP_PATTERN_n the function and argument names are misleading. Please rename it to something like "dp_catalog_ctrl_set_pattern_state_bit()" and the "pattern" argument within that function to "state_bit". Thanks, Bjorn > if (ret) > return ret; > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >