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

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

On 8/26/2025 1:28 AM, Dmitry Baryshkov wrote:
> On Mon, Aug 25, 2025 at 10:15:51PM +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, which makes it harder to support MST.
>>
>> This patch separates the configuration into two functions:
> 
> git grep "This patch" Documentation/process
> Got it. Will fix it next patch.
>> - msm_dp_ctrl_config_ctrl_link(): handles link-related fields
>> - msm_dp_ctrl_config_ctrl_streams(): handles stream-specific fields
>>
>> It also moves the link-related configuration out of
>> msm_dp_ctrl_configure_source_params().
> 
> Why? And it looks like a separate patch...
> 
Because the patches following msm_dp_ctrl_configure_source_params() will 
take a panel parameter, and lane_mapping and peripheral_flush seem to be 
more closely related to the link, they have been moved out.
>>
>> Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_ctrl.c | 49 +++++++++++++++++++++++++---------------
>>   1 file changed, 31 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> index e1ff4c6bb4f0eed2e1ff931f12ba891cf81feffb..45d6c9a7f7ddaa049443253cbf4c6fc5feda3177 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> @@ -380,26 +380,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);
>> +}
>> +
>> +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);
>> @@ -413,10 +428,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);
>>   }
>> @@ -439,10 +451,7 @@ static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl
>>   {
>>   	u32 colorimetry_cfg, test_bits_depth, misc_val;
>>   
>> -	msm_dp_ctrl_lane_mapping(ctrl);
>> -	msm_dp_setup_peripheral_flush(ctrl);
>> -
>> -	msm_dp_ctrl_config_ctrl(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);
>> @@ -1614,7 +1623,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;
>> @@ -2524,6 +2533,10 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl)
>>   	 */
>>   	reinit_completion(&ctrl->video_comp);
>>   
>> +	msm_dp_ctrl_lane_mapping(ctrl);
>> +	msm_dp_setup_peripheral_flush(ctrl);
>> +	msm_dp_ctrl_config_ctrl_link(ctrl);
>> +
>>   	msm_dp_ctrl_configure_source_params(ctrl);
>>   
>>   	msm_dp_ctrl_config_msa(ctrl,
>>
>> -- 
>> 2.34.1
>>
>