Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/mediatek: DSI device is never resumed causing bus hangs --
commit 6c75df0d91773376b0709ec5e7caceb104ea2e16 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. Reviewed-by: CK Hu <[email protected]> Signed-off-by: AngeloGioacchino Del Regno <[email protected]> > 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] Will mtk_ddp_comp_power_on() actually resume the DSI device? Looking at mtk_crtc_atomic_enable() in drivers/gpu/drm/mediatek/mtk_crtc.c, mtk_ddp_comp_power_on() is only called for the first pipeline component: mtk_crtc_atomic_enable() { ... struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0]; ... ret = mtk_ddp_comp_power_on(comp); ... } Because the DSI device is at the end of the pipeline rather than the first component, it won't be resumed here. > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index 6f176515e2587..4355103725cf6 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -1734,6 +1734,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: High] Since PM runtime leaves the device in a suspended state by default, won't this cause bus hangs or system error crashes when accessing hardware registers? Furthermore, DRM atomic helpers call bridge atomic_pre_enable() callbacks before CRTC atomic_enable() callbacks. This means mtk_dsi_host_transfer() in drivers/gpu/drm/mediatek/mtk_dsi.c will attempt to write to hardware registers before any CRTC-level power-on is attempted: mtk_dsi_host_transfer() { ... dsi_mode = readl(dsi->regs + dsi->driver_data->reg_main[DSI_MODE_CTRL]); ... } Will this result in accessing a powered-off PM domain? > 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=11
