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