[PATCH 2/6] drm/amdgpu: add dedicated vmid field in vm struct

[Christian König]

Am 26.04.2017 um 11:14 schrieb zhoucm1:
>
>
> On 2017年04月26日 17:10, Christian König wrote:
>> Am 26.04.2017 um 11:05 schrieb zhoucm1:
>>>
>>>
>>> On 2017年04月26日 16:49, Christian König wrote:
>>>> Am 26.04.2017 um 09:09 schrieb Zhang, Jerry (Junwei):
>>>>> On 04/24/2017 01:57 PM, Chunming Zhou wrote:
>>>>>> Change-Id: Id728e20366c8a1ae90d4e901dc80e136e2a613bb
>>>>>> Signed-off-by: Chunming Zhou <David1.Zhou at amd.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 ++++++++++++++++-
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 ++
>>>>>>   2 files changed, 18 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> index eb429c5..acf9102 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> @@ -2144,10 +2144,12 @@ int amdgpu_vm_init(struct amdgpu_device 
>>>>>> *adev, struct amdgpu_vm *vm,
>>>>>>       unsigned ring_instance;
>>>>>>       struct amdgpu_ring *ring;
>>>>>>       struct amd_sched_rq *rq;
>>>>>> -    int r;
>>>>>> +    int r, i;
>>>>>>
>>>>>>       vm->va = RB_ROOT;
>>>>>>       vm->client_id = 
>>>>>> atomic64_inc_return(&adev->vm_manager.client_counter);
>>>>>> +    for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
>>>>>> +        vm->dedicated_vmid[i] = NULL;
>>>>>
>>>>> Maybe it's better to give it a consistent name as resv_vmid, or 
>>>>> anything like that.
>>>>
>>>> Yes, agree.
>>> I think the reserved vmid is dedicated to this vm, I don't know 
>>> where this name doesn't make sense.
>>>
>>>> And if I'm not completely mistaken we still should only apply that 
>>>> to the GFX hub on Vega10.
>>> David Mao required mmhub as well. IIRC, we don't have necessary to 
>>> argue more on this.
>>
>> I think we still have. There is no technical reason why we should use 
>> the reserved/dedicated VM for other engines than the one involved in 
>> the SQ trace.
> How about removing SQ trace reason? is it ok just that one process 
> wants to do experiment for other purpose? :)

In this case I would clearly NAK the whole approach.

Regards,
Christian.

>
> Regards,
> David Zhou
>>
>> So I would say we should limit this to GFX and Compute jobs and only 
>> allocate the dedicated VMID for those.
>>
>> Regards,
>> Christian.
>>
>>>
>>> Regards,
>>> David Zhou
>>>>
>>>> Christian.
>>>>
>>>>>
>>>>> Jerry
>>>>>> spin_lock_init(&vm->status_lock);
>>>>>>       INIT_LIST_HEAD(&vm->invalidated);
>>>>>>       INIT_LIST_HEAD(&vm->cleared);
>>>>>> @@ -2250,6 +2252,7 @@ void amdgpu_vm_fini(struct amdgpu_device 
>>>>>> *adev, struct amdgpu_vm *vm)
>>>>>>   {
>>>>>>       struct amdgpu_bo_va_mapping *mapping, *tmp;
>>>>>>       bool prt_fini_needed = !!adev->gart.gart_funcs->set_prt;
>>>>>> +    int i;
>>>>>>
>>>>>>       if (vm->is_kfd_vm) {
>>>>>>           struct amdgpu_vm_id_manager *id_mgr =
>>>>>> @@ -2292,6 +2295,18 @@ void amdgpu_vm_fini(struct amdgpu_device 
>>>>>> *adev, struct amdgpu_vm *vm)
>>>>>>
>>>>>>       amdgpu_vm_free_levels(&vm->root);
>>>>>>       fence_put(vm->last_dir_update);
>>>>>> +    for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
>>>>>> +        struct amdgpu_vm_id_manager *id_mgr =
>>>>>> +            &adev->vm_manager.id_mgr[i];
>>>>>> +
>>>>>> +        mutex_lock(&id_mgr->lock);
>>>>>> +        if (vm->dedicated_vmid[i]) {
>>>>>> + list_add(&vm->dedicated_vmid[i]->list,
>>>>>> +                 &id_mgr->ids_lru);
>>>>>> +            vm->dedicated_vmid[i] = NULL;
>>>>>> +        }
>>>>>> +        mutex_unlock(&id_mgr->lock);
>>>>>> +    }
>>>>>>   }
>>>>>>
>>>>>>   /**
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>>>> index 62dbace..23981ee 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>>>> @@ -122,6 +122,8 @@ struct amdgpu_vm {
>>>>>>
>>>>>>       /* client id */
>>>>>>       u64                     client_id;
>>>>>> +    /* dedicated vmid */
>>>>>> +    struct amdgpu_vm_id *dedicated_vmid[AMDGPU_MAX_VMHUBS];
>>>>>>       /* each VM will map on CSA */
>>>>>>       struct amdgpu_bo_va *csa_bo_va;
>>>>>>
>>>>>>
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx at lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>>
>>>>
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>>
>