Am 21.03.2018 um 11:31 schrieb 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.
I've already tried to give the list a certain order in
amdgpu_vm_bo_invalidate(), e.g. we add kernel BOs (page tables) to the
front and normal BOs to the back of the list.
What you could do is to splice the evicted list to a local copy in
amdgpu_vm_validate_pt_bos(), then use list_sort() from linux/list_sort.h.
As criteria we could use the BO in kernel priority + some new user
definable priority, this way page tables are still validated first.
Regards,
Christian.
>
> 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);
>>>
>>
>
