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.kuehl...@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;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to