Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/imx: dc-prg: unclocked MMIO and RPM underflow on dc_prg_configure() error --
commit dc9803b183fdde7606d3c14739f864581b99c2a3 Author: Liu Ying <[email protected]> drm/imx: dc: Add PRG support This commit adds a platform driver to support the Display Prefetch Resolve Gasket (PRG), which sits between a Display Prefetch Resolve Channel (DPRC) and a FetchUnit. > diff --git a/drivers/gpu/drm/imx/dc/dc-prg.c b/drivers/gpu/drm/imx/dc/dc-prg.c > new file mode 100644 > index 0000000000000..46108f7e09af3 > --- /dev/null > +++ b/drivers/gpu/drm/imx/dc/dc-prg.c [ ... ] > +void dc_prg_enable(struct dc_prg *prg) > +{ > + regmap_write(prg->reg, PRG_CTRL + CLR, BYPASS); > +} > + > +void dc_prg_disable(struct dc_prg *prg) > +{ > + regmap_write(prg->reg, PRG_CTRL, BYPASS); > + > + pm_runtime_put(prg->dev); > +} [ ... ] > +void dc_prg_configure(struct dc_prg *prg, > + unsigned int width, unsigned int height, > + unsigned int stride, unsigned int bits_per_pixel, > + dma_addr_t baddr, bool start) > +{ > + struct device *dev = prg->dev; > + unsigned int burst_size; > + u32 val; > + int ret; > + > + if (start) { > + ret = pm_runtime_resume_and_get(dev); > + if (ret < 0) { > + dev_err(dev, "failed to get RPM: %d\n", ret); > + return; > + } > + } [Severity: High] As reported by Sashiko on v7 [1], what happens if pm_runtime_resume_and_get() fails in dc_prg_configure() in drivers/gpu/drm/imx/dc/dc-prg.c when start is true? Because dc_prg_configure() returns void, the caller is not notified that the PRG device remains runtime-suspended with its clocks disabled. Subsequent calls to dc_prg_enable(), dc_prg_reg_update(), dc_prg_shadow_enable(), or later dc_prg_configure() updates with start set to false will perform MMIO writes to PRG registers while the hardware is unclocked and unpowered, which can trigger a bus abort on i.MX8 SoCs. Additionally, when the plane or CRTC is later disabled, dc_prg_disable() writes to PRG_CTRL while unclocked and unconditionally calls pm_runtime_put(prg->dev), underflowing the runtime PM usage counter on prg->dev. Should dc_prg_configure() return an error code or track the active state so callers do not access unclocked registers or drop an unacquired runtime PM reference? [1] https://lore.kernel.org/r/[email protected] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
