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

[David Rosca]

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 <david.ro...@amd.com>
Fixes: cb1e657ccac8 ("drm/amdgpu: handle GDS and SPM without a VM fence")
Signed-off-by: Alex Deucher <alexander.deuc...@amd.com>
---
  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;

With that, Reviewed-by: David Rosca <david.ro...@amd.com>

@@ -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);