Re: [PATCH 2/2] drm/v3d: Idle AXI transactions before disabling the clock on suspend

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Hardware quiescence timeouts are ignored, allowing the driver to cut the clock while transactions are still in flight.
- [High] Direct assignment to the V3D_GMP_CFG register in v3d_idle_axi() clobbers existing bits, breaking the newly introduced read-modify-write undo logic.
--

--- Patch [2]: [PATCH 2/2] drm/v3d: Idle AXI transactions before disabling the clock on suspend ---
commit 586b3ea31f7f24c40a5d3a66c4418d7273686a72
Author: Maíra Canal <mcanal@igalia.com>

drm/v3d: Idle AXI transactions before disabling the clock on suspend

Currently, v3d_power_suspend() removes the GPU clock without first
quiescing the GPU's memory interface (AXI). If the clock is cut while the
core still has outstanding AXI transactions in flight, the hardware is
frozen mid-transaction. That corrupted state survives the power cycle, and
the first job submitted after the next resume will cause a GPU hang.

Link: https://github.com/raspberrypi/linux/issues/7443
Link: https://github.com/raspberrypi/linux/issues/7488

> diff --git a/drivers/gpu/drm/v3d/v3d_power.c b/drivers/gpu/drm/v3d/v3d_power.c
> index ade8e932fb9c5..f608e6d5516a1 100644
> --- a/drivers/gpu/drm/v3d/v3d_power.c
> +++ b/drivers/gpu/drm/v3d/v3d_power.c
> @@ -56,8 +56,15 @@ int v3d_power_suspend(struct device *dev)
>  
>  	v3d_clean_caches(v3d);
>  
> +	/* Wait until V3D has no active or pending AXI transactions. */
> +	v3d_idle_axi(v3d, 0);
> +	v3d_idle_gca(v3d);
> +

[Severity: High]
If v3d_idle_axi() times out, it logs an error but returns void:

v3d_idle_axi() {
    ...
    if (wait_for(... == 0, 100)) {
        drm_err(&v3d->drm, "Failed to wait for safe GMP shutdown\n");
    }
}

Does this allow the suspend operation to blindly proceed and disable the clock
while AXI transactions are still in flight?

>  	ret = v3d_suspend_sms(v3d);
>  	if (ret) {
> +		/* Staying active: undo the GMP STOP_REQ from v3d_idle_axi(). */
> +		V3D_WRITE(V3D_GMP_CFG(v3d->ver),
> +			  V3D_READ(V3D_GMP_CFG(v3d->ver)) & ~V3D_GMP_CFG_STOP_REQ);

[Severity: High]
Since v3d_idle_axi() uses direct assignment to set V3D_GMP_CFG_STOP_REQ:

v3d_idle_axi() {
    ...
    V3D_WRITE(V3D_GMP_CFG(v3d->ver), V3D_GMP_CFG_STOP_REQ);
}

Does this clobber other bits in the register, making the read-modify-write
undo logic here write 0 and fail to restore the hardware's original
configuration?

>  		v3d_irq_enable(v3d);
>  		return ret;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718-v3d-pm-axi-transactions-v1-0-4ecd7729ed70@igalia.com?part=2