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 <[email protected]> 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/[email protected]?part=6
