Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Low] Unnecessary device tree string matching on every iteration due to missing short-circuit evaluation. Pre-existing issues: - [High] Silent failure on `-EPROBE_DEFER` for `mdp1-mem` prevents the SDM845 workaround from applying. -- commit 25610098a6bc164e41324a820ebd766373a58240 Author: Sam Day <[email protected]> drm/msm/mdss: keep mdp1-mem interconnect alive during suspend on SDM845 If the peak vote for mdp1-mem is allowed to drop to zero, it seems to cause the fabric to collapse that path entirely, which causes the device to bus stall and fatally reset. > diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c > index 9087c4b290dba..c635380b2ac3c 100644 > --- a/drivers/gpu/drm/msm/msm_mdss.c > +++ b/drivers/gpu/drm/msm/msm_mdss.c [ ... ] > @@ -284,8 +284,12 @@ static int msm_mdss_disable(struct msm_mdss *msm_mdss) > > clk_bulk_disable_unprepare(msm_mdss->num_clocks, msm_mdss->clocks); > > - for (i = 0; i < msm_mdss->num_mdp_paths; i++) > - icc_set_bw(msm_mdss->mdp_path[i], 0, 0); > + for (i = 0; i < msm_mdss->num_mdp_paths; i++) { > + if (of_device_is_compatible(msm_mdss->dev->of_node, > "qcom,sdm845-mdss") && i == 1) [Severity: Low] Does this code perform unnecessary device tree string matching on every iteration? In C, the logical AND operator evaluates left-to-right. Thus, of_device_is_compatible() is called on the first iteration (i == 0), despite i == 1 being false. Swapping the conditions to i == 1 && of_device_is_compatible(...) would properly short-circuit the expensive device tree string matching. [Severity: High] This isn't a bug introduced by this patch, but does the silent failure on -EPROBE_DEFER for mdp1-mem prevent this SDM845 workaround from applying? Looking at msm_mdss_parse_data_bus_icc_path(): drivers/gpu/drm/msm/msm_mdss.c:msm_mdss_parse_data_bus_icc_path() { ... path1 = devm_of_icc_get(dev, "mdp1-mem"); if (!IS_ERR_OR_NULL(path1)) { ... } During device probe, if the interconnect provider for mdp1-mem is not yet ready, devm_of_icc_get() returns ERR_PTR(-EPROBE_DEFER). The condition !IS_ERR_OR_NULL(path1) handles this by silently skipping mdp1-mem rather than returning -EPROBE_DEFER to defer the probe. If this happens on SDM845, num_mdp_paths remains 1, meaning the newly added workaround loop here in msm_mdss_disable() never reaches i == 1. Consequently, the vote for mdp1-mem drops to zero when the interconnect core syncs state, which triggers the bus stall and fatal reset that this patch is explicitly trying to fix. > + icc_set_bw(msm_mdss->mdp_path[i], 0, 1); > + else > + icc_set_bw(msm_mdss->mdp_path[i], 0, 0); > + } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260627-rfc-sdm845-interconnect-collapse-workaround-v1-1-608a38de3...@samcday.com?part=1
