[PATCH 4/6] create shadow bo using amdgpu_bo_create_shadow()

[Nirmoy]

On 4/23/21 9:55 AM, Christian König wrote:
> Am 22.04.21 um 17:40 schrieb Nirmoy Das:
>> Shadow BOs are only needed for vm code so call amdgpu_bo_create_shadow()
>> directly instead of depending on amdgpu_bo_create().
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 70 +++++++++++++++++---------
>>   1 file changed, 47 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 577148a4ffaa..bb5506ff80dd 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -850,35 +850,63 @@ static int amdgpu_vm_clear_bo(struct 
>> amdgpu_device *adev,
>>   }
>>     /**
>> - * amdgpu_vm_bo_param - fill in parameters for PD/PT allocation
>> + * amdgpu_vm_pt_create - create bo for PD/PT
>>    *
>>    * @adev: amdgpu_device pointer
>>    * @vm: requesting vm
>>    * @level: the page table level
>>    * @immediate: use a immediate update
>> - * @bp: resulting BO allocation parameters
>> + * @bo: pointer to the buffer object pointer
>>    */
>> -static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct 
>> amdgpu_vm *vm,
>> +static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
>> +                   struct amdgpu_vm *vm,
>>                      int level, bool immediate,
>> -                   struct amdgpu_bo_param *bp)
>> +                   struct amdgpu_bo **bo)
>>   {
>> -    memset(bp, 0, sizeof(*bp));
>> +    struct amdgpu_bo_param bp;
>
>> +    bool create_shadow = false;
>
> As far as I can see this variable is only set and never used.
>
> Please clean that up, apart from that looks good to me.


Sorry I missed that :/ . Re-sending!


>
> Christian.
>
>> +    int r;
>>   -    bp->size = amdgpu_vm_bo_size(adev, level);
>> -    bp->byte_align = AMDGPU_GPU_PAGE_SIZE;
>> -    bp->domain = AMDGPU_GEM_DOMAIN_VRAM;
>> -    bp->domain = amdgpu_bo_get_preferred_pin_domain(adev, bp->domain);
>> -    bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
>> +    memset(&bp, 0, sizeof(bp));
>> +
>> +    bp.size = amdgpu_vm_bo_size(adev, level);
>> +    bp.byte_align = AMDGPU_GPU_PAGE_SIZE;
>> +    bp.domain = AMDGPU_GEM_DOMAIN_VRAM;
>> +    bp.domain = amdgpu_bo_get_preferred_pin_domain(adev, bp.domain);
>> +    bp.flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
>>           AMDGPU_GEM_CREATE_CPU_GTT_USWC;
>> -    bp->bo_ptr_size = sizeof(struct amdgpu_bo);
>> +    bp.bo_ptr_size = sizeof(struct amdgpu_bo);
>>       if (vm->use_cpu_for_update)
>> -        bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>> +        bp.flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>>       else if (!vm->root.base.bo || vm->root.base.bo->shadow)
>> -        bp->flags |= AMDGPU_GEM_CREATE_SHADOW;
>> -    bp->type = ttm_bo_type_kernel;
>> -    bp->no_wait_gpu = immediate;
>> +        create_shadow = true;
>> +
>> +    bp.type = ttm_bo_type_kernel;
>> +    bp.no_wait_gpu = immediate;
>>       if (vm->root.base.bo)
>> -        bp->resv = vm->root.base.bo->tbo.base.resv;
>> +        bp.resv = vm->root.base.bo->tbo.base.resv;
>> +
>> +    r = amdgpu_bo_create(adev, &bp, bo);
>> +    if (r)
>> +        return r;
>> +
>> +    if (vm->is_compute_context && (adev->flags & AMD_IS_APU))
>> +        return 0;
>> +
>> +    if (!bp.resv)
>> +        WARN_ON(dma_resv_lock((*bo)->tbo.base.resv,
>> +                      NULL));
>> +    r = amdgpu_bo_create_shadow(adev, bp.size, *bo);
>> +
>> +    if (!bp.resv)
>> +        dma_resv_unlock((*bo)->tbo.base.resv);
>> +
>> +    if (r) {
>> +        amdgpu_bo_unref(bo);
>> +        return r;
>> +    }
>> +
>> +    return 0;
>>   }
>>     /**
>> @@ -901,7 +929,6 @@ static int amdgpu_vm_alloc_pts(struct 
>> amdgpu_device *adev,
>>                      bool immediate)
>>   {
>>       struct amdgpu_vm_pt *entry = cursor->entry;
>> -    struct amdgpu_bo_param bp;
>>       struct amdgpu_bo *pt;
>>       int r;
>>   @@ -919,9 +946,7 @@ static int amdgpu_vm_alloc_pts(struct 
>> amdgpu_device *adev,
>>       if (entry->base.bo)
>>           return 0;
>>   -    amdgpu_vm_bo_param(adev, vm, cursor->level, immediate, &bp);
>> -
>> -    r = amdgpu_bo_create(adev, &bp, &pt);
>> +    r = amdgpu_vm_pt_create(adev, vm, cursor->level, immediate, &pt);
>>       if (r)
>>           return r;
>>   @@ -2784,7 +2809,6 @@ long amdgpu_vm_wait_idle(struct amdgpu_vm 
>> *vm, long timeout)
>>    */
>>   int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm 
>> *vm, u32 pasid)
>>   {
>> -    struct amdgpu_bo_param bp;
>>       struct amdgpu_bo *root;
>>       int r, i;
>>   @@ -2835,8 +2859,8 @@ int amdgpu_vm_init(struct amdgpu_device 
>> *adev, struct amdgpu_vm *vm, u32 pasid)
>>       mutex_init(&vm->eviction_lock);
>>       vm->evicting = false;
>>   -    amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, 
>> false, &bp);
>> -    r = amdgpu_bo_create(adev, &bp, &root);
>> +    r = amdgpu_vm_pt_create(adev, vm, adev->vm_manager.root_level,
>> +                false, &root);
>>       if (r)
>>           goto error_free_delayed;
>