Am 26.04.2018 um 05:01 schrieb Zhang, Jerry (Junwei):
On 04/24/2018 03:35 PM, Chunming Zhou wrote:
Change-Id: Ia3e57dbacff05f32b6c02e29aeadabd36f08028e
Signed-off-by: Chunming Zhou <[email protected]>
Reviewed-by: Junwei Zhang <[email protected]>

some trivial comments.

Both a nice cleanup as well as the right comments.

With Jerry's comments addressed Reviewed-by: Christian König <[email protected]> as well.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 75 ++++++++++++++++++----------------
  1 file changed, 40 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8c34060e130f..c75b96433ee7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -94,6 +94,9 @@ struct amdgpu_prt_cb {
      struct dma_fence_cb cb;
  };

+static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
+                   struct amdgpu_vm *vm,
+                   struct amdgpu_bo *bo);

Move the function upper, we could avoid such declaration.

  /**
   * amdgpu_vm_level_shift - return the addr shift for each level
   *
@@ -446,11 +449,9 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
              */
              pt->parent = amdgpu_bo_ref(parent->base.bo);

-            entry->base.vm = vm;
-            entry->base.bo = pt;
-            list_add_tail(&entry->base.bo_list, &pt->va);
+            amdgpu_vm_bo_base_init(&entry->base, vm, pt);
              spin_lock(&vm->status_lock);
-            list_add(&entry->base.vm_status, &vm->relocated);
+            list_move(&entry->base.vm_status, &vm->relocated);
              spin_unlock(&vm->status_lock);
          }

@@ -1821,6 +1822,35 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
      return r;
  }

+static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
+                   struct amdgpu_vm *vm,
+                   struct amdgpu_bo *bo)
+{
+    base->vm = vm;
+    base->bo = bo;
+    INIT_LIST_HEAD(&base->bo_list);
+    INIT_LIST_HEAD(&base->vm_status);
+
+    if (!bo)
+        return;
+    list_add_tail(&base->bo_list, &bo->va);
+
+    if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
+        return;
+
+    if (bo->preferred_domains &
+        amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type))
+        return;
+
+    /*
+     * we checked all the prerequisites, but it looks like this per vm bo +     * is currently evicted. add the bo to the evicted list to make sure it
+     * is validated on next vm use to avoid fault.
+     * */

/*
 *
 */  is expected

+    spin_lock(&vm->status_lock);
+    list_move_tail(&base->vm_status, &vm->evicted);
+    spin_unlock(&vm->status_lock);
+}
  /**
   * amdgpu_vm_bo_add - add a bo to a specific vm
   *
@@ -1844,36 +1874,12 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
      if (bo_va == NULL) {
          return NULL;
      }
-    bo_va->base.vm = vm;
-    bo_va->base.bo = bo;
-    INIT_LIST_HEAD(&bo_va->base.bo_list);
-    INIT_LIST_HEAD(&bo_va->base.vm_status);
+    amdgpu_vm_bo_base_init(&bo_va->base, vm, bo);

      bo_va->ref_count = 1;
      INIT_LIST_HEAD(&bo_va->valids);
      INIT_LIST_HEAD(&bo_va->invalids);

-    if (!bo)
-        return bo_va;
-
-    list_add_tail(&bo_va->base.bo_list, &bo->va);
-
-    if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
-        return bo_va;
-
-    if (bo->preferred_domains &
-        amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type))
-        return bo_va;
-
-    /*
-     * We checked all the prerequisites, but it looks like this per VM BO -     * is currently evicted. add the BO to the evicted list to make sure it
-     * is validated on next VM use to avoid fault.
-     * */
-    spin_lock(&vm->status_lock);
-    list_move_tail(&bo_va->base.vm_status, &vm->evicted);
-    spin_unlock(&vm->status_lock);
-
      return bo_va;
  }

@@ -2373,6 +2379,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
             int vm_context, unsigned int pasid)
  {
      struct amdgpu_bo_param bp;
+    struct amdgpu_bo *root;
      const unsigned align = min(AMDGPU_VM_PTB_ALIGN_SIZE,
          AMDGPU_VM_PTE_COUNT(adev) * 8);
      unsigned ring_instance;
@@ -2434,23 +2441,21 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
      bp.flags = flags;
      bp.type = ttm_bo_type_kernel;
      bp.resv = NULL;
-    r = amdgpu_bo_create(adev, &bp, &vm->root.base.bo);
+    r = amdgpu_bo_create(adev, &bp, &root);
      if (r)
          goto error_free_sched_entity;

-    r = amdgpu_bo_reserve(vm->root.base.bo, true);
+    r = amdgpu_bo_reserve(root, true);
      if (r)
          goto error_free_root;

-    r = amdgpu_vm_clear_bo(adev, vm, vm->root.base.bo,
+    r = amdgpu_vm_clear_bo(adev, vm, root,
                     adev->vm_manager.root_level,
                     vm->pte_support_ats);
      if (r)
          goto error_unreserve;

-    vm->root.base.vm = vm;
-    list_add_tail(&vm->root.base.bo_list, &vm->root.base.bo->va);
-    list_add_tail(&vm->root.base.vm_status, &vm->evicted);
+    amdgpu_vm_bo_base_init(&vm->root.base, vm, root);
      amdgpu_bo_unreserve(vm->root.base.bo);

      if (pasid) {


_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to