Re: [PATCH RFC] drm/msm/mdss: keep mdp1-mem interconnect alive during suspend on SDM845

[sashiko-bot@kernel.org]

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 <me@samcday.com>

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-608a38de3715@samcday.com?part=1