On 10/16/26 13:41, Khatri, Sunil wrote:
> On 16-10-2026 05:04 pm, Christian König wrote:
>>
>> On 10/13/26 13:21, Sunil Khatri wrote:
>>> These functions only ever hand out the dedicated SPM VMID now, not a
>>> generic reserved VMID. Rename them to amdgpu_vmid_alloc_spm() and
>>> amdgpu_vmid_free_spm() to say so. Pure rename, no functional change.
>>>
>>> Suggested-by: Christian König <[email protected]>
>>> Signed-off-by: Sunil Khatri <[email protected]>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 8 ++++----
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h | 4 ++--
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++---
>>> 3 files changed, 9 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>>> index e5d84a786c4f2..f15702993eeab 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>>> @@ -589,7 +589,7 @@ bool amdgpu_vmid_uses_reserved(struct amdgpu_vm *vm,
>>> unsigned int vmhub)
>>> }
>>> /*
>>> - * amdgpu_vmid_alloc_reserved - reserve a specific VMID for this vm
>>> + * amdgpu_vmid_alloc_spm - reserve a specific VMID for this vm
>>> * @adev: amdgpu device structure
>>> * @vm: the VM to reserve an ID for
>>> * @vmhub: the VMHUB which should be used
>>> @@ -598,7 +598,7 @@ bool amdgpu_vmid_uses_reserved(struct amdgpu_vm *vm,
>>> unsigned int vmhub)
>>> *
>>> * Returns: 0 for success, -ENOENT if an ID is already reserved.
>>> */
>>> -int amdgpu_vmid_alloc_reserved(struct amdgpu_device *adev, struct
>>> amdgpu_vm *vm,
>>> +int amdgpu_vmid_alloc_spm(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>> unsigned vmhub)
>> I think we also need to make sure that the hub is a graphics VMHUB.
>
> amdgpu_vmid_free_spm/alloc passed explicitly AMDGPU_GFXHUB(0) only... i left
> the function deliberately to be generic but caller is within the kernel and
> we hold control to call with GFXHUB only4
Yeah passing AMDGPU_GFXHUB(*0*) is not 100% correct because we need the vmhub
depending on the partition the VM uses.
But I think SPM is not really used on that HW or otherwise somebody would have
complained already.
Regards,
Christian.
>
> Regards
> Sunil Khatri
>
>>
>> There is really no use case for an MMHUB here.
>>
>> Apart from that looks good to me,
>> Christian.
>>
>>> {
>>> struct amdgpu_kq_vmid_mgr *id_mgr = &adev->vm_manager.id_mgr[vmhub];
>>> @@ -629,7 +629,7 @@ int amdgpu_vmid_alloc_reserved(struct amdgpu_device
>>> *adev, struct amdgpu_vm *vm,
>>> }
>>> /*
>>> - * amdgpu_vmid_free_reserved - free up a reserved VMID again
>>> + * amdgpu_vmid_free_spm - free up a reserved VMID again
>>> * @adev: amdgpu device structure
>>> * @vm: the VM with the reserved ID
>>> * @vmhub: the VMHUB which should be used
>>> @@ -637,7 +637,7 @@ int amdgpu_vmid_alloc_reserved(struct amdgpu_device
>>> *adev, struct amdgpu_vm *vm,
>>> * Waits for all work that used the reserved VMID to finish before
>>> unbinding
>>> * its page table, so submissions still in flight don't suddenly fault.
>>> */
>>> -void amdgpu_vmid_free_reserved(struct amdgpu_device *adev, struct
>>> amdgpu_vm *vm,
>>> +void amdgpu_vmid_free_spm(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>> unsigned vmhub)
>>> {
>>> struct amdgpu_kq_vmid_mgr *id_mgr = &adev->vm_manager.id_mgr[vmhub];
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
>>> index b85aa2ee19b19..c134000eecfdc 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
>>> @@ -125,9 +125,9 @@ void amdgpu_pasid_mgr_cleanup(void);
>>> bool amdgpu_vmid_had_gpu_reset(struct amdgpu_device *adev,
>>> struct amdgpu_vmid *id);
>>> bool amdgpu_vmid_uses_reserved(struct amdgpu_vm *vm, unsigned int vmhub);
>>> -int amdgpu_vmid_alloc_reserved(struct amdgpu_device *adev, struct
>>> amdgpu_vm *vm,
>>> +int amdgpu_vmid_alloc_spm(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>> unsigned vmhub);
>>> -void amdgpu_vmid_free_reserved(struct amdgpu_device *adev, struct
>>> amdgpu_vm *vm,
>>> +void amdgpu_vmid_free_spm(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>> unsigned vmhub);
>>> int amdgpu_vmid_grab(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
>>> struct amdgpu_job *job, struct dma_fence **fence);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 7fd6215510a94..8d4c13f2ef7fb 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -2894,7 +2894,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev,
>>> struct amdgpu_vm *vm)
>>> dma_fence_put(vm->last_update);
>>> for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
>>> - amdgpu_vmid_free_reserved(adev, vm, i);
>>> + amdgpu_vmid_free_spm(adev, vm, i);
>>> }
>>> ttm_lru_bulk_move_fini(&adev->mman.bdev, &vm->lru_bulk_move);
>>> @@ -2997,7 +2997,7 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void
>>> *data, struct drm_file *filp)
>>> int r;
>>> /* We only have requirement to reserve vmid from gfxhub */
>>> - r = amdgpu_vmid_alloc_reserved(adev, vm, AMDGPU_GFXHUB(0));
>>> + r = amdgpu_vmid_alloc_spm(adev, vm, AMDGPU_GFXHUB(0));
>>> if (!r && !amdgpu_bo_reserve(vm->root.bo, true)) {
>>> amdgpu_vm_spm_update(adev, vm);
>>> amdgpu_bo_unreserve(vm->root.bo);
>>> @@ -3005,7 +3005,7 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void
>>> *data, struct drm_file *filp)
>>> return r;
>>> }
>>> case AMDGPU_VM_OP_UNRESERVE_VMID:
>>> - amdgpu_vmid_free_reserved(adev, vm, AMDGPU_GFXHUB(0));
>>> + amdgpu_vmid_free_spm(adev, vm, AMDGPU_GFXHUB(0));
>>> break;
>>> default:
>>> return -EINVAL;