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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing cleanup of DSI host registration on devm_pm_runtime_enable() failure.
- [High] The DSI device is never runtime-resumed by mtk_ddp_comp_power_on(), leaving its power domain off and causing hardware access faults.
- [High] devm_pm_runtime_enable() is called too late in the probe sequence, creating a race condition with DRM device binding.
--

--- Patch [8]: [PATCH v2 08/10] drm/mediatek: mtk_dsi: Enable PM Runtime on probe ---
commit a3706ccca44411c23c1950cb6ce7edfa10269c16
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.
    
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> 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.

[Severity: High]
Does mtk_ddp_comp_power_on() actually get called for the DSI device?

Looking at mtk_crtc_atomic_enable(), it 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 the DSI device is an encoder/bridge at the end of the pipeline (e.g.,
ddp_comp[6]), it won't be resumed here, leaving it in RPM_SUSPENDED state.
Will this cause hardware access faults when DSI registers are subsequently
accessed?

> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 5af44c13bd5ac..5fbaa7d580d35 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -1684,6 +1684,10 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>  	dsi->bridge.of_node = dev->of_node;
>  	dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
>  
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		return ret;

[Severity: High]
If this returns early, will it leak the DSI host registration?

Earlier in mtk_dsi_probe(), mipi_dsi_host_register(&dsi->host) adds the host
to a global list. Since there is no mipi_dsi_host_unregister() call on this
error path, and the devres-managed mtk_dsi structure will be freed, does this
leave a dangling pointer in the MIPI DSI host list that could lead to a
use-after-free?

[Severity: High]
Is it safe to enable PM runtime after registering the DSI host?

By calling mipi_dsi_host_register() earlier in the function, the DSI host
becomes visible to dependent panel drivers that may probe synchronously:

mtk_dsi_probe()
  mipi_dsi_host_register()
    -> panel probe triggers
      mipi_dsi_attach()
        mtk_dsi_host_attach()
          component_add()
            -> DRM master bind triggers if all components are ready

Since devm_pm_runtime_enable() hasn't executed yet, PM runtime isn't ready
when the display pipeline starts initializing. Would it be better to move
devm_pm_runtime_enable() before mipi_dsi_host_register() to avoid this race?

> +
>  	return 0;
>  }
>

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