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;

Reply via email to