[PATCH 1/1] drm/amdgpu: Fix compute VM BO params after rebase

[Christian König]

Am 06.09.2018 um 13:46 schrieb Russell, Kent:
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken at gmail.com>
> Sent: Thursday, September 06, 2018 3:36 AM
> To: Kuehling, Felix <Felix.Kuehling at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: Russell, Kent <Kent.Russell at amd.com>
> Subject: Re: [PATCH 1/1] drm/amdgpu: Fix compute VM BO params after rebase
>
> Am 06.09.2018 um 02:28 schrieb Felix Kuehling:
>> The intent of two commits was lost in the last rebase:
>>
>> 810955b drm/amdgpu: Fix acquiring VM on large-BAR systems b5d21aa
>> drm/amdgpu: Don't use shadow BO for compute context
>>
>> This commit restores the original behaviour:
>> * Don't set AMDGPU_GEM_CREATE_NO_CPU_ACCESS for page directories
>>     to allow them to be reused for compute VMs
>> * Don't create shadow BOs for page tables in compute VMs
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 +++++++++++------
>>    1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index ea5e277..5e7a3de 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -577,10 +577,13 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>>     *
>>     * @adev: amdgpu_device pointer
>>     * @vm: requesting vm
>> + * @level: level in the page table hierarchy
>> + * @no_shadow: disable creation of shadow BO for this VM
>>     * @bp: resulting BO allocation parameters
>>     */
>>    static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>> -			       int level, struct amdgpu_bo_param *bp)
>> +			       int level, bool no_shadow,
>> +			       struct amdgpu_bo_param *bp)
>>    {
>>    	memset(bp, 0, sizeof(*bp));
>>    
>> @@ -595,9 +598,8 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>    		AMDGPU_GEM_CREATE_CPU_GTT_USWC;
>>    	if (vm->use_cpu_for_update)
>>    		bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>> -	else
>> -		bp->flags |= AMDGPU_GEM_CREATE_SHADOW |
>> -			AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
>> +	else if (!no_shadow)
>> +		bp->flags |= AMDGPU_GEM_CREATE_SHADOW;
>>    	bp->type = ttm_bo_type_kernel;
>>    	if (vm->root.base.bo)
>>    		bp->resv = vm->root.base.bo->tbo.resv; @@ -626,6 +628,7 @@ static
>> int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>>    				  unsigned level, bool ats)
>>    {
>>    	unsigned shift = amdgpu_vm_level_shift(adev, level);
>> +	bool no_shadow = !vm->root.base.bo->shadow;
> Please move that logic into amdgpu_vm_bo_param().
> [KR] The issue with that is that in the other call below, we don't have access to the vm context via the vm struct, so we can't move the logic in for both calls.

Yeah, but you can look at the root PD (if present) and see if it has a 
shadow BO.

And for the root PD we can clear the flag manually after calling 
amdgpu_vm_bo_param().

Christian.

>
> Christian.
>
>>    	struct amdgpu_bo_param bp;
>>    	unsigned pt_idx, from, to;
>>    	int r;
>> @@ -650,7 +653,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>>    	saddr = saddr & ((1 << shift) - 1);
>>    	eaddr = eaddr & ((1 << shift) - 1);
>>    
>> -	amdgpu_vm_bo_param(adev, vm, level, &bp);
>> +	amdgpu_vm_bo_param(adev, vm, level, no_shadow, &bp);
>>    
>>    	/* walk over the address space and allocate the page tables */
>>    	for (pt_idx = from; pt_idx <= to; ++pt_idx) { @@ -2709,6 +2712,7 @@
>> void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t min_vm_size,
>>    int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>    		   int vm_context, unsigned int pasid)
>>    {
>> +	bool no_shadow = (vm_context == AMDGPU_VM_CONTEXT_COMPUTE);
>>    	struct amdgpu_bo_param bp;
>>    	struct amdgpu_bo *root;
>>    	int r, i;
>> @@ -2748,7 +2752,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>    		  "CPU update of VM recommended only for large BAR system\n");
>>    	vm->last_update = NULL;
>>    
>> -	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, &bp);
>> +	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, no_shadow,
>> +			   &bp);
>>    	r = amdgpu_bo_create(adev, &bp, &root);
>>    	if (r)
>>    		goto error_free_sched_entity;