Re: [PATCH] drm/amdgpu: handle pipeline sync without a VM fence

On Fri, Aug 14, 2026 at 1:14 PM David Rosca <[email protected]> wrote:
>
>
> On 8/14/26 17:42, Alex Deucher wrote:
> > If we end up emitting a VM fence keep pipeline sync
> > associated with that fence.  If not, emit them as
> > part of the IB fence.
> >
> > Cc: David Rosca <[email protected]>
> > Fixes: cb1e657ccac8 ("drm/amdgpu: handle GDS and SPM without a VM fence")
> > Signed-off-by: Alex Deucher <[email protected]>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 6 +++++-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 ++++--
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 +-
> >   3 files changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > index da4dc489e80bd..360e6f00cb7c0 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > @@ -222,7 +222,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned int num_ibs,
> >               vm_af = job->hw_vm_fence;
> >               /* VM sequence */
> >               vm_af->ib_wptr = ring->wptr;
> > -             amdgpu_vm_flush(ring, job, need_pipe_sync, &emit_spm_needed,
> > +             amdgpu_vm_flush(ring, job, &need_pipe_sync, &emit_spm_needed,
> >                               &emit_gds_needed);
> >               vm_af->ib_dw_size =
> >                       amdgpu_ring_get_dw_distance(ring, vm_af->ib_wptr, ring->wptr);
> > @@ -235,6 +235,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned int num_ibs,
> >       if (ring->funcs->insert_start)
> >               ring->funcs->insert_start(ring);
> >
> > +     /* this may have been handled by amdgpu_vm_flush */
> > +     if (need_pipe_sync)
> > +             amdgpu_ring_emit_pipeline_sync(ring);
> > +
> >       if (emit_spm_needed)
> >               adev->gfx.rlc.funcs->update_spm_vmid(adev, ring->xcc_id, ring, job->vmid);
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > index 71050a86bcc3a..4bd9368b13d97 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > @@ -772,7 +772,7 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
> >    * Emit a VM flush when it is necessary.
> >    */
> >   void amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job,
> > -                  bool need_pipe_sync, bool *emit_spm_needed,
> > +                  bool *need_pipe_sync, bool *emit_spm_needed,
> >                    bool *emit_gds_needed)
> >   {
> >       struct amdgpu_device *adev = ring->adev;
> need_pipe_sync should also be removed from this condition below:
>
>      if (!vm_flush_needed && !gds_switch_needed && !need_pipe_sync &&
>          !cleaner_shader_needed && !spm_update_needed)
>          return;

It's still needed, we just need to dereference it first.

Alex

>
> With that, Reviewed-by: David Rosca <[email protected]>
>
> > @@ -847,8 +847,10 @@ void amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job,
> >               patch = amdgpu_ring_init_cond_exec(ring,
> >                                                  ring->cond_exe_gpu_addr);
> >
> > -     if (need_pipe_sync)
> > +     if (emit_fence && *need_pipe_sync) {
> >               amdgpu_ring_emit_pipeline_sync(ring);
> > +             *need_pipe_sync = false;
> > +     }
> >
> >       if (cleaner_shader_needed)
> >               ring->funcs->emit_cleaner_shader(ring);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > index 7f2ba728e3ed3..d32183cd9e0fc 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > @@ -512,7 +512,7 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> >                      int (*callback)(void *p, struct amdgpu_bo *bo),
> >                      void *param);
> >   void amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job,
> > -                  bool need_pipe_sync, bool *emit_spm_needed,
> > +                  bool *need_pipe_sync, bool *emit_spm_needed,
> >                    bool *emit_gds_needed);
> >   int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
> >                         struct amdgpu_vm *vm, bool immediate);