Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The rcar_dsc driver does not deassert its hardware reset, leaving the DSC block permanently in reset. - [Medium] Mixing devm_drm_bridge_add with manual pm_runtime_disable creates a devres cleanup ordering bug and a use-after-disable window. --
commit ffeafc5aca0f12828e91ec48615ef59b3d5989f2 Author: Marek Vasut <[email protected]> arm64: dts: renesas: r8a779g0: Add DSC This commit adds the Renesas DSC Display Stream Compression bridge to the r8a779g0 device tree. The node is placed between the DU and DSI1 units to make the DSI1 pipeline functional. > --- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi > @@ -2298,6 +2298,35 @@ du_out_dsi0: endpoint { [ ... ] > + dsc: bridge@feb8d000 { > + compatible = "renesas,r8a779g0-dsc"; [Severity: Medium] Does the driver backing this device introduce a devres cleanup regression? Looking at rcar_dsc.c, rcar_dsc_probe() uses a managed function for the bridge but manually enables PM runtime: drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c:rcar_dsc_probe() { ... pm_runtime_enable(&pdev->dev); ret = devm_drm_bridge_add(dev, &dsc->bridge); ... } Then rcar_dsc_remove() manually disables PM runtime: drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c:rcar_dsc_remove() { pm_runtime_disable(&pdev->dev); } If the driver is unbound, rcar_dsc_remove() manually calls pm_runtime_disable(). Only after this returns do the devres handlers run to call drm_bridge_remove(). If a DRM atomic commit executes during this window, rcar_dsc_atomic_enable() will call pm_runtime_get_sync(). Because PM runtime is disabled, won't this fail, trigger a warning, and leave the hardware unpowered? Using devm_pm_runtime_enable() instead would guarantee the correct reverse cleanup order. > + reg = <0 0xfeb8d000 0 0x400>; > + interrupts = <GIC_SPI 559 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 2819>; > + power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>; > + resets = <&cpg 2819>; [Severity: High] Is there a regression here that leaves the DSC block permanently in reset? Looking at rcar_dsc_probe() in drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c, it doesn't appear to request or explicitly deassert the hardware reset line defined here. If the DSC block is left in its initial reset state, won't it fail to pass pixel data and break the display pipeline? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
