Hello everybody, > That HW is ancient!
It is — and still in daily service here, which is exactly why I care. :) > The question is really if that works reliable or if that could break anything? Fair frame, so let me answer it directly. 1) The success path is bit-identical to stock. When the first attempt works (cold boot, healthy resumes), the added code executes zero extra instructions — the loop breaks immediately. 2) The failure path cannot leave the system worse than stock. Today a failed ring test already ends in -EINVAL with UVD dead for the session. The patch only adds up to two more bounded attempts (~250 ms each: start, ring test, stop, 20 ms settle) and then falls through to the same failure state. The display rings (0/3) and the rest of the resume path never touch this code — and years of stock behavior prove the machine resumes fine with UVD dead. 3) The one real hazard — MMIO to an unclocked UVD block in the retry's stop path — was caught by the Sashiko review and is fixed in the [PATCH v3] already on the list: stop is now called only when clock setup succeeded. v3 also drops the warm-up cycle and the widened poll window I had in v2 — both reviews were right that they didn't belong; our measurements agree (neither prevented first-attempt failures). Measured on the target machine (HD 5650M, kernel 7.0.0): stock survives resume with working UVD in ~50% of 42 measured cycles; patched has been 12/12 on the test stand plus every daily resume since. If you prefer a tighter bound, I'm fine with 2 attempts instead of 3. > when resume doesn't work at all any more then that would be really bad Agreed — and that scenario is exactly what the v3 guard is about. Thanks, Oleg ср, 21 жовт. 2026 р. о 17:28 Christian König <[email protected]> пише: > > That HW is ancient! > > The question is really if that works reliable or if that could break anything? > > I mean when UVD doesn't work after resume is ugly, but when resume doesn't > work at all any more then that would be really bad. > > Regards, > Christian. > > On 10/21/26 15:29, Alex Deucher wrote: > > + Leo, Christian > > > > On Tue, Oct 20, 2026 at 3:46 AM ukrainer8 <[email protected]> wrote: > >> > >> On Evergreen/Redwood (and likely other UVD v1/v2 hardware), resume from > >> suspend intermittently fails the UVD ring test: > >> > >> radeon 0000:01:00.0: [drm] *ERROR* radeon: ring 5 test failed > >> (0xCAFEDEAD) > >> radeon 0000:01:00.0: failed initializing UVD (-22). > >> > >> Cold boot always succeeds; resume succeeds only in about half of the cycles > >> (42 measured suspend/resume cycles on a Radeon HD 5650M: OK counts per > >> 6-cycle run were 2, 3, 5, 6, 1, 5, 3). Once the ring test fails, UVD stays > >> dead for the rest of the session and hardware video decode silently falls > >> back to CPU. > >> > >> The resume path runs the same full asic startup as cold boot > >> (evergreen_resume() -> evergreen_startup()), so the difference is the > >> hardware state at entry, not a skipped call. Field data shows the first > >> ring instance after resume is dead functionally, not by timeout: the ring > >> test fails fast and deterministically, and only a full stop+init cycle > >> revives the engine. > >> > >> Fix this the way the Windows driver effectively does it: silently cycle > >> the engine once (clocks up, start, stop, 20 ms settle) before testing, > >> then run the init sequence in a retry loop - on failure of clock setup, > >> VCPU start or the ring test, fully stop UVD, let it settle for 20 ms and > >> try again (up to 3 attempts). Each retry logs the failing step and the > >> attempt number, which also makes future reports of this failure mode > >> self-diagnosing. The ring-test polling window is widened from 100 ms to > >> 2000 ms as spare capacity; the early-exit poll keeps healthy hardware on > >> the fast path. > >> > >> On the same machine the patched driver went 6/6 OK across repeated test > >> runs, with the warm-up absorbing the dead first instance and zero error > >> lines in dmesg. > >> > >> v2: guard uvd_v1_0_stop() behind successful clock setup, per Sashiko AI > >> review - MMIO writes to an unclocked UVD block can cause a PCI bus master > >> abort and hard-hang the system. > >> > >> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1867 > >> Link: https://bugs.launchpad.net/bugs/1865130 > >> Tested-by: ukrainer8 <[email protected]> # Radeon HD 5650M, kernel > >> 7.0.0 > >> Signed-off-by: ukrainer8 <[email protected]> > >> --- > >> v1 -> v2: keep the clock-setup result in a separate variable and only > >> call uvd_v1_0_stop() when clocks were actually raised (Sashiko AI review). > >> Also folded in the silent warm-up cycle and the wider ring-test window > >> validated in field testing since v1. > >> drivers/gpu/drm/radeon/uvd_v1_0.c | 57 > >> ++++++++++++++++++++++++++------------- > >> --- a/drivers/gpu/drm/radeon/uvd_v1_0.c > >> +++ b/drivers/gpu/drm/radeon/uvd_v1_0.c > >> @@ -158,24 +158,58 @@ > >> { > >> struct radeon_ring *ring = &rdev->ring[R600_RING_TYPE_UVD_INDEX]; > >> uint32_t tmp; > >> - int r; > >> + int r, retry, clk_err; > >> > >> - /* raise clocks while booting up the VCPU */ > >> + /* warm-up: silently cycle the engine once (like the Windows > >> driver), > >> + * the first ring instance after resume is dead functionally */ > >> if (rdev->family < CHIP_RV740) > >> radeon_set_uvd_clocks(rdev, 10000, 10000); > >> else > >> radeon_set_uvd_clocks(rdev, 53300, 40000); > >> + uvd_v1_0_start(rdev); > >> + uvd_v1_0_stop(rdev); > >> + mdelay(20); > >> > >> - r = uvd_v1_0_start(rdev); > >> - if (r) > >> - goto done; > >> + /* after resume the first ring test can race with the hardware; > >> retry */ > >> + for (retry = 0; retry < 3; retry++) { > >> + /* raise clocks while booting up the VCPU */ > >> + if (rdev->family < CHIP_RV740) > >> + clk_err = radeon_set_uvd_clocks(rdev, 10000, > >> 10000); > >> + else > >> + clk_err = radeon_set_uvd_clocks(rdev, 53300, > >> 40000); > >> + r = clk_err; > >> + if (r) > >> + drm_warn(&rdev->ddev, "radeon: raising UVD clocks > >> failed (%d), attempt %d\n", > >> + r, retry + 1); > >> + > >> + if (!r) { > >> + r = uvd_v1_0_start(rdev); > >> + if (r) > >> + drm_warn(&rdev->ddev, "radeon: starting > >> UVD failed (%d), attempt %d\n", > >> + r, retry + 1); > >> + } > >> > >> - ring->ready = true; > >> - r = radeon_ring_test(rdev, R600_RING_TYPE_UVD_INDEX, ring); > >> - if (r) { > >> - ring->ready = false; > >> - goto done; > >> + if (!r) { > >> + ring->ready = true; > >> + r = radeon_ring_test(rdev, > >> R600_RING_TYPE_UVD_INDEX, ring); > >> + if (r) { > >> + ring->ready = false; > >> + drm_warn(&rdev->ddev, "radeon: UVD ring > >> test failed (%d), attempt %d\n", > >> + r, retry + 1); > >> + } > >> + } > >> + > >> + if (!r) > >> + break; > >> + > >> + /* MMIO is only safe when the block is actually clocked */ > >> + if (!clk_err) > >> + uvd_v1_0_stop(rdev); > >> + if (retry < 2) > >> + mdelay(20); > >> } > >> + if (r) > >> + goto done; > >> > >> r = radeon_ring_lock(rdev, ring, 10); > >> if (r) { > >> @@ -421,7 +455,8 @@ > >> int uvd_v1_0_ring_test(struct radeon_device *rdev, struct radeon_ring > >> *ring) > >> { > >> uint32_t tmp = 0; > >> - unsigned i; > >> + /* after resume the ring buffer controller needs ~1.5-2s to settle > >> */ > >> + unsigned i, timeout = 2000000; > >> int r; > >> > >> WREG32(UVD_CONTEXT_ID, 0xCAFEDEAD); > >> @@ -434,14 +469,14 @@ > >> radeon_ring_write(ring, PACKET0(UVD_CONTEXT_ID, 0)); > >> radeon_ring_write(ring, 0xDEADBEEF); > >> radeon_ring_unlock_commit(rdev, ring, false); > >> - for (i = 0; i < rdev->usec_timeout; i++) { > >> + for (i = 0; i < timeout; i++) { > >> tmp = RREG32(UVD_CONTEXT_ID); > >> if (tmp == 0xDEADBEEF) > >> break; > >> udelay(1); > >> } > >> > >> - if (i < rdev->usec_timeout) { > >> + if (i < timeout) { > >> drm_info(&rdev->ddev, "ring test on %d succeeded in %d > >> usecs\n", > >> ring->idx, i); > >> } else { >
