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 in dc_prg_configure() --
commit d1c395ad8b2bd23da43b9ddd3c832ab32084d33e Author: Liu Ying <[email protected]> drm/imx: dc: Add PRG support This commit adds a platform driver for the Display Prefetch Resolve Gasket (PRG), which sits between a Display Prefetch Resolve Channel (DPRC) and a FetchUnit as part of the prefetch engine. > 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..b80ce5e641173 > --- /dev/null > +++ b/drivers/gpu/drm/imx/dc/dc-prg.c > @@ -0,0 +1,317 @@ [ ... ] > +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 v8 [1], in drivers/gpu/drm/imx/dc/dc-prg.c, what happens if pm_runtime_resume_and_get() fails in dc_prg_configure() when start is true? Because dc_prg_configure() returns void, the error is not propagated to the caller and the PRG device remains runtime-suspended with its clocks disabled in dc_prg_runtime_resume(). Callers that proceed after dc_prg_configure() will then call dc_prg_enable(), dc_prg_reg_update(), or dc_prg_shadow_enable(), performing MMIO writes to PRG_CTRL and PRG_REG_UPDATE while the PRG clocks are off. Subsequent updates with start set to false will also write to the PRG registers without clocks enabled. Furthermore, when the plane or CRTC is later disabled, dc_prg_disable() will write to PRG_CTRL while unclocked and call 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 runtime PM active state so subsequent register accesses and pm_runtime_put() are skipped when resume fails? [1] https://lore.kernel.org/r/[email protected] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
