Am 27.05.21 um 13:53 schrieb Nirmoy Das:
The subclass, amdgpu_bo_vm is intended for PT/PD BOs which are also
shadowed, so switch to amdgpu_bo_vm BO for PT/PD BOs.

v3: simplify code.
     check also if shadow bo exist, instead of checking only bo's type.
v2: squash three related patches.

Signed-off-by: Nirmoy Das <[email protected]>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c      | 93 +++++++++++++--------
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 18 ++--
  2 files changed, 68 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 6bc7566cc193..d723873df765 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -652,15 +652,15 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device 
*adev,
        spin_lock(&adev->mman.bdev.lru_lock);
        list_for_each_entry(bo_base, &vm->idle, vm_status) {
                struct amdgpu_bo *bo = bo_base->bo;
+               struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo);

                if (!bo->parent)
                        continue;

                ttm_bo_move_to_lru_tail(&bo->tbo, &bo->tbo.mem,
                                        &vm->lru_bulk_move);
-               if (bo->shadow)
-                       ttm_bo_move_to_lru_tail(&bo->shadow->tbo,
-                                               &bo->shadow->tbo.mem,
+               if (shadow)
+                       ttm_bo_move_to_lru_tail(&shadow->tbo, &shadow->tbo.mem,
                                                &vm->lru_bulk_move);
        }
        spin_unlock(&adev->mman.bdev.lru_lock);
@@ -692,12 +692,13 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,

        list_for_each_entry_safe(bo_base, tmp, &vm->evicted, vm_status) {
                struct amdgpu_bo *bo = bo_base->bo;
+               struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo);

                r = validate(param, bo);
                if (r)
                        return r;
-               if (bo->shadow) {
-                       r = validate(param, bo->shadow);
+               if (shadow) {
+                       r = validate(param, shadow);
                        if (r)
                                return r;
                }
@@ -754,6 +755,7 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
        unsigned level = adev->vm_manager.root_level;
        struct amdgpu_vm_update_params params;
        struct amdgpu_bo *ancestor = bo;
+       struct amdgpu_bo *shadow;
        unsigned entries, ats_entries;
        uint64_t addr;
        int r;
@@ -793,9 +795,9 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
        if (r)
                return r;

-       if (bo->shadow) {
-               r = ttm_bo_validate(&bo->shadow->tbo, &bo->shadow->placement,
-                                   &ctx);
+       shadow = amdgpu_bo_shadowed(bo);
+       if (shadow) {
+               r = ttm_bo_validate(&shadow->tbo, &shadow->placement, &ctx);
                if (r)
                        return r;
        }
@@ -863,14 +865,16 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
   * @vm: requesting vm
   * @level: the page table level
   * @immediate: use a immediate update
- * @bo: pointer to the buffer object pointer
+ * @vmbo: pointer to the buffer object pointer
   */
  static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
                               struct amdgpu_vm *vm,
                               int level, bool immediate,
-                              struct amdgpu_bo **bo)
+                              struct amdgpu_bo_vm **vmbo)
  {
        struct amdgpu_bo_param bp;
+       struct amdgpu_bo *bo;
+       struct dma_resv *resv;
        int r;

        memset(&bp, 0, sizeof(bp));
@@ -881,7 +885,7 @@ static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
        bp.domain = amdgpu_bo_get_preferred_pin_domain(adev, bp.domain);
        bp.flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
                AMDGPU_GEM_CREATE_CPU_GTT_USWC;
-       bp.bo_ptr_size = sizeof(struct amdgpu_bo);
+       bp.bo_ptr_size = sizeof(struct amdgpu_bo_vm);
        if (vm->use_cpu_for_update)
                bp.flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;

@@ -890,26 +894,41 @@ static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
        if (vm->root.base.bo)
                bp.resv = vm->root.base.bo->tbo.base.resv;

-       r = amdgpu_bo_create(adev, &bp, bo);
+       r = amdgpu_bo_create_vm(adev, &bp, vmbo);
        if (r)
                return r;

-       if (vm->is_compute_context && (adev->flags & AMD_IS_APU))
+       bo = &(*vmbo)->bo;
+       if (vm->is_compute_context && (adev->flags & AMD_IS_APU)) {
+               (*vmbo)->shadow = NULL;
                return 0;
+       }

        if (!bp.resv)
-               WARN_ON(dma_resv_lock((*bo)->tbo.base.resv,
+               WARN_ON(dma_resv_lock(bo->tbo.base.resv,
                                      NULL));
-       r = amdgpu_bo_create_shadow(adev, bp.size, *bo);
+       resv = bp.resv;
+       memset(&bp, 0, sizeof(bp));
+       bp.size = amdgpu_vm_bo_size(adev, level);
+       bp.domain = AMDGPU_GEM_DOMAIN_GTT;
+       bp.flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC;
+       bp.type = ttm_bo_type_kernel;
+       bp.resv = bo->tbo.base.resv;
+       bp.bo_ptr_size = sizeof(struct amdgpu_bo);

-       if (!bp.resv)
-               dma_resv_unlock((*bo)->tbo.base.resv);
+       r = amdgpu_bo_create(adev, &bp, &(*vmbo)->shadow);
+
+       if (!resv)
+               dma_resv_unlock(bo->tbo.base.resv);

        if (r) {
-               amdgpu_bo_unref(bo);
+               amdgpu_bo_unref(&bo);
                return r;
        }

+       (*vmbo)->shadow->parent = amdgpu_bo_ref(bo);
+       amdgpu_bo_add_to_shadow_list((*vmbo)->shadow);
+
        return 0;
  }

@@ -933,7 +952,8 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
                               bool immediate)
  {
        struct amdgpu_vm_pt *entry = cursor->entry;
-       struct amdgpu_bo *pt;
+       struct amdgpu_bo *pt_bo;
+       struct amdgpu_bo_vm *pt;
        int r;

        if (cursor->level < AMDGPU_VM_PTB && !entry->entries) {
@@ -957,10 +977,11 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
        /* Keep a reference to the root directory to avoid
         * freeing them up in the wrong order.
         */
-       pt->parent = amdgpu_bo_ref(cursor->parent->base.bo);
-       amdgpu_vm_bo_base_init(&entry->base, vm, pt);
+       pt_bo = &pt->bo;
+       pt_bo->parent = amdgpu_bo_ref(cursor->parent->base.bo);
+       amdgpu_vm_bo_base_init(&entry->base, vm, pt_bo);

-       r = amdgpu_vm_clear_bo(adev, vm, pt, immediate);
+       r = amdgpu_vm_clear_bo(adev, vm, pt_bo, immediate);
        if (r)
                goto error_free_pt;

@@ -968,7 +989,7 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,

  error_free_pt:
        amdgpu_bo_unref(&pt->shadow);
-       amdgpu_bo_unref(&pt);
+       amdgpu_bo_unref(&pt_bo);
        return r;
  }

@@ -979,10 +1000,13 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device 
*adev,
   */
  static void amdgpu_vm_free_table(struct amdgpu_vm_pt *entry)
  {
+       struct amdgpu_bo *shadow;
+
        if (entry->base.bo) {
+               shadow = amdgpu_bo_shadowed(entry->base.bo);
                entry->base.bo->vm_bo = NULL;
                list_del(&entry->base.vm_status);
-               amdgpu_bo_unref(&entry->base.bo->shadow);
+               amdgpu_bo_unref(&shadow);
                amdgpu_bo_unref(&entry->base.bo);
        }
        kvfree(entry->entries);
@@ -2674,7 +2698,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
        struct amdgpu_vm_bo_base *bo_base;

        /* shadow bo doesn't have bo base, its validation needs its parent */
-       if (bo->parent && bo->parent->shadow == bo)
+       if (bo->parent && (amdgpu_bo_shadowed(bo->parent) == bo))
                bo = bo->parent;

        for (bo_base = bo->vm_bo; bo_base; bo_base = bo_base->next) {
@@ -2843,7 +2867,8 @@ long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long 
timeout)
   */
  int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 
pasid)
  {
-       struct amdgpu_bo *root;
+       struct amdgpu_bo *root_bo;
+       struct amdgpu_bo_vm *root;
        int r, i;

        vm->va = RB_ROOT_CACHED;
@@ -2897,18 +2922,18 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm, u32 pasid)
                                false, &root);
        if (r)
                goto error_free_delayed;
-
-       r = amdgpu_bo_reserve(root, true);
+       root_bo = &root->bo;
+       r = amdgpu_bo_reserve(root_bo, true);
        if (r)
                goto error_free_root;

-       r = dma_resv_reserve_shared(root->tbo.base.resv, 1);
+       r = dma_resv_reserve_shared(root_bo->tbo.base.resv, 1);
        if (r)
                goto error_unreserve;

-       amdgpu_vm_bo_base_init(&vm->root.base, vm, root);
+       amdgpu_vm_bo_base_init(&vm->root.base, vm, root_bo);

-       r = amdgpu_vm_clear_bo(adev, vm, root, false);
+       r = amdgpu_vm_clear_bo(adev, vm, root_bo, false);
        if (r)
                goto error_unreserve;

@@ -2935,8 +2960,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm, u32 pasid)
        amdgpu_bo_unreserve(vm->root.base.bo);

  error_free_root:
-       amdgpu_bo_unref(&vm->root.base.bo->shadow);
-       amdgpu_bo_unref(&vm->root.base.bo);
+       amdgpu_bo_unref(&root->shadow);
+       amdgpu_bo_unref(&root_bo);
        vm->root.base.bo = NULL;

  error_free_delayed:
@@ -3078,7 +3103,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
        }

        /* Free the shadow bo for compute VM */
-       amdgpu_bo_unref(&vm->root.base.bo->shadow);
+       amdgpu_bo_unref(&to_amdgpu_bo_vm(vm->root.base.bo)->shadow);

        if (pasid)
                vm->pasid = pasid;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
index a83a646759c5..c9ef025c43f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
@@ -41,10 +41,7 @@ static int amdgpu_vm_sdma_map_table(struct amdgpu_bo *table)
        if (r)
                return r;

-       if (table->shadow)
-               r = amdgpu_ttm_alloc_gart(&table->shadow->tbo);
-
-       return r;
+       return amdgpu_ttm_alloc_gart(&to_amdgpu_bo_vm(table)->shadow->tbo);

Here you also need to check if shadow isn't NULL.

I think it would be simpler if you make the parameter to amdgpu_vm_sdma_map_table a vmbo in the first place.

  }

  /**
@@ -201,17 +198,20 @@ static int amdgpu_vm_sdma_update(struct 
amdgpu_vm_update_params *p,
                                 uint64_t addr, unsigned count, uint32_t incr,
                                 uint64_t flags)
  {
+       struct amdgpu_bo *shadow;
        enum amdgpu_ib_pool_type pool = p->immediate ? AMDGPU_IB_POOL_IMMEDIATE
                : AMDGPU_IB_POOL_DELAYED;
        unsigned int i, ndw, nptes;
        uint64_t *pte;
        int r;

+       shadow = amdgpu_bo_shadowed(bo);

Same for amdgpu_vm_sdma_map_table(), just make the parameter a vmbo in the first place.

Apart from that patch looks good to me.

Christian.

        /* Wait for PD/PT moves to be completed */
        r = amdgpu_sync_fence(&p->job->sync, bo->tbo.moving);
        if (r)
                return r;

+
        do {
                ndw = p->num_dw_left;
                ndw -= p->job->ibs->length_dw;
@@ -238,8 +238,8 @@ static int amdgpu_vm_sdma_update(struct 
amdgpu_vm_update_params *p,

                if (!p->pages_addr) {
                        /* set page commands needed */
-                       if (bo->shadow)
-                               amdgpu_vm_sdma_set_ptes(p, bo->shadow, pe, addr,
+                       if (shadow)
+                               amdgpu_vm_sdma_set_ptes(p, shadow, pe, addr,
                                                        count, incr, flags);
                        amdgpu_vm_sdma_set_ptes(p, bo, pe, addr, count,
                                                incr, flags);
@@ -248,7 +248,7 @@ static int amdgpu_vm_sdma_update(struct 
amdgpu_vm_update_params *p,

                /* copy commands needed */
                ndw -= p->adev->vm_manager.vm_pte_funcs->copy_pte_num_dw *
-                       (bo->shadow ? 2 : 1);
+                       (shadow ? 2 : 1);

                /* for padding */
                ndw -= 7;
@@ -263,8 +263,8 @@ static int amdgpu_vm_sdma_update(struct 
amdgpu_vm_update_params *p,
                        pte[i] |= flags;
                }

-               if (bo->shadow)
-                       amdgpu_vm_sdma_copy_ptes(p, bo->shadow, pe, nptes);
+               if (shadow)
+                       amdgpu_vm_sdma_copy_ptes(p, shadow, pe, nptes);
                amdgpu_vm_sdma_copy_ptes(p, bo, pe, nptes);

                pe += nptes * 8;
--
2.31.1


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

Reply via email to