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 <[email protected]>; Lazar, Lijo
<[email protected]>; [email protected]
Cc: Deucher, Alexander <[email protected]>; Prosyak, Vitaly
<[email protected]>
Subject: RE: [PATCH v2 09/14] drm/amdgpu: defer JPEG and VCN idle work across
GPU reset



Regards,
       Prike

-----Original Message-----
From: Koenig, Christian <[email protected]>
Sent: Thursday, October 8, 2026 8:34 PM
To: Lazar, Lijo <[email protected]>; Liang, Prike
<[email protected]>; amd- [email protected]
Cc: Deucher, Alexander <[email protected]>; Prosyak, Vitaly
<[email protected]>
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 <[email protected]>
---
   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?

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)



Reply via email to