Re: [PATCH v3 08/10] drm/mediatek: mtk_dsi: Enable PM Runtime on probe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing PM Runtime Resume / System Crash
- [Medium] Resource Leak / Async PM Runtime Race
--

--- Patch [8]: [PATCH v3 08/10] drm/mediatek: mtk_dsi: Enable PM Runtime on probe ---
commit b7932646865a258fb9d31cc5baae149e48665e6a
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

drm/mediatek: mtk_dsi: Enable PM Runtime on probe

Enable PM Runtime at the end of mtk_dsi_probe() so that the
mediatek-drm mtk_ddp_comp_power_on() function is able to do
pm_runtime_resume_and_get() and, analogously, the function
mtk_ddp_comp_power_off() to pm_runtime_put() for the DSI
device.

This makes sure that the power domains for the DSI IP are
turned on and off when necessary.

> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 573299f0e63b6..6e1482b201196 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -1716,6 +1716,10 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>  	if (ret)
>  		return dev_err_probe(&pdev->dev, ret, "Failed to request DSI irq\n");
>  
> +	ret = devm_pm_runtime_enable(dev);

[Severity: High]
Does this actually resume the DSI device when needed?

The commit message mentions that mtk_ddp_comp_power_on() will handle the
pm_runtime_resume_and_get() for the DSI device. However, looking at
mtk_crtc_atomic_enable(), mtk_ddp_comp_power_on() seems to only be called
for the very first component in the display pipeline:

drivers/gpu/drm/mediatek/mtk_crtc.c:mtk_crtc_atomic_enable() {
	...
	struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
	...
	ret = mtk_ddp_comp_power_on(comp);
	...
}

Since DSI is typically an encoder at the end of the pipeline, it appears
it is never passed to this function. Will this lead to the device remaining
in RPM_SUSPENDED, potentially causing synchronous external aborts (crashes)
when DSI hardware registers are accessed later in mtk_dsi_poweron() or
mtk_dsi_host_transfer()?

[Severity: Medium]
Can this lead to a PM domain reference leak during driver unbind?

The commit message notes that mtk_ddp_comp_power_off() handles the
pm_runtime_put() for the DSI device. Because pm_runtime_put() queues an
asynchronous suspend, if the display is disabled during module unload,
the CRTC disable path will queue this async suspend right before
mtk_dsi_remove() executes.

The devres cleanup registered here by devm_pm_runtime_enable() will then
call pm_runtime_disable(), which cancels all pending asynchronous PM work.
Does this leave the device in RPM_ACTIVE and leak the PM domain reference?

> +	if (ret)
> +		return ret;
> +
>  	ret = mipi_dsi_host_register(&dsi->host);
>  	if (ret < 0)
>  		return dev_err_probe(dev, ret, "Failed to register DSI host\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713130103.34654-1-angelogioacchino.delregno@collabora.com?part=8