Re: [PATCH 4/4] ARM: dts: bcm2835: Add the V3D reset line

[sashiko-bot@kernel.org]

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 <mcanal@igalia.com>

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/20260816-vc4-reset-control-v1-0-896444373716@igalia.com?part=4