I'm not sure this one, since you remove status lock for vm_status list, I think we need to test this carefully.

Regards,

David Zhou


On 2017年08月25日 17:38, Christian König wrote:
From: Christian König <christian.koe...@amd.com>

We changed this to use an extra list a while back, but for the next
series I need a separate flag again.

Signed-off-by: Christian König <christian.koe...@amd.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 37 ++++++++++++++----------------
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h     |  3 ---
  3 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index a288fa6..e613ba4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -55,6 +55,9 @@ struct amdgpu_bo_va {
        /* mappings for this bo_va */
        struct list_head                invalids;
        struct list_head                valids;
+
+       /* If the mappings are cleared or filled */
+       bool                            cleared;
  };
struct amdgpu_bo {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f621dba..16148ef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1787,10 +1787,13 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
        else
                flags = 0x0;
- spin_lock(&vm->status_lock);
-       if (!list_empty(&bo_va->base.vm_status))
+       /* We access vm_status without the status lock here, but that is ok
+        * because when we don't clear the BO is locked and so the status can't
+        * change
+        */
+       if ((!clear && !list_empty(&bo_va->base.vm_status)) ||
+           bo_va->cleared != clear)
                list_splice_init(&bo_va->valids, &bo_va->invalids);
-       spin_unlock(&vm->status_lock);
list_for_each_entry(mapping, &bo_va->invalids, list) {
                r = amdgpu_vm_bo_split_mapping(adev, exclusive, pages_addr, vm,
@@ -1800,25 +1803,22 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
                        return r;
        }
- if (trace_amdgpu_vm_bo_mapping_enabled()) {
-               list_for_each_entry(mapping, &bo_va->valids, list)
-                       trace_amdgpu_vm_bo_mapping(mapping);
-
-               list_for_each_entry(mapping, &bo_va->invalids, list)
-                       trace_amdgpu_vm_bo_mapping(mapping);
+       if (vm->use_cpu_for_update) {
+               /* Flush HDP */
+               mb();
+               amdgpu_gart_flush_gpu_tlb(adev, 0);
        }
spin_lock(&vm->status_lock);
-       list_splice_init(&bo_va->invalids, &bo_va->valids);
        list_del_init(&bo_va->base.vm_status);
-       if (clear)
-               list_add(&bo_va->base.vm_status, &vm->cleared);
        spin_unlock(&vm->status_lock);
- if (vm->use_cpu_for_update) {
-               /* Flush HDP */
-               mb();
-               amdgpu_gart_flush_gpu_tlb(adev, 0);
+       list_splice_init(&bo_va->invalids, &bo_va->valids);
+       bo_va->cleared = clear;
+
+       if (trace_amdgpu_vm_bo_mapping_enabled()) {
+               list_for_each_entry(mapping, &bo_va->valids, list)
+                       trace_amdgpu_vm_bo_mapping(mapping);
        }
return 0;
@@ -2419,9 +2419,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
list_for_each_entry(bo_base, &bo->va, bo_list) {
                spin_lock(&bo_base->vm->status_lock);
-               if (list_empty(&bo_base->vm_status))
-                       list_add(&bo_base->vm_status,
-                                &bo_base->vm->moved);
+               list_move(&bo_base->vm_status, &bo_base->vm->moved);
                spin_unlock(&bo_base->vm->status_lock);
        }
  }
@@ -2508,7 +2506,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
                vm->reserved_vmid[i] = NULL;
        spin_lock_init(&vm->status_lock);
        INIT_LIST_HEAD(&vm->moved);
-       INIT_LIST_HEAD(&vm->cleared);
        INIT_LIST_HEAD(&vm->freed);
/* create scheduler entity for page table updates */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 9347d28..e705f0f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -126,9 +126,6 @@ struct amdgpu_vm {
        /* BOs moved, but not yet updated in the PT */
        struct list_head        moved;
- /* BOs cleared in the PT because of a move */
-       struct list_head        cleared;
-
        /* BO mappings freed, but not yet updated in the PT */
        struct list_head        freed;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to