Re: [PATCH v5 09/11] drm/mediatek: mtk_dsi: Enable PM Runtime on probe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Enabling PM runtime causes a kernel panic because the DSI device is never resumed, leaving its power domain off during register access.
--

--- Patch [9]: [PATCH v5 09/11] drm/mediatek: mtk_dsi: Enable PM Runtime on probe ---
commit a4d6ef16908de6080f4fae3e792889c6f09b7a22
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

drm/mediatek: mtk_dsi: Enable PM Runtime on probe

This commit enables PM runtime for the Mediatek DSI device during probe.
It assumes the display pipeline's power-on functions will handle resuming
and suspending the DSI device to manage its power domains.

> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 4d8a02cbee385..dc0c4e867179c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -1733,6 +1733,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);
> +	if (ret)
> +		return ret;

[Severity: Critical]
Will this cause a kernel panic when the DSI registers are accessed later?

The commit message indicates that mtk_ddp_comp_power_on() will handle
resuming the DSI device. However, looking at the display initialization,
mtk_crtc_atomic_enable() only powers on the first component in the 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 ddp_comp[0] is typically an OVL or RDMA component rather than the DSI,
the DSI device is never resumed and remains in the RPM_SUSPENDED state
with its power domain off.

When mtk_dsi_poweron() is subsequently called, it attempts to access the
suspended hardware registers:

drivers/gpu/drm/mediatek/mtk_dsi.c:mtk_dsi_poweron() {
        ...
        mtk_dsi_enable(dsi);

        /* Bypass shadow and force commit only if the register is present */
        if (data->reg_adv[DSI_SHADOW_DEBUG])
                writel(FORCE_COMMIT | BYPASS_SHADOW,
                       dsi->regs + data->reg_adv[DSI_SHADOW_DEBUG]);

        mtk_dsi_reset_engine(dsi);
        mtk_dsi_phy_timconfig(dsi);
        ...
}

Does mtk_dsi_poweron() or an earlier path need to call
pm_runtime_resume_and_get() directly before accessing these registers to
prevent a synchronous external abort?

> +
>  	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/20260715114916.19690-1-angelogioacchino.delregno@collabora.com?part=9