On Mon, Jun 29, 2026 at 10:14:34PM +0800, Yongxing Mou wrote:
> From: Abhinav Kumar <[email protected]>
> 
> 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.

Last two phrases are redundat. Drop the "Introduce..." one.

> 
> Signed-off-by: Abhinav Kumar <[email protected]>
> Signed-off-by: Yongxing Mou <[email protected]>
> ---
>  drivers/gpu/drm/msm/dp/dp_audio.c   |  2 +-
>  drivers/gpu/drm/msm/dp/dp_display.c | 28 +++++++++++++++++-----------
>  drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
>  3 files changed, 19 insertions(+), 13 deletions(-)
> 
> @@ -670,14 +670,10 @@ static int msm_dp_display_enable(struct 
> msm_dp_display_private *dp,
>       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, 
> msm_dp_display->mst_active);
> -     if (!rc)
> -             msm_dp_display->power_on = true;
> +
> +     msm_dp_display->active_stream_cnt++;

Current power_on prevents from calling msm_dp_ctrl_on_stream() several
times. How is it prevented after the change?

>  
>       return rc;
>  }
> @@ -726,14 +722,14 @@ 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)
> +     if (!msm_dp_display->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;
> +     msm_dp_display->active_stream_cnt--;
>  
>       drm_dbg_dp(dp->drm_dev, "sink count: %d\n", dp->link->sink_count);
>       return 0;
> @@ -850,10 +846,10 @@ void msm_dp_snapshot(struct msm_disp_state *disp_state, 
> struct msm_dp *dp)
>        * if we are reading registers we need the link clocks to be on
>        * however till DP cable is connected this will not happen as we
>        * do not know the resolution to power up with. Hence check the
> -      * power_on status before dumping DP registers to avoid crash due
> +      * active_stream_cnt status before dumping DP registers to avoid crash 
> due
>        * to unclocked access
>        */
> -     if (!dp->power_on)
> +     if (!dp->active_stream_cnt)
>               return;
>  
>       msm_disp_snapshot_add_block(disp_state, msm_dp_display->ahb_len,
> @@ -1535,6 +1531,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 (!msm_dp_display->active_stream_cnt) {
> +             drm_dbg_dp(dp->drm_dev, "no active streams\n");
> +             return;
> +     }

If you get here, there was at least one active stream, wasn't it?

> +
>       msm_dp_ctrl_push_vcpf(dp->ctrl, msm_dp_panel);
>       msm_dp_ctrl_mst_stream_channel_slot_setup(dp->ctrl);
>       msm_dp_ctrl_mst_send_act(dp->ctrl, msm_dp_panel);
> @@ -1555,6 +1556,11 @@ void msm_dp_display_unprepare(struct msm_dp 
> *msm_dp_display)
>  
>       dp = container_of(msm_dp_display, struct msm_dp_display_private, 
> msm_dp_display);
>  
> +     if (msm_dp_display->active_stream_cnt) {
> +             drm_dbg_dp(dp->drm_dev, "stream still active, return\n");
> +             return;

Why?

General comment. You are not just replacing power_on with
active_stream_cnt. You are also introducing sequence changes, but it is
not covered in the cover letter. It's not clear, why those changes are
legitimage or why they are required.

> +     }
> +
>       /* 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 55ffa22bb233..75dc40261723 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.h
> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
> @@ -18,7 +18,7 @@ struct msm_dp {
>       struct drm_bridge *next_bridge;
>       struct drm_bridge *bridge;
>       bool audio_enabled;
> -     bool power_on;
> +     u32 active_stream_cnt;
>       bool mst_active;
>       unsigned int connector_type;
>       bool is_edp;
> 
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

Reply via email to