* [PATCH v2] drm/radeon: retry UVD v1 init on failure after resume
@ 2026-10-19 14:49 ukrainer8
2026-10-20 8:03 ` sashiko-bot
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: ukrainer8 @ 2026-10-19 14:49 UTC (permalink / raw)
To: dri-devel
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 <kolomiiec@gmail.com> # Radeon HD 5650M, kernel 7.0.0
Signed-off-by: ukrainer8 <kolomiiec@gmail.com>
---
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 {
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/radeon: retry UVD v1 init on failure after resume
2026-10-19 14:49 [PATCH v2] drm/radeon: retry UVD v1 init on failure after resume ukrainer8
@ 2026-10-20 8:03 ` sashiko-bot
2026-10-21 13:29 ` Alex Deucher
2026-10-22 15:11 ` Alex Deucher
2 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-20 8:03 UTC (permalink / raw)
To: ukrainer8, dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/radeon: unclocked UVD MMIO in uvd_v1_0_init() when warm-up clocks fail
- [Medium] drm/radeon: excessive busy-wait and redundant unguarded warm-up in uvd_v1_0_init()
--
commit 6ca2d05b74a007ba26b71ef0af031a5857a4aa3e
Author: ukrainer8 <kolomiiec@gmail.com>
drm/radeon: retry UVD v1 init on failure after resume
This patch adds an unconditional warm-up cycle and a 3-attempt retry loop to
uvd_v1_0_init(), and increases the polling timeout in uvd_v1_0_ring_test() to
2 seconds to recover from UVD initialization failures after system resume.
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1867
Link: https://bugs.launchpad.net/bugs/1865130
> diff --git a/drivers/gpu/drm/radeon/uvd_v1_0.c b/drivers/gpu/drm/radeon/uvd_v1_0.c
> index 5e6607e16244b..79d3b8cd32388 100644
> --- a/drivers/gpu/drm/radeon/uvd_v1_0.c
> +++ b/drivers/gpu/drm/radeon/uvd_v1_0.c
> @@ -158,24 +158,58 @@ int uvd_v1_0_init(struct radeon_device *rdev)
> {
> 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);
[Severity: High]
What happens in uvd_v1_0_init() if radeon_set_uvd_clocks() fails during this
warm-up block?
In the retry loop right below, the return value of radeon_set_uvd_clocks() is
saved in clk_err, and both uvd_v1_0_start() and uvd_v1_0_stop() are guarded by
!clk_err because MMIO accesses to an unclocked UVD block can cause a PCI bus
master abort and hard-hang the system.
Here in the warm-up block, the return value of radeon_set_uvd_clocks() is
ignored (for example, if radeon_uvd_send_upll_ctlreq(), sumo_set_uvd_clock(),
or cik_set_uvd_clock() returns -ETIMEDOUT, or radeon_uvd_calc_upll_dividers()
returns -EINVAL), and uvd_v1_0_start() and uvd_v1_0_stop() are called
unconditionally. Won't that immediately perform MMIO reads and writes to the
unclocked UVD registers (such as UVD_CGC_GATE, UVD_MASTINT_EN, UVD_LMI_CTRL2,
UVD_RB_ARB_CTRL, UVD_SOFT_RESET, and UVD_STATUS) before the guarded retry loop
is ever reached?
In addition, is this unconditional warm-up cycle necessary given that the
3-attempt retry loop below already stops UVD, delays 20 ms, and retries from
scratch when a failure occurs? Running the warm-up unconditionally on every
cold boot and healthy resume across all R600 through CIK GPUs adds ~200 ms of
busy-wait mdelay() overhead (programming UPLL, starting and stopping the VCPU,
waiting 20 ms, and then immediately reprogramming UPLL a second time on the
first retry iteration while clocks are already raised), and if the VCPU is
unresponsive, uvd_v1_0_start() busy-waits for ~10.2 seconds of mdelay() in
the warm-up before repeating it 3 more times in the retry loop (~40.8 seconds
total).
> +
> + /* 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);
> + }
>
> - r = uvd_v1_0_start(rdev);
> - if (r)
> - 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);
> + }
> + }
>
> - ring->ready = true;
> - r = radeon_ring_test(rdev, R600_RING_TYPE_UVD_INDEX, ring);
> - if (r) {
> - ring->ready = false;
> - goto done;
> + 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 @@ void uvd_v1_0_stop(struct radeon_device *rdev)
> 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 @@ int uvd_v1_0_ring_test(struct radeon_device *rdev, struct radeon_ring *ring)
> 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);
> }
[Severity: Medium]
Could replacing rdev->usec_timeout (100,000 usec) with a hardcoded timeout of
2,000,000 iterations in uvd_v1_0_ring_test() stall a CPU core for several
seconds per failed attempt?
Because uvd_v1_0_ring_test() has no early-exit condition on failure and only
fails after exhausting all loop iterations, any failed ring test attempt will
execute 2,000,000 iterations of synchronous PCIe MMIO RREG32(UVD_CONTEXT_ID)
plus udelay(1) in a non-sleeping busy-wait loop. Across 3 failed attempts in
uvd_v1_0_init(), this will busy-wait for 6 to 15+ seconds during boot, system
resume, or GPU reset.
Also, the comment here states that the ring buffer controller needs ~1.5-2s to
settle after resume, whereas the commit message states that the first ring
instance after resume is dead functionally, not by timeout, and only a full
stop+init cycle revives the engine. If a stop+init cycle is what revives the
engine, is widening the busy-wait poll window from 100 ms to 2000 ms needed?
>
> - 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 {
့
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019144916.108589-1-kolomiiec@gmail.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/radeon: retry UVD v1 init on failure after resume
2026-10-19 14:49 [PATCH v2] drm/radeon: retry UVD v1 init on failure after resume ukrainer8
2026-10-20 8:03 ` sashiko-bot
@ 2026-10-21 13:29 ` Alex Deucher
2026-10-21 14:28 ` Christian König
2026-10-22 15:11 ` Alex Deucher
2 siblings, 1 reply; 7+ messages in thread
From: Alex Deucher @ 2026-10-21 13:29 UTC (permalink / raw)
To: ukrainer8, Leo Liu, Christian Koenig; +Cc: dri-devel
+ Leo, Christian
On Tue, Oct 20, 2026 at 3:46 AM ukrainer8 <kolomiiec@gmail.com> 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 <kolomiiec@gmail.com> # Radeon HD 5650M, kernel 7.0.0
> Signed-off-by: ukrainer8 <kolomiiec@gmail.com>
> ---
> 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 {
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/radeon: retry UVD v1 init on failure after resume
2026-10-21 13:29 ` Alex Deucher
@ 2026-10-21 14:28 ` Christian König
2026-10-21 21:47 ` О К
0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2026-10-21 14:28 UTC (permalink / raw)
To: Alex Deucher, ukrainer8, Leo Liu, amd-gfx@lists.freedesktop.org; +Cc: dri-devel
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 <kolomiiec@gmail.com> 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 <kolomiiec@gmail.com> # Radeon HD 5650M, kernel 7.0.0
>> Signed-off-by: ukrainer8 <kolomiiec@gmail.com>
>> ---
>> 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 {
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/radeon: retry UVD v1 init on failure after resume
2026-10-21 14:28 ` Christian König
@ 2026-10-21 21:47 ` О К
2026-10-22 12:50 ` Christian König
0 siblings, 1 reply; 7+ messages in thread
From: О К @ 2026-10-21 21:47 UTC (permalink / raw)
To: Christian König
Cc: Alex Deucher, Leo Liu, amd-gfx@lists.freedesktop.org, dri-devel,
sashiko-bot
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 <christian.koenig@amd.com> пише:
>
> 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 <kolomiiec@gmail.com> 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 <kolomiiec@gmail.com> # Radeon HD 5650M, kernel 7.0.0
> >> Signed-off-by: ukrainer8 <kolomiiec@gmail.com>
> >> ---
> >> 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 {
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/radeon: retry UVD v1 init on failure after resume
2026-10-21 21:47 ` О К
@ 2026-10-22 12:50 ` Christian König
0 siblings, 0 replies; 7+ messages in thread
From: Christian König @ 2026-10-22 12:50 UTC (permalink / raw)
To: О К
Cc: Alex Deucher, Leo Liu, amd-gfx@lists.freedesktop.org, dri-devel
On 10/21/26 23:47, О К wrote:
> 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.
Oh well, that is not even remotely true :)
The problem is that those old HW generations work with a rather rudimentary UVD FW signing. When that FW signing HW block thinks you are tampering with the UVD FW the device usually spontaneously falls of the bus.
In other words you can't read/write the PCI config space of the device any more, reads to MMIO BARs only return 0xffffffff and a few seconds later the system becomes completely unresponsive and you can only power cycle it to get out of that.
> 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.
Yeah that sounds unproblematic, the bit extra delay is not really a problem.
> 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).
Yeah that is definitely a good idea as well.
IIRC there is also a bit in the LMI register which is called *_VPU_STALL_* or something like that. Setting that before a retry and waiting a few miliseconds is usually a good idea as well.
> 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.
That is not really the problem, question is rather have you seen any hard crashes with this?
Regards,
Christian.
>
>> 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 <christian.koenig@amd.com> пише:
>>
>> 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 <kolomiiec@gmail.com> 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 <kolomiiec@gmail.com> # Radeon HD 5650M, kernel 7.0.0
>>>> Signed-off-by: ukrainer8 <kolomiiec@gmail.com>
>>>> ---
>>>> 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 {
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/radeon: retry UVD v1 init on failure after resume
2026-10-19 14:49 [PATCH v2] drm/radeon: retry UVD v1 init on failure after resume ukrainer8
2026-10-20 8:03 ` sashiko-bot
2026-10-21 13:29 ` Alex Deucher
@ 2026-10-22 15:11 ` Alex Deucher
2 siblings, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2026-10-22 15:11 UTC (permalink / raw)
To: ukrainer8; +Cc: dri-devel
On Tue, Oct 20, 2026 at 3:46 AM ukrainer8 <kolomiiec@gmail.com> 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 <kolomiiec@gmail.com> # Radeon HD 5650M, kernel 7.0.0
> Signed-off-by: ukrainer8 <kolomiiec@gmail.com>
We'll see how this goes. If anyone reports any issues, we can revert it.
Acked-by: Alex Deucher <alexander.deucher@amd.com>
and applied.
> ---
> 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 {
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-22 15:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-19 14:49 [PATCH v2] drm/radeon: retry UVD v1 init on failure after resume ukrainer8
2026-10-20 8:03 ` sashiko-bot
2026-10-21 13:29 ` Alex Deucher
2026-10-21 14:28 ` Christian König
2026-10-21 21:47 ` О К
2026-10-22 12:50 ` Christian König
2026-10-22 15:11 ` Alex Deucher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox