AMD General
Regards,
Prike
> -----Original Message-----
> From: Lazar, Lijo <[email protected]>
> Sent: Friday, October 9, 2026 2:36 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 11:49 AM, 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.
> >
>
> 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.
By “ungated,” I mean that if the VCN idle worker schedules and fails to acquire
the reset semaphore during GPU reset holds reset semaphore
and returns without rescheduling, it skips power gating through
amdgpu_device_ip_set_powergating_state(). VCN may then remain ungated
if no new VCN jobs arrive to schedule another idle worker.
The VCN ring/IB tests run during the recovery resume phase while the reset path
may still hold the semaphore. If idle work
scheduled by those tests runs before the semaphore is released, hardware
accesses from the worker can trigger warnings and
may be unreliable. Using a trylock and rescheduling on contention lets the
worker safely gate VCN after GPU reset completes,
while avoiding deadlocks and unsafe hardware accesses.
Thanks,
Prike
> 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)
> >>>>>>
> >>>
> >
