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 <[email protected]> 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/[email protected]?part=8
