[PATCH v1 0/2] fix DP phy compliance test

[PATCH v1 0/2] fix DP phy compliance test

[Kuogee Hsieh]

Current DP phy compliance test failed due to test pattern generation was
terminated premature.

Kuogee Hsieh (2):
  drm/msm/dp: reset DP controller before transmit phy test pattern
  drm/msm/dp: do not stop transmitting phy test pattern during DP phy
    compliance test

 drivers/gpu/drm/msm/dp/dp_ctrl.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v1 0/2] fix DP phy compliance test

[Kuogee Hsieh]

Current DP phy compliance test failed due to test pattern generation was
terminated premature.

Kuogee Hsieh (2):
  drm/msm/dp: reset DP controller before transmit phy test pattern
  drm/msm/dp: do not stop transmitting phy test pattern during DP phy
    compliance test

 drivers/gpu/drm/msm/dp/dp_ctrl.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v1 1/2] drm/msm/dp: reset DP controller before transmit phy test pattern

[Kuogee Hsieh]

DP controller state can not switch from video ready state to
transmit phy pattern state at run time. DP mainlink has to be
teared down followed by reset controller to default state to have
DP controller switch to transmit phy test pattern state and start
generate specified phy test pattern to sinker once main link setup
again.

Fixes: ee35444be0c8 ("drm/msm/dp: use dp_ctrl_off_link_stream during PHY compliance test run")
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 5356856..193cc1a 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1532,7 +1532,7 @@ static int dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl)
 	 * running. Add the global reset just before disabling the
 	 * link clocks and core clocks.
 	 */
-	ret = dp_ctrl_off_link_stream(&ctrl->dp_ctrl);
+	ret = dp_ctrl_off(&ctrl->dp_ctrl);
 	if (ret) {
 		DRM_ERROR("failed to disable DP controller\n");
 		return ret;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v1 1/2] drm/msm/dp: reset DP controller before transmit phy test pattern

[Kuogee Hsieh]

DP controller state can not switch from video ready state to
transmit phy pattern state at run time. DP mainlink has to be
teared down followed by reset controller to default state to have
DP controller switch to transmit phy test pattern state and start
generate specified phy test pattern to sinker once main link setup
again.

Fixes: ee35444be0c8 ("drm/msm/dp: use dp_ctrl_off_link_stream during PHY compliance test run")
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 5356856..193cc1a 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1532,7 +1532,7 @@ static int dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl)
 	 * running. Add the global reset just before disabling the
 	 * link clocks and core clocks.
 	 */
-	ret = dp_ctrl_off_link_stream(&ctrl->dp_ctrl);
+	ret = dp_ctrl_off(&ctrl->dp_ctrl);
 	if (ret) {
 		DRM_ERROR("failed to disable DP controller\n");
 		return ret;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v1 2/2] drm/msm/dp: do not stop transmitting phy test pattern during DP phy compliance test

[Kuogee Hsieh]

At normal operation, transmit phy test pattern has to be terminated before
DP controller switch to video ready state. However during phy compliance
testing, transmit phy test pattern should not be terminated until end of
compliance test which usually indicated by unplugged interrupt.

Fixes: 64e190e720a7 ("drm/msm/dp: DisplayPort PHY compliance tests fixup")
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 193cc1a..f99e173 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1699,8 +1699,6 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
 		ctrl->link->link_params.rate,
 		ctrl->link->link_params.num_lanes, ctrl->dp_ctrl.pixel_rate);
 
-	ctrl->link->phy_params.p_level = 0;
-	ctrl->link->phy_params.v_level = 0;
 
 	rc = dp_ctrl_enable_mainlink_clocks(ctrl);
 	if (rc)
@@ -1822,12 +1820,6 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
 		}
 	}
 
-	if (!dp_ctrl_channel_eq_ok(ctrl))
-		dp_ctrl_link_retrain(ctrl);
-
-	/* stop txing train pattern to end link training */
-	dp_ctrl_clear_training_pattern(ctrl);
-
 	ret = dp_ctrl_enable_stream_clocks(ctrl);
 	if (ret) {
 		DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
@@ -1839,6 +1831,13 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
 		return 0;
 	}
 
+	if (!dp_ctrl_channel_eq_ok(ctrl)) {
+		dp_ctrl_link_retrain(ctrl);
+	}
+
+	/* stop txing train pattern to end link training */
+	dp_ctrl_clear_training_pattern(ctrl);
+
 	/*
 	 * Set up transfer unit values and set controller state to send
 	 * video.
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v1 2/2] drm/msm/dp: do not stop transmitting phy test pattern during DP phy compliance test

[Kuogee Hsieh]

At normal operation, transmit phy test pattern has to be terminated before
DP controller switch to video ready state. However during phy compliance
testing, transmit phy test pattern should not be terminated until end of
compliance test which usually indicated by unplugged interrupt.

Fixes: 64e190e720a7 ("drm/msm/dp: DisplayPort PHY compliance tests fixup")
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 193cc1a..f99e173 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1699,8 +1699,6 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
 		ctrl->link->link_params.rate,
 		ctrl->link->link_params.num_lanes, ctrl->dp_ctrl.pixel_rate);
 
-	ctrl->link->phy_params.p_level = 0;
-	ctrl->link->phy_params.v_level = 0;
 
 	rc = dp_ctrl_enable_mainlink_clocks(ctrl);
 	if (rc)
@@ -1822,12 +1820,6 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
 		}
 	}
 
-	if (!dp_ctrl_channel_eq_ok(ctrl))
-		dp_ctrl_link_retrain(ctrl);
-
-	/* stop txing train pattern to end link training */
-	dp_ctrl_clear_training_pattern(ctrl);
-
 	ret = dp_ctrl_enable_stream_clocks(ctrl);
 	if (ret) {
 		DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
@@ -1839,6 +1831,13 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
 		return 0;
 	}
 
+	if (!dp_ctrl_channel_eq_ok(ctrl)) {
+		dp_ctrl_link_retrain(ctrl);
+	}
+
+	/* stop txing train pattern to end link training */
+	dp_ctrl_clear_training_pattern(ctrl);
+
 	/*
 	 * Set up transfer unit values and set controller state to send
 	 * video.
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v1 1/2] drm/msm/dp: reset DP controller before transmit phy test pattern

[Stephen Boyd]

Quoting Kuogee Hsieh (2022-04-25 15:11:02)
> DP controller state can not switch from video ready state to
> transmit phy pattern state at run time. DP mainlink has to be
> teared down followed by reset controller to default state to have
> DP controller switch to transmit phy test pattern state and start
> generate specified phy test pattern to sinker once main link setup
> again.
>
> Fixes: ee35444be0c8 ("drm/msm/dp: use dp_ctrl_off_link_stream during PHY compliance test run")

What commit is this? I think it's supposed to be

Fixes: 52352fe2f866 ("drm/msm/dp: use dp_ctrl_off_link_stream during
PHY compliance test run")

> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---

Otherwise,

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v1 1/2] drm/msm/dp: reset DP controller before transmit phy test pattern

[Stephen Boyd]

Quoting Kuogee Hsieh (2022-04-25 15:11:02)
> DP controller state can not switch from video ready state to
> transmit phy pattern state at run time. DP mainlink has to be
> teared down followed by reset controller to default state to have
> DP controller switch to transmit phy test pattern state and start
> generate specified phy test pattern to sinker once main link setup
> again.
>
> Fixes: ee35444be0c8 ("drm/msm/dp: use dp_ctrl_off_link_stream during PHY compliance test run")

What commit is this? I think it's supposed to be

Fixes: 52352fe2f866 ("drm/msm/dp: use dp_ctrl_off_link_stream during
PHY compliance test run")

> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---

Otherwise,

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v1 2/2] drm/msm/dp: do not stop transmitting phy test pattern during DP phy compliance test

[Stephen Boyd]

Quoting Kuogee Hsieh (2022-04-25 15:11:03)
> At normal operation, transmit phy test pattern has to be terminated before
> DP controller switch to video ready state. However during phy compliance
> testing, transmit phy test pattern should not be terminated until end of
> compliance test which usually indicated by unplugged interrupt.

And what does the patch do to fix it? We need more details in the commit
text. I think it should continue with:

Only stop sending the train pattern in dp_ctrl_on_stream() if we're not
doing compliance testing. We also no longer reset 'p_level' and
'v_level' because XYZ and it's OK/better to retrain the link after
enabling the stream clks because XYZ.

>
> Fixes: 64e190e720a7 ("drm/msm/dp: DisplayPort PHY compliance tests fixup")

Should be

Fixes: 6625e2637d93 ("drm/msm/dp: DisplayPort PHY compliance tests fixup")

> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_ctrl.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 193cc1a..f99e173 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1699,8 +1699,6 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
>                 ctrl->link->link_params.rate,
>                 ctrl->link->link_params.num_lanes, ctrl->dp_ctrl.pixel_rate);
>
> -       ctrl->link->phy_params.p_level = 0;
> -       ctrl->link->phy_params.v_level = 0;

Why is this removed? Can you mention in the commit text why we don't
need to zero it out anymore?

>
>         rc = dp_ctrl_enable_mainlink_clocks(ctrl);
>         if (rc)
> @@ -1822,12 +1820,6 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
>                 }
>         }
>
> -       if (!dp_ctrl_channel_eq_ok(ctrl))
> -               dp_ctrl_link_retrain(ctrl);
> -
> -       /* stop txing train pattern to end link training */
> -       dp_ctrl_clear_training_pattern(ctrl);
> -
>         ret = dp_ctrl_enable_stream_clocks(ctrl);
>         if (ret) {
>                 DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
> @@ -1839,6 +1831,13 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
>                 return 0;
>         }
>
> +       if (!dp_ctrl_channel_eq_ok(ctrl)) {
> +               dp_ctrl_link_retrain(ctrl);
> +       }

Braces not needed. Please remove.

> +
> +       /* stop txing train pattern to end link training */
> +       dp_ctrl_clear_training_pattern(ctrl);
> +

Re: [PATCH v1 2/2] drm/msm/dp: do not stop transmitting phy test pattern during DP phy compliance test

[Stephen Boyd]

Quoting Kuogee Hsieh (2022-04-25 15:11:03)
> At normal operation, transmit phy test pattern has to be terminated before
> DP controller switch to video ready state. However during phy compliance
> testing, transmit phy test pattern should not be terminated until end of
> compliance test which usually indicated by unplugged interrupt.

And what does the patch do to fix it? We need more details in the commit
text. I think it should continue with:

Only stop sending the train pattern in dp_ctrl_on_stream() if we're not
doing compliance testing. We also no longer reset 'p_level' and
'v_level' because XYZ and it's OK/better to retrain the link after
enabling the stream clks because XYZ.

>
> Fixes: 64e190e720a7 ("drm/msm/dp: DisplayPort PHY compliance tests fixup")

Should be

Fixes: 6625e2637d93 ("drm/msm/dp: DisplayPort PHY compliance tests fixup")

> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_ctrl.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 193cc1a..f99e173 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1699,8 +1699,6 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
>                 ctrl->link->link_params.rate,
>                 ctrl->link->link_params.num_lanes, ctrl->dp_ctrl.pixel_rate);
>
> -       ctrl->link->phy_params.p_level = 0;
> -       ctrl->link->phy_params.v_level = 0;

Why is this removed? Can you mention in the commit text why we don't
need to zero it out anymore?

>
>         rc = dp_ctrl_enable_mainlink_clocks(ctrl);
>         if (rc)
> @@ -1822,12 +1820,6 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
>                 }
>         }
>
> -       if (!dp_ctrl_channel_eq_ok(ctrl))
> -               dp_ctrl_link_retrain(ctrl);
> -
> -       /* stop txing train pattern to end link training */
> -       dp_ctrl_clear_training_pattern(ctrl);
> -
>         ret = dp_ctrl_enable_stream_clocks(ctrl);
>         if (ret) {
>                 DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
> @@ -1839,6 +1831,13 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
>                 return 0;
>         }
>
> +       if (!dp_ctrl_channel_eq_ok(ctrl)) {
> +               dp_ctrl_link_retrain(ctrl);
> +       }

Braces not needed. Please remove.

> +
> +       /* stop txing train pattern to end link training */
> +       dp_ctrl_clear_training_pattern(ctrl);
> +