> 2020年3月13日 22:13，Koenig, Christian <Christian.Koenig@xxxxxxx> 写道：
>
> Am 13.03.20 um 15:07 schrieb xinhui pan:
>> Provide resv as a parameter for vm sdma commit.
>> Job fence on page table should be a shared one, so add it to the resv.
>>
>> Cc: Christian König <christian.koenig@xxxxxxx>
>> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
>> Cc: Felix Kuehling <Felix.Kuehling@xxxxxxx>
>> Suggested-by: Christian König <christian.koenig@xxxxxxx>
>> Signed-off-by: xinhui pan <xinhui.pan@xxxxxxx>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 ++--
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 5 +++++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 7 +++++++
>> 3 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 73398831196f..809ca6e8f40f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1579,6 +1579,7 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>> dma_addr_t *pages_addr,
>> struct dma_fence **fence)
>> {
>> + struct amdgpu_bo *root = vm->root.base.bo;
>> struct amdgpu_vm_update_params params;
>> enum amdgpu_sync_mode sync_mode;
>> int r;
>> @@ -1604,8 +1605,6 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>> }
>> if (flags & AMDGPU_PTE_VALID) {
>> - struct amdgpu_bo *root = vm->root.base.bo;
>> -
>> if (!dma_fence_is_signaled(vm->last_direct))
>> amdgpu_bo_fence(root, vm->last_direct, true);
>> @@ -1613,6 +1612,7 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>> amdgpu_bo_fence(root, vm->last_delayed, true);
>> }
>> + params.resv = root->tbo.base.resv;
>> r = vm->update_funcs->prepare(¶ms, resv, sync_mode);
>> if (r)
>> goto error_unlock;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> index b5705fcfc935..ca6021b4200b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> @@ -226,6 +226,11 @@ struct amdgpu_vm_update_params {
>> * @num_dw_left: number of dw left for the IB
>> */
>> unsigned int num_dw_left;
>> +
>> + /**
>> + * @resv: sync the resv and add job fence to it conditionally.
>> + */
>> + struct dma_resv *resv;
>> };
>> struct amdgpu_vm_update_funcs {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
>> index 4cc7881f438c..a1b270a4da8e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
>> @@ -111,6 +111,13 @@ static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p,
>> swap(p->vm->last_delayed, tmp);
>> dma_fence_put(tmp);
>> + /* add job fence to resv.
>> + * MM notifier path is an exception as we can not grab the
>> + * resv lock.
>> + */
>> + if (!p->direct && p->resv)
>
> You can just use p->vm->root.base.bo->tbo.base.resv here, no need for a new field in the paramater structure.
>
yep, make sense.
> And it would probably be best to also remove the vm->last_delayed field entirely.
>
> In other words use something like this here
>
> if (p->direct) {
> tmp = dma_fence_get(f);
> swap(p->vm->last_direct, tmp);
> dma_fence_put(tmp);
> } else {
> dma_resv_add_shared_fence(p->vm->root.base.bo->tbo.base.resv, f);
> }
>
I think we still need udpate the last_delayed. looks like eviction and some other ioctls test this field to determine the vm state.
this should be done by a new patch if possible.
> Regards,
> Christian.
>
>> + dma_resv_add_shared_fence(p->resv, f);
>> +
>> if (fence && !p->direct)
>> swap(*fence, f);
>> dma_fence_put(f);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
