On 2025/6/9 20:59, Dmitry Baryshkov wrote:
Emm, sorry for forget this case.. Whern MST enabled, msm_dp_display_prepare() will be called from mst_bridge_atomic_pre_enable, that means, when second stream called this func, it already prepared, so we should skip here. so this condition will really hit in MST case..On Mon, Jun 09, 2025 at 08:21:22PM +0800, Yongxing Mou wrote:From: Abhinav Kumar <quic_abhin...@quicinc.com> dp_display_enable() currently re-trains the link if needed and then enables the pixel clock, programs the controller to start sending the pixel stream. Splite these two parts into prepare/enable APIs, to support MST bridges_enable insertetyposthe MST payloads funcs between enable stream_clks and programe register. Signed-off-by: Abhinav Kumar <quic_abhin...@quicinc.com> Signed-off-by: Yongxing Mou <quic_yong...@quicinc.com> --- drivers/gpu/drm/msm/dp/dp_ctrl.c | 57 +++++++++++++-------- drivers/gpu/drm/msm/dp/dp_ctrl.h | 3 +- drivers/gpu/drm/msm/dp/dp_display.c | 99 +++++++++++++++++++++++++++---------- drivers/gpu/drm/msm/dp/dp_display.h | 1 + 4 files changed, 111 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index a50bfafbb4ea85c114c958ea0ed24362a1f23136..1e13ca81b0155a37a4ed7a2e83c918293d703a37 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1980,40 +1980,61 @@ static int msm_dp_ctrl_link_retrain(struct msm_dp_ctrl_private *ctrl) return msm_dp_ctrl_setup_main_link(ctrl, &training_step); }-int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train)+int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train) { int ret = 0; - bool mainlink_ready = false; struct msm_dp_ctrl_private *ctrl; - unsigned long pixel_rate; - unsigned long pixel_rate_orig;if (!msm_dp_ctrl)return -EINVAL;ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl); - pixel_rate = pixel_rate_orig = ctrl->panel->msm_dp_mode.drm_mode.clock;- - if (msm_dp_ctrl->wide_bus_en || ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420) - pixel_rate >>= 1; - - 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, pixel_rate); + drm_dbg_dp(ctrl->drm_dev, "rate=%d, num_lanes=%d\n", + ctrl->link->link_params.rate, + ctrl->link->link_params.num_lanes);Please don't mix whitespace changes with the actual code changes. It makes reviewing the patch much harder.drm_dbg_dp(ctrl->drm_dev,- "core_clk_on=%d link_clk_on=%d stream_clk_on=%d\n", - ctrl->core_clks_on, ctrl->link_clks_on, ctrl->stream_clks_on); + "core_clk_on=%d link_clk_on=%d stream_clk_on=%d\n", + ctrl->core_clks_on, ctrl->link_clks_on, ctrl->stream_clks_on);if (!ctrl->link_clks_on) { /* link clk is off */ret = msm_dp_ctrl_enable_mainlink_clocks(ctrl); if (ret) { DRM_ERROR("Failed to start link clocks. ret=%d\n", ret); - goto end; + return ret; } }+ if (force_link_train || !msm_dp_ctrl_channel_eq_ok(ctrl))+ msm_dp_ctrl_link_retrain(ctrl); + + /* stop txing train pattern to end link training */ + msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX); + + return ret; +} + +int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl) +{ + int ret = 0; + bool mainlink_ready = false; + struct msm_dp_ctrl_private *ctrl; + unsigned long pixel_rate; + unsigned long pixel_rate_orig; + + if (!msm_dp_ctrl) + return -EINVAL; + + ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl); + + pixel_rate = pixel_rate_orig = ctrl->panel->msm_dp_mode.drm_mode.clock; + + if (msm_dp_ctrl->wide_bus_en || ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420) + pixel_rate >>= 1; + + drm_dbg_dp(ctrl->drm_dev, "pixel_rate=%lu\n", pixel_rate); + ret = clk_set_rate(ctrl->pixel_clk, pixel_rate * 1000); if (ret) { DRM_ERROR("Failed to set pixel clock rate. ret=%d\n", ret); @@ -2031,12 +2052,6 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train ctrl->stream_clks_on = true; }- if (force_link_train || !msm_dp_ctrl_channel_eq_ok(ctrl))- msm_dp_ctrl_link_retrain(ctrl); - - /* stop txing train pattern to end link training */ - msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX); - /* * Set up transfer unit values and set controller state to send * video. diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h index b7abfedbf5749c25877a0b8ba3af3d8ed4b23d67..42745c912adbad7221c78f5cecefa730bfda1e75 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h @@ -18,7 +18,8 @@ struct msm_dp_ctrl { struct phy;int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl);-int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train); +int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl); +int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *dp_ctrl, bool force_link_train); void msm_dp_ctrl_off_link_stream(struct msm_dp_ctrl *msm_dp_ctrl); void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl); void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl); diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 9d2db9cbd2552470a36a63f70f517c35436f7280..5ac5dcf35b789f2bda052a2c17aae20aa48d8e18 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -831,7 +831,37 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display, return 0; }-static int msm_dp_display_enable(struct msm_dp_display_private *dp, bool force_link_train)+static int msm_dp_display_prepare(struct msm_dp_display_private *dp) +{ + int rc = 0; + struct msm_dp *msm_dp_display = &dp->msm_dp_display; + bool force_link_train = false; + + drm_dbg_dp(dp->drm_dev, "sink_count=%d\n", dp->link->sink_count); + if (msm_dp_display->prepared) { + drm_dbg_dp(dp->drm_dev, "Link already setup, return\n"); + return 0; + }How can it be prepared here? It is called at the beginning of the .atomic_enable() only, so there is no way this can be true.
+ + rc = pm_runtime_resume_and_get(&msm_dp_display->pdev->dev); + if (rc) { + DRM_ERROR("failed to pm_runtime_resume\n"); + return rc; + } + + if (dp->hpd_state == ST_CONNECTED && !msm_dp_display->power_on) { + msm_dp_display_host_phy_init(dp); + force_link_train = true; + } + + rc = msm_dp_ctrl_prepare_stream_on(dp->ctrl, force_link_train); + if (!rc) + msm_dp_display->prepared = true; + + return rc; +} + +static int msm_dp_display_enable(struct msm_dp_display_private *dp) { int rc = 0; struct msm_dp *msm_dp_display = &dp->msm_dp_display; @@ -842,7 +872,7 @@ static int msm_dp_display_enable(struct msm_dp_display_private *dp, bool force_l return 0; }- rc = msm_dp_ctrl_on_stream(dp->ctrl, force_link_train);+ rc = msm_dp_ctrl_on_stream(dp->ctrl); if (!rc) msm_dp_display->power_on = true;@@ -872,13 +902,10 @@ static int msm_dp_display_post_enable(struct msm_dp *msm_dp_display)return 0; }-static int msm_dp_display_disable(struct msm_dp_display_private *dp)+static void msm_dp_display_audio_notify_disable(struct msm_dp_display_private *dp) { struct msm_dp *msm_dp_display = &dp->msm_dp_display;- if (!msm_dp_display->power_on)- return 0; - /* wait only if audio was enabled */ if (msm_dp_display->audio_enabled) { /* signal the disconnect event */ @@ -889,6 +916,14 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp) }msm_dp_display->audio_enabled = false;+} + +static int msm_dp_display_disable(struct msm_dp_display_private *dp) +{ + struct msm_dp *msm_dp_display = &dp->msm_dp_display; + + if (!msm_dp_display->power_on) + return 0;if (dp->link->sink_count == 0) {/* @@ -1506,9 +1541,8 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge, struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(drm_bridge); struct msm_dp *dp = msm_dp_bridge->msm_dp_display; int rc = 0; + struct msm_dp_display_private *msm_dp_display; - u32 hpd_state; - bool force_link_train = false;msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display); @@ -1516,29 +1550,23 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,msm_dp_hpd_plug_handle(msm_dp_display, 0);mutex_lock(&msm_dp_display->event_mutex);- if (pm_runtime_resume_and_get(&dp->pdev->dev)) { - DRM_ERROR("failed to pm_runtime_resume\n"); - mutex_unlock(&msm_dp_display->event_mutex); - return; - }- hpd_state = msm_dp_display->hpd_state;- if (hpd_state == ST_DISCONNECT_PENDING) { + rc = msm_dp_display_prepare(msm_dp_display); + if (rc) { + DRM_ERROR("DP display prepare failed, rc=%d\n", rc); mutex_unlock(&msm_dp_display->event_mutex); return; }- if (hpd_state == ST_CONNECTED && !dp->power_on) {- msm_dp_display_host_phy_init(msm_dp_display); - force_link_train = true; - } - - msm_dp_display_enable(msm_dp_display, force_link_train); - - rc = msm_dp_display_post_enable(dp); - if (rc) { - DRM_ERROR("DP display post enable failed, rc=%d\n", rc); - msm_dp_display_disable(msm_dp_display); + if (dp->prepared) { + rc = msm_dp_display_enable(msm_dp_display); + if (rc) + DRM_ERROR("DP display enable failed, rc=%d\n", rc); + rc = msm_dp_display_post_enable(dp); + if (rc) { + DRM_ERROR("DP display post enable failed, rc=%d\n", rc); + msm_dp_display_disable(msm_dp_display); + } }/* completed connection */@@ -1560,6 +1588,20 @@ void msm_dp_bridge_atomic_disable(struct drm_bridge *drm_bridge, msm_dp_ctrl_push_idle(msm_dp_display->ctrl); }+static void msm_dp_display_unprepare(struct msm_dp_display_private *dp)+{ + struct msm_dp *msm_dp_display = &dp->msm_dp_display; + + if (!msm_dp_display->prepared) { + drm_dbg_dp(dp->drm_dev, "Link already setup, return\n"); + return; + }Why/ how is it possible?+ + pm_runtime_put_sync(&msm_dp_display->pdev->dev); + + msm_dp_display->prepared = false; +} + void msm_dp_bridge_atomic_post_disable(struct drm_bridge *drm_bridge, struct drm_atomic_state *state) { @@ -1580,6 +1622,8 @@ void msm_dp_bridge_atomic_post_disable(struct drm_bridge *drm_bridge, drm_dbg_dp(dp->drm_dev, "type=%d wrong hpd_state=%d\n", dp->connector_type, hpd_state);+ msm_dp_display_audio_notify_disable(msm_dp_display);+ msm_dp_display_disable(msm_dp_display);hpd_state = msm_dp_display->hpd_state;@@ -1588,9 +1632,10 @@ void msm_dp_bridge_atomic_post_disable(struct drm_bridge *drm_bridge, msm_dp_display->hpd_state = ST_DISCONNECTED; }+ msm_dp_display_unprepare(msm_dp_display);+ drm_dbg_dp(dp->drm_dev, "type=%d Done\n", dp->connector_type);- pm_runtime_put_sync(&dp->pdev->dev);mutex_unlock(&msm_dp_display->event_mutex); }diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.hindex cc6e2cab36e9c0b1527ff292e547cbb4d69fd95c..2394840e9f28e136705004c3e6af93fbe13c33c5 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.h +++ b/drivers/gpu/drm/msm/dp/dp_display.h @@ -19,6 +19,7 @@ struct msm_dp { bool link_ready; bool audio_enabled; bool power_on; + bool prepared; unsigned int connector_type; bool is_edp; bool internal_hpd; -- 2.34.1
