The GPU has no system-sleep callbacks, only runtime PM ones. If it is runtime-active when the system suspends, genpd drops the power domain behind the driver's back and the first firmware operation after resume wedges its caller in an unkillable D state. Patch 4 is that fix, and is what went out as v1 and v2.
The v2 review showed that wiring the force helpers in raw was not safe as the driver stood. The cause is one design fault: the runtime PM callbacks use drm_dev_enter() as a "device lost" test and hold the SRCU section while waiting for the watchdog worker. That makes a lost device fail every later suspend and makes pm_runtime_force_suspend() deadlock against a worker that loses the device. Patch 1 fixes the callbacks, which lets patch 2 unplug first in pvr_remove() and lets patch 4 go back to v1's one-liner. Patch 3 fixes a watchdog-vs-teardown race found on the way. On the v2 review's remaining finding, that pvr_power_fw_disable() can return without re-arming the watchdog: after a failed runtime suspend the device is in runtime_error with a zero usage count, and the worker's pm_runtime_get_if_in_use() returns 0 in that state, so a re-armed watchdog could not act. No change made for it. All four were measured on mt8173 (Rogue GX6250), both arms from one build switched at runtime; the details are in the individual messages. Lockdep cannot report the patch 1 deadlock: cancel_delayed_work_sync() only touches the work's lockdep map when a worker is executing, so the srcu -> work edge is never recorded unless the cancel races a running worker. Signed-off-by: Ryan Brue <[email protected]> --- Changes in v3: - Restructured as a four-patch series. Instead of wrapping the force helpers to work around the runtime PM callbacks, 1/4 fixes the callbacks: they test pvr_dev->lost rather than drm_dev_enter(), and hold no SRCU section while waiting for the watchdog worker. That removes the [Critical] AB-BA deadlock and the [High] TOCTOU at their source, and 4/4 returns to v1's raw pm_runtime_force_suspend()/resume(). - 2/4 fixes the pre-existing remove() ordering the review reported, and can now put drm_dev_unplug() first. - 3/4 fixes a pre-existing watchdog-vs-teardown race found while reviewing the above. - The review's "watchdog not re-armed" finding is answered in the cover letter rather than patched; see above for why. - Added hardware evidence for both arms of every patch; v2 asserted the failure without showing it. - Link to v2: https://patch.msgid.link/20260909-rbrue-suez-upstreaming-pvr-suspend-gpu-system-sleep-v2-1-66a938e92...@gmail.com Changes in v2: - Don't wire pm_runtime_force_suspend/resume in directly. Once the GPU has been lost, drm_dev_enter() fails, pvr_power_device_suspend() returns -EIO, and the force helper hands that to the PM core, which aborts the system suspend -- permanently, since a lost device never becomes suspendable again. v1 would have traded a GPU that dies on suspend for a machine that cannot suspend at all. v2 wraps the helpers and skips the transition when the device is already unplugged. Caught by the Sashiko AI review bot. - Commit message: say why the wrappers exist, and note that pvr_power_fw_disable() leaves the watchdog cancelled on its error path -- pre-existing, but system sleep is a new way to reach it. - Cc'd Chen-Yu Tsai, Icenowy Zheng and YoungJoon Lee, who are testing this same GPU in the MT8173 powervr thread [1] and are the people best placed to say whether this reproduces on a Chromebook. That thread doesn't mention suspend at all. - Dropped [email protected] and [email protected], both of which bounced on v1. [1] https://lore.kernel.org/all/[email protected]/ - Link to v1: https://patch.msgid.link/20260909-rbrue-suez-upstreaming-pvr-suspend-gpu-system-sleep-v1-1-f45847bc9...@gmail.com --- Ryan Brue (4): drm/imagination: don't gate the runtime PM callbacks on drm_dev_enter() drm/imagination: unplug the device before tearing it down in pvr_remove() drm/imagination: stop the watchdog before tearing the device down drm/imagination: suspend the GPU for system sleep, not just runtime PM drivers/gpu/drm/imagination/pvr_drv.c | 17 ++++++++++--- drivers/gpu/drm/imagination/pvr_power.c | 44 +++++++++++++-------------------- 2 files changed, 31 insertions(+), 30 deletions(-) --- base-commit: df2908090cda368b01ff43709f51890076c56157 change-id: 20260909-rbrue-suez-upstreaming-pvr-suspend-gpu-system-sleep-bfb9dfd7007b Best regards, -- Ryan Brue <[email protected]>
