Re: [PATCH v4 05/39] drm/msm/dp: splite msm_dp_ctrl_config_ctrl() into link parts and stream parts

[Yongxing Mou <yongxing.mou@oss.qualcomm.com>]

On 5/20/2026 5:23 PM, Dmitry Baryshkov wrote:
> On Tue, May 19, 2026 at 03:46:36PM +0800, Yongxing Mou wrote:
>>
>>
>> On 4/12/2026 1:34 AM, Dmitry Baryshkov wrote:
>>> On Fri, Apr 10, 2026 at 05:33:40PM +0800, Yongxing Mou wrote:
>>>> The DP_CONFIGURATION_CTRL register contains both link-level and
>>>> stream-specific fields. Currently, msm_dp_ctrl_config_ctrl() configures
>>>> all of them together. Separates the configuration into link parts and
>>>> streams part for support MST.
>>>>
>>>> Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
>>>> ---
>>>>    drivers/gpu/drm/msm/dp/dp_ctrl.c | 43 ++++++++++++++++++++++++++--------------
>>>>    1 file changed, 28 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>>> index 476346e3ac19..85315467b5d0 100644
>>>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>>> @@ -388,26 +388,41 @@ void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl)
>>>>    	drm_dbg_dp(ctrl->drm_dev, "mainlink off\n");
>>>>    }
>>>> -static void msm_dp_ctrl_config_ctrl(struct msm_dp_ctrl_private *ctrl)
>>>> +static void msm_dp_ctrl_config_ctrl_streams(struct msm_dp_ctrl_private *ctrl,
>>>> +					    struct msm_dp_panel *msm_dp_panel)
>>>>    {
>>>>    	u32 config = 0, tbd;
>>>> +
>>>> +	config = msm_dp_read_link(ctrl, REG_DP_CONFIGURATION_CTRL);
>>>> +
>>>> +	if (msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420)
>>>> +		config |= DP_CONFIGURATION_CTRL_RGB_YUV; /* YUV420 */
>>>> +
>>>> +	tbd = msm_dp_link_get_test_bits_depth(ctrl->link,
>>>> +					      msm_dp_panel->msm_dp_mode.bpp);
>>>> +
>>>> +	config |= tbd << DP_CONFIGURATION_CTRL_BPC_SHIFT;
>>>> +
>>>> +	if (msm_dp_panel->psr_cap.version)
>>>> +		config |= DP_CONFIGURATION_CTRL_SEND_VSC;
>>>> +
>>>> +	drm_dbg_dp(ctrl->drm_dev, "stream DP_CONFIGURATION_CTRL=0x%x\n", config);
>>>> +
>>>> +	msm_dp_write_link(ctrl, REG_DP_CONFIGURATION_CTRL, config);
>>>
>>> You have an RMW cycle here. Please document what prevents it from racing
>>> with the concurrent msm_dp_ctrl_config_ctrl_link().
>>>
>> Here protected by mst_lock in MST case. Will add a comment.
> 
> And in SST case?
>
Emm, do you think this comments fine?

   /*
    * RMW: in SST, config_ctrl_link and config_ctrl_streams are called
    * sequentially on the same thread. In MST, caller holds mst_lock.
    */

>>>> +}
>>>> +
>>>> +static void msm_dp_ctrl_config_ctrl_link(struct msm_dp_ctrl_private *ctrl)
>>>> +{
>>>> +	u32 config = 0;
>>>>    	const u8 *dpcd = ctrl->panel->dpcd;
>>>>    	/* Default-> LSCLK DIV: 1/4 LCLK  */
>>>>    	config |= (2 << DP_CONFIGURATION_CTRL_LSCLK_DIV_SHIFT);
>>>> -	if (ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420)
>>>> -		config |= DP_CONFIGURATION_CTRL_RGB_YUV; /* YUV420 */
>>>> -
>>>>    	/* Scrambler reset enable */
>>>>    	if (drm_dp_alternate_scrambler_reset_cap(dpcd))
>>>>    		config |= DP_CONFIGURATION_CTRL_ASSR;
>>>> -	tbd = msm_dp_link_get_test_bits_depth(ctrl->link,
>>>> -			ctrl->panel->msm_dp_mode.bpp);
>>>> -
>>>> -	config |= tbd << DP_CONFIGURATION_CTRL_BPC_SHIFT;
>>>> -
>>>>    	/* Num of Lanes */
>>>>    	config |= ((ctrl->link->link_params.num_lanes - 1)
>>>>    			<< DP_CONFIGURATION_CTRL_NUM_OF_LANES_SHIFT);
>>>> @@ -421,10 +436,7 @@ static void msm_dp_ctrl_config_ctrl(struct msm_dp_ctrl_private *ctrl)
>>>>    	config |= DP_CONFIGURATION_CTRL_STATIC_DYNAMIC_CN;
>>>>    	config |= DP_CONFIGURATION_CTRL_SYNC_ASYNC_CLK;
>>>> -	if (ctrl->panel->psr_cap.version)
>>>> -		config |= DP_CONFIGURATION_CTRL_SEND_VSC;
>>>> -
>>>> -	drm_dbg_dp(ctrl->drm_dev, "DP_CONFIGURATION_CTRL=0x%x\n", config);
>>>> +	drm_dbg_dp(ctrl->drm_dev, "link DP_CONFIGURATION_CTRL=0x%x\n", config);
>>>>    	msm_dp_write_link(ctrl, REG_DP_CONFIGURATION_CTRL, config);
>>>>    }
>>>> @@ -450,7 +462,8 @@ static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl
>>>>    	msm_dp_ctrl_lane_mapping(ctrl);
>>>>    	msm_dp_setup_peripheral_flush(ctrl);
>>>> -	msm_dp_ctrl_config_ctrl(ctrl);
>>>> +	msm_dp_ctrl_config_ctrl_link(ctrl);
>>>> +	msm_dp_ctrl_config_ctrl_streams(ctrl, ctrl->panel);
>>>>    	test_bits_depth = msm_dp_link_get_test_bits_depth(ctrl->link, ctrl->panel->msm_dp_mode.bpp);
>>>>    	colorimetry_cfg = msm_dp_link_get_colorimetry_config(ctrl->link);
>>>> @@ -1628,7 +1641,7 @@ static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
>>>>    	u8 assr;
>>>>    	struct msm_dp_link_info link_info = {0};
>>>> -	msm_dp_ctrl_config_ctrl(ctrl);
>>>> +	msm_dp_ctrl_config_ctrl_link(ctrl);
>>>>    	link_info.num_lanes = ctrl->link->link_params.num_lanes;
>>>>    	link_info.rate = ctrl->link->link_params.rate;
>>>>
>>>> -- 
>>>> 2.43.0
>>>>
>>>
>>
>