[PATCH] drm/amdgpu: re-validate per VM BOs if required

[zhoucm1]

On 2018年03月20日 15:49, zhoucm1 wrote:
>
>
> On 2018年03月19日 18:50, Christian König wrote:
>> If a per VM BO ends up in a allowed domain it never moves back into the
>> prefered domain.
>>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
> Yeah, it's better than mine, Reviewed-by: Chunming Zhou 
> <david1.zhou at amd.com>
>
> the left problem is BOs validation order.
> For old bo list usage, it has fixed order for BOs in bo list,
> but for per-vm-bo feature, the order isn't fixed, which will result in 
> the performance is undulate.
> e.g. steam game F1 generally is 40fps when using old bo list, it's 
> very stable, but when enabling per-vm-bo feature, the fps is between 
> 37~40fps.
even worse, sometime, fps could drop to 18fps.
the root cause is some *KEY* BOs are randomly placed to allowed domain 
without fixed validation order.
For old bo list case, its later BOs can be evictable, so the front BOs 
are validated with preferred domain first, that is also why the 
performance is stable to 40fps when using old bo list.

Some more thinking:
Could user space pass validation order for per-vm BOs? or set BOs index 
for every per-vm BO?

Any comment?


Regards,
David Zhou

>
> Any thought?
>
> Regards,
> David Zhou
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 +++++++++++++--
>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 24474294c92a..e8b515dd032c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1770,14 +1770,16 @@ int amdgpu_vm_handle_moved(struct 
>> amdgpu_device *adev,
>>         spin_lock(&vm->status_lock);
>>       while (!list_empty(&vm->moved)) {
>> -        struct amdgpu_bo_va *bo_va;
>>           struct reservation_object *resv;
>> +        struct amdgpu_bo_va *bo_va;
>> +        struct amdgpu_bo *bo;
>>             bo_va = list_first_entry(&vm->moved,
>>               struct amdgpu_bo_va, base.vm_status);
>>           spin_unlock(&vm->status_lock);
>>   -        resv = bo_va->base.bo->tbo.resv;
>> +        bo = bo_va->base.bo;
>> +        resv = bo->tbo.resv;
>>             /* Per VM BOs never need to bo cleared in the page tables */
>>           if (resv == vm->root.base.bo->tbo.resv)
>> @@ -1797,6 +1799,15 @@ int amdgpu_vm_handle_moved(struct 
>> amdgpu_device *adev,
>>               reservation_object_unlock(resv);
>>             spin_lock(&vm->status_lock);
>> +
>> +        /* If the BO prefers to be in VRAM, but currently isn't add it
>> +         * back to the evicted list so that it gets validated again on
>> +         * the next command submission.
>> +         */
>> +        if (resv == vm->root.base.bo->tbo.resv &&
>> +            bo->preferred_domains == AMDGPU_GEM_DOMAIN_VRAM &&
>> +            bo->tbo.mem.mem_type != TTM_PL_VRAM)
>> +            list_add_tail(&bo_va->base.vm_status, &vm->evicted);
>>       }
>>       spin_unlock(&vm->status_lock);
>