AMD General
Regards,
Prike
> -----Original Message-----
> From: Lazar, Lijo <[email protected]>
> Sent: Friday, October 9, 2026 12:50 PM
> To: Liang, Prike <[email protected]>; Koenig, Christian
> <[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
>
>
>
> 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?
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.
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)
> >>>>
> >
