Re: [PATCH 15/16] drm/ttm: remove bo->moving

On Wed, Apr 06, 2022 at 09:51:31AM +0200, Christian König wrote:

This is now handled by the DMA-buf framework in the dma_resv obj.

Also remove the workaround inside VMWGFX to update the moving fence.

Signed-off-by: Christian König christian.koenig@amd.com

Looks all reasonable.

Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch

---

.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 13 ++++--- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 5 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c | 11 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 11 ++++-- drivers/gpu/drm/ttm/ttm_bo.c | 10 ++---- drivers/gpu/drm/ttm/ttm_bo_util.c | 7 ---- drivers/gpu/drm/ttm/ttm_bo_vm.c | 34 +++++++------------ drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 6 ---- include/drm/ttm/ttm_bo_api.h | 2 -- 9 files changed, 39 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 808e21dcb517..a4955ef76cfc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -2447,6 +2447,8 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info, struct dma_fence **ef) struct amdgpu_bo *bo = mem->bo; uint32_t domain = mem->domain; struct kfd_mem_attachment *attachment;

• struct dma_resv_iter cursor;
  

• struct dma_fence *fence;
  
  

  total_size += amdgpu_bo_size(bo);

@@ -2461,10 +2463,13 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info, struct dma_fence **ef) goto validate_map_fail; } }

• ret = amdgpu_sync_fence(&sync_obj, bo->tbo.moving);
  

• if (ret) {
  

• 	pr_debug("Memory eviction: Sync BO fence failed. Try again\n");
  

• 	goto validate_map_fail;
  

• dma_resv_for_each_fence(&cursor, bo->tbo.base.resv,
  

• 			DMA_RESV_USAGE_KERNEL, fence) {
  

• 	ret = amdgpu_sync_fence(&sync_obj, fence);
  

• 	if (ret) {
  

• 		pr_debug("Memory eviction: Sync BO fence failed. Try again\n");
  

• 		goto validate_map_fail;
  

• 	}
  

  } list_for_each_entry(attachment, &mem->attachments, list) { if (!attachment->is_mapped)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 5832c05ab10d..ef93abec13b9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -612,9 +612,8 @@ int amdgpu_bo_create(struct amdgpu_device *adev, if (unlikely(r)) goto fail_unreserve;

• amdgpu_bo_fence(bo, fence, false);
  

• dma_fence_put(bo->tbo.moving);
  

• bo->tbo.moving = dma_fence_get(fence);
  

• dma_resv_add_fence(bo->tbo.base.resv, fence,
  

• 		   DMA_RESV_USAGE_KERNEL);
  

  dma_fence_put(fence); } if (!bp->resv)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c index e3fbf0f10add..31913ae86de6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c @@ -74,13 +74,12 @@ static int amdgpu_vm_cpu_update(struct amdgpu_vm_update_params *p, { unsigned int i; uint64_t value;

• int r;

• long r;

• if (vmbo->bo.tbo.moving) {

• r = dma_fence_wait(vmbo->bo.tbo.moving, true);
  

• if (r)
  

• 	return r;
  

• }

• r = dma_resv_wait_timeout(vmbo->bo.tbo.base.resv, DMA_RESV_USAGE_KERNEL,

• 		  true, MAX_SCHEDULE_TIMEOUT);
  

• if (r < 0)

• return r;
  
  

  pe += (unsigned long)amdgpu_bo_kptr(&vmbo->bo);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c index 9485b541947e..9cd6f41896c0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c @@ -205,14 +205,19 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p, struct amdgpu_bo *bo = &vmbo->bo; enum amdgpu_ib_pool_type pool = p->immediate ? AMDGPU_IB_POOL_IMMEDIATE : AMDGPU_IB_POOL_DELAYED;

• struct dma_resv_iter cursor; unsigned int i, ndw, nptes;

• struct dma_fence *fence; uint64_t *pte; int r;

  /* Wait for PD/PT moves to be completed */

• r = amdgpu_sync_fence(&p->job->sync, bo->tbo.moving);
• if (r)

• return r;
  

• dma_resv_for_each_fence(&cursor, bo->tbo.base.resv,

• 		DMA_RESV_USAGE_KERNEL, fence) {
  

• r = amdgpu_sync_fence(&p->job->sync, fence);
  

• if (r)
  

• 	return r;
  

• }

  do { ndw = p->num_dw_left;

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 360f980c7e10..015a94f766de 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -418,7 +418,6 @@ static void ttm_bo_release(struct kref *kref) dma_resv_unlock(bo->base.resv);

atomic_dec(&ttm_glob.bo_count);

• dma_fence_put(bo->moving); bo->destroy(bo);

}

@@ -714,9 +713,8 @@ void ttm_bo_unpin(struct ttm_buffer_object *bo) EXPORT_SYMBOL(ttm_bo_unpin);

/*

• • Add the last move fence to the BO and reserve a new shared slot. We only use
• • a shared slot to avoid unecessary sync and rely on the subsequent bo move to
• • either stall or use an exclusive fence respectively set bo->moving.

• • Add the last move fence to the BO as kernel dependency and reserve a new
• • fence slot.
  */

static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo, struct ttm_resource_manager *man, @@ -746,9 +744,6 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo, dma_fence_put(fence); return ret; }

• dma_fence_put(bo->moving);
• bo->moving = fence; return 0;

}

@@ -951,7 +946,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, bo->bdev = bdev; bo->type = type; bo->page_alignment = page_alignment;

• bo->moving = NULL; bo->pin_count = 0; bo->sg = sg; bo->bulk_move = NULL;

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 99deb45894f4..bc5190340b9c 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -228,7 +228,6 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,

atomic_inc(&ttm_glob.bo_count); INIT_LIST_HEAD(&fbo->base.ddestroy);

• fbo->base.moving = NULL; drm_vma_node_reset(&fbo->base.base.vma_node);

  kref_init(&fbo->base.kref);

@@ -500,9 +499,6 @@ static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo, * operation has completed. */

• dma_fence_put(bo->moving);
• bo->moving = dma_fence_get(fence);
• ret = ttm_buffer_object_transfer(bo, &ghost_obj); if (ret) return ret;

@@ -546,9 +542,6 @@ static void ttm_bo_move_pipeline_evict(struct ttm_buffer_object *bo, spin_unlock(&from->move_lock);

ttm_resource_free(bo, &bo->resource);

• dma_fence_put(bo->moving);
• bo->moving = dma_fence_get(fence);

}

int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo, diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 08ba083a80d2..5b324f245265 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -46,17 +46,13 @@ static vm_fault_t ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo, struct vm_fault *vmf) {

• vm_fault_t ret = 0;
• int err = 0;
• if (likely(!bo->moving))

• goto out_unlock;
  

• long err = 0;

  /*

  • Quick non-stalling check for idle.

  */

• if (dma_fence_is_signaled(bo->moving))

• goto out_clear;
  

• if (dma_resv_test_signaled(bo->base.resv, DMA_RESV_USAGE_KERNEL))

• return 0;
  
  

  /*

  • If possible, avoid waiting for GPU with mmap_lock

@@ -64,34 +60,30 @@ static vm_fault_t ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo, * is the first attempt. */ if (fault_flag_allow_retry_first(vmf->flags)) {

• ret = VM_FAULT_RETRY;
  

  if (vmf->flags & FAULT_FLAG_RETRY_NOWAIT)

• 	goto out_unlock;
  

• 	return VM_FAULT_RETRY;
  
  

  ttm_bo_get(bo); mmap_read_unlock(vmf->vma->vm_mm);

• (void) dma_fence_wait(bo->moving, true);
  

• (void)dma_resv_wait_timeout(bo->base.resv,
  

• 			    DMA_RESV_USAGE_KERNEL, true,
  

• 			    MAX_SCHEDULE_TIMEOUT);
  

  dma_resv_unlock(bo->base.resv); ttm_bo_put(bo);

• goto out_unlock;
  

• return VM_FAULT_RETRY;
  

  }

  /*

  • Ordinary wait.

  */

• err = dma_fence_wait(bo->moving, true);
• if (unlikely(err != 0)) {

• ret = (err != -ERESTARTSYS) ? VM_FAULT_SIGBUS :
  

• err = dma_resv_wait_timeout(bo->base.resv, DMA_RESV_USAGE_KERNEL, true,

• 		    MAX_SCHEDULE_TIMEOUT);
  

• if (unlikely(err < 0)) {

• return (err != -ERESTARTSYS) ? VM_FAULT_SIGBUS :
  VM_FAULT_NOPAGE;
  

• goto out_unlock;
  

  }

-out_clear:

• dma_fence_put(bo->moving);
• bo->moving = NULL;

-out_unlock:

• return ret;

• return 0;

}

static unsigned long ttm_bo_io_mem_pfn(struct ttm_buffer_object *bo, diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c index a84d1d5628d0..a7d62a4eb47b 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c @@ -1161,12 +1161,6 @@ int vmw_resources_clean(struct vmw_buffer_object *vbo, pgoff_t start, *num_prefault = __KERNEL_DIV_ROUND_UP(last_cleaned - res_start, PAGE_SIZE); vmw_bo_fence_single(bo, NULL);

• if (bo->moving)
  

• 	dma_fence_put(bo->moving);
  

• return dma_resv_get_singleton(bo->base.resv,
  

• 			      DMA_RESV_USAGE_WRITE,
  

• 			      &bo->moving);
  

  }

  return 0;

diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index c76932b68a33..2d524f8b0802 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -94,7 +94,6 @@ struct ttm_tt;

• @deleted: True if the object is only a zombie and already deleted.
• @ddestroy: List head for the delayed destroy list.
• @swap: List head for swap LRU list.

• • @moving: Fence set when BO is moving
  • @offset: The current GPU offset, which can have different meanings
  • depending on the memory type. For SYSTEM type memory, it should be 0.
  • @cur_placement: Hint of current placement.

@@ -147,7 +146,6 @@ struct ttm_buffer_object { * Members protected by a bo reservation. */

• struct dma_fence *moving; unsigned priority; unsigned pin_count;

-- 2.25.1