Re: [PATCH v2 09/14] drm/amdgpu: defer JPEG and VCN idle work across GPU reset

[Lazar, Lijo]

On 09-Oct-26 11:49 AM, Liang, Prike wrote:
AMD General

Regards,
       Prike

-----Original Message-----
From: Lazar, Lijo <lijo.la...@amd.com>
Sent: Friday, October 9, 2026 12:50 PM
To: Liang, Prike <prike.li...@amd.com>; Koenig, Christian
<christian.koe...@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <alexander.deuc...@amd.com>; Prosyak, Vitaly
<vitaly.pros...@amd.com>
Subject: Re: [PATCH v2 09/14] drm/amdgpu: defer JPEG and VCN idle work across
GPU reset



On 09-Oct-26 9:13 AM, Liang, Prike wrote:
AMD General

Regards,
        Prike

-----Original Message-----
From: Liang, Prike
Sent: Thursday, October 8, 2026 8:42 PM
To: Koenig, Christian <christian.koe...@amd.com>; Lazar, Lijo
<lijo.la...@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <alexander.deuc...@amd.com>; Prosyak, Vitaly
<vitaly.pros...@amd.com>
Subject: RE: [PATCH v2 09/14] drm/amdgpu: defer JPEG and VCN idle
work across GPU reset



Regards,
        Prike

-----Original Message-----
From: Koenig, Christian <christian.koe...@amd.com>
Sent: Thursday, October 8, 2026 8:34 PM
To: Lazar, Lijo <lijo.la...@amd.com>; Liang, Prike
<prike.li...@amd.com>; amd- g...@lists.freedesktop.org
Cc: Deucher, Alexander <alexander.deuc...@amd.com>; Prosyak, Vitaly
<vitaly.pros...@amd.com>
Subject: Re: [PATCH v2 09/14] drm/amdgpu: defer JPEG and VCN idle
work across GPU reset

On 10/8/26 14:17, Lazar, Lijo wrote:


On 29-Sep-26 9:16 AM, Prike Liang wrote:
Protect media idle workers with a reset-domain read trylock. On
contention, requeue the idle timeout and return without hardware
access. Keep the ordinary end-use scheduling event, including from
reset-time ring tests, so otherwise idle engines eventually power
down and VCN
drops its profile.

A blocking read acquisition would deadlock with reset-time
synchronous cancellation of these workers, so the workers must
only try the
lock.

Signed-off-by: Prike Liang <prike.li...@amd.com>
---
    drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c | 7 +++++++
    drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c  | 7 +++++++
    2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
index 9887a4f99aed..7b10260ad0d4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
@@ -114,6 +114,12 @@ static void
amdgpu_jpeg_idle_work_handler(struct
work_struct *work)
        unsigned int fences = 0;
        unsigned int i, j;
    +    /* Reset may cancel this worker synchronously while holding
the writer. */
+    if (!down_read_trylock(&adev->reset_domain->sem)) {
+        schedule_delayed_work(&adev->jpeg.idle_work,
+JPEG_IDLE_TIMEOUT);

Is there any need to schedule this work here? Being not able to get
the lock
indicates that a reset is in progress already.

Yeah agree. We should just block for the GPU reset to finish.

So a simple down_read(&adev->reset_domain->sem) should do it.

This patch wants to schedule a delay idle work during processing a
gpu reset process, and then retry to run the VCN idle at the next scheduled 
loop.

A potential deadlock can occur if the VCN/JPEG idle worker runs during a GPU
reset while performing a blocking wait for the reset to complete. This patch 
resolves
the issue by using a retry-based wait on the GPU reset semaphore within the idle
worker callback.


VCN is going to be idle after a reset. The next idle worker will be scheduled 
after a
fresh job submission is complete. Why we need to reschedule idle work here?

If the VCN idle worker runs while GPU reset holds the semaphore, returning 
without rescheduling could leave VCN ungated if no new jobs arrive. However, 
blocking on the semaphore in VCN idle worker could deadlock because the reset 
path may synchronously cancel the worker while holding the semaphore. Using a 
trylock and rescheduling on contention avoids this deadlock while ensuring VCN 
can be power gated once reset completes.


Could you clarify on the ungated part? Typically after reset, VCN 
ring/ib tests are run. After that it should go to idle similar to a 
driver load scenario.

Thanks,
Lijo

Thanks,
Prike

Thanks,
Lijo

Thanks,
   Prike

Regards,
Christian.


Thanks,
Lijo

+        return;
+    }
+
        for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
            if (adev->jpeg.harvest_config & (1U << i))
                continue;
@@ -129,6 +135,7 @@ static void
amdgpu_jpeg_idle_work_handler(struct
work_struct *work)
            mutex_unlock(&adev->jpeg.jpeg_pg_lock);
        } else
            schedule_delayed_work(&adev->jpeg.idle_work,
JPEG_IDLE_TIMEOUT);
+    up_read(&adev->reset_domain->sem);
    }
      void amdgpu_jpeg_ring_begin_use(struct amdgpu_ring *ring)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 17db7264269e..f851a6ecea1b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -470,6 +470,12 @@ static void
amdgpu_vcn_idle_work_handler(struct
work_struct *work)
        if (adev->vcn.harvest_config & (1 << i))
            return;
    +    /* Reset may cancel this worker synchronously while holding
the writer. */
+    if (!down_read_trylock(&adev->reset_domain->sem)) {
+        schedule_delayed_work(&vcn_inst->idle_work,
+VCN_IDLE_TIMEOUT);
+        return;
+    }
+
        for (j = 0; j < adev->vcn.inst[i].num_enc_rings; ++j)
            fence[i] +=
amdgpu_fence_count_emitted(&vcn_inst->ring_enc[j]);
    @@ -499,6 +505,7 @@ static void
amdgpu_vcn_idle_work_handler(struct
work_struct *work)
        } else {
            schedule_delayed_work(&vcn_inst->idle_work,
VCN_IDLE_TIMEOUT);
        }
+    up_read(&adev->reset_domain->sem);
    }
      void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)