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.
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)
> > >
