Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] Enabling PM runtime for the DSI device causes its power domain to be turned off, but it is never properly resumed, and multiple hardware access paths lack PM runtime checks. This leads to bus faults, system crashes, or hard lockups on register access. --
--- Patch [9]: [PATCH v6 09/11] drm/mediatek: mtk_dsi: Enable PM Runtime on probe --- commit 7ae5fcbe79f5f5488a2a9688fd56572adf6bd2e9 Author: AngeloGioacchino Del Regno <[email protected]> drm/mediatek: mtk_dsi: Enable PM Runtime on probe This commit enables PM Runtime at the end of mtk_dsi_probe() to allow the DSI device's power domain to be managed dynamically. > 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 appears to only call mtk_ddp_comp_power_on() for the 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); ... } If the DSI device is at the end of the pipeline and bypassed here, wouldn't it remain powered off? > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index d2fdeaeb97bea..9f69b237c9116 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] Once PM runtime is enabled and the device can be powered off, does this introduce bus faults in paths that access hardware without PM runtime checks? For example, mtk_dsi_host_transfer() accesses registers directly without a preceding pm_runtime_resume_and_get(): drivers/gpu/drm/mediatek/mtk_dsi.c:mtk_dsi_host_transfer() { ... dsi_mode = readl(dsi->regs + dsi->driver_data->reg_main[DSI_MODE_CTRL]); ... } If downstream panel drivers invoke this asynchronously during probe, could this access unpowered registers and trigger a bus fault? Similarly, if the interrupt handler triggers while the device is powered off, could it read ~0u from DSI_INTSTA and enter an infinite loop? drivers/gpu/drm/mediatek/mtk_dsi.c:mtk_dsi_irq() { ... status = readl(dsi->regs + dsi->driver_data->reg_main[DSI_INTSTA]) & flag; ... } Would it be necessary to use pm_runtime_get_if_active() in the interrupt handler and ensure other hardware access paths resume the device first? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
