Re: [PATCH 07/13] drm/amdgpu: drop immediate updates from amdgpu_vm_update_range

["Timur Kristóf" <timur.kristof@gmail.com>]

On 2026. május 29., péntek 13:24:09 közép-európai nyári idő Christian König 
wrote:
> That case is handled by amdgpu_vm_update_leaves now.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

This commit seems to be doing more than suggested by the (very short) commit 
message. It removes immediate updates not only from amdgpu_vm_update_range() 
but also amdgpu_vm_pt_clear() and amdgpu_vm_pt_create().

Otherwise the code looks good.

Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c    | 21 +++++++++------------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h    | 16 +++++++---------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 19 ++++++-------------
>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c      |  4 ++--
>  4 files changed, 24 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 94632a660b79..edc8b1ca2d3e
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1084,7 +1084,6 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params
> *params, *
>   * @adev: amdgpu_device pointer to use for commands
>   * @vm: the VM to update the range
> - * @immediate: immediate submission in a page fault
>   * @unlocked: unlocked invalidation during MM callback
>   * @flush_tlb: trigger tlb invalidation after update completed
>   * @allow_override: change MTYPE for local NUMA nodes
> @@ -1104,12 +1103,11 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params
> *params, * 0 for success, negative erro code for failure.
>   */
>  int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm
> *vm, -			   bool immediate, bool unlocked, bool 
flush_tlb,
> -			   bool allow_override, struct amdgpu_sync 
*sync,
> -			   uint64_t start, uint64_t last, uint64_t 
flags,
> -			   uint64_t offset, uint64_t vram_base,
> -			   struct ttm_resource *res, dma_addr_t 
*pages_addr,
> -			   struct dma_fence **fence)
> +			   bool unlocked, bool flush_tlb, bool 
allow_override,
> +			   struct amdgpu_sync *sync, uint64_t start,
> +			   uint64_t last, uint64_t flags, uint64_t 
offset,
> +			   uint64_t vram_base, struct ttm_resource 
*res,
> +			   dma_addr_t *pages_addr, struct dma_fence 
**fence)
>  {
>  	struct amdgpu_vm_tlb_seq_struct *tlb_cb;
>  	struct amdgpu_vm_update_params params;
> @@ -1139,7 +1137,6 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev,
> struct amdgpu_vm *vm, memset(&params, 0, sizeof(params));
>  	params.adev = adev;
>  	params.vm = vm;
> -	params.immediate = immediate;
>  	params.pages_addr = pages_addr;
>  	params.unlocked = unlocked;
>  	params.needs_flush = flush_tlb;
> @@ -1365,7 +1362,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
> struct amdgpu_bo_va *bo_va,
> 
>  		trace_amdgpu_vm_bo_update(mapping);
> 
> -		r = amdgpu_vm_update_range(adev, vm, false, false, 
flush_tlb,
> +		r = amdgpu_vm_update_range(adev, vm, false, flush_tlb,
>  					   !uncached, &sync, 
mapping->start,
>  					   mapping->last, 
update_flags,
>  					   mapping->offset, 
vram_base, mem,
> @@ -1568,7 +1565,7 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>  			struct amdgpu_bo_va_mapping, list);
>  		list_del(&mapping->list);
> 
> -		r = amdgpu_vm_update_range(adev, vm, false, false, 
true, false,
> +		r = amdgpu_vm_update_range(adev, vm, false, true, false,
>  					   &sync, mapping-
>start, mapping->last,
>  					   0, 0, 0, NULL, 
NULL, &f);
>  		amdgpu_vm_free_mapping(adev, vm, mapping, f);
> @@ -2617,7 +2614,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
> amdgpu_vm *vm, vm->tlb_fence_context = dma_fence_context_alloc(1);
> 
>  	r = amdgpu_vm_pt_create(adev, vm, adev->vm_manager.root_level,
> -				false, &root, xcp_id);
> +				&root, xcp_id);
>  	if (r)
>  		goto error_free_delayed;
> 
> @@ -2633,7 +2630,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
> amdgpu_vm *vm, if (r)
>  		goto error_free_root;
> 
> -	r = amdgpu_vm_pt_clear(adev, vm, root, false);
> +	r = amdgpu_vm_pt_clear(adev, vm, root);
>  	if (r)
>  		goto error_free_root;
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index 04b32accfa3f..3e86a2a470f0
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -530,12 +530,11 @@ int amdgpu_vm_flush_compute_tlb(struct amdgpu_device
> *adev, void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
>  			    struct amdgpu_vm *vm, struct amdgpu_bo 
*bo);
>  int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm
> *vm, -			   bool immediate, bool unlocked, bool 
flush_tlb,
> -			   bool allow_override, struct amdgpu_sync 
*sync,
> -			   uint64_t start, uint64_t last, uint64_t 
flags,
> -			   uint64_t offset, uint64_t vram_base,
> -			   struct ttm_resource *res, dma_addr_t 
*pages_addr,
> -			   struct dma_fence **fence);
> +			   bool unlocked, bool flush_tlb, bool 
allow_override,
> +			   struct amdgpu_sync *sync, uint64_t start,
> +			   uint64_t last, uint64_t flags, uint64_t 
offset,
> +			   uint64_t vram_base, struct ttm_resource 
*res,
> +			   dma_addr_t *pages_addr, struct dma_fence 
**fence);
>  int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>  			struct amdgpu_bo_va *bo_va,
>  			bool clear);
> @@ -602,10 +601,9 @@ void amdgpu_vm_get_memory(struct amdgpu_vm *vm,
>  			  struct amdgpu_mem_stats 
stats[__AMDGPU_PL_NUM]);
> 
>  int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> -		       struct amdgpu_bo_vm *vmbo, bool immediate);
> +		       struct amdgpu_bo_vm *vmbo);
>  int amdgpu_vm_pt_create(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> -			int level, bool immediate, struct 
amdgpu_bo_vm **vmbo,
> -			int32_t xcp_id);
> +			int level, struct amdgpu_bo_vm **vmbo, 
int32_t xcp_id);
>  void amdgpu_vm_pt_free_root(struct amdgpu_device *adev, struct amdgpu_vm
> *vm);
> 
>  int amdgpu_vm_pde_update(struct amdgpu_vm_update_params *params,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index
> 9766b6b9aecc..6f5415d5a1bc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> @@ -351,7 +351,6 @@ static void amdgpu_vm_pt_next_dfs(struct amdgpu_device
> *adev, * @adev: amdgpu_device pointer
>   * @vm: VM to clear BO from
>   * @vmbo: BO to clear
> - * @immediate: use an immediate update
>   *
>   * Root PD needs to be reserved when calling this.
>   *
> @@ -359,7 +358,7 @@ static void amdgpu_vm_pt_next_dfs(struct amdgpu_device
> *adev, * 0 on success, errno otherwise.
>   */
>  int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> -		       struct amdgpu_bo_vm *vmbo, bool immediate)
> +		       struct amdgpu_bo_vm *vmbo)
>  {
>  	unsigned int level = adev->vm_manager.root_level;
>  	struct ttm_operation_ctx ctx = { true, false };
> @@ -396,7 +395,6 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
> struct amdgpu_vm *vm, memset(&params, 0, sizeof(params));
>  	params.adev = adev;
>  	params.vm = vm;
> -	params.immediate = immediate;
> 
>  	r = vm->update_funcs->prepare(&params, NULL,
>  				      
AMDGPU_KERNEL_JOB_ID_VM_PT_CLEAR);
> @@ -434,13 +432,11 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
> struct amdgpu_vm *vm, * @adev: amdgpu_device pointer
>   * @vm: requesting vm
>   * @level: the page table level
> - * @immediate: use a immediate update
>   * @vmbo: pointer to the buffer object pointer
>   * @xcp_id: GPU partition id
>   */
>  int amdgpu_vm_pt_create(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> -			int level, bool immediate, struct 
amdgpu_bo_vm **vmbo,
> -			int32_t xcp_id)
> +			int level, struct amdgpu_bo_vm **vmbo, 
int32_t xcp_id)
>  {
>  	struct amdgpu_bo_param bp;
>  	unsigned int num_entries;
> @@ -470,7 +466,6 @@ int amdgpu_vm_pt_create(struct amdgpu_device *adev,
> struct amdgpu_vm *vm, bp.flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> 
>  	bp.type = ttm_bo_type_kernel;
> -	bp.no_wait_gpu = immediate;
>  	bp.xcp_id_plus1 = xcp_id + 1;
> 
>  	if (vm->root.bo)
> @@ -485,7 +480,6 @@ int amdgpu_vm_pt_create(struct amdgpu_device *adev,
> struct amdgpu_vm *vm, * @adev: amdgpu_device pointer
>   * @vm: VM to allocate page tables for
>   * @cursor: Which page table to allocate
> - * @immediate: use an immediate update
>   *
>   * Make sure a specific page table or directory is allocated.
>   *
> @@ -495,8 +489,7 @@ int amdgpu_vm_pt_create(struct amdgpu_device *adev,
> struct amdgpu_vm *vm, */
>  static int amdgpu_vm_pt_alloc(struct amdgpu_device *adev,
>  			      struct amdgpu_vm *vm,
> -			      struct amdgpu_vm_pt_cursor *cursor,
> -			      bool immediate)
> +			      struct amdgpu_vm_pt_cursor *cursor)
>  {
>  	struct amdgpu_vm_bo_base *entry = cursor->entry;
>  	struct amdgpu_bo *pt_bo;
> @@ -507,7 +500,7 @@ static int amdgpu_vm_pt_alloc(struct amdgpu_device
> *adev, return 0;
> 
>  	amdgpu_vm_eviction_unlock(vm);
> -	r = amdgpu_vm_pt_create(adev, vm, cursor->level, immediate, &pt,
> +	r = amdgpu_vm_pt_create(adev, vm, cursor->level, &pt,
>  				vm->root.bo->xcp_id);
>  	amdgpu_vm_eviction_lock(vm);
>  	if (r)
> @@ -519,7 +512,7 @@ static int amdgpu_vm_pt_alloc(struct amdgpu_device
> *adev, pt_bo = &pt->bo;
>  	pt_bo->parent = amdgpu_bo_ref(cursor->parent->bo);
>  	amdgpu_vm_bo_base_init(entry, vm, pt_bo);
> -	r = amdgpu_vm_pt_clear(adev, vm, pt, immediate);
> +	r = amdgpu_vm_pt_clear(adev, vm, pt);
>  	if (r)
>  		goto error_free_pt;
> 
> @@ -813,7 +806,7 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params
> *params, * address range are actually allocated
>  			 */
>  			r = amdgpu_vm_pt_alloc(params->adev, params-
>vm,
> -					       &cursor, 
params->immediate);
> +					       &cursor);
>  			if (r)
>  				return r;
>  		}
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c index 72cfb4a6ab3e..37b5166e9a14
> 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -1372,7 +1372,7 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev,
> struct amdgpu_vm *vm, return -EINVAL;
>  	}
> 
> -	return amdgpu_vm_update_range(adev, vm, false, true, true, false, 
NULL,
> gpu_start, +	return amdgpu_vm_update_range(adev, vm, true, true, 
false,
> NULL, gpu_start, gpu_end, init_pte_value, 0, 0, NULL, NULL,
>  				      fence);
>  }
> @@ -1489,7 +1489,7 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd,
> struct svm_range *prange, (last_domain == SVM_RANGE_VRAM_DOMAIN) ? 1 : 0,
>  			 pte_flags);
> 
> -		r = amdgpu_vm_update_range(adev, vm, false, false, 
flush_tlb, true,
> +		r = amdgpu_vm_update_range(adev, vm, false, flush_tlb, 
true,
>  					   NULL, gpu_start, 
gpu_end,
>  					   pte_flags,
>  					   (last_start - 
prange->start) << PAGE_SHIFT,