[PATCH v8 0/2] force link training for display resolution change

[PATCH v8 0/2] force link training for display resolution change

[Kuogee Hsieh]

1) force link training for display resolution change
2) remove pixel_rate from struct dp_ctrl

Kuogee Hsieh (2):
  drm/msm/dp: force link training for display resolution change
  drm/msm/dp: clean up pixel_rate from dp_ctrl.c

 drivers/gpu/drm/msm/dp/dp_ctrl.c    | 147 ++++++++++++++++++++----------------
 drivers/gpu/drm/msm/dp/dp_ctrl.h    |   3 +-
 drivers/gpu/drm/msm/dp/dp_display.c |  13 ++--
 3 files changed, 88 insertions(+), 75 deletions(-)

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

[PATCH v8 1/2] drm/msm/dp: force link training for display resolution change

[Kuogee Hsieh]

Display resolution change is implemented through drm modeset. Older
modeset (resolution) has to be disabled first before newer modeset
(resolution) can be enabled. Display disable will turn off both
pixel clock and main link clock so that main link have to be
re-trained during display enable to have new video stream flow
again. At current implementation, display enable function manually
kicks up irq_hpd_handle which will read panel link status and start
link training if link status is not in sync state.

However, there is rare case that a particular panel links status keep
staying in sync for some period of time after main link had been shut
down previously at display disabled. In this case, main link retraining
will not be executed by irq_hdp_handle(). Hence video stream of newer
display resolution will fail to be transmitted to panel due to main
link is not in sync between host and panel.

This patch will bypass irq_hpd_handle() in favor of directly call
dp_ctrl_on_stream() to always perform link training in regardless of
main link status. So that no unexpected exception resolution change
failure cases will happen. Also this implementation are more efficient
than manual kicking off irq_hpd_handle function.

Changes in v2:
-- set force_link_train flag on DP only (is_edp == false)

Changes in v3:
-- revise commit  text
-- add Fixes tag

Changes in v4:
-- revise commit  text

Changes in v5:
-- fix spelling at commit text

Changes in v6:
-- split dp_ctrl_on_stream() for phy test case
-- revise commit text for modeset

Changes in v7:
-- drop 0 assignment at local variable (ret = 0)

Changes in v8:
-- add patch to remove pixel_rate from dp_ctrl

Fixes: 62671d2ef24b ("drm/msm/dp: fixes wrong connection state caused by failure of link train")
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c    | 31 +++++++++++++++++++++++--------
 drivers/gpu/drm/msm/dp/dp_ctrl.h    |  3 ++-
 drivers/gpu/drm/msm/dp/dp_display.c | 13 ++++++-------
 3 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index af7a80c..01028b5 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1551,7 +1551,7 @@ static int dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl)
 
 	ret = dp_ctrl_on_link(&ctrl->dp_ctrl);
 	if (!ret)
-		ret = dp_ctrl_on_stream(&ctrl->dp_ctrl);
+		ret = dp_ctrl_on_stream_phy_test_report(&ctrl->dp_ctrl);
 	else
 		DRM_ERROR("failed to enable DP link controller\n");
 
@@ -1807,7 +1807,27 @@ static int dp_ctrl_link_retrain(struct dp_ctrl_private *ctrl)
 	return dp_ctrl_setup_main_link(ctrl, &training_step);
 }
 
-int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
+int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl)
+{
+	int ret;
+	struct dp_ctrl_private *ctrl;
+
+	ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
+
+	ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
+
+	ret = dp_ctrl_enable_stream_clocks(ctrl);
+	if (ret) {
+		DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
+		return ret;
+	}
+
+	dp_ctrl_send_phy_test_pattern(ctrl);
+
+	return 0;
+}
+
+int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train)
 {
 	int ret = 0;
 	bool mainlink_ready = false;
@@ -1843,12 +1863,7 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
 		goto end;
 	}
 
-	if (ctrl->link->sink_request & DP_TEST_LINK_PHY_TEST_PATTERN) {
-		dp_ctrl_send_phy_test_pattern(ctrl);
-		return 0;
-	}
-
-	if (!dp_ctrl_channel_eq_ok(ctrl))
+	if (force_link_train || !dp_ctrl_channel_eq_ok(ctrl))
 		dp_ctrl_link_retrain(ctrl);
 
 	/* stop txing train pattern to end link training */
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 0745fde..9a39b00 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -21,7 +21,8 @@ struct dp_ctrl {
 };
 
 int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl);
-int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl);
+int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train);
+int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl);
 int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl);
 int dp_ctrl_off_link(struct dp_ctrl *dp_ctrl);
 int dp_ctrl_off(struct dp_ctrl *dp_ctrl);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index c388323..b6d25ab 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -872,7 +872,7 @@ static int dp_display_enable(struct dp_display_private *dp, u32 data)
 		return 0;
 	}
 
-	rc = dp_ctrl_on_stream(dp->ctrl);
+	rc = dp_ctrl_on_stream(dp->ctrl, data);
 	if (!rc)
 		dp_display->power_on = true;
 
@@ -1654,6 +1654,7 @@ void dp_bridge_enable(struct drm_bridge *drm_bridge)
 	int rc = 0;
 	struct dp_display_private *dp_display;
 	u32 state;
+	bool force_link_train = false;
 
 	dp_display = container_of(dp, struct dp_display_private, dp_display);
 	if (!dp_display->dp_mode.drm_mode.clock) {
@@ -1688,10 +1689,12 @@ void dp_bridge_enable(struct drm_bridge *drm_bridge)
 
 	state =  dp_display->hpd_state;
 
-	if (state == ST_DISPLAY_OFF)
+	if (state == ST_DISPLAY_OFF) {
 		dp_display_host_phy_init(dp_display);
+		force_link_train = true;
+	}
 
-	dp_display_enable(dp_display, 0);
+	dp_display_enable(dp_display, force_link_train);
 
 	rc = dp_display_post_enable(dp);
 	if (rc) {
@@ -1700,10 +1703,6 @@ void dp_bridge_enable(struct drm_bridge *drm_bridge)
 		dp_display_unprepare(dp);
 	}
 
-	/* manual kick off plug event to train link */
-	if (state == ST_DISPLAY_OFF)
-		dp_add_event(dp_display, EV_IRQ_HPD_INT, 0, 0);
-
 	/* completed connection */
 	dp_display->hpd_state = ST_CONNECTED;
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v8 2/2] drm/msm/dp: clean up pixel_rate from dp_ctrl.c

[Kuogee Hsieh]

dp_ctrl keep an local cache of pixel_rate which increase confusing
in regrading how pixel_rate being used. This patch refer pixel_rate
directly from dp_panel to eliminate unnecessary pixel_rate variable
from struct dp_ctrl.

Changes in v8:
-- add this patch to remove pixel_rate from dp_ctrl

Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c | 158 +++++++++++++++++++--------------------
 drivers/gpu/drm/msm/dp/dp_ctrl.h |   2 -
 2 files changed, 79 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 01028b5..6fddddd 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1237,8 +1237,6 @@ static int dp_ctrl_link_train_2(struct dp_ctrl_private *ctrl,
 	return -ETIMEDOUT;
 }
 
-static int dp_ctrl_reinitialize_mainlink(struct dp_ctrl_private *ctrl);
-
 static int dp_ctrl_link_train(struct dp_ctrl_private *ctrl,
 			int *training_step)
 {
@@ -1337,7 +1335,8 @@ static void dp_ctrl_set_clock_rate(struct dp_ctrl_private *ctrl,
 				name, rate);
 }
 
-static int dp_ctrl_enable_mainlink_clocks(struct dp_ctrl_private *ctrl)
+static int dp_ctrl_enable_mainlink_clocks(struct dp_ctrl_private *ctrl,
+					unsigned long pixel_rate)
 {
 	int ret = 0;
 	struct dp_io *dp_io = &ctrl->parser->io;
@@ -1358,25 +1357,25 @@ static int dp_ctrl_enable_mainlink_clocks(struct dp_ctrl_private *ctrl)
 	if (ret)
 		DRM_ERROR("Unable to start link clocks. ret=%d\n", ret);
 
-	drm_dbg_dp(ctrl->drm_dev, "link rate=%d pixel_clk=%d\n",
-		ctrl->link->link_params.rate, ctrl->dp_ctrl.pixel_rate);
+	drm_dbg_dp(ctrl->drm_dev, "link rate=%d pixel_clk=%lu\n",
+		ctrl->link->link_params.rate, pixel_rate);
 
 	return ret;
 }
 
-static int dp_ctrl_enable_stream_clocks(struct dp_ctrl_private *ctrl)
+static int dp_ctrl_enable_stream_clocks(struct dp_ctrl_private *ctrl,
+					unsigned long pixel_rate)
 {
-	int ret = 0;
+	int ret;
 
-	dp_ctrl_set_clock_rate(ctrl, DP_STREAM_PM, "stream_pixel",
-					ctrl->dp_ctrl.pixel_rate * 1000);
+	dp_ctrl_set_clock_rate(ctrl, DP_STREAM_PM, "stream_pixel", pixel_rate * 1000);
 
 	ret = dp_power_clk_enable(ctrl->power, DP_STREAM_PM, true);
 	if (ret)
 		DRM_ERROR("Unabled to start pixel clocks. ret=%d\n", ret);
 
-	drm_dbg_dp(ctrl->drm_dev, "link rate=%d pixel_clk=%d\n",
-			ctrl->link->link_params.rate, ctrl->dp_ctrl.pixel_rate);
+	drm_dbg_dp(ctrl->drm_dev, "link rate=%d pixel_clk=%lu\n",
+			ctrl->link->link_params.rate, pixel_rate);
 
 	return ret;
 }
@@ -1441,7 +1440,8 @@ static bool dp_ctrl_use_fixed_nvid(struct dp_ctrl_private *ctrl)
 	return false;
 }
 
-static int dp_ctrl_reinitialize_mainlink(struct dp_ctrl_private *ctrl)
+static int dp_ctrl_reinitialize_mainlink(struct dp_ctrl_private *ctrl,
+					unsigned long pixel_rate)
 {
 	int ret = 0;
 	struct dp_io *dp_io = &ctrl->parser->io;
@@ -1465,7 +1465,7 @@ static int dp_ctrl_reinitialize_mainlink(struct dp_ctrl_private *ctrl)
 	/* hw recommended delay before re-enabling clocks */
 	msleep(20);
 
-	ret = dp_ctrl_enable_mainlink_clocks(ctrl);
+	ret = dp_ctrl_enable_mainlink_clocks(ctrl, pixel_rate);
 	if (ret) {
 		DRM_ERROR("Failed to enable mainlink clks. ret=%d\n", ret);
 		return ret;
@@ -1513,8 +1513,6 @@ static int dp_ctrl_link_maintenance(struct dp_ctrl_private *ctrl)
 	ctrl->link->phy_params.p_level = 0;
 	ctrl->link->phy_params.v_level = 0;
 
-	ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
-
 	ret = dp_ctrl_setup_main_link(ctrl, &training_step);
 	if (ret)
 		goto end;
@@ -1528,36 +1526,6 @@ static int dp_ctrl_link_maintenance(struct dp_ctrl_private *ctrl)
 	return ret;
 }
 
-static int dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl)
-{
-	int ret = 0;
-
-	if (!ctrl->link->phy_params.phy_test_pattern_sel) {
-		drm_dbg_dp(ctrl->drm_dev,
-			"no test pattern selected by sink\n");
-		return ret;
-	}
-
-	/*
-	 * The global reset will need DP link related clocks to be
-	 * running. Add the global reset just before disabling the
-	 * link clocks and core clocks.
-	 */
-	ret = dp_ctrl_off(&ctrl->dp_ctrl);
-	if (ret) {
-		DRM_ERROR("failed to disable DP controller\n");
-		return ret;
-	}
-
-	ret = dp_ctrl_on_link(&ctrl->dp_ctrl);
-	if (!ret)
-		ret = dp_ctrl_on_stream_phy_test_report(&ctrl->dp_ctrl);
-	else
-		DRM_ERROR("failed to enable DP link controller\n");
-
-	return ret;
-}
-
 static bool dp_ctrl_send_phy_test_pattern(struct dp_ctrl_private *ctrl)
 {
 	bool success = false;
@@ -1610,6 +1578,56 @@ static bool dp_ctrl_send_phy_test_pattern(struct dp_ctrl_private *ctrl)
 	return success;
 }
 
+int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl)
+{
+	int ret = 0;
+	struct dp_ctrl_private *ctrl;
+	unsigned long pixel_rate;
+
+	ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
+
+	pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
+	ret = dp_ctrl_enable_stream_clocks(ctrl, pixel_rate);
+	if (ret) {
+		DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
+		return ret;
+	}
+
+	dp_ctrl_send_phy_test_pattern(ctrl);
+
+	return 0;
+}
+
+static int dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl)
+{
+	int ret = 0;
+
+	if (!ctrl->link->phy_params.phy_test_pattern_sel) {
+		drm_dbg_dp(ctrl->drm_dev,
+			"no test pattern selected by sink\n");
+		return ret;
+	}
+
+	/*
+	 * The global reset will need DP link related clocks to be
+	 * running. Add the global reset just before disabling the
+	 * link clocks and core clocks.
+	 */
+	ret = dp_ctrl_off(&ctrl->dp_ctrl);
+	if (ret) {
+		DRM_ERROR("failed to disable DP controller\n");
+		return ret;
+	}
+
+	ret = dp_ctrl_on_link(&ctrl->dp_ctrl);
+	if (!ret)
+		ret = dp_ctrl_on_stream_phy_test_report(&ctrl->dp_ctrl);
+	else
+		DRM_ERROR("failed to enable DP link controller\n");
+
+	return ret;
+}
+
 void dp_ctrl_handle_sink_request(struct dp_ctrl *dp_ctrl)
 {
 	struct dp_ctrl_private *ctrl;
@@ -1685,6 +1703,7 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
 	u32 const phy_cts_pixel_clk_khz = 148500;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 	unsigned int training_step;
+	unsigned long pixel_rate;
 
 	if (!dp_ctrl)
 		return -EINVAL;
@@ -1695,29 +1714,30 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
 
 	dp_power_clk_enable(ctrl->power, DP_CORE_PM, true);
 
+	pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
+
 	if (ctrl->link->sink_request & DP_TEST_LINK_PHY_TEST_PATTERN) {
 		drm_dbg_dp(ctrl->drm_dev,
 				"using phy test link parameters\n");
-		if (!ctrl->panel->dp_mode.drm_mode.clock)
-			ctrl->dp_ctrl.pixel_rate = phy_cts_pixel_clk_khz;
+		if (!pixel_rate)
+			pixel_rate = phy_cts_pixel_clk_khz;
 	} else {
 		ctrl->link->link_params.rate = rate;
 		ctrl->link->link_params.num_lanes =
 			ctrl->panel->link_info.num_lanes;
-		ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
 	}
 
-	drm_dbg_dp(ctrl->drm_dev, "rate=%d, num_lanes=%d, pixel_rate=%d\n",
+	drm_dbg_dp(ctrl->drm_dev, "rate=%d, num_lanes=%d, pixel_rate=%lu\n",
 		ctrl->link->link_params.rate, ctrl->link->link_params.num_lanes,
-		ctrl->dp_ctrl.pixel_rate);
+		pixel_rate);
 
 
-	rc = dp_ctrl_enable_mainlink_clocks(ctrl);
+	rc = dp_ctrl_enable_mainlink_clocks(ctrl, pixel_rate);
 	if (rc)
 		return rc;
 
 	while (--link_train_max_retries) {
-		rc = dp_ctrl_reinitialize_mainlink(ctrl);
+		rc = dp_ctrl_reinitialize_mainlink(ctrl, pixel_rate);
 		if (rc) {
 			DRM_ERROR("Failed to reinitialize mainlink. rc=%d\n",
 					rc);
@@ -1807,31 +1827,12 @@ static int dp_ctrl_link_retrain(struct dp_ctrl_private *ctrl)
 	return dp_ctrl_setup_main_link(ctrl, &training_step);
 }
 
-int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl)
-{
-	int ret;
-	struct dp_ctrl_private *ctrl;
-
-	ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
-
-	ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
-
-	ret = dp_ctrl_enable_stream_clocks(ctrl);
-	if (ret) {
-		DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
-		return ret;
-	}
-
-	dp_ctrl_send_phy_test_pattern(ctrl);
-
-	return 0;
-}
-
 int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train)
 {
 	int ret = 0;
 	bool mainlink_ready = false;
 	struct dp_ctrl_private *ctrl;
+	unsigned long pixel_rate;
 	unsigned long pixel_rate_orig;
 
 	if (!dp_ctrl)
@@ -1839,25 +1840,24 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train)
 
 	ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
 
-	ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
+	pixel_rate = pixel_rate_orig = ctrl->panel->dp_mode.drm_mode.clock;
 
-	pixel_rate_orig = ctrl->dp_ctrl.pixel_rate;
 	if (dp_ctrl->wide_bus_en)
-		ctrl->dp_ctrl.pixel_rate >>= 1;
+		pixel_rate >>= 1;
 
-	drm_dbg_dp(ctrl->drm_dev, "rate=%d, num_lanes=%d, pixel_rate=%d\n",
+	drm_dbg_dp(ctrl->drm_dev, "rate=%d, num_lanes=%d, pixel_rate=%lu\n",
 		ctrl->link->link_params.rate,
-		ctrl->link->link_params.num_lanes, ctrl->dp_ctrl.pixel_rate);
+		ctrl->link->link_params.num_lanes, pixel_rate);
 
 	if (!dp_power_clk_status(ctrl->power, DP_CTRL_PM)) { /* link clk is off */
-		ret = dp_ctrl_enable_mainlink_clocks(ctrl);
+		ret = dp_ctrl_enable_mainlink_clocks(ctrl, pixel_rate);
 		if (ret) {
 			DRM_ERROR("Failed to start link clocks. ret=%d\n", ret);
 			goto end;
 		}
 	}
 
-	ret = dp_ctrl_enable_stream_clocks(ctrl);
+	ret = dp_ctrl_enable_stream_clocks(ctrl, pixel_rate);
 	if (ret) {
 		DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
 		goto end;
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 9a39b00..9f29734 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -16,13 +16,11 @@
 struct dp_ctrl {
 	bool orientation;
 	atomic_t aborted;
-	u32 pixel_rate;
 	bool wide_bus_en;
 };
 
 int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl);
 int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train);
-int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl);
 int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl);
 int dp_ctrl_off_link(struct dp_ctrl *dp_ctrl);
 int dp_ctrl_off(struct dp_ctrl *dp_ctrl);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v8 2/2] drm/msm/dp: clean up pixel_rate from dp_ctrl.c

[Stephen Boyd]

Quoting Kuogee Hsieh (2022-06-16 10:09:21)
> dp_ctrl keep an local cache of pixel_rate which increase confusing
> in regrading how pixel_rate being used. This patch refer pixel_rate
> directly from dp_panel to eliminate unnecessary pixel_rate variable
> from struct dp_ctrl.
>
> Changes in v8:
> -- add this patch to remove pixel_rate from dp_ctrl
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

I can send a proper patch for this myself later.

> ---
>  drivers/gpu/drm/msm/dp/dp_ctrl.c | 158 +++++++++++++++++++--------------------
>  drivers/gpu/drm/msm/dp/dp_ctrl.h |   2 -
>  2 files changed, 79 insertions(+), 81 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 01028b5..6fddddd 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1528,36 +1526,6 @@ static int dp_ctrl_link_maintenance(struct dp_ctrl_private *ctrl)
>         return ret;
>  }
>
> -static int dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl)
> -{
> -       int ret = 0;
> -
> -       if (!ctrl->link->phy_params.phy_test_pattern_sel) {
> -               drm_dbg_dp(ctrl->drm_dev,
> -                       "no test pattern selected by sink\n");
> -               return ret;
> -       }
> -
> -       /*
> -        * The global reset will need DP link related clocks to be
> -        * running. Add the global reset just before disabling the
> -        * link clocks and core clocks.
> -        */
> -       ret = dp_ctrl_off(&ctrl->dp_ctrl);
> -       if (ret) {
> -               DRM_ERROR("failed to disable DP controller\n");
> -               return ret;
> -       }
> -
> -       ret = dp_ctrl_on_link(&ctrl->dp_ctrl);
> -       if (!ret)
> -               ret = dp_ctrl_on_stream_phy_test_report(&ctrl->dp_ctrl);
> -       else
> -               DRM_ERROR("failed to enable DP link controller\n");
> -
> -       return ret;
> -}
> -
>  static bool dp_ctrl_send_phy_test_pattern(struct dp_ctrl_private *ctrl)
>  {
>         bool success = false;
> @@ -1610,6 +1578,56 @@ static bool dp_ctrl_send_phy_test_pattern(struct dp_ctrl_private *ctrl)
>         return success;
>  }
>
> +int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl)
> +{
> +       int ret = 0;
> +       struct dp_ctrl_private *ctrl;
> +       unsigned long pixel_rate;
> +
> +       ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
> +
> +       pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
> +       ret = dp_ctrl_enable_stream_clocks(ctrl, pixel_rate);
> +       if (ret) {
> +               DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
> +               return ret;
> +       }
> +
> +       dp_ctrl_send_phy_test_pattern(ctrl);
> +
> +       return 0;
> +}
> +
> +static int dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl)
> +{
> +       int ret = 0;
> +
> +       if (!ctrl->link->phy_params.phy_test_pattern_sel) {
> +               drm_dbg_dp(ctrl->drm_dev,
> +                       "no test pattern selected by sink\n");
> +               return ret;
> +       }
> +
> +       /*
> +        * The global reset will need DP link related clocks to be
> +        * running. Add the global reset just before disabling the
> +        * link clocks and core clocks.
> +        */
> +       ret = dp_ctrl_off(&ctrl->dp_ctrl);
> +       if (ret) {
> +               DRM_ERROR("failed to disable DP controller\n");
> +               return ret;
> +       }
> +
> +       ret = dp_ctrl_on_link(&ctrl->dp_ctrl);
> +       if (!ret)
> +               ret = dp_ctrl_on_stream_phy_test_report(&ctrl->dp_ctrl);
> +       else
> +               DRM_ERROR("failed to enable DP link controller\n");
> +
> +       return ret;
> +}
> +
>  void dp_ctrl_handle_sink_request(struct dp_ctrl *dp_ctrl)
>  {
>         struct dp_ctrl_private *ctrl;

I'd prefer these hunks to be part of a different patch. Either squashed
into the previous patch, or after the previous patch to show that a
forward declaration isn't necessary, but helped minimize the diff of
that patch.

> @@ -1685,6 +1703,7 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
>         u32 const phy_cts_pixel_clk_khz = 148500;
>         u8 link_status[DP_LINK_STATUS_SIZE];
>         unsigned int training_step;
> +       unsigned long pixel_rate;
>
>         if (!dp_ctrl)
>                 return -EINVAL;

Re: [PATCH v8 1/2] drm/msm/dp: force link training for display resolution change

[Stephen Boyd]

Quoting Kuogee Hsieh (2022-06-16 10:09:20)
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> index 0745fde..9a39b00 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> @@ -21,7 +21,8 @@ struct dp_ctrl {
>  };
>
>  int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl);
> -int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl);
> +int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train);
> +int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl);

Instead of declaring it here how about forward declaring it in the C
file and making it static?

>  int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl);
>  int dp_ctrl_off_link(struct dp_ctrl *dp_ctrl);
>  int dp_ctrl_off(struct dp_ctrl *dp_ctrl);

Re: [PATCH v8 2/2] drm/msm/dp: clean up pixel_rate from dp_ctrl.c

[Kuogee Hsieh]

On 6/16/2022 1:07 PM, Stephen Boyd wrote:
> Quoting Kuogee Hsieh (2022-06-16 10:09:21)
>> dp_ctrl keep an local cache of pixel_rate which increase confusing
>> in regrading how pixel_rate being used. This patch refer pixel_rate
>> directly from dp_panel to eliminate unnecessary pixel_rate variable
>> from struct dp_ctrl.
>>
>> Changes in v8:
>> -- add this patch to remove pixel_rate from dp_ctrl
>>
>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> I can send a proper patch for this myself later.
ok, then I will drop this patch
>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_ctrl.c | 158 +++++++++++++++++++--------------------
>>   drivers/gpu/drm/msm/dp/dp_ctrl.h |   2 -
>>   2 files changed, 79 insertions(+), 81 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> index 01028b5..6fddddd 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> @@ -1528,36 +1526,6 @@ static int dp_ctrl_link_maintenance(struct dp_ctrl_private *ctrl)
>>          return ret;
>>   }
>>
>> -static int dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl)
>> -{
>> -       int ret = 0;
>> -
>> -       if (!ctrl->link->phy_params.phy_test_pattern_sel) {
>> -               drm_dbg_dp(ctrl->drm_dev,
>> -                       "no test pattern selected by sink\n");
>> -               return ret;
>> -       }
>> -
>> -       /*
>> -        * The global reset will need DP link related clocks to be
>> -        * running. Add the global reset just before disabling the
>> -        * link clocks and core clocks.
>> -        */
>> -       ret = dp_ctrl_off(&ctrl->dp_ctrl);
>> -       if (ret) {
>> -               DRM_ERROR("failed to disable DP controller\n");
>> -               return ret;
>> -       }
>> -
>> -       ret = dp_ctrl_on_link(&ctrl->dp_ctrl);
>> -       if (!ret)
>> -               ret = dp_ctrl_on_stream_phy_test_report(&ctrl->dp_ctrl);
>> -       else
>> -               DRM_ERROR("failed to enable DP link controller\n");
>> -
>> -       return ret;
>> -}
>> -
>>   static bool dp_ctrl_send_phy_test_pattern(struct dp_ctrl_private *ctrl)
>>   {
>>          bool success = false;
>> @@ -1610,6 +1578,56 @@ static bool dp_ctrl_send_phy_test_pattern(struct dp_ctrl_private *ctrl)
>>          return success;
>>   }
>>
>> +int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl)
>> +{
>> +       int ret = 0;
>> +       struct dp_ctrl_private *ctrl;
>> +       unsigned long pixel_rate;
>> +
>> +       ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
>> +
>> +       pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
>> +       ret = dp_ctrl_enable_stream_clocks(ctrl, pixel_rate);
>> +       if (ret) {
>> +               DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       dp_ctrl_send_phy_test_pattern(ctrl);
>> +
>> +       return 0;
>> +}
>> +
>> +static int dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl)
>> +{
>> +       int ret = 0;
>> +
>> +       if (!ctrl->link->phy_params.phy_test_pattern_sel) {
>> +               drm_dbg_dp(ctrl->drm_dev,
>> +                       "no test pattern selected by sink\n");
>> +               return ret;
>> +       }
>> +
>> +       /*
>> +        * The global reset will need DP link related clocks to be
>> +        * running. Add the global reset just before disabling the
>> +        * link clocks and core clocks.
>> +        */
>> +       ret = dp_ctrl_off(&ctrl->dp_ctrl);
>> +       if (ret) {
>> +               DRM_ERROR("failed to disable DP controller\n");
>> +               return ret;
>> +       }
>> +
>> +       ret = dp_ctrl_on_link(&ctrl->dp_ctrl);
>> +       if (!ret)
>> +               ret = dp_ctrl_on_stream_phy_test_report(&ctrl->dp_ctrl);
>> +       else
>> +               DRM_ERROR("failed to enable DP link controller\n");
>> +
>> +       return ret;
>> +}
>> +
>>   void dp_ctrl_handle_sink_request(struct dp_ctrl *dp_ctrl)
>>   {
>>          struct dp_ctrl_private *ctrl;
> I'd prefer these hunks to be part of a different patch. Either squashed
> into the previous patch, or after the previous patch to show that a
> forward declaration isn't necessary, but helped minimize the diff of
> that patch.
>
>> @@ -1685,6 +1703,7 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
>>          u32 const phy_cts_pixel_clk_khz = 148500;
>>          u8 link_status[DP_LINK_STATUS_SIZE];
>>          unsigned int training_step;
>> +       unsigned long pixel_rate;
>>
>>          if (!dp_ctrl)
>>                  return -EINVAL;

Re: [PATCH v8 2/2] drm/msm/dp: clean up pixel_rate from dp_ctrl.c

[kernel test robot]

Hi Kuogee,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm/drm-next]
[also build test WARNING on drm-exynos/exynos-drm-next drm-tip/drm-tip linus/master v5.19-rc2 next-20220616]
[cannot apply to drm-intel/for-linux-next tegra-drm/drm/tegra/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Kuogee-Hsieh/force-link-training-for-display-resolution-change/20220617-011110
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20220617/202206170505.2U1jLZVk-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/b04f0b39a03a9fc3728e9414157f9d5f0b8b2366
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Kuogee-Hsieh/force-link-training-for-display-resolution-change/20220617-011110
        git checkout b04f0b39a03a9fc3728e9414157f9d5f0b8b2366
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/gpu/drm/msm/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/msm/dp/dp_ctrl.c:1587:5: warning: no previous prototype for 'dp_ctrl_on_stream_phy_test_report' [-Wmissing-prototypes]
    1587 | int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl)
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/dp_ctrl_on_stream_phy_test_report +1587 drivers/gpu/drm/msm/dp/dp_ctrl.c

  1586	
> 1587	int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl)
  1588	{
  1589		int ret = 0;
  1590		struct dp_ctrl_private *ctrl;
  1591		unsigned long pixel_rate;
  1592	
  1593		ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
  1594	
  1595		pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
  1596		ret = dp_ctrl_enable_stream_clocks(ctrl, pixel_rate);
  1597		if (ret) {
  1598			DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
  1599			return ret;
  1600		}
  1601	
  1602		dp_ctrl_send_phy_test_pattern(ctrl);
  1603	
  1604		return 0;
  1605	}
  1606	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v8 2/2] drm/msm/dp: clean up pixel_rate from dp_ctrl.c

[kernel test robot]

Hi Kuogee,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm/drm-next]
[also build test WARNING on drm-exynos/exynos-drm-next]
[cannot apply to drm-intel/for-linux-next tegra-drm/drm/tegra/for-next airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Kuogee-Hsieh/force-link-training-for-display-resolution-change/20220617-011110
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20220617/202206172356.J5CB8zDf-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project f0e608de27b3d568000046eebf3712ab542979d6)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/b04f0b39a03a9fc3728e9414157f9d5f0b8b2366
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Kuogee-Hsieh/force-link-training-for-display-resolution-change/20220617-011110
        git checkout b04f0b39a03a9fc3728e9414157f9d5f0b8b2366
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/gpu/drm/msm/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/msm/dp/dp_ctrl.c:1587:5: warning: no previous prototype for function 'dp_ctrl_on_stream_phy_test_report' [-Wmissing-prototypes]
   int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl)
       ^
   drivers/gpu/drm/msm/dp/dp_ctrl.c:1587:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl)
   ^
   static 
   1 warning generated.


vim +/dp_ctrl_on_stream_phy_test_report +1587 drivers/gpu/drm/msm/dp/dp_ctrl.c

  1586	
> 1587	int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl)
  1588	{
  1589		int ret = 0;
  1590		struct dp_ctrl_private *ctrl;
  1591		unsigned long pixel_rate;
  1592	
  1593		ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
  1594	
  1595		pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
  1596		ret = dp_ctrl_enable_stream_clocks(ctrl, pixel_rate);
  1597		if (ret) {
  1598			DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
  1599			return ret;
  1600		}
  1601	
  1602		dp_ctrl_send_phy_test_pattern(ctrl);
  1603	
  1604		return 0;
  1605	}
  1606	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v8 1/2] drm/msm/dp: force link training for display resolution change

[Dmitry Baryshkov]

On 16/06/2022 20:09, Kuogee Hsieh wrote:
> Display resolution change is implemented through drm modeset. Older
> modeset (resolution) has to be disabled first before newer modeset
> (resolution) can be enabled. Display disable will turn off both
> pixel clock and main link clock so that main link have to be
> re-trained during display enable to have new video stream flow
> again. At current implementation, display enable function manually
> kicks up irq_hpd_handle which will read panel link status and start
> link training if link status is not in sync state.
> 
> However, there is rare case that a particular panel links status keep
> staying in sync for some period of time after main link had been shut
> down previously at display disabled. In this case, main link retraining
> will not be executed by irq_hdp_handle(). Hence video stream of newer
> display resolution will fail to be transmitted to panel due to main
> link is not in sync between host and panel.
> 
> This patch will bypass irq_hpd_handle() in favor of directly call
> dp_ctrl_on_stream() to always perform link training in regardless of
> main link status. So that no unexpected exception resolution change
> failure cases will happen. Also this implementation are more efficient
> than manual kicking off irq_hpd_handle function.
> 
> Changes in v2:
> -- set force_link_train flag on DP only (is_edp == false)
> 
> Changes in v3:
> -- revise commit  text
> -- add Fixes tag
> 
> Changes in v4:
> -- revise commit  text
> 
> Changes in v5:
> -- fix spelling at commit text
> 
> Changes in v6:
> -- split dp_ctrl_on_stream() for phy test case
> -- revise commit text for modeset
> 
> Changes in v7:
> -- drop 0 assignment at local variable (ret = 0)
> 
> Changes in v8:
> -- add patch to remove pixel_rate from dp_ctrl
> 
> Fixes: 62671d2ef24b ("drm/msm/dp: fixes wrong connection state caused by failure of link train")
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_ctrl.c    | 31 +++++++++++++++++++++++--------
>   drivers/gpu/drm/msm/dp/dp_ctrl.h    |  3 ++-
>   drivers/gpu/drm/msm/dp/dp_display.c | 13 ++++++-------
>   3 files changed, 31 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index af7a80c..01028b5 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1551,7 +1551,7 @@ static int dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl)
>   
>   	ret = dp_ctrl_on_link(&ctrl->dp_ctrl);
>   	if (!ret)
> -		ret = dp_ctrl_on_stream(&ctrl->dp_ctrl);
> +		ret = dp_ctrl_on_stream_phy_test_report(&ctrl->dp_ctrl);
>   	else
>   		DRM_ERROR("failed to enable DP link controller\n");
>   
> @@ -1807,7 +1807,27 @@ static int dp_ctrl_link_retrain(struct dp_ctrl_private *ctrl)
>   	return dp_ctrl_setup_main_link(ctrl, &training_step);
>   }
>   
> -int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
> +int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl)
> +{
> +	int ret;
> +	struct dp_ctrl_private *ctrl;
> +
> +	ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
> +
> +	ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;

Stephen has raised an interesting question. Comparing this to the 
dp_ctrl_on_stream(), he noticed that we do not halve the pixel clock 
here (if the wide bus is supported). So, the question is if this is 
correct or not.

> +
> +	ret = dp_ctrl_enable_stream_clocks(ctrl);
> +	if (ret) {
> +		DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
> +		return ret;
> +	}
> +
> +	dp_ctrl_send_phy_test_pattern(ctrl);
> +
> +	return 0;
> +}
> +
> +int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train)
>   {
>   	int ret = 0;
>   	bool mainlink_ready = false;
> @@ -1843,12 +1863,7 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
>   		goto end;
>   	}
>   
> -	if (ctrl->link->sink_request & DP_TEST_LINK_PHY_TEST_PATTERN) {
> -		dp_ctrl_send_phy_test_pattern(ctrl);
> -		return 0;
> -	}
> -
> -	if (!dp_ctrl_channel_eq_ok(ctrl))
> +	if (force_link_train || !dp_ctrl_channel_eq_ok(ctrl))
>   		dp_ctrl_link_retrain(ctrl);
>   
>   	/* stop txing train pattern to end link training */


-- 
With best wishes
Dmitry

Re: [PATCH v8 1/2] drm/msm/dp: force link training for display resolution change

[Kuogee Hsieh]

On 6/22/2022 12:26 AM, Dmitry Baryshkov wrote:
> On 16/06/2022 20:09, Kuogee Hsieh wrote:
>> Display resolution change is implemented through drm modeset. Older
>> modeset (resolution) has to be disabled first before newer modeset
>> (resolution) can be enabled. Display disable will turn off both
>> pixel clock and main link clock so that main link have to be
>> re-trained during display enable to have new video stream flow
>> again. At current implementation, display enable function manually
>> kicks up irq_hpd_handle which will read panel link status and start
>> link training if link status is not in sync state.
>>
>> However, there is rare case that a particular panel links status keep
>> staying in sync for some period of time after main link had been shut
>> down previously at display disabled. In this case, main link retraining
>> will not be executed by irq_hdp_handle(). Hence video stream of newer
>> display resolution will fail to be transmitted to panel due to main
>> link is not in sync between host and panel.
>>
>> This patch will bypass irq_hpd_handle() in favor of directly call
>> dp_ctrl_on_stream() to always perform link training in regardless of
>> main link status. So that no unexpected exception resolution change
>> failure cases will happen. Also this implementation are more efficient
>> than manual kicking off irq_hpd_handle function.
>>
>> Changes in v2:
>> -- set force_link_train flag on DP only (is_edp == false)
>>
>> Changes in v3:
>> -- revise commit  text
>> -- add Fixes tag
>>
>> Changes in v4:
>> -- revise commit  text
>>
>> Changes in v5:
>> -- fix spelling at commit text
>>
>> Changes in v6:
>> -- split dp_ctrl_on_stream() for phy test case
>> -- revise commit text for modeset
>>
>> Changes in v7:
>> -- drop 0 assignment at local variable (ret = 0)
>>
>> Changes in v8:
>> -- add patch to remove pixel_rate from dp_ctrl
>>
>> Fixes: 62671d2ef24b ("drm/msm/dp: fixes wrong connection state caused 
>> by failure of link train")
>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_ctrl.c    | 31 
>> +++++++++++++++++++++++--------
>>   drivers/gpu/drm/msm/dp/dp_ctrl.h    |  3 ++-
>>   drivers/gpu/drm/msm/dp/dp_display.c | 13 ++++++-------
>>   3 files changed, 31 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c 
>> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> index af7a80c..01028b5 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> @@ -1551,7 +1551,7 @@ static int 
>> dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl)
>>         ret = dp_ctrl_on_link(&ctrl->dp_ctrl);
>>       if (!ret)
>> -        ret = dp_ctrl_on_stream(&ctrl->dp_ctrl);
>> +        ret = dp_ctrl_on_stream_phy_test_report(&ctrl->dp_ctrl);
>>       else
>>           DRM_ERROR("failed to enable DP link controller\n");
>>   @@ -1807,7 +1807,27 @@ static int dp_ctrl_link_retrain(struct 
>> dp_ctrl_private *ctrl)
>>       return dp_ctrl_setup_main_link(ctrl, &training_step);
>>   }
>>   -int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
>> +int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl)
>> +{
>> +    int ret;
>> +    struct dp_ctrl_private *ctrl;
>> +
>> +    ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
>> +
>> +    ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
>
> Stephen has raised an interesting question. Comparing this to the 
> dp_ctrl_on_stream(), he noticed that we do not halve the pixel clock 
> here (if the wide bus is supported). So, the question is if this is 
> correct or not.
>
pixel is for video stream which has nothing to do phy test.

Therefore no half pixel clock rate required for phy test.

>> +
>> +    ret = dp_ctrl_enable_stream_clocks(ctrl);
>> +    if (ret) {
>> +        DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    dp_ctrl_send_phy_test_pattern(ctrl);
>> +
>> +    return 0;
>> +}
>> +
>> +int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train)
>>   {
>>       int ret = 0;
>>       bool mainlink_ready = false;
>> @@ -1843,12 +1863,7 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
>>           goto end;
>>       }
>>   -    if (ctrl->link->sink_request & DP_TEST_LINK_PHY_TEST_PATTERN) {
>> -        dp_ctrl_send_phy_test_pattern(ctrl);
>> -        return 0;
>> -    }
>> -
>> -    if (!dp_ctrl_channel_eq_ok(ctrl))
>> +    if (force_link_train || !dp_ctrl_channel_eq_ok(ctrl))
>>           dp_ctrl_link_retrain(ctrl);
>>         /* stop txing train pattern to end link training */
>
>