Re: [PATCH v3 1/2] drm/amdgpu: Add job fence to resv conditionally

[Christian König]

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.koe...@amd.com>
Cc: Alex Deucher <alexander.deuc...@amd.com>
Cc: Felix Kuehling <felix.kuehl...@amd.com>
Suggested-by: Christian König <christian.koe...@amd.com>
Signed-off-by: xinhui pan <xinhui....@amd.com>
---
  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(&params, 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.

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);
}

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx