Re: [PATCH RESEND v5 01/25] drm/msm/dp: introduce stream_id for each DP panel

[Dmitry Baryshkov]

On Mon, Jun 29, 2026 at 10:14:22PM +0800, Yongxing Mou wrote:
> From: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> 
> With MST, each DP controller can handle multiple streams. There shall be
> one dp_panel for each stream but the dp_display object shall be shared
> among them. To represent this abstraction, create a stream_id for each DP

You are not creating IDs.

> panel which shall be dynamically assigned to actual stream IDs by the MST
> path. For SST, default this to stream 0.
> 
> In the MST path, panels are dynamically assigned to actual stream IDs at
> stream enable time by the MST layer.
> 
> Use the stream ID to control the pixel clock of that respective stream by
> extending the clock handles and state tracking of the DP pixel clock to
> an array of max supported streams. The maximum streams currently is 4.
> 
> Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> Signed-off-by: Yongxing Mou <yongxing.mou@xxxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/msm/dp/dp_ctrl.c    | 67 +++++++++++++++++++++++--------------
>  drivers/gpu/drm/msm/dp/dp_ctrl.h    |  2 +-
>  drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
>  drivers/gpu/drm/msm/dp/dp_panel.c   |  1 +
>  drivers/gpu/drm/msm/dp/dp_panel.h   | 11 ++++++
>  5 files changed, 55 insertions(+), 28 deletions(-)
> 
> @@ -2188,38 +2188,39 @@ static bool msm_dp_ctrl_send_phy_test_pattern(struct msm_dp_ctrl_private *ctrl)
>  	return success;
>  }
>  
> -static int msm_dp_ctrl_on_pixel_clk(struct msm_dp_ctrl_private *ctrl, unsigned long pixel_rate)
> +static int msm_dp_ctrl_on_pixel_clk(struct msm_dp_ctrl_private *ctrl, unsigned long pixel_rate,
> +				    enum msm_dp_stream_id stream_id)
>  {
>  	int ret;
>  
> -	ret = clk_set_rate(ctrl->pixel_clk, pixel_rate * 1000);
> +	ret = clk_set_rate(ctrl->pixel_clk[stream_id], pixel_rate * 1000);
>  	if (ret) {
>  		DRM_ERROR("Failed to set pixel clock rate. ret=%d\n", ret);
>  		return ret;
>  	}
>  
> -	if (WARN_ON_ONCE(ctrl->stream_clks_on))
> +	if (WARN_ON_ONCE(ctrl->stream_clks_on[stream_id]))
>  		return 0;

Is it an error, defensive coding, DT mismatch or something else? Why are
we warning the users _and_ returning success here?

>  
> -	ret = clk_prepare_enable(ctrl->pixel_clk);
> +	ret = clk_prepare_enable(ctrl->pixel_clk[stream_id]);
>  	if (ret) {
>  		DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
>  		return ret;
>  	}
> -	ctrl->stream_clks_on = true;
> +	ctrl->stream_clks_on[stream_id] = true;
>  
>  	return ret;
>  }
>  

-- 
With best wishes
Dmitry