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

[Yongxing Mou]

On 7/12/2026 7:11 PM, Dmitry Baryshkov wrote:
> 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.
Got it. Will modify it next patchset.
> > 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?
This is primarily defensive code. The normal execution flow is not 
expected to reach this path, so we only print a warning 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;
> >   }
> >