AMD General
Regards,
Prike
> -----Original Message-----
> From: Koenig, Christian <[email protected]>
> Sent: Thursday, October 29, 2026 11:05 PM
> To: Zhang, Jesse(Jie) <[email protected]>; [email protected]
> Cc: Deucher, Alexander <[email protected]>; Liang, Prike
> <[email protected]>; Khatri, Sunil <[email protected]>
> Subject: Re: [PATCH 1/3] drm/amdgpu: program the SPM VMID for jobs on the
> reserved VMID again
>
> On 10/29/26 08:03, Jesse Zhang wrote:
> > Commit 38e07c072833 ("drm/amdgpu: remove the now-redundant runtime SPM
> > VMID update") dropped the RLC_SPM_VMID update on submission, as the
> > SPM VMID is fixed and programmed once at init. RLC_SPM_MC_CNTL doesn't
> > keep its value over GFXOFF though, the RLC brings it back as 0 when it
> > powers GFX up again. So once the GPU idled, the job of a process which
> > reserved the VMID for SPM runs with the SPM writing through VMID 0.
>
> That sounds like a pretty clear bug in the RLC handling to me.
>
> @Alex do you know where the save/restore register list can be found?
>
> This here is not really an option since we want to completely nuke the
> update_spm_vmid() function.
The registers that the RLC saves and restores are listed in register_restore.h
(or an equivalent header).
SPM-related registers such as RLC_SPM_MC_CNTL are in the Navi3x save/restore
list but are missing in the Navi4x S/R list.
The RLC team may need to add them to the new save/restore list for handling
such save/restore problem.
Regards,
Prike
> Regards,
> Christian.
>
> >
> > On navi48 RLC_SPM_MC_CNTL reads 0xf right after hw_init, 0 once GFXOFF
> > was entered, and stays 0xf with GFXOFF disabled. A COPY_DATA of the
> > register from the IB of a process with a reserved VMID gives 0 instead
> > of the reserved VMID 15:
> >
> > $ umr -i 0 -go 0; umr -i 0 -r '*.*.regRLC_SPM_MC_CNTL'
> > gfx1201.regRLC_SPM_MC_CNTL => 0x00000000
> > $ amd_spm --run-subtest reserved-vmid-spm-vmid
> > RLC_SPM_MC_CNTL in the job: 0x00000000
> >
> > Update it on submission again, but only for the jobs on the reserved
> > VMID and with the fixed SPM VMID. Unlike before that commit, a GPU
> > reset must not force the update for every job: RLC_SPM_MC_CNTL is
> > shared, and a job of another VM would retarget the SPM of the profiled
> > process to its own VMID. The reserved path requests the update on every
> submission anyway.
> > With that it's back to 15 inside the job.
> >
> > Fixes: 38e07c072833 ("drm/amdgpu: remove the now-redundant runtime SPM
> > VMID update")
> > Signed-off-by: Jesse Zhang <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 8 +++++++-
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 1 +
> > drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 1 +
> > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 ++++++++++++++-
> > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 ++-
> > 5 files changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > index f98a7c6797b3..ec0883df91b1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > @@ -126,6 +126,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, struct
> amdgpu_job *job,
> > struct amdgpu_fence *af;
> > struct amdgpu_fence *vm_af;
> > bool need_ctx_switch;
> > + bool emit_spm_needed = false;
> > bool emit_gds_needed = false;
> > uint64_t fence_ctx;
> > uint32_t status = 0, alloc_size;
> > @@ -194,7 +195,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
> > struct amdgpu_job *job,
> >
> > /* VM sequence */
> > vm_af->ib_wptr = ring->wptr;
> > - amdgpu_vm_flush(ring, job, &need_pipe_sync, &emit_gds_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);
> >
> > @@ -209,6 +211,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, struct
> amdgpu_job *job,
> > 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,
> > + adev->vm_manager.spm_vmid);
> > +
> > if (emit_gds_needed)
> > amdgpu_ring_emit_gds_switch(ring, job->vmid, job->gds_base,
> > job->gds_size, job->gws_base, diff
> > --git
> > a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> > index 9c879ac68352..124fb875c651 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> > @@ -434,6 +434,7 @@ static int amdgpu_vmid_grab_reserved(struct
> amdgpu_vm *vm,
> > return r;
> >
> > job->vm_needs_flush = needs_flush;
> > + job->spm_update_needed = true;
> > return 0;
> > }
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > index f6577f82e6b9..ae569b4497f6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > @@ -75,6 +75,7 @@ struct amdgpu_job {
> > uint32_t preemption_status;
> > bool vm_needs_flush;
> > bool gds_switch_needed;
> > + bool spm_update_needed;
> > uint64_t vm_pd_addr;
> > unsigned vmid;
> > unsigned pasid;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > index 000c982816a7..7a8e363449a6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > @@ -809,12 +809,14 @@ bool amdgpu_vm_need_pipeline_sync(struct
> amdgpu_ring *ring,
> > * @ring: ring to use for flush
> > * @job: related job
> > * @need_pipe_sync: is pipe sync needed
> > + * @emit_spm_needed: does the caller need to emit spm
> > * @emit_gds_needed: does the caller need to emit gds
> > *
> > * 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_gds_needed)
> > + bool *need_pipe_sync, bool *emit_spm_needed,
> > + bool *emit_gds_needed)
> > {
> > struct amdgpu_device *adev = ring->adev;
> > struct amdgpu_isolation *isolation = @@ -823,6 +825,8 @@ void
> > amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job,
> > unsigned vmhub = ring->vm_hub;
> > struct amdgpu_kq_vmid_mgr *id_mgr = &adev-
> >vm_manager.id_mgr[vmhub];
> > struct amdgpu_vmid *id = &id_mgr->ids[job->vmid];
> > + bool spm_update_needed = adev->gfx.rlc.funcs->update_spm_vmid &&
> > + job->spm_update_needed;
> > bool gds_switch_needed = ring->funcs->emit_gds_switch &&
> > job->gds_switch_needed;
> > bool vm_flush_needed = job->vm_needs_flush; @@ -845,6 +849,7 @@ void
> > amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job,
> > mutex_unlock(&id_mgr->lock);
> >
> > gds_switch_needed &= !!ring->funcs->emit_gds_switch;
> > + spm_update_needed &= !!adev->gfx.rlc.funcs->update_spm_vmid;
> > vm_flush_needed &= !!ring->funcs->emit_vm_flush &&
> > job->vm_pd_addr != AMDGPU_BO_INVALID_OFFSET;
> > pasid_mapping_needed &= adev->gmc.gmc_funcs->emit_pasid_mapping
> &&
> > @@ -858,6 +863,10 @@ void amdgpu_vm_flush(struct amdgpu_ring *ring, struct
> amdgpu_job *job,
> > emit_fence = vm_flush_needed || pasid_mapping_needed ||
> > cleaner_shader_needed;
> >
> > + *emit_spm_needed = spm_update_needed;
> > + if (spm_update_needed && emit_fence)
> > + *emit_spm_needed = false;
> > +
> > *emit_gds_needed = gds_switch_needed;
> > if (gds_switch_needed && emit_fence)
> > *emit_gds_needed = false;
> > @@ -897,6 +906,10 @@ void amdgpu_vm_flush(struct amdgpu_ring *ring, struct
> amdgpu_job *job,
> > if (pasid_mapping_needed)
> > amdgpu_gmc_emit_pasid_mapping(ring, job->vmid, job->pasid);
> >
> > + if (spm_update_needed)
> > + adev->gfx.rlc.funcs->update_spm_vmid(adev, ring->xcc_id,
> > + adev->vm_manager.spm_vmid);
> > +
> > if (gds_switch_needed)
> > amdgpu_ring_emit_gds_switch(ring, job->vmid, job->gds_base,
> > job->gds_size,
> > job->gws_base, diff --
> git
> > a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > index 06e0ddd60c7b..973ad7576d9e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > @@ -449,7 +449,8 @@ 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_gds_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); int
> > amdgpu_vm_clear_freed(struct amdgpu_device *adev,
