[1/2] drm/amdgpu: abstract bo_base init function

Submitted by Zhou, David(ChunMing) on April 24, 2018, 7:35 a.m.

Details

Message ID 20180424073503.30175-1-david1.zhou@amd.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Zhou, David(ChunMing) April 24, 2018, 7:35 a.m.
Change-Id: Ia3e57dbacff05f32b6c02e29aeadabd36f08028e
Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 75 ++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 35 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8c34060e130f..c75b96433ee7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -94,6 +94,9 @@  struct amdgpu_prt_cb {
 	struct dma_fence_cb cb;
 };
 
+static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
+				   struct amdgpu_vm *vm,
+				   struct amdgpu_bo *bo);
 /**
  * amdgpu_vm_level_shift - return the addr shift for each level
  *
@@ -446,11 +449,9 @@  static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
 			*/
 			pt->parent = amdgpu_bo_ref(parent->base.bo);
 
-			entry->base.vm = vm;
-			entry->base.bo = pt;
-			list_add_tail(&entry->base.bo_list, &pt->va);
+			amdgpu_vm_bo_base_init(&entry->base, vm, pt);
 			spin_lock(&vm->status_lock);
-			list_add(&entry->base.vm_status, &vm->relocated);
+			list_move(&entry->base.vm_status, &vm->relocated);
 			spin_unlock(&vm->status_lock);
 		}
 
@@ -1821,6 +1822,35 @@  int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
 	return r;
 }
 
+static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
+				   struct amdgpu_vm *vm,
+				   struct amdgpu_bo *bo)
+{
+	base->vm = vm;
+	base->bo = bo;
+	INIT_LIST_HEAD(&base->bo_list);
+	INIT_LIST_HEAD(&base->vm_status);
+
+	if (!bo)
+		return;
+	list_add_tail(&base->bo_list, &bo->va);
+
+	if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
+		return;
+
+	if (bo->preferred_domains &
+	    amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type))
+		return;
+
+	/*
+	 * we checked all the prerequisites, but it looks like this per vm bo
+	 * is currently evicted. add the bo to the evicted list to make sure it
+	 * is validated on next vm use to avoid fault.
+	 * */
+	spin_lock(&vm->status_lock);
+	list_move_tail(&base->vm_status, &vm->evicted);
+	spin_unlock(&vm->status_lock);
+}
 /**
  * amdgpu_vm_bo_add - add a bo to a specific vm
  *
@@ -1844,36 +1874,12 @@  struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
 	if (bo_va == NULL) {
 		return NULL;
 	}
-	bo_va->base.vm = vm;
-	bo_va->base.bo = bo;
-	INIT_LIST_HEAD(&bo_va->base.bo_list);
-	INIT_LIST_HEAD(&bo_va->base.vm_status);
+	amdgpu_vm_bo_base_init(&bo_va->base, vm, bo);
 
 	bo_va->ref_count = 1;
 	INIT_LIST_HEAD(&bo_va->valids);
 	INIT_LIST_HEAD(&bo_va->invalids);
 
-	if (!bo)
-		return bo_va;
-
-	list_add_tail(&bo_va->base.bo_list, &bo->va);
-
-	if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
-		return bo_va;
-
-	if (bo->preferred_domains &
-	    amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type))
-		return bo_va;
-
-	/*
-	 * We checked all the prerequisites, but it looks like this per VM BO
-	 * is currently evicted. add the BO to the evicted list to make sure it
-	 * is validated on next VM use to avoid fault.
-	 * */
-	spin_lock(&vm->status_lock);
-	list_move_tail(&bo_va->base.vm_status, &vm->evicted);
-	spin_unlock(&vm->status_lock);
-
 	return bo_va;
 }
 
@@ -2373,6 +2379,7 @@  int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		   int vm_context, unsigned int pasid)
 {
 	struct amdgpu_bo_param bp;
+	struct amdgpu_bo *root;
 	const unsigned align = min(AMDGPU_VM_PTB_ALIGN_SIZE,
 		AMDGPU_VM_PTE_COUNT(adev) * 8);
 	unsigned ring_instance;
@@ -2434,23 +2441,21 @@  int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	bp.flags = flags;
 	bp.type = ttm_bo_type_kernel;
 	bp.resv = NULL;
-	r = amdgpu_bo_create(adev, &bp, &vm->root.base.bo);
+	r = amdgpu_bo_create(adev, &bp, &root);
 	if (r)
 		goto error_free_sched_entity;
 
-	r = amdgpu_bo_reserve(vm->root.base.bo, true);
+	r = amdgpu_bo_reserve(root, true);
 	if (r)
 		goto error_free_root;
 
-	r = amdgpu_vm_clear_bo(adev, vm, vm->root.base.bo,
+	r = amdgpu_vm_clear_bo(adev, vm, root,
 			       adev->vm_manager.root_level,
 			       vm->pte_support_ats);
 	if (r)
 		goto error_unreserve;
 
-	vm->root.base.vm = vm;
-	list_add_tail(&vm->root.base.bo_list, &vm->root.base.bo->va);
-	list_add_tail(&vm->root.base.vm_status, &vm->evicted);
+	amdgpu_vm_bo_base_init(&vm->root.base, vm, root);
 	amdgpu_bo_unreserve(vm->root.base.bo);
 
 	if (pasid) {

Comments

On 04/24/2018 03:35 PM, Chunming Zhou wrote:
> Change-Id: Ia3e57dbacff05f32b6c02e29aeadabd36f08028e
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

some trivial comments.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 75 ++++++++++++++++++----------------
>   1 file changed, 40 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 8c34060e130f..c75b96433ee7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -94,6 +94,9 @@ struct amdgpu_prt_cb {
>   	struct dma_fence_cb cb;
>   };
>
> +static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
> +				   struct amdgpu_vm *vm,
> +				   struct amdgpu_bo *bo);

Move the function upper, we could avoid such declaration.

>   /**
>    * amdgpu_vm_level_shift - return the addr shift for each level
>    *
> @@ -446,11 +449,9 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   			*/
>   			pt->parent = amdgpu_bo_ref(parent->base.bo);
>
> -			entry->base.vm = vm;
> -			entry->base.bo = pt;
> -			list_add_tail(&entry->base.bo_list, &pt->va);
> +			amdgpu_vm_bo_base_init(&entry->base, vm, pt);
>   			spin_lock(&vm->status_lock);
> -			list_add(&entry->base.vm_status, &vm->relocated);
> +			list_move(&entry->base.vm_status, &vm->relocated);
>   			spin_unlock(&vm->status_lock);
>   		}
>
> @@ -1821,6 +1822,35 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>   	return r;
>   }
>
> +static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
> +				   struct amdgpu_vm *vm,
> +				   struct amdgpu_bo *bo)
> +{
> +	base->vm = vm;
> +	base->bo = bo;
> +	INIT_LIST_HEAD(&base->bo_list);
> +	INIT_LIST_HEAD(&base->vm_status);
> +
> +	if (!bo)
> +		return;
> +	list_add_tail(&base->bo_list, &bo->va);
> +
> +	if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
> +		return;
> +
> +	if (bo->preferred_domains &
> +	    amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type))
> +		return;
> +
> +	/*
> +	 * we checked all the prerequisites, but it looks like this per vm bo
> +	 * is currently evicted. add the bo to the evicted list to make sure it
> +	 * is validated on next vm use to avoid fault.
> +	 * */

/*
  *
  */  is expected

> +	spin_lock(&vm->status_lock);
> +	list_move_tail(&base->vm_status, &vm->evicted);
> +	spin_unlock(&vm->status_lock);
> +}
>   /**
>    * amdgpu_vm_bo_add - add a bo to a specific vm
>    *
> @@ -1844,36 +1874,12 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>   	if (bo_va == NULL) {
>   		return NULL;
>   	}
> -	bo_va->base.vm = vm;
> -	bo_va->base.bo = bo;
> -	INIT_LIST_HEAD(&bo_va->base.bo_list);
> -	INIT_LIST_HEAD(&bo_va->base.vm_status);
> +	amdgpu_vm_bo_base_init(&bo_va->base, vm, bo);
>
>   	bo_va->ref_count = 1;
>   	INIT_LIST_HEAD(&bo_va->valids);
>   	INIT_LIST_HEAD(&bo_va->invalids);
>
> -	if (!bo)
> -		return bo_va;
> -
> -	list_add_tail(&bo_va->base.bo_list, &bo->va);
> -
> -	if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
> -		return bo_va;
> -
> -	if (bo->preferred_domains &
> -	    amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type))
> -		return bo_va;
> -
> -	/*
> -	 * We checked all the prerequisites, but it looks like this per VM BO
> -	 * is currently evicted. add the BO to the evicted list to make sure it
> -	 * is validated on next VM use to avoid fault.
> -	 * */
> -	spin_lock(&vm->status_lock);
> -	list_move_tail(&bo_va->base.vm_status, &vm->evicted);
> -	spin_unlock(&vm->status_lock);
> -
>   	return bo_va;
>   }
>
> @@ -2373,6 +2379,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		   int vm_context, unsigned int pasid)
>   {
>   	struct amdgpu_bo_param bp;
> +	struct amdgpu_bo *root;
>   	const unsigned align = min(AMDGPU_VM_PTB_ALIGN_SIZE,
>   		AMDGPU_VM_PTE_COUNT(adev) * 8);
>   	unsigned ring_instance;
> @@ -2434,23 +2441,21 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	bp.flags = flags;
>   	bp.type = ttm_bo_type_kernel;
>   	bp.resv = NULL;
> -	r = amdgpu_bo_create(adev, &bp, &vm->root.base.bo);
> +	r = amdgpu_bo_create(adev, &bp, &root);
>   	if (r)
>   		goto error_free_sched_entity;
>
> -	r = amdgpu_bo_reserve(vm->root.base.bo, true);
> +	r = amdgpu_bo_reserve(root, true);
>   	if (r)
>   		goto error_free_root;
>
> -	r = amdgpu_vm_clear_bo(adev, vm, vm->root.base.bo,
> +	r = amdgpu_vm_clear_bo(adev, vm, root,
>   			       adev->vm_manager.root_level,
>   			       vm->pte_support_ats);
>   	if (r)
>   		goto error_unreserve;
>
> -	vm->root.base.vm = vm;
> -	list_add_tail(&vm->root.base.bo_list, &vm->root.base.bo->va);
> -	list_add_tail(&vm->root.base.vm_status, &vm->evicted);
> +	amdgpu_vm_bo_base_init(&vm->root.base, vm, root);
>   	amdgpu_bo_unreserve(vm->root.base.bo);
>
>   	if (pasid) {
>
Am 26.04.2018 um 05:01 schrieb Zhang, Jerry (Junwei):
> On 04/24/2018 03:35 PM, Chunming Zhou wrote:
>> Change-Id: Ia3e57dbacff05f32b6c02e29aeadabd36f08028e
>> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>
>
> some trivial comments.

Both a nice cleanup as well as the right comments.

With Jerry's comments addressed Reviewed-by: Christian König 
<christian.koenig@amd.com> as well.

>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 75 
>> ++++++++++++++++++----------------
>>   1 file changed, 40 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 8c34060e130f..c75b96433ee7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -94,6 +94,9 @@ struct amdgpu_prt_cb {
>>       struct dma_fence_cb cb;
>>   };
>>
>> +static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
>> +                   struct amdgpu_vm *vm,
>> +                   struct amdgpu_bo *bo);
>
> Move the function upper, we could avoid such declaration.
>
>>   /**
>>    * amdgpu_vm_level_shift - return the addr shift for each level
>>    *
>> @@ -446,11 +449,9 @@ static int amdgpu_vm_alloc_levels(struct 
>> amdgpu_device *adev,
>>               */
>>               pt->parent = amdgpu_bo_ref(parent->base.bo);
>>
>> -            entry->base.vm = vm;
>> -            entry->base.bo = pt;
>> -            list_add_tail(&entry->base.bo_list, &pt->va);
>> +            amdgpu_vm_bo_base_init(&entry->base, vm, pt);
>>               spin_lock(&vm->status_lock);
>> -            list_add(&entry->base.vm_status, &vm->relocated);
>> +            list_move(&entry->base.vm_status, &vm->relocated);
>>               spin_unlock(&vm->status_lock);
>>           }
>>
>> @@ -1821,6 +1822,35 @@ int amdgpu_vm_handle_moved(struct 
>> amdgpu_device *adev,
>>       return r;
>>   }
>>
>> +static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
>> +                   struct amdgpu_vm *vm,
>> +                   struct amdgpu_bo *bo)
>> +{
>> +    base->vm = vm;
>> +    base->bo = bo;
>> +    INIT_LIST_HEAD(&base->bo_list);
>> +    INIT_LIST_HEAD(&base->vm_status);
>> +
>> +    if (!bo)
>> +        return;
>> +    list_add_tail(&base->bo_list, &bo->va);
>> +
>> +    if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
>> +        return;
>> +
>> +    if (bo->preferred_domains &
>> +        amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type))
>> +        return;
>> +
>> +    /*
>> +     * we checked all the prerequisites, but it looks like this per 
>> vm bo
>> +     * is currently evicted. add the bo to the evicted list to make 
>> sure it
>> +     * is validated on next vm use to avoid fault.
>> +     * */
>
> /*
>  *
>  */  is expected
>
>> +    spin_lock(&vm->status_lock);
>> +    list_move_tail(&base->vm_status, &vm->evicted);
>> +    spin_unlock(&vm->status_lock);
>> +}
>>   /**
>>    * amdgpu_vm_bo_add - add a bo to a specific vm
>>    *
>> @@ -1844,36 +1874,12 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
>> amdgpu_device *adev,
>>       if (bo_va == NULL) {
>>           return NULL;
>>       }
>> -    bo_va->base.vm = vm;
>> -    bo_va->base.bo = bo;
>> -    INIT_LIST_HEAD(&bo_va->base.bo_list);
>> -    INIT_LIST_HEAD(&bo_va->base.vm_status);
>> +    amdgpu_vm_bo_base_init(&bo_va->base, vm, bo);
>>
>>       bo_va->ref_count = 1;
>>       INIT_LIST_HEAD(&bo_va->valids);
>>       INIT_LIST_HEAD(&bo_va->invalids);
>>
>> -    if (!bo)
>> -        return bo_va;
>> -
>> -    list_add_tail(&bo_va->base.bo_list, &bo->va);
>> -
>> -    if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
>> -        return bo_va;
>> -
>> -    if (bo->preferred_domains &
>> -        amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type))
>> -        return bo_va;
>> -
>> -    /*
>> -     * We checked all the prerequisites, but it looks like this per 
>> VM BO
>> -     * is currently evicted. add the BO to the evicted list to make 
>> sure it
>> -     * is validated on next VM use to avoid fault.
>> -     * */
>> -    spin_lock(&vm->status_lock);
>> -    list_move_tail(&bo_va->base.vm_status, &vm->evicted);
>> -    spin_unlock(&vm->status_lock);
>> -
>>       return bo_va;
>>   }
>>
>> @@ -2373,6 +2379,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, 
>> struct amdgpu_vm *vm,
>>              int vm_context, unsigned int pasid)
>>   {
>>       struct amdgpu_bo_param bp;
>> +    struct amdgpu_bo *root;
>>       const unsigned align = min(AMDGPU_VM_PTB_ALIGN_SIZE,
>>           AMDGPU_VM_PTE_COUNT(adev) * 8);
>>       unsigned ring_instance;
>> @@ -2434,23 +2441,21 @@ int amdgpu_vm_init(struct amdgpu_device 
>> *adev, struct amdgpu_vm *vm,
>>       bp.flags = flags;
>>       bp.type = ttm_bo_type_kernel;
>>       bp.resv = NULL;
>> -    r = amdgpu_bo_create(adev, &bp, &vm->root.base.bo);
>> +    r = amdgpu_bo_create(adev, &bp, &root);
>>       if (r)
>>           goto error_free_sched_entity;
>>
>> -    r = amdgpu_bo_reserve(vm->root.base.bo, true);
>> +    r = amdgpu_bo_reserve(root, true);
>>       if (r)
>>           goto error_free_root;
>>
>> -    r = amdgpu_vm_clear_bo(adev, vm, vm->root.base.bo,
>> +    r = amdgpu_vm_clear_bo(adev, vm, root,
>>                      adev->vm_manager.root_level,
>>                      vm->pte_support_ats);
>>       if (r)
>>           goto error_unreserve;
>>
>> -    vm->root.base.vm = vm;
>> -    list_add_tail(&vm->root.base.bo_list, &vm->root.base.bo->va);
>> -    list_add_tail(&vm->root.base.vm_status, &vm->evicted);
>> +    amdgpu_vm_bo_base_init(&vm->root.base, vm, root);
>>       amdgpu_bo_unreserve(vm->root.base.bo);
>>
>>       if (pasid) {
>>