Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] GPU hang recovery is permanently broken for older BCM2835 device trees that lack the `resets` property. - [Medium] Missing `pm_runtime_mark_last_busy()` in `vc4_v3d_pm_put()` causes premature device autosuspend. --
--- Patch [4]: [PATCH 4/4] ARM: dts: bcm2835: Add the V3D reset line --- commit 911a73ec97f6adb52d8c5867c354702414eb11dd Author: Maíra Canal <[email protected]> ARM: dts: bcm2835: Add the V3D reset line The PM block exposes a reset line for the V3D block, which the vc4 driver uses to reset the GPU when recovering from a hang instead of forcing its power domain down. Wire it up in the shared V3D node, covering BCM2835, BCM2836 and BCM2837. > diff --git a/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi > b/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi > index 1e76b290510d3..39eea8ce10583 100644 > --- a/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi > +++ b/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi > @@ -139,6 +139,7 @@ v3d: v3d@7ec00000 { > compatible = "brcm,bcm2835-v3d"; > reg = <0x7ec00000 0x1000>; > interrupts = <1 10>; > + resets = <&pm BCM2835_RESET_V3D>; [Severity: High] Does this change mean GPU hang recovery is permanently broken for older BCM2835 device trees that lack the resets property? Looking at vc4_reset() in drivers/gpu/drm/vc4/vc4_gem.c, it explicitly skips resetting the hardware if the reset line is absent: if (v3d->reset) { drm_info(dev, "Resetting GPU.\n"); ret = reset_control_reset(v3d->reset); if (ret) drm_err(dev, "Failed to reset the GPU: %d\n", ret); vc4_v3d_init_hw(dev); } else { drm_info_once(dev, "No reset line; GPU state is not reset.\n"); } Will this cause the system to be unable to recover from a GPU hang on older device trees, making the GPU effectively unusable after the first hang, and breaking Device Tree backward compatibility guarantees? [Severity: Medium] Also, does the driver suffer from a performance regression due to premature device autosuspend? Looking at vc4_v3d_pm_put() in drivers/gpu/drm/vc4/vc4_v3d.c: void vc4_v3d_pm_put(struct vc4_dev *vc4) { if (WARN_ON_ONCE(vc4->gen > VC4_GEN_4)) return; pm_runtime_put_autosuspend(&vc4->v3d->pdev->dev); } Because pm_runtime_mark_last_busy() is not called before dropping the reference, the autosuspend delay is calculated from the resume time, not the idle time. If the job took longer than the autosuspend delay, could the device suspend immediately upon dropping the reference, causing power-cycling overhead between back-to-back jobs? > }; > > }; -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
