On Fri, Aug 28, 2026 at 11:45 AM Timur Kristóf <[email protected]> wrote:
>
> On Friday, August 28, 2026 4:20:19 PM Central European Summer Time Lazar, Lijo
> wrote:
> > On 28-Aug-26 5:37 PM, Timur Kristóf wrote:
> > > The code was exactly the same between SDMA v5.0 and v5.2
> > > furthermore the exact same implementation can be shared
> > > between all SDMA versions that don't use MES.
> > >
> > > Signed-off-by: Timur Kristóf <[email protected]>
> > > ---
> > >
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 37 ++++++++++++++++++++++++
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h | 4 +++
> > > drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 25 +---------------
> > > drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 25 +---------------
> > > 4 files changed, 43 insertions(+), 48 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c index
> > > 66f278f77f71..9eebd8380834 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> > > @@ -635,3 +635,40 @@ int amdgpu_sdma_reset_engine(struct amdgpu_device
> > > *adev, uint32_t instance_id,>
> > > return ret;
> > >
> > > }
> > >
> > > +
> > > +/**
> > > + * amdgpu_sdma_reset_queue_legacy() - Reset legacy SDMA queue after
> > > timeout (without MES) + *
> > > + * @ring: Pointer to the ring of the SDMA queue
> > > + * @vmid: VMID of the timed out job
> > > + * @timedout_fence: Fence of the timed out job
> > > + *
> > > + * Common implementation for resetting SDMA queues without MES (legacy).
> > > + * This relies on the proper amdgpu_sdma_funcs to be set up
> > > + * for the given ring.
> > > + *
> > > + * Applicable to SDMA versions that don't rely on the MES yet,
> > > + * that is all versions up to SDMA v5.x and older.
> > > + */
> > > +int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
> > > + unsigned int vmid,
> > > + struct amdgpu_fence
> *timedout_fence)
> > > +{
> > > + struct amdgpu_device *adev = ring->adev;
> > > + int r;
> > > +
> > > + if (ring->me >= adev->sdma.num_instances) {
> > > + dev_err(adev->dev, "sdma instance not found\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + amdgpu_ring_reset_helper_begin(ring, timedout_fence);
> > > +
> > > + amdgpu_amdkfd_suspend(adev, true);
> > > + r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> > > + amdgpu_amdkfd_resume(adev, true);
> > > + if (r)
> > > + return r;
> > > +
> > > + return amdgpu_ring_reset_helper_end(ring, timedout_fence);
> > > +}
> >
> > Instead of moving it here - sdma_v5_x_reset_queue and using it for 5.2
> > is better. This may not work in the same way for all legacy queues.
> >
> > Thanks,
> > Lijo
>
> Hi,
>
> As far as I see, this function can be reused for all generations that don't
> use MES. If you don't think so, please explain why not.
I think the context is missing that you want to extend this to older
generations as well and it's still applicable there.
Alex
>
> Thanks,
> Timur
>
> >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h index
> > > 4f4e56022c97..7c4e145ca0c1 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> > > @@ -160,6 +160,10 @@ struct amdgpu_buffer_funcs {
> > >
> > > int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t
> > > instance_id,>
> > > bool caller_handles_kernel_queues);
> > >
> > > +int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
> > > + unsigned int vmid,
> > > + struct amdgpu_fence
> *timedout_fence);
> > > +
> > >
> > > #define amdgpu_emit_copy_buffer(adev, ib, s, d, b, t)
> > > (adev)->mman.buffer_funcs->emit_copy_buffer((ib), (s), (d), (b), (t))
> > > #define amdgpu_emit_fill_buffer(adev, ib, s, d, b)
> > > (adev)->mman.buffer_funcs->emit_fill_buffer((ib), (s), (d), (b))>
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> > > b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c index 0da54c335822..76f8765fb175
> > > 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> > > @@ -1518,29 +1518,6 @@ static int sdma_v5_0_wait_for_idle(struct
> > > amdgpu_ip_block *ip_block)>
> > > return -ETIMEDOUT;
> > >
> > > }
> > >
> > > -static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring,
> > > - unsigned int vmid,
> > > - struct amdgpu_fence
> *timedout_fence)
> > > -{
> > > - struct amdgpu_device *adev = ring->adev;
> > > - int r;
> > > -
> > > - if (ring->me >= adev->sdma.num_instances) {
> > > - dev_err(adev->dev, "sdma instance not found\n");
> > > - return -EINVAL;
> > > - }
> > > -
> > > - amdgpu_ring_reset_helper_begin(ring, timedout_fence);
> > > -
> > > - amdgpu_amdkfd_suspend(adev, true);
> > > - r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> > > - amdgpu_amdkfd_resume(adev, true);
> > > - if (r)
> > > - return r;
> > > -
> > > - return amdgpu_ring_reset_helper_end(ring, timedout_fence);
> > > -}
> > > -
> > >
> > > static int sdma_v5_0_stop_queue(struct amdgpu_ring *ring)
> > > {
> > >
> > > u32 f32_cntl, freeze, cntl, stat1_reg;
> > >
> > > @@ -1936,7 +1913,7 @@ static const struct amdgpu_ring_funcs
> > > sdma_v5_0_ring_funcs = {>
> > > .emit_reg_write_reg_wait =
> sdma_v5_0_ring_emit_reg_write_reg_wait,
> > > .init_cond_exec = sdma_v5_0_ring_init_cond_exec,
> > > .preempt_ib = sdma_v5_0_ring_preempt_ib,
> > >
> > > - .reset = sdma_v5_0_reset_queue,
> > > + .reset = amdgpu_sdma_reset_queue_legacy,
> > >
> > > };
> > >
> > > static void sdma_v5_0_set_ring_funcs(struct amdgpu_device *adev)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > > b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c index 5543e381dcca..e7f4b74f27b4
> > > 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > > @@ -1436,29 +1436,6 @@ static int sdma_v5_2_wait_for_idle(struct
> > > amdgpu_ip_block *ip_block)>
> > > return -ETIMEDOUT;
> > >
> > > }
> > >
> > > -static int sdma_v5_2_reset_queue(struct amdgpu_ring *ring,
> > > - unsigned int vmid,
> > > - struct amdgpu_fence
> *timedout_fence)
> > > -{
> > > - struct amdgpu_device *adev = ring->adev;
> > > - int r;
> > > -
> > > - if (ring->me >= adev->sdma.num_instances) {
> > > - dev_err(adev->dev, "sdma instance not found\n");
> > > - return -EINVAL;
> > > - }
> > > -
> > > - amdgpu_ring_reset_helper_begin(ring, timedout_fence);
> > > -
> > > - amdgpu_amdkfd_suspend(adev, true);
> > > - r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> > > - amdgpu_amdkfd_resume(adev, true);
> > > - if (r)
> > > - return r;
> > > -
> > > - return amdgpu_ring_reset_helper_end(ring, timedout_fence);
> > > -}
> > > -
> > >
> > > static int sdma_v5_2_stop_queue(struct amdgpu_ring *ring)
> > > {
> > >
> > > u32 f32_cntl, freeze, cntl, stat1_reg;
> > >
> > > @@ -1951,7 +1928,7 @@ static const struct amdgpu_ring_funcs
> > > sdma_v5_2_ring_funcs = {>
> > > .emit_reg_write_reg_wait =
> sdma_v5_2_ring_emit_reg_write_reg_wait,
> > > .init_cond_exec = sdma_v5_2_ring_init_cond_exec,
> > > .preempt_ib = sdma_v5_2_ring_preempt_ib,
> > >
> > > - .reset = sdma_v5_2_reset_queue,
> > > + .reset = amdgpu_sdma_reset_queue_legacy,
> > >
> > > };
> > >
> > > static void sdma_v5_2_set_ring_funcs(struct amdgpu_device *adev)
>
>
>
>
