[PATCH 8/8] drm/amdgpu: rework page directory filling

Deucher, Alexander Alexander.Deucher at amd.com
Thu Aug 10 15:01:09 UTC 2017


> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Christian König
> Sent: Thursday, August 10, 2017 10:51 AM
> To: amd-gfx at lists.freedesktop.org
> Subject: [PATCH 8/8] drm/amdgpu: rework page directory filling
> 
> From: Christian König <christian.koenig at amd.com>
> 
> Keep track off relocated PDs/PTs instead of walking and checking all PDs.
> 
> Signed-off-by: Christian König <christian.koenig at amd.com>

Some comments on patch 4.  The rest are:
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 85
> ++++++++++++++++++++++------------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  3 ++
>  2 files changed, 59 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 8e2ce91..8056e4a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -204,7 +204,10 @@ int amdgpu_vm_validate_pt_bos(struct
> amdgpu_device *adev, struct amdgpu_vm *vm,
>  		}
> 
>  		spin_lock(&vm->status_lock);
> -		list_del_init(&bo_base->vm_status);
> +		if (bo->parent)
> +			list_move(&bo_base->vm_status, &vm->relocated);
> +		else
> +			list_del_init(&bo_base->vm_status);
>  	}
>  	spin_unlock(&vm->status_lock);
> 
> @@ -303,8 +306,10 @@ static int amdgpu_vm_alloc_levels(struct
> amdgpu_device *adev,
>  			entry->base.vm = vm;
>  			entry->base.bo = pt;
>  			list_add_tail(&entry->base.bo_list, &pt->va);
> -			INIT_LIST_HEAD(&entry->base.vm_status);
> -			entry->addr = 0;
> +			spin_lock(&vm->status_lock);
> +			list_add(&entry->base.vm_status, &vm->relocated);
> +			spin_unlock(&vm->status_lock);
> +			entry->addr = ~0ULL;
>  		}
> 
>  		if (level < adev->vm_manager.num_level) {
> @@ -989,18 +994,17 @@ static int amdgpu_vm_wait_pd(struct
> amdgpu_device *adev, struct amdgpu_vm *vm,
>   */
>  static int amdgpu_vm_update_level(struct amdgpu_device *adev,
>  				  struct amdgpu_vm *vm,
> -				  struct amdgpu_vm_pt *parent,
> -				  unsigned level)
> +				  struct amdgpu_vm_pt *parent)
>  {
>  	struct amdgpu_bo *shadow;
>  	struct amdgpu_ring *ring = NULL;
>  	uint64_t pd_addr, shadow_addr = 0;
> -	uint32_t incr = amdgpu_vm_bo_size(adev, level + 1);
>  	uint64_t last_pde = ~0, last_pt = ~0, last_shadow = ~0;
>  	unsigned count = 0, pt_idx, ndw = 0;
>  	struct amdgpu_job *job;
>  	struct amdgpu_pte_update_params params;
>  	struct dma_fence *fence = NULL;
> +	uint32_t incr;
> 
>  	int r;
> 
> @@ -1048,12 +1052,17 @@ static int amdgpu_vm_update_level(struct
> amdgpu_device *adev,
> 
>  	/* walk over the address space and update the directory */
>  	for (pt_idx = 0; pt_idx <= parent->last_entry_used; ++pt_idx) {
> -		struct amdgpu_bo *bo = parent->entries[pt_idx].base.bo;
> +		struct amdgpu_vm_pt *entry = &parent->entries[pt_idx];
> +		struct amdgpu_bo *bo = entry->base.bo;
>  		uint64_t pde, pt;
> 
>  		if (bo == NULL)
>  			continue;
> 
> +		spin_lock(&vm->status_lock);
> +		list_del_init(&entry->base.vm_status);
> +		spin_unlock(&vm->status_lock);
> +
>  		pt = amdgpu_bo_gpu_offset(bo);
>  		pt = amdgpu_gart_get_vm_pde(adev, pt);
>  		if ((parent->entries[pt_idx].addr & AMDGPU_PDE_PTE) ||
> @@ -1063,6 +1072,7 @@ static int amdgpu_vm_update_level(struct
> amdgpu_device *adev,
>  		parent->entries[pt_idx].addr = pt | AMDGPU_PTE_VALID;
> 
>  		pde = pd_addr + pt_idx * 8;
> +		incr = amdgpu_bo_size(bo);
>  		if (((last_pde + 8 * count) != pde) ||
>  		    ((last_pt + incr * count) != pt) ||
>  		    (count == AMDGPU_VM_MAX_UPDATE_SIZE)) {
> @@ -1123,20 +1133,6 @@ static int amdgpu_vm_update_level(struct
> amdgpu_device *adev,
>  			dma_fence_put(fence);
>  		}
>  	}
> -	/*
> -	 * 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) {
> -		struct amdgpu_vm_pt *entry = &parent->entries[pt_idx];
> -
> -		if (!entry->base.bo)
> -			continue;
> -
> -		r = amdgpu_vm_update_level(adev, vm, entry, level + 1);
> -		if (r)
> -			return r;
> -	}
> 
>  	return 0;
> 
> @@ -1152,7 +1148,8 @@ static int amdgpu_vm_update_level(struct
> amdgpu_device *adev,
>   *
>   * Mark all PD level as invalid after an error.
>   */
> -static void amdgpu_vm_invalidate_level(struct amdgpu_vm_pt *parent)
> +static void amdgpu_vm_invalidate_level(struct amdgpu_vm *vm,
> +				       struct amdgpu_vm_pt *parent)
>  {
>  	unsigned pt_idx;
> 
> @@ -1167,7 +1164,10 @@ static void amdgpu_vm_invalidate_level(struct
> amdgpu_vm_pt *parent)
>  			continue;
> 
>  		entry->addr = ~0ULL;
> -		amdgpu_vm_invalidate_level(entry);
> +		spin_lock(&vm->status_lock);
> +		list_move(&entry->base.vm_status, &vm->relocated);
> +		spin_unlock(&vm->status_lock);
> +		amdgpu_vm_invalidate_level(vm, entry);
>  	}
>  }
> 
> @@ -1185,9 +1185,31 @@ int amdgpu_vm_update_directories(struct
> amdgpu_device *adev,
>  {
>  	int r;
> 
> -	r = amdgpu_vm_update_level(adev, vm, &vm->root, 0);
> -	if (r)
> -		amdgpu_vm_invalidate_level(&vm->root);
> +	spin_lock(&vm->status_lock);
> +	while (!list_empty(&vm->relocated)) {
> +		struct amdgpu_vm_bo_base *bo_base, *parent;
> +		struct amdgpu_vm_pt *pt;
> +		struct amdgpu_bo *bo;
> +
> +		bo_base = list_first_entry(&vm->relocated,
> +					   struct amdgpu_vm_bo_base,
> +					   vm_status);
> +		spin_unlock(&vm->status_lock);
> +
> +		bo = bo_base->bo->parent;
> +		parent = list_first_entry(&bo->va, struct
> amdgpu_vm_bo_base,
> +					  bo_list);
> +		pt = container_of(parent, struct amdgpu_vm_pt, base);
> +
> +		r = amdgpu_vm_update_level(adev, vm, pt);
> +		if (r) {
> +			amdgpu_vm_invalidate_level(vm, &vm->root);
> +			break;
> +		}
> +
> +		spin_lock(&vm->status_lock);
> +	}
> +	spin_unlock(&vm->status_lock);
> 
>  	if (vm->use_cpu_for_update) {
>  		/* Flush HDP */
> @@ -1575,7 +1597,7 @@ static int amdgpu_vm_bo_update_mapping(struct
> amdgpu_device *adev,
> 
>  error_free:
>  	amdgpu_job_free(job);
> -	amdgpu_vm_invalidate_level(&vm->root);
> +	amdgpu_vm_invalidate_level(vm, &vm->root);
>  	return r;
>  }
> 
> @@ -2374,9 +2396,13 @@ void amdgpu_vm_bo_invalidate(struct
> amdgpu_device *adev,
>  			continue;
>  		}
> 
> -		/* Don't add page tables to the moved state */
> -		if (bo->tbo.type == ttm_bo_type_kernel)
> +		if (bo->tbo.type == ttm_bo_type_kernel) {
> +			spin_lock(&bo_base->vm->status_lock);
> +			if (list_empty(&bo_base->vm_status))
> +				list_add(&bo_base->vm_status, &vm-
> >relocated);
> +			spin_unlock(&bo_base->vm->status_lock);
>  			continue;
> +		}
> 
>  		spin_lock(&bo_base->vm->status_lock);
>  		if (list_empty(&bo_base->vm_status))
> @@ -2451,6 +2477,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev,
> struct amdgpu_vm *vm,
>  		vm->reserved_vmid[i] = NULL;
>  	spin_lock_init(&vm->status_lock);
>  	INIT_LIST_HEAD(&vm->evicted);
> +	INIT_LIST_HEAD(&vm->relocated);
>  	INIT_LIST_HEAD(&vm->moved);
>  	INIT_LIST_HEAD(&vm->cleared);
>  	INIT_LIST_HEAD(&vm->freed);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index a649950..bc1c995 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -131,6 +131,9 @@ struct amdgpu_vm {
>  	/* BOs who needs a validation */
>  	struct list_head	evicted;
> 
> +	/* PT BOs which relocated and their parent need an update */
> +	struct list_head	relocated;
> +
>  	/* BOs moved, but not yet updated in the PT */
>  	struct list_head	moved;
> 
> --
> 2.7.4
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


More information about the amd-gfx mailing list