On Mon, Aug 25, 2025 at 10:16:10PM +0800, Yongxing Mou wrote:
> From: Abhinav Kumar <quic_abhin...@quicinc.com>
>
> For DP MST, the link clock and power domain resources stay on until
> both streams have been disabled OR we receive hotplug. Introduce an
> active_stream_cnt to track the number of active streams and necessary
> state handling. Replace the power_on variable with active_stream_cnt
> as power_on boolean works only for a single stream.
>
> Signed-off-by: Abhinav Kumar <quic_abhin...@quicinc.com>
> Signed-off-by: Yongxing Mou <yongxing....@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/dp/dp_audio.c | 2 +-
> drivers/gpu/drm/msm/dp/dp_display.c | 50
> ++++++++++++++++++++++++-------------
> drivers/gpu/drm/msm/dp/dp_display.h | 3 ++-
> 3 files changed, 36 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c
> b/drivers/gpu/drm/msm/dp/dp_audio.c
> index
> 41018e82efa10ec863eb4b60d8df66c23c432fa5..e84c8b8e83d9ec689c0d29e8ac69860a745a4877
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_audio.c
> +++ b/drivers/gpu/drm/msm/dp/dp_audio.c
> @@ -284,7 +284,7 @@ int msm_dp_audio_prepare(struct drm_bridge *bridge,
> * such cases check for connection status and bail out if not
> * connected.
> */
> - if (!msm_dp_display->power_on) {
> + if (!msm_dp_display_get_active_stream_cnt(msm_dp_display)) {
Why can't it be a part of the public structure?
> rc = -EINVAL;
> goto end;
> }
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index
> 0815973e6597492e09f33359d9777c0e8ce31e0d..e2e6b0ea2f9dbfe49a599ca19b1d205669365c4c
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -73,6 +73,8 @@ struct msm_dp_display_private {
>
> bool wide_bus_supported;
>
> + u32 active_stream_cnt;
> +
> struct msm_dp_audio *audio;
>
> void __iomem *ahb_base;
> @@ -175,6 +177,15 @@ static const struct of_device_id msm_dp_dt_match[] = {
> {}
> };
>
> +int msm_dp_display_get_active_stream_cnt(struct msm_dp *msm_dp_display)
> +{
> + struct msm_dp_display_private *dp;
> +
> + dp = container_of(msm_dp_display, struct msm_dp_display_private,
> msm_dp_display);
> +
> + return dp->active_stream_cnt;
> +}
> +
> static struct msm_dp_display_private *dev_get_dp_display_private(struct
> device *dev)
> {
> struct msm_dp *dp = dev_get_drvdata(dev);
> @@ -709,15 +720,17 @@ static int msm_dp_display_prepare(struct
> msm_dp_display_private *dp)
> if (dp->link->sink_count == 0)
> return rc;
>
> - if (msm_dp_display->link_ready && !msm_dp_display->power_on) {
> + if (msm_dp_display->link_ready && !dp->active_stream_cnt) {
> msm_dp_display_host_phy_init(dp);
> force_link_train = true;
> }
>
> - rc = msm_dp_ctrl_on_link(dp->ctrl, msm_dp_display->mst_active);
> - if (rc) {
> - DRM_ERROR("Failed link training (rc=%d)\n", rc);
> - msm_dp_display->connector->state->link_status =
> DRM_LINK_STATUS_BAD;
> + if (!dp->active_stream_cnt) {
Such changes need at least some explanation in the commit message or,
better, split to a separate patch.
> + rc = msm_dp_ctrl_on_link(dp->ctrl, msm_dp_display->mst_active);
> + if (rc) {
> + DRM_ERROR("Failed link training (rc=%d)\n", rc);
> + msm_dp_display->connector->state->link_status =
> DRM_LINK_STATUS_BAD;
> + }
> }
>
> rc = msm_dp_ctrl_prepare_stream_on(dp->ctrl, force_link_train);
> @@ -731,17 +744,10 @@ static int msm_dp_display_enable(struct
> msm_dp_display_private *dp,
> struct msm_dp_panel *msm_dp_panel)
> {
> int rc = 0;
> - struct msm_dp *msm_dp_display = &dp->msm_dp_display;
>
> drm_dbg_dp(dp->drm_dev, "sink_count=%d\n", dp->link->sink_count);
> - if (msm_dp_display->power_on) {
> - drm_dbg_dp(dp->drm_dev, "Link already setup, return\n");
> - return 0;
> - }
>
> rc = msm_dp_ctrl_on_stream(dp->ctrl, msm_dp_panel, dp->max_stream);
> - if (!rc)
> - msm_dp_display->power_on = true;
>
> return rc;
> }
> @@ -788,16 +794,14 @@ static void msm_dp_display_audio_notify_disable(struct
> msm_dp_display_private *d
> static int msm_dp_display_disable(struct msm_dp_display_private *dp,
> struct msm_dp_panel *msm_dp_panel)
> {
> - struct msm_dp *msm_dp_display = &dp->msm_dp_display;
> -
> - if (!msm_dp_display->power_on)
> + if (!dp->active_stream_cnt)
> return 0;
>
> msm_dp_panel_disable_vsc_sdp(msm_dp_panel);
>
> msm_dp_ctrl_off_pixel_clk(dp->ctrl, msm_dp_panel->stream_id);
>
> - msm_dp_display->power_on = false;
> + dp->active_stream_cnt--;
>
> drm_dbg_dp(dp->drm_dev, "sink count: %d\n", dp->link->sink_count);
> return 0;
> @@ -934,7 +938,7 @@ void msm_dp_snapshot(struct msm_disp_state *disp_state,
> struct msm_dp *dp)
> * power_on status before dumping DP registers to avoid crash due
> * to unclocked access
> */
> - if (!dp->power_on)
> + if (!msm_dp_display->active_stream_cnt)
> return;
>
> msm_disp_snapshot_add_block(disp_state, msm_dp_display->ahb_len,
> @@ -1543,6 +1547,8 @@ void msm_dp_display_enable_helper(struct msm_dp
> *msm_dp_display, struct msm_dp_p
> }
> }
>
> + dp->active_stream_cnt++;
This should be a part of the 'stream_on' function. Also, which locking
ensures that there is no concurrent modification of this var?
> +
> drm_dbg_dp(msm_dp_display->drm_dev, "type=%d Done\n",
> msm_dp_display->connector_type);
> }
>
> @@ -1564,6 +1570,11 @@ void msm_dp_display_disable_helper(struct msm_dp
> *msm_dp_display,
>
> dp = container_of(msm_dp_display, struct msm_dp_display_private,
> msm_dp_display);
>
> + if (!dp->active_stream_cnt) {
> + drm_dbg_dp(dp->drm_dev, "no active streams\n");
> + return;
> + }
> +
> msm_dp_ctrl_push_idle(dp->ctrl, msm_dp_panel);
> msm_dp_ctrl_mst_stream_channel_slot_setup(dp->ctrl,
> dp->max_stream);
> @@ -1590,6 +1601,11 @@ void msm_dp_display_unprepare(struct msm_dp
> *msm_dp_display)
> return;
> }
>
> + if (dp->active_stream_cnt) {
> + drm_dbg_dp(dp->drm_dev, "stream still active, return\n");
> + return;
> + }
> +
> /* dongle is still connected but sinks are disconnected */
> if (dp->link->sink_count == 0)
> msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true);
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
> b/drivers/gpu/drm/msm/dp/dp_display.h
> index
> 20b7ed735b3f428e894b82ae2756d0efcfa47624..b1ea027438d952c94f3ae80725c92e46c631bdb2
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.h
> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
> @@ -19,7 +19,6 @@ struct msm_dp {
> struct drm_bridge *bridge;
> bool link_ready;
> bool audio_enabled;
> - bool power_on;
> bool prepared;
> bool mst_active;
> unsigned int connector_type;
> @@ -64,4 +63,6 @@ void msm_dp_display_atomic_post_disable_helper(struct
> msm_dp *msm_dp_display,
>
> void msm_dp_display_unprepare(struct msm_dp *dp);
>
> +int msm_dp_display_get_active_stream_cnt(struct msm_dp *msm_dp_display);
> +
> #endif /* _DP_DISPLAY_H_ */
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
