AMD General
Regards,
Prike
> -----Original Message-----
> From: Koenig, Christian <[email protected]>
> Sent: Friday, October 9, 2026 3:42 PM
> To: Liang, Prike <[email protected]>; Lazar, Lijo <[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/9/26 08:19, Liang, Prike wrote:
> > 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.
>
> In this case absolutely clear NAK to that! Approaches like that are
> completely illegal
> upstream.
>
> The reset work handler *CAN'T* depend on the idle work and so also can't
> synchronously cancel it. That is an absolutely clear NO-GO.
>
> Just use down_read() to make sure that lockdep starts complaining when the
> reset
> handler does such nasty things.
Thanks for the suggestion, will use a blocking reset read lock in the idle
worker, and remove synchronous dependencies on that worker from paths holding
the reset write lock.
Thanks,
Prike
> Regards,
> Christian.
>
> >
> > 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)
> >>>>>>
> >>>
> >
