[PATCH 1/1] drm/amdgpu: Wait for newly allocated PTs to be idle

[Kuehling, Felix]

When we use SDMA, we don't wait on the CPU. The GPU scheduler waits for 
the fences on the root PD reservation before executing the SDMA IB. 
amdgpu_vm_bo_update_mapping gets those fences and builds the sync object 
for the scheduler after all the page tables have been allocated, so it 
should be no problem.

Regards,
   Felix

On 2019-03-12 6:13 p.m., Liu, Shaoyun wrote:
> Hi,
>
> I think even use SDMA to update PTE we may still need to wait the clear
> job to be completed if we can not guarantee the clear and set PTE job
> will use the exact same SDMA engine ( Did we use a dedicate SDMA engine
> for PTE update including clear? ).  But if we didn't use the  same
> engine , it may explain why the  test failed occasionally.
>
> Regards
>
> shaoyun.liu
>
>
>
> On 2019-03-12 5:20 p.m., Kuehling, Felix wrote:
>> When page table are updated by the CPU, synchronize with the
>> allocation and initialization of newly allocated page tables.
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 20 +++++++++++++-------
>>    1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 8603c85..4303436 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -899,17 +899,17 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>    }
>>    
>>    /**
>> - * amdgpu_vm_alloc_pts - Allocate page tables.
>> + * amdgpu_vm_alloc_pts - Allocate a specific page table
>>     *
>>     * @adev: amdgpu_device pointer
>>     * @vm: VM to allocate page tables for
>> - * @saddr: Start address which needs to be allocated
>> - * @size: Size from start address we need.
>> + * @cursor: Which page table to allocate
>>     *
>> - * Make sure the page directories and page tables are allocated
>> + * Make sure a specific page table or directory is allocated.
>>     *
>>     * Returns:
>> - * 0 on success, errno otherwise.
>> + * 1 if page table needed to be allocated, 0 if page table was already
>> + * allocated, negative errno if an error occurred.
>>     */
>>    static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>>    			       struct amdgpu_vm *vm,
>> @@ -956,7 +956,7 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>>    	if (r)
>>    		goto error_free_pt;
>>    
>> -	return 0;
>> +	return 1;
>>    
>>    error_free_pt:
>>    	amdgpu_bo_unref(&pt->shadow);
>> @@ -1621,10 +1621,12 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
>>    		unsigned shift, parent_shift, mask;
>>    		uint64_t incr, entry_end, pe_start;
>>    		struct amdgpu_bo *pt;
>> +		bool need_to_sync;
>>    
>>    		r = amdgpu_vm_alloc_pts(params->adev, params->vm, &cursor);
>> -		if (r)
>> +		if (r < 0)
>>    			return r;
>> +		need_to_sync = (r && params->vm->use_cpu_for_update);
>>    
>>    		pt = cursor.entry->base.bo;
>>    
>> @@ -1672,6 +1674,10 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
>>    		entry_end += cursor.pfn & ~(entry_end - 1);
>>    		entry_end = min(entry_end, end);
>>    
>> +		if (need_to_sync)
>> +			r = amdgpu_bo_sync_wait(params->vm->root.base.bo,
>> +						AMDGPU_FENCE_OWNER_VM, true);
>> +
>>    		do {
>>    			uint64_t upd_end = min(entry_end, frag_end);
>>    			unsigned nptes = (upd_end - frag_start) >> shift;