Re: [PATCH v7 04/15] drm/msm/dp: split msm_dp_ctrl_config_ctrl() into link parts and stream parts

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Splitting `msm_dp_ctrl_config_ctrl()` causes stream configuration state to be permanently lost during dynamic link maintenance, leading to display corruption.
--

commit 83fda55d30fe1cd098ad8cc76e151a0398a09b6a
Author: Yongxing Mou <yongxing.mou@oss.qualcomm.com>

drm/msm/dp: split msm_dp_ctrl_config_ctrl() into link parts and stream parts
    
The DP_CONFIGURATION_CTRL register contains both link-level and
stream-specific fields. Currently, msm_dp_ctrl_config_ctrl() configures
all of them together. Separate the configuration into link parts and
stream parts to support MST.

> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 86ef8c89ad443..ed2ba47881fd0 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
[ ... ]
> +static void msm_dp_ctrl_config_ctrl_link(struct msm_dp_ctrl_private *ctrl)
> +{
> +	u32 config = 0;
[ ... ]
> -	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);
>  }
[ ... ]
> @@ -1628,7 +1644,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);

[Severity: High]
Does splitting this configuration cause the stream state to be permanently lost
during dynamic link maintenance?

Before this patch, msm_dp_ctrl_config_ctrl() programmed both link and stream
parameters. Now, msm_dp_ctrl_config_ctrl_link() initializes config = 0 and
overwrites REG_DP_CONFIGURATION_CTRL, which erases any existing stream
configuration.

While normal initialization restores stream parameters by calling
msm_dp_ctrl_configure_source_params() after link training, dynamic link
maintenance paths (like msm_dp_ctrl_link_maintenance() calling
msm_dp_ctrl_setup_main_link()) perform link training and then transition
directly to DP_STATE_CTRL_SEND_VIDEO without re-applying the stream
parameters:

msm_dp_ctrl_link_maintenance()
    ...
    ret = msm_dp_ctrl_setup_main_link(ctrl, &training_step);
    ...
    msm_dp_write_link(ctrl, REG_DP_STATE_CTRL, DP_STATE_CTRL_SEND_VIDEO);

Since msm_dp_ctrl_setup_main_link() eventually triggers
msm_dp_ctrl_link_train() which calls msm_dp_ctrl_config_ctrl_link(), will this
result in the DP stream coming back up with critical parameters cleared (e.g.,
0 BPC, no VSC, incorrect color format), breaking the display output?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-dp_mstclean-v7-0-ea04113e8233@oss.qualcomm.com?part=4