Re: [PATCH v8 06/15] drm/msm/dp: split link setup from source params

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] msm_dp_ctrl_config_ctrl_streams() performs an incomplete Read-Modify-Write (RMW) on REG_DP_CONFIGURATION_CTRL without clearing stream-specific bitfields, leading to potential hardware state corruption.
--

--- Patch [6]: [PATCH v8 06/15] drm/msm/dp: split link setup from source params ---
commit ea5efedb46078b56862c196153fb89423c8cb660
Author: Yongxing Mou <yongxing.mou@oss.qualcomm.com>

drm/msm/dp: split link setup from source params

msm_dp_ctrl_configure_source_params() should only handle stream-related
configuration. Move the link setup out of it so MST can program link and
stream settings separately.

> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index ac24c958852b8..711579de899d8 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -482,10 +482,6 @@ static void msm_dp_ctrl_config_misc1_misc0(struct msm_dp_ctrl_private *ctrl,
>  
>  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_link(ctrl);
>  	msm_dp_ctrl_config_ctrl_streams(ctrl, ctrl->panel);

[Severity: Medium]
By splitting the link setup from the stream setup here, are we exposing an
incomplete read-modify-write in msm_dp_ctrl_config_ctrl_streams()?

When MST programs the link and stream settings separately,
msm_dp_ctrl_config_ctrl_streams() will be called independently. Looking at
msm_dp_ctrl_config_ctrl_streams():

drivers/gpu/drm/msm/dp/dp_ctrl.c:msm_dp_ctrl_config_ctrl_streams() {
    ...
    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;
    ...
}

Since this function reads the register and bitwise-ORs the new values
without clearing the existing stream-specific bitfields (like the BPC or
YUV bits), won't this cause bit accumulation and hardware state corruption
across multiple streams if the link isn't reset in between?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-dp_mstclean-v8-0-bea261bf4e7d@oss.qualcomm.com?part=6