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

[zhoucm1]

On 2018年03月20日 17:13, zhoucm1 wrote:
>
>
> 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?
Ping...
If no objection, I will try to make a bo list for per-vm case to 
determine the validation order.

Regards,
David Zhou
>
> 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);
>>
>