diff mbox series

drm/msm/dp: delete DP_RECOVERED_CLOCK_OUT_EN to fix tps4

Message ID 1659382970-17477-1-git-send-email-quic_khsieh@quicinc.com (mailing list archive)
State Superseded
Headers show
Series drm/msm/dp: delete DP_RECOVERED_CLOCK_OUT_EN to fix tps4 | expand

Commit Message

Kuogee Hsieh Aug. 1, 2022, 7:42 p.m. UTC
Data Symbols scrambled is required for tps4 at link training 2.
Therefore SCRAMBLING_DISABLE bit should not be set for tps4 to
work.
RECOVERED_CLOCK_OUT_EN is for enable simple EYE test for jitter
measurement with minimal equipment for embedded applications purpose
and is not required to be set during normal operation.
Current implementation always have RECOVERED_CLOCK_OUT_EN bit set
which cause SCRAMBLING_DISABLE bit wrongly set at tps4 which prevent
tps4 from working.
This patch delete setting RECOVERED_CLOCK_OUT_EN to fix SCRAMBLING_DISABLE
be wrongly set at tps4.

Fixes: 956653250b21 ("drm/msm/dp: add support of tps4 (training pattern 4) for HBR3")

Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stephen Boyd Aug. 1, 2022, 7:51 p.m. UTC | #1
Quoting Kuogee Hsieh (2022-08-01 12:42:50)
> Data Symbols scrambled is required for tps4 at link training 2.
> Therefore SCRAMBLING_DISABLE bit should not be set for tps4 to
> work.
> RECOVERED_CLOCK_OUT_EN is for enable simple EYE test for jitter
> measurement with minimal equipment for embedded applications purpose
> and is not required to be set during normal operation.
> Current implementation always have RECOVERED_CLOCK_OUT_EN bit set
> which cause SCRAMBLING_DISABLE bit wrongly set at tps4 which prevent
> tps4 from working.
> This patch delete setting RECOVERED_CLOCK_OUT_EN to fix SCRAMBLING_DISABLE
> be wrongly set at tps4.
>
> Fixes: 956653250b21 ("drm/msm/dp: add support of tps4 (training pattern 4) for HBR3")
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_ctrl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index ab6aa13..013ca02 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1214,7 +1214,7 @@ static int dp_ctrl_link_train_2(struct dp_ctrl_private *ctrl,
>         if (ret)
>                 return ret;
>
> -       dp_ctrl_train_pattern_set(ctrl, pattern | DP_RECOVERED_CLOCK_OUT_EN);
> +       dp_ctrl_train_pattern_set(ctrl, pattern);

This line is from the first patch introducing this driver. Even if this
is fixing tps4 support, it sounds like the bit should never have been
enabled in the first place. Why isn't the fixes tag targeted at the
first commit? Does it hurt to apply it without commit 956653250b21?
Kuogee Hsieh Aug. 1, 2022, 8:08 p.m. UTC | #2
On 8/1/2022 12:51 PM, Stephen Boyd wrote:
> Quoting Kuogee Hsieh (2022-08-01 12:42:50)
>> Data Symbols scrambled is required for tps4 at link training 2.
>> Therefore SCRAMBLING_DISABLE bit should not be set for tps4 to
>> work.
>> RECOVERED_CLOCK_OUT_EN is for enable simple EYE test for jitter
>> measurement with minimal equipment for embedded applications purpose
>> and is not required to be set during normal operation.
>> Current implementation always have RECOVERED_CLOCK_OUT_EN bit set
>> which cause SCRAMBLING_DISABLE bit wrongly set at tps4 which prevent
>> tps4 from working.
>> This patch delete setting RECOVERED_CLOCK_OUT_EN to fix SCRAMBLING_DISABLE
>> be wrongly set at tps4.
>>
>> Fixes: 956653250b21 ("drm/msm/dp: add support of tps4 (training pattern 4) for HBR3")
>>
>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_ctrl.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> index ab6aa13..013ca02 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> @@ -1214,7 +1214,7 @@ static int dp_ctrl_link_train_2(struct dp_ctrl_private *ctrl,
>>          if (ret)
>>                  return ret;
>>
>> -       dp_ctrl_train_pattern_set(ctrl, pattern | DP_RECOVERED_CLOCK_OUT_EN);
>> +       dp_ctrl_train_pattern_set(ctrl, pattern);
> This line is from the first patch introducing this driver. Even if this
> is fixing tps4 support, it sounds like the bit should never have been
> enabled in the first place. Why isn't the fixes tag targeted at the
> first commit? Does it hurt to apply it without commit 956653250b21?
agree, it should be fixed to first patch
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index ab6aa13..013ca02 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1214,7 +1214,7 @@  static int dp_ctrl_link_train_2(struct dp_ctrl_private *ctrl,
 	if (ret)
 		return ret;
 
-	dp_ctrl_train_pattern_set(ctrl, pattern | DP_RECOVERED_CLOCK_OUT_EN);
+	dp_ctrl_train_pattern_set(ctrl, pattern);
 
 	for (tries = 0; tries <= maximum_retries; tries++) {
 		drm_dp_link_train_channel_eq_delay(ctrl->aux, ctrl->panel->dpcd);