[PATCH 4/8] drm/amdgpu: remove last_entry_used from the VM code

Chunming Zhou zhoucm1 at amd.com
Mon Dec 11 05:52:43 UTC 2017



On 2017年12月09日 00:41, Christian König wrote:
> Not needed any more.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 52 +++++++++++++++++++---------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  1 -
>   2 files changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 400a00fababd..ae5451bf5873 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -329,9 +329,6 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   	    to >= amdgpu_vm_num_entries(adev, level))
>   		return -EINVAL;
>   
> -	if (to > parent->last_entry_used)
> -		parent->last_entry_used = to;
> -
>   	++level;
>   	saddr = saddr & ((1 << shift) - 1);
>   	eaddr = eaddr & ((1 << shift) - 1);
> @@ -1187,16 +1184,19 @@ static int amdgpu_vm_update_pde(struct amdgpu_device *adev,
>    *
>    * Mark all PD level as invalid after an error.
>    */
> -static void amdgpu_vm_invalidate_level(struct amdgpu_vm *vm,
> -				       struct amdgpu_vm_pt *parent)
> +static void amdgpu_vm_invalidate_level(struct amdgpu_device *adev,
> +				       struct amdgpu_vm *vm,
> +				       struct amdgpu_vm_pt *parent,
> +				       unsigned level)
can we move level to struct amdgpu_vm_pt?
otherwise, it looks ok to me.

Regards,
David Zhou
>   {
> -	unsigned pt_idx;
> +	unsigned pt_idx, num_entries;
>   
>   	/*
>   	 * Recurse into the subdirectories. This recursion is harmless because
>   	 * we only have a maximum of 5 layers.
>   	 */
> -	for (pt_idx = 0; pt_idx <= parent->last_entry_used; ++pt_idx) {
> +	num_entries = amdgpu_vm_num_entries(adev, level);
> +	for (pt_idx = 0; pt_idx < num_entries; ++pt_idx) {
>   		struct amdgpu_vm_pt *entry = &parent->entries[pt_idx];
>   
>   		if (!entry->base.bo)
> @@ -1207,7 +1207,7 @@ static void amdgpu_vm_invalidate_level(struct amdgpu_vm *vm,
>   		if (list_empty(&entry->base.vm_status))
>   			list_add(&entry->base.vm_status, &vm->relocated);
>   		spin_unlock(&vm->status_lock);
> -		amdgpu_vm_invalidate_level(vm, entry);
> +		amdgpu_vm_invalidate_level(adev, vm, entry, level + 1);
>   	}
>   }
>   
> @@ -1249,7 +1249,8 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
>   
>   			r = amdgpu_vm_update_pde(adev, vm, pt, entry);
>   			if (r) {
> -				amdgpu_vm_invalidate_level(vm, &vm->root);
> +				amdgpu_vm_invalidate_level(adev, vm,
> +							   &vm->root, 0);
>   				return r;
>   			}
>   			spin_lock(&vm->status_lock);
> @@ -1652,7 +1653,7 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>   
>   error_free:
>   	amdgpu_job_free(job);
> -	amdgpu_vm_invalidate_level(vm, &vm->root);
> +	amdgpu_vm_invalidate_level(adev, vm, &vm->root, 0);
>   	return r;
>   }
>   
> @@ -2716,26 +2717,31 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   /**
>    * amdgpu_vm_free_levels - free PD/PT levels
>    *
> - * @level: PD/PT starting level to free
> + * @adev: amdgpu device structure
> + * @parent: PD/PT starting level to free
> + * @level: level of parent structure
>    *
>    * Free the page directory or page table level and all sub levels.
>    */
> -static void amdgpu_vm_free_levels(struct amdgpu_vm_pt *level)
> +static void amdgpu_vm_free_levels(struct amdgpu_device *adev,
> +				  struct amdgpu_vm_pt *parent,
> +				  unsigned level)
>   {
> -	unsigned i;
> +	unsigned i, num_entries = amdgpu_vm_num_entries(adev, level);
>   
> -	if (level->base.bo) {
> -		list_del(&level->base.bo_list);
> -		list_del(&level->base.vm_status);
> -		amdgpu_bo_unref(&level->base.bo->shadow);
> -		amdgpu_bo_unref(&level->base.bo);
> +	if (parent->base.bo) {
> +		list_del(&parent->base.bo_list);
> +		list_del(&parent->base.vm_status);
> +		amdgpu_bo_unref(&parent->base.bo->shadow);
> +		amdgpu_bo_unref(&parent->base.bo);
>   	}
>   
> -	if (level->entries)
> -		for (i = 0; i <= level->last_entry_used; i++)
> -			amdgpu_vm_free_levels(&level->entries[i]);
> +	if (parent->entries)
> +		for (i = 0; i < num_entries; i++)
> +			amdgpu_vm_free_levels(adev, &parent->entries[i],
> +					      level + 1);
>   
> -	kvfree(level->entries);
> +	kvfree(parent->entries);
>   }
>   
>   /**
> @@ -2793,7 +2799,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>   	if (r) {
>   		dev_err(adev->dev, "Leaking page tables because BO reservation failed\n");
>   	} else {
> -		amdgpu_vm_free_levels(&vm->root);
> +		amdgpu_vm_free_levels(adev, &vm->root, 0);
>   		amdgpu_bo_unreserve(root);
>   	}
>   	amdgpu_bo_unref(&root);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 43ea131dd411..7a308a1ea048 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -141,7 +141,6 @@ struct amdgpu_vm_pt {
>   
>   	/* array of page tables, one for each directory entry */
>   	struct amdgpu_vm_pt		*entries;
> -	unsigned			last_entry_used;
>   };
>   
>   #define AMDGPU_VM_FAULT(pasid, addr) (((u64)(pasid) << 48) | (addr))



More information about the amd-gfx mailing list