Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The error code returned by `msm_dp_ctrl_on_pixel_clk()` is ignored in `msm_dp_ctrl_process_phy_test_request()`, allowing execution to proceed even if clock enablement fails. -- commit 63742fe8cd27d164a8457069750896b0e989b7f3 Author: Abhinav Kumar <[email protected]> drm/msm/dp: split dp_ctrl_off() into stream and link parts This commit splits dp_ctrl_off() into stream and link parts so that for MST cases we can control the link and pixel parts separately. > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c > b/drivers/gpu/drm/msm/dp/dp_ctrl.c > index 87c3a5517911f..90fba03de7f0b 100644 > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c [ ... ] > @@ -2240,7 +2240,8 @@ static int msm_dp_ctrl_process_phy_test_request(struct > msm_dp_ctrl_private *ctrl > * running. Add the global reset just before disabling the > * link clocks and core clocks. > */ > - msm_dp_ctrl_off(&ctrl->msm_dp_ctrl, panel); > + msm_dp_ctrl_off_pixel_clk(&ctrl->msm_dp_ctrl); > + msm_dp_ctrl_off_link(&ctrl->msm_dp_ctrl, panel); > > ret = msm_dp_ctrl_on_link(&ctrl->msm_dp_ctrl, panel); > if (ret) { [Severity: High] This isn't a bug introduced by this patch, but while looking at the rest of msm_dp_ctrl_process_phy_test_request(), the return value from enabling the pixel clock appears to be completely ignored: ret = msm_dp_ctrl_on_pixel_clk(ctrl, pixel_rate); msm_dp_ctrl_send_phy_test_pattern(ctrl); return 0; If msm_dp_ctrl_on_pixel_clk() fails to prepare and enable the clock, it returns a negative error code, but the code still continues execution. Does processing the PHY test pattern and commanding the DP controller without a successfully enabled stream clock leave the hardware in an undefined state? Should we be checking this return value and exiting early on failure? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
