Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The DRM bridge remains registered and globally accessible after `pm_runtime_disable()` is called, creating a teardown order race during driver removal. - [Medium] The sleeping function `pm_runtime_get_sync()` is called from an atomic context path (`rcar_dsc_atomic_enable`). --
commit a8616c71b764dc9fa4ae244e47795371e7fead44 Author: Marek Vasut <[email protected]> drm/rcar-du: dsc: Add rudimentary Renesas R-Car V4H DSC driver The Renesas DSC Display Stream Compression is a bridge embedded in the Renesas R-Car V4H SoC. The bridge performs VESA DSC encoding of up to 8k or 400 Mpixel/s. Add rudimentary driver, which currently acts as a pass-through bridge and allows DSI1 to be operational on R-Car V4H. > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c > b/drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c > new file mode 100644 > --- /dev/null > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c [ ... ] > +static void rcar_dsc_atomic_enable(struct drm_bridge *bridge, > + struct drm_atomic_commit *commit) > +{ > + struct rcar_dsc *dsc = bridge_to_rcar_dsc(bridge); > + > + WARN_ON(pm_runtime_get_sync(dsc->dev) < 0); [Severity: Medium] Is it safe to call the sleeping function pm_runtime_get_sync() here? Since rcar_dsc_atomic_enable() is called during non-blocking atomic commits, doesn't this path execute in atomic context where sleeping is forbidden and can lead to kernel warnings or deadlocks? > +} [ ... ] > +static int rcar_dsc_probe(struct platform_device *pdev) > +{ [ ... ] > + pm_runtime_enable(&pdev->dev); > + > + ret = devm_drm_bridge_add(dev, &dsc->bridge); > + if (ret) > + goto err_runtime_disable; > + > + return 0; > + > +err_runtime_disable: > + pm_runtime_disable(&pdev->dev); > + > + return ret; > +} > + > +static void rcar_dsc_remove(struct platform_device *pdev) > +{ > + pm_runtime_disable(&pdev->dev); > +} [Severity: High] Does this create a teardown order race during driver removal? Because devm_drm_bridge_add() defers unregistration until after rcar_dsc_remove() returns, the bridge remains globally accessible to the DRM subsystem after pm_runtime_disable() has already shut down power management. If a concurrent modeset calls rcar_dsc_atomic_enable() during this window, won't pm_runtime_get_sync() return -EACCES and trigger the WARN_ON since power management is disabled? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
