[PATCH 1/5] drm/amdgpu: minor cleanup for amdgpu_ttm_bind

[PATCH 1/5] drm/amdgpu: minor cleanup for amdgpu_ttm_bind

[Christian König]

From: Christian König <christian.koenig@amd.com>

Filter the placement mask before using it. In theory it could be that we
have other flags set here as well.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index dcdfb8d176fc..32c822f3db11 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -910,7 +910,8 @@ int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct ttm_mem_reg *bo_mem)
 	placement.busy_placement = &placements;
 	placements.fpfn = 0;
 	placements.lpfn = adev->mc.gart_size >> PAGE_SHIFT;
-	placements.flags = bo->mem.placement | TTM_PL_FLAG_TT;
+	placements.flags = (bo->mem.placement & ~TTM_PL_MASK_MEM) |
+		TTM_PL_FLAG_TT;
 
 	r = ttm_bo_mem_space(bo, &placement, &tmp, true, false);
 	if (unlikely(r))
-- 
2.11.0

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

[PATCH 2/5] drm/amdgpu: nuke amdgpu_ttm_is_bound()

[Christian König]

From: Christian König <christian.koenig@amd.com>

use amdgpu_gtt_mgr_is_allocated() instead.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 15 +++++----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  1 -
 4 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8b4ed8a98a18..71a0c1a477ae 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -980,7 +980,7 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
 {
 	WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_SYSTEM);
 	WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_TT &&
-		     !amdgpu_ttm_is_bound(bo->tbo.ttm));
+		     !amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem));
 	WARN_ON_ONCE(!ww_mutex_is_locked(&bo->tbo.resv->lock) &&
 		     !bo->pin_count);
 	WARN_ON_ONCE(bo->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 428aae048f4b..3affcc497be6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -187,7 +187,7 @@ static inline u64 amdgpu_bo_mmap_offset(struct amdgpu_bo *bo)
 static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo)
 {
 	switch (bo->tbo.mem.mem_type) {
-	case TTM_PL_TT: return amdgpu_ttm_is_bound(bo->tbo.ttm);
+	case TTM_PL_TT: return amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem);
 	case TTM_PL_VRAM: return true;
 	default: return false;
 	}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 32c822f3db11..1c9faa1a53ce 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -861,8 +861,10 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt *ttm,
 	    bo_mem->mem_type == AMDGPU_PL_OA)
 		return -EINVAL;
 
-	if (!amdgpu_gtt_mgr_is_allocated(bo_mem))
+	if (!amdgpu_gtt_mgr_is_allocated(bo_mem)) {
+		gtt->offset = AMDGPU_BO_INVALID_OFFSET;
 		return 0;
+	}
 
 	spin_lock(&gtt->adev->gtt_list_lock);
 	flags = amdgpu_ttm_tt_pte_flags(gtt->adev, ttm, bo_mem);
@@ -882,13 +884,6 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt *ttm,
 	return r;
 }
 
-bool amdgpu_ttm_is_bound(struct ttm_tt *ttm)
-{
-	struct amdgpu_ttm_tt *gtt = (void *)ttm;
-
-	return gtt && !list_empty(&gtt->list);
-}
-
 int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct ttm_mem_reg *bo_mem)
 {
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
@@ -899,7 +894,7 @@ int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct ttm_mem_reg *bo_mem)
 	struct ttm_place placements;
 	int r;
 
-	if (!ttm || amdgpu_ttm_is_bound(ttm))
+	if (!ttm || amdgpu_gtt_mgr_is_allocated(bo_mem))
 		return 0;
 
 	tmp = bo->mem;
@@ -960,7 +955,7 @@ static int amdgpu_ttm_backend_unbind(struct ttm_tt *ttm)
 	if (gtt->userptr)
 		amdgpu_ttm_tt_unpin_userptr(ttm);
 
-	if (!amdgpu_ttm_is_bound(ttm))
+	if (gtt->offset == AMDGPU_BO_INVALID_OFFSET)
 		return 0;
 
 	/* unbind shouldn't be done for GDS/GWS/OA in ttm_bo_clean_mm */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index abd4084982a3..89a71abc71e0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -90,7 +90,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
 			struct dma_fence **fence);
 
 int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
-bool amdgpu_ttm_is_bound(struct ttm_tt *ttm);
 int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct ttm_mem_reg *bo_mem);
 int amdgpu_ttm_recover_gart(struct amdgpu_device *adev);
 
-- 
2.11.0

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

[PATCH 3/5] drm/amdgpu: remove extra parameter from amdgpu_ttm_bind()

[Christian König]

From: Christian König <christian.koenig@amd.com>

We always use the BO mem now.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     | 4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 6 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index f7fceb63413c..9f0a462bf70f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -675,7 +675,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 	if (!r && p->uf_entry.robj) {
 		struct amdgpu_bo *uf = p->uf_entry.robj;
 
-		r = amdgpu_ttm_bind(&uf->tbo, &uf->tbo.mem);
+		r = amdgpu_ttm_bind(&uf->tbo);
 		p->job->uf_addr += amdgpu_bo_gpu_offset(uf);
 	}
 
@@ -1591,5 +1591,5 @@ int amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser,
 			return r;
 	}
 
-	return amdgpu_ttm_bind(&(*bo)->tbo, &(*bo)->tbo.mem);
+	return amdgpu_ttm_bind(&(*bo)->tbo);
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 71a0c1a477ae..aecc62b158d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -690,7 +690,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
 
 	bo->pin_count = 1;
 	if (gpu_addr != NULL) {
-		r = amdgpu_ttm_bind(&bo->tbo, &bo->tbo.mem);
+		r = amdgpu_ttm_bind(&bo->tbo);
 		if (unlikely(r)) {
 			dev_err(adev->dev, "%p bind failed\n", bo);
 			goto error;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 1c9faa1a53ce..a0944a150d9a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -884,7 +884,7 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt *ttm,
 	return r;
 }
 
-int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct ttm_mem_reg *bo_mem)
+int amdgpu_ttm_bind(struct ttm_buffer_object *bo)
 {
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
 	struct ttm_tt *ttm = bo->ttm;
@@ -894,7 +894,7 @@ int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct ttm_mem_reg *bo_mem)
 	struct ttm_place placements;
 	int r;
 
-	if (!ttm || amdgpu_gtt_mgr_is_allocated(bo_mem))
+	if (!ttm || amdgpu_gtt_mgr_is_allocated(&bo->mem))
 		return 0;
 
 	tmp = bo->mem;
@@ -1627,7 +1627,7 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
 	}
 
 	if (bo->tbo.mem.mem_type == TTM_PL_TT) {
-		r = amdgpu_ttm_bind(&bo->tbo, &bo->tbo.mem);
+		r = amdgpu_ttm_bind(&bo->tbo);
 		if (r)
 			return r;
 	}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 89a71abc71e0..39140ed88273 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -90,7 +90,7 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
 			struct dma_fence **fence);
 
 int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
-int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct ttm_mem_reg *bo_mem);
+int amdgpu_ttm_bind(struct ttm_buffer_object *bo);
 int amdgpu_ttm_recover_gart(struct amdgpu_device *adev);
 
 int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages);
-- 
2.11.0

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

[PATCH 4/5] drm/amdgpu: move GART recovery into GTT manager

[Christian König]

From: Christian König <christian.koenig@amd.com>

The GTT manager handles the GART address space anyway, so it is
completely pointless to keep the same information around twice.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h         |  3 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |  8 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 53 +++++++++++++++++++++--------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 51 ++++++++-------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  3 +-
 5 files changed, 59 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 774edc1a3118..bf34076eedca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1631,9 +1631,6 @@ struct amdgpu_device {
 	/* link all shadow bo */
 	struct list_head                shadow_list;
 	struct mutex                    shadow_list_lock;
-	/* link all gtt */
-	spinlock_t			gtt_list_lock;
-	struct list_head                gtt_list;
 	/* keep an lru list of rings by HW IP */
 	struct list_head		ring_lru_list;
 	spinlock_t			ring_lru_list_lock;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 99acf2978134..4f99d17aa582 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2177,9 +2177,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	INIT_LIST_HEAD(&adev->shadow_list);
 	mutex_init(&adev->shadow_list_lock);
 
-	INIT_LIST_HEAD(&adev->gtt_list);
-	spin_lock_init(&adev->gtt_list_lock);
-
 	INIT_LIST_HEAD(&adev->ring_lru_list);
 	spin_lock_init(&adev->ring_lru_list_lock);
 
@@ -2885,7 +2882,7 @@ int amdgpu_sriov_gpu_reset(struct amdgpu_device *adev, struct amdgpu_job *job)
 	amdgpu_sriov_reinit_early(adev);
 
 	/* we need recover gart prior to run SMC/CP/SDMA resume */
-	amdgpu_ttm_recover_gart(adev);
+	amdgpu_gtt_mgr_recover(&adev->mman.bdev.man[TTM_PL_TT]);
 
 	/* now we are okay to resume SMC/CP/SDMA */
 	amdgpu_sriov_reinit_late(adev);
@@ -3026,7 +3023,8 @@ int amdgpu_gpu_reset(struct amdgpu_device *adev)
 				DRM_ERROR("VRAM is lost!\n");
 				atomic_inc(&adev->vram_lost_counter);
 			}
-			r = amdgpu_ttm_recover_gart(adev);
+			r = amdgpu_gtt_mgr_recover(
+				&adev->mman.bdev.man[TTM_PL_TT]);
 			if (r)
 				goto out;
 			r = amdgpu_resume_phase2(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 33535d347734..f276fa7f088b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -31,6 +31,11 @@ struct amdgpu_gtt_mgr {
 	atomic64_t available;
 };
 
+struct amdgpu_gtt_node {
+	struct drm_mm_node node;
+	struct ttm_buffer_object *tbo;
+};
+
 /**
  * amdgpu_gtt_mgr_init - init GTT manager and DRM MM
  *
@@ -93,9 +98,9 @@ static int amdgpu_gtt_mgr_fini(struct ttm_mem_type_manager *man)
  */
 bool amdgpu_gtt_mgr_is_allocated(struct ttm_mem_reg *mem)
 {
-	struct drm_mm_node *node = mem->mm_node;
+	struct amdgpu_gtt_node *node = mem->mm_node;
 
-	return (node->start != AMDGPU_BO_INVALID_OFFSET);
+	return (node->node.start != AMDGPU_BO_INVALID_OFFSET);
 }
 
 /**
@@ -115,7 +120,7 @@ static int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 {
 	struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
 	struct amdgpu_gtt_mgr *mgr = man->priv;
-	struct drm_mm_node *node = mem->mm_node;
+	struct amdgpu_gtt_node *node = mem->mm_node;
 	enum drm_mm_insert_mode mode;
 	unsigned long fpfn, lpfn;
 	int r;
@@ -138,13 +143,13 @@ static int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 		mode = DRM_MM_INSERT_HIGH;
 
 	spin_lock(&mgr->lock);
-	r = drm_mm_insert_node_in_range(&mgr->mm, node,
-					mem->num_pages, mem->page_alignment, 0,
-					fpfn, lpfn, mode);
+	r = drm_mm_insert_node_in_range(&mgr->mm, &node->node, mem->num_pages,
+					mem->page_alignment, 0, fpfn, lpfn,
+					mode);
 	spin_unlock(&mgr->lock);
 
 	if (!r)
-		mem->start = node->start;
+		mem->start = node->node.start;
 
 	return r;
 }
@@ -165,7 +170,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_mem_type_manager *man,
 			      struct ttm_mem_reg *mem)
 {
 	struct amdgpu_gtt_mgr *mgr = man->priv;
-	struct drm_mm_node *node;
+	struct amdgpu_gtt_node *node;
 	int r;
 
 	spin_lock(&mgr->lock);
@@ -183,8 +188,9 @@ static int amdgpu_gtt_mgr_new(struct ttm_mem_type_manager *man,
 		goto err_out;
 	}
 
-	node->start = AMDGPU_BO_INVALID_OFFSET;
-	node->size = mem->num_pages;
+	node->node.start = AMDGPU_BO_INVALID_OFFSET;
+	node->node.size = mem->num_pages;
+	node->tbo = tbo;
 	mem->mm_node = node;
 
 	if (place->fpfn || place->lpfn || place->flags & TTM_PL_FLAG_TOPDOWN) {
@@ -196,7 +202,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_mem_type_manager *man,
 			goto err_out;
 		}
 	} else {
-		mem->start = node->start;
+		mem->start = node->node.start;
 	}
 
 	return 0;
@@ -220,14 +226,14 @@ static void amdgpu_gtt_mgr_del(struct ttm_mem_type_manager *man,
 			       struct ttm_mem_reg *mem)
 {
 	struct amdgpu_gtt_mgr *mgr = man->priv;
-	struct drm_mm_node *node = mem->mm_node;
+	struct amdgpu_gtt_node *node = mem->mm_node;
 
 	if (!node)
 		return;
 
 	spin_lock(&mgr->lock);
-	if (node->start != AMDGPU_BO_INVALID_OFFSET)
-		drm_mm_remove_node(node);
+	if (node->node.start != AMDGPU_BO_INVALID_OFFSET)
+		drm_mm_remove_node(&node->node);
 	spin_unlock(&mgr->lock);
 	atomic64_add(mem->num_pages, &mgr->available);
 
@@ -250,6 +256,25 @@ uint64_t amdgpu_gtt_mgr_usage(struct ttm_mem_type_manager *man)
 	return (result > 0 ? result : 0) * PAGE_SIZE;
 }
 
+int amdgpu_gtt_mgr_recover(struct ttm_mem_type_manager *man)
+{
+	struct amdgpu_gtt_mgr *mgr = man->priv;
+	struct amdgpu_gtt_node *node;
+	struct drm_mm_node *mm_node;
+	int r = 0;
+
+	spin_lock(&mgr->lock);
+	drm_mm_for_each_node(mm_node, &mgr->mm) {
+		node = container_of(mm_node, struct amdgpu_gtt_node, node);
+		r = amdgpu_ttm_recover_gart(node->tbo);
+		if (r)
+			break;
+	}
+	spin_unlock(&mgr->lock);
+
+	return r;
+}
+
 /**
  * amdgpu_gtt_mgr_debug - dump VRAM table
  *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index a0944a150d9a..1eb6177d4af0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -690,7 +690,6 @@ struct amdgpu_ttm_tt {
 	struct list_head        guptasks;
 	atomic_t		mmu_invalidations;
 	uint32_t		last_set_pages;
-	struct list_head        list;
 };
 
 int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
@@ -866,21 +865,14 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt *ttm,
 		return 0;
 	}
 
-	spin_lock(&gtt->adev->gtt_list_lock);
 	flags = amdgpu_ttm_tt_pte_flags(gtt->adev, ttm, bo_mem);
 	gtt->offset = (u64)bo_mem->start << PAGE_SHIFT;
 	r = amdgpu_gart_bind(gtt->adev, gtt->offset, ttm->num_pages,
 		ttm->pages, gtt->ttm.dma_address, flags);
 
-	if (r) {
+	if (r)
 		DRM_ERROR("failed to bind %lu pages at 0x%08llX\n",
 			  ttm->num_pages, gtt->offset);
-		goto error_gart_bind;
-	}
-
-	list_add_tail(&gtt->list, &gtt->adev->gtt_list);
-error_gart_bind:
-	spin_unlock(&gtt->adev->gtt_list_lock);
 	return r;
 }
 
@@ -922,29 +914,23 @@ int amdgpu_ttm_bind(struct ttm_buffer_object *bo)
 	return r;
 }
 
-int amdgpu_ttm_recover_gart(struct amdgpu_device *adev)
+int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo)
 {
-	struct amdgpu_ttm_tt *gtt, *tmp;
-	struct ttm_mem_reg bo_mem;
+	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
+	struct amdgpu_ttm_tt *gtt = (void *)tbo->ttm;
 	uint64_t flags;
 	int r;
 
-	bo_mem.mem_type = TTM_PL_TT;
-	spin_lock(&adev->gtt_list_lock);
-	list_for_each_entry_safe(gtt, tmp, &adev->gtt_list, list) {
-		flags = amdgpu_ttm_tt_pte_flags(gtt->adev, &gtt->ttm.ttm, &bo_mem);
-		r = amdgpu_gart_bind(adev, gtt->offset, gtt->ttm.ttm.num_pages,
-				     gtt->ttm.ttm.pages, gtt->ttm.dma_address,
-				     flags);
-		if (r) {
-			spin_unlock(&adev->gtt_list_lock);
-			DRM_ERROR("failed to bind %lu pages at 0x%08llX\n",
-				  gtt->ttm.ttm.num_pages, gtt->offset);
-			return r;
-		}
-	}
-	spin_unlock(&adev->gtt_list_lock);
-	return 0;
+	if (!gtt)
+		return 0;
+
+	flags = amdgpu_ttm_tt_pte_flags(adev, &gtt->ttm.ttm, &tbo->mem);
+	r = amdgpu_gart_bind(adev, gtt->offset, gtt->ttm.ttm.num_pages,
+			     gtt->ttm.ttm.pages, gtt->ttm.dma_address, flags);
+	if (r)
+		DRM_ERROR("failed to bind %lu pages at 0x%08llX\n",
+			  gtt->ttm.ttm.num_pages, gtt->offset);
+	return r;
 }
 
 static int amdgpu_ttm_backend_unbind(struct ttm_tt *ttm)
@@ -959,16 +945,10 @@ static int amdgpu_ttm_backend_unbind(struct ttm_tt *ttm)
 		return 0;
 
 	/* unbind shouldn't be done for GDS/GWS/OA in ttm_bo_clean_mm */
-	spin_lock(&gtt->adev->gtt_list_lock);
 	r = amdgpu_gart_unbind(gtt->adev, gtt->offset, ttm->num_pages);
-	if (r) {
+	if (r)
 		DRM_ERROR("failed to unbind %lu pages at 0x%08llX\n",
 			  gtt->ttm.ttm.num_pages, gtt->offset);
-		goto error_unbind;
-	}
-	list_del_init(&gtt->list);
-error_unbind:
-	spin_unlock(&gtt->adev->gtt_list_lock);
 	return r;
 }
 
@@ -1005,7 +985,6 @@ static struct ttm_tt *amdgpu_ttm_tt_create(struct ttm_bo_device *bdev,
 		kfree(gtt);
 		return NULL;
 	}
-	INIT_LIST_HEAD(&gtt->list);
 	return &gtt->ttm.ttm;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 39140ed88273..3c3406d9fc64 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -69,6 +69,7 @@ extern const struct ttm_mem_type_manager_func amdgpu_vram_mgr_func;
 
 bool amdgpu_gtt_mgr_is_allocated(struct ttm_mem_reg *mem);
 uint64_t amdgpu_gtt_mgr_usage(struct ttm_mem_type_manager *man);
+int amdgpu_gtt_mgr_recover(struct ttm_mem_type_manager *man);
 
 uint64_t amdgpu_vram_mgr_usage(struct ttm_mem_type_manager *man);
 uint64_t amdgpu_vram_mgr_vis_usage(struct ttm_mem_type_manager *man);
@@ -91,7 +92,7 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
 
 int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
 int amdgpu_ttm_bind(struct ttm_buffer_object *bo);
-int amdgpu_ttm_recover_gart(struct amdgpu_device *adev);
+int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
 
 int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages);
 void amdgpu_ttm_tt_set_user_pages(struct ttm_tt *ttm, struct page **pages);
-- 
2.11.0

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

[PATCH 5/5] drm/amdgpu: don't flush the TLB before initializing GART

[Christian König]

From: Christian König <christian.koenig@amd.com>

No point in doing this.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index f4370081f6e6..fe818501c520 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -332,12 +332,13 @@ int amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
 		adev->gart.pages[p] = pagelist[i];
 #endif
 
-	if (adev->gart.ptr) {
-		r = amdgpu_gart_map(adev, offset, pages, dma_addr, flags,
-			    adev->gart.ptr);
-		if (r)
-			return r;
-	}
+	if (!adev->gart.ptr)
+		return 0;
+
+	r = amdgpu_gart_map(adev, offset, pages, dma_addr, flags,
+		    adev->gart.ptr);
+	if (r)
+		return r;
 
 	mb();
 	amdgpu_gart_flush_gpu_tlb(adev, 0);
-- 
2.11.0

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

Re: [PATCH 1/5] drm/amdgpu: minor cleanup for amdgpu_ttm_bind

[Michel Dänzer]

On 20/10/17 11:20 AM, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
> 
> Filter the placement mask before using it. In theory it could be that we
> have other flags set here as well.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index dcdfb8d176fc..32c822f3db11 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -910,7 +910,8 @@ int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct ttm_mem_reg *bo_mem)
>  	placement.busy_placement = &placements;
>  	placements.fpfn = 0;
>  	placements.lpfn = adev->mc.gart_size >> PAGE_SHIFT;
> -	placements.flags = bo->mem.placement | TTM_PL_FLAG_TT;
> +	placements.flags = (bo->mem.placement & ~TTM_PL_MASK_MEM) |
> +		TTM_PL_FLAG_TT;
>  
>  	r = ttm_bo_mem_space(bo, &placement, &tmp, true, false);
>  	if (unlikely(r))
> 

Patches 1, 3 & 5 are

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/5] drm/amdgpu: nuke amdgpu_ttm_is_bound()

[Chunming Zhou]

using ttm->state == tt_bound seems make more sense.


On 2017年10月20日 17:20, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> use amdgpu_gtt_mgr_is_allocated() instead.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 15 +++++----------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  1 -
>   4 files changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 8b4ed8a98a18..71a0c1a477ae 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -980,7 +980,7 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
>   {
>   	WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_SYSTEM);
>   	WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_TT &&
> -		     !amdgpu_ttm_is_bound(bo->tbo.ttm));
> +		     !amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem));
>   	WARN_ON_ONCE(!ww_mutex_is_locked(&bo->tbo.resv->lock) &&
>   		     !bo->pin_count);
>   	WARN_ON_ONCE(bo->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 428aae048f4b..3affcc497be6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -187,7 +187,7 @@ static inline u64 amdgpu_bo_mmap_offset(struct amdgpu_bo *bo)
>   static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo)
>   {
>   	switch (bo->tbo.mem.mem_type) {
> -	case TTM_PL_TT: return amdgpu_ttm_is_bound(bo->tbo.ttm);
> +	case TTM_PL_TT: return amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem);
>   	case TTM_PL_VRAM: return true;
>   	default: return false;
>   	}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 32c822f3db11..1c9faa1a53ce 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -861,8 +861,10 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt *ttm,
>   	    bo_mem->mem_type == AMDGPU_PL_OA)
>   		return -EINVAL;
>   
> -	if (!amdgpu_gtt_mgr_is_allocated(bo_mem))
> +	if (!amdgpu_gtt_mgr_is_allocated(bo_mem)) {
> +		gtt->offset = AMDGPU_BO_INVALID_OFFSET;
>   		return 0;
> +	}
>   
>   	spin_lock(&gtt->adev->gtt_list_lock);
>   	flags = amdgpu_ttm_tt_pte_flags(gtt->adev, ttm, bo_mem);
> @@ -882,13 +884,6 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt *ttm,
>   	return r;
>   }
>   
> -bool amdgpu_ttm_is_bound(struct ttm_tt *ttm)
> -{
> -	struct amdgpu_ttm_tt *gtt = (void *)ttm;
> -
> -	return gtt && !list_empty(&gtt->list);
> -}
> -
>   int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct ttm_mem_reg *bo_mem)
>   {
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
> @@ -899,7 +894,7 @@ int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct ttm_mem_reg *bo_mem)
>   	struct ttm_place placements;
>   	int r;
>   
> -	if (!ttm || amdgpu_ttm_is_bound(ttm))
> +	if (!ttm || amdgpu_gtt_mgr_is_allocated(bo_mem))
>   		return 0;
>   
>   	tmp = bo->mem;
> @@ -960,7 +955,7 @@ static int amdgpu_ttm_backend_unbind(struct ttm_tt *ttm)
>   	if (gtt->userptr)
>   		amdgpu_ttm_tt_unpin_userptr(ttm);
>   
> -	if (!amdgpu_ttm_is_bound(ttm))
> +	if (gtt->offset == AMDGPU_BO_INVALID_OFFSET)
>   		return 0;
>   
>   	/* unbind shouldn't be done for GDS/GWS/OA in ttm_bo_clean_mm */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index abd4084982a3..89a71abc71e0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -90,7 +90,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
>   			struct dma_fence **fence);
>   
>   int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
> -bool amdgpu_ttm_is_bound(struct ttm_tt *ttm);
>   int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct ttm_mem_reg *bo_mem);
>   int amdgpu_ttm_recover_gart(struct amdgpu_device *adev);
>   

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

Re: [PATCH 2/5] drm/amdgpu: nuke amdgpu_ttm_is_bound()

[Christian König]

Am 23.10.2017 um 09:33 schrieb Chunming Zhou:
> using ttm->state == tt_bound seems make more sense.

That won't work. We set ttm->state = tt_bound even when we haven't 
allocated GART space.

That's also a reason why I wanted to remove it, the name is confusing.

Christian.

>
>
> On 2017年10月20日 17:20, Christian König wrote:
>> From: Christian König <christian.koenig@amd.com>
>>
>> use amdgpu_gtt_mgr_is_allocated() instead.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 15 +++++----------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  1 -
>>   4 files changed, 7 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 8b4ed8a98a18..71a0c1a477ae 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -980,7 +980,7 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
>>   {
>>       WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_SYSTEM);
>>       WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_TT &&
>> -             !amdgpu_ttm_is_bound(bo->tbo.ttm));
>> + !amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem));
>> WARN_ON_ONCE(!ww_mutex_is_locked(&bo->tbo.resv->lock) &&
>>                !bo->pin_count);
>>       WARN_ON_ONCE(bo->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> index 428aae048f4b..3affcc497be6 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> @@ -187,7 +187,7 @@ static inline u64 amdgpu_bo_mmap_offset(struct 
>> amdgpu_bo *bo)
>>   static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo)
>>   {
>>       switch (bo->tbo.mem.mem_type) {
>> -    case TTM_PL_TT: return amdgpu_ttm_is_bound(bo->tbo.ttm);
>> +    case TTM_PL_TT: return amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem);
>>       case TTM_PL_VRAM: return true;
>>       default: return false;
>>       }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 32c822f3db11..1c9faa1a53ce 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -861,8 +861,10 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt 
>> *ttm,
>>           bo_mem->mem_type == AMDGPU_PL_OA)
>>           return -EINVAL;
>>   -    if (!amdgpu_gtt_mgr_is_allocated(bo_mem))
>> +    if (!amdgpu_gtt_mgr_is_allocated(bo_mem)) {
>> +        gtt->offset = AMDGPU_BO_INVALID_OFFSET;
>>           return 0;
>> +    }
>>         spin_lock(&gtt->adev->gtt_list_lock);
>>       flags = amdgpu_ttm_tt_pte_flags(gtt->adev, ttm, bo_mem);
>> @@ -882,13 +884,6 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt 
>> *ttm,
>>       return r;
>>   }
>>   -bool amdgpu_ttm_is_bound(struct ttm_tt *ttm)
>> -{
>> -    struct amdgpu_ttm_tt *gtt = (void *)ttm;
>> -
>> -    return gtt && !list_empty(&gtt->list);
>> -}
>> -
>>   int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct 
>> ttm_mem_reg *bo_mem)
>>   {
>>       struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
>> @@ -899,7 +894,7 @@ int amdgpu_ttm_bind(struct ttm_buffer_object *bo, 
>> struct ttm_mem_reg *bo_mem)
>>       struct ttm_place placements;
>>       int r;
>>   -    if (!ttm || amdgpu_ttm_is_bound(ttm))
>> +    if (!ttm || amdgpu_gtt_mgr_is_allocated(bo_mem))
>>           return 0;
>>         tmp = bo->mem;
>> @@ -960,7 +955,7 @@ static int amdgpu_ttm_backend_unbind(struct 
>> ttm_tt *ttm)
>>       if (gtt->userptr)
>>           amdgpu_ttm_tt_unpin_userptr(ttm);
>>   -    if (!amdgpu_ttm_is_bound(ttm))
>> +    if (gtt->offset == AMDGPU_BO_INVALID_OFFSET)
>>           return 0;
>>         /* unbind shouldn't be done for GDS/GWS/OA in ttm_bo_clean_mm */
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> index abd4084982a3..89a71abc71e0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> @@ -90,7 +90,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
>>               struct dma_fence **fence);
>>     int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
>> -bool amdgpu_ttm_is_bound(struct ttm_tt *ttm);
>>   int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct 
>> ttm_mem_reg *bo_mem);
>>   int amdgpu_ttm_recover_gart(struct amdgpu_device *adev);
>

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

Re: [PATCH 2/5] drm/amdgpu: nuke amdgpu_ttm_is_bound()

[Chunming Zhou]

Yes, I see the checking in backend_bind:

         if (!amdgpu_gtt_mgr_is_allocated(bo_mem))
                 return 0;

OK, then my only concern is your ’instead‘ assumes 
amdgpu_gtt_mgr_is_allocated means gart is bound, how about:

bool amdgpu_gtt_is_bound(struct ttm_tt *ttm)
{
     return amdgpu_gtt_mgr_is_allocated(bo_mem);
}

Why I prefer to keep is_bound for naming is we only can get gpu offset 
after gtt is bound not check if node is allocated.

Regards,
David Zhou
On 2017年10月23日 16:11, Christian König wrote:
> Am 23.10.2017 um 09:33 schrieb Chunming Zhou:
>> using ttm->state == tt_bound seems make more sense.
>
> That won't work. We set ttm->state = tt_bound even when we haven't 
> allocated GART space.
>
> That's also a reason why I wanted to remove it, the name is confusing.
>
> Christian.
>
>>
>>
>> On 2017年10月20日 17:20, Christian König wrote:
>>> From: Christian König <christian.koenig@amd.com>
>>>
>>> use amdgpu_gtt_mgr_is_allocated() instead.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  2 +-
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +-
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 15 +++++----------
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  1 -
>>>   4 files changed, 7 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> index 8b4ed8a98a18..71a0c1a477ae 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> @@ -980,7 +980,7 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
>>>   {
>>>       WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_SYSTEM);
>>>       WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_TT &&
>>> -             !amdgpu_ttm_is_bound(bo->tbo.ttm));
>>> + !amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem));
>>> WARN_ON_ONCE(!ww_mutex_is_locked(&bo->tbo.resv->lock) &&
>>>                !bo->pin_count);
>>>       WARN_ON_ONCE(bo->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> index 428aae048f4b..3affcc497be6 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> @@ -187,7 +187,7 @@ static inline u64 amdgpu_bo_mmap_offset(struct 
>>> amdgpu_bo *bo)
>>>   static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo)
>>>   {
>>>       switch (bo->tbo.mem.mem_type) {
>>> -    case TTM_PL_TT: return amdgpu_ttm_is_bound(bo->tbo.ttm);
>>> +    case TTM_PL_TT: return amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem);
>>>       case TTM_PL_VRAM: return true;
>>>       default: return false;
>>>       }
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 32c822f3db11..1c9faa1a53ce 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -861,8 +861,10 @@ static int amdgpu_ttm_backend_bind(struct 
>>> ttm_tt *ttm,
>>>           bo_mem->mem_type == AMDGPU_PL_OA)
>>>           return -EINVAL;
>>>   -    if (!amdgpu_gtt_mgr_is_allocated(bo_mem))
>>> +    if (!amdgpu_gtt_mgr_is_allocated(bo_mem)) {
>>> +        gtt->offset = AMDGPU_BO_INVALID_OFFSET;
>>>           return 0;
>>> +    }
>>>         spin_lock(&gtt->adev->gtt_list_lock);
>>>       flags = amdgpu_ttm_tt_pte_flags(gtt->adev, ttm, bo_mem);
>>> @@ -882,13 +884,6 @@ static int amdgpu_ttm_backend_bind(struct 
>>> ttm_tt *ttm,
>>>       return r;
>>>   }
>>>   -bool amdgpu_ttm_is_bound(struct ttm_tt *ttm)
>>> -{
>>> -    struct amdgpu_ttm_tt *gtt = (void *)ttm;
>>> -
>>> -    return gtt && !list_empty(&gtt->list);
>>> -}
>>> -
>>>   int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct 
>>> ttm_mem_reg *bo_mem)
>>>   {
>>>       struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
>>> @@ -899,7 +894,7 @@ int amdgpu_ttm_bind(struct ttm_buffer_object 
>>> *bo, struct ttm_mem_reg *bo_mem)
>>>       struct ttm_place placements;
>>>       int r;
>>>   -    if (!ttm || amdgpu_ttm_is_bound(ttm))
>>> +    if (!ttm || amdgpu_gtt_mgr_is_allocated(bo_mem))
>>>           return 0;
>>>         tmp = bo->mem;
>>> @@ -960,7 +955,7 @@ static int amdgpu_ttm_backend_unbind(struct 
>>> ttm_tt *ttm)
>>>       if (gtt->userptr)
>>>           amdgpu_ttm_tt_unpin_userptr(ttm);
>>>   -    if (!amdgpu_ttm_is_bound(ttm))
>>> +    if (gtt->offset == AMDGPU_BO_INVALID_OFFSET)
>>>           return 0;
>>>         /* unbind shouldn't be done for GDS/GWS/OA in 
>>> ttm_bo_clean_mm */
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> index abd4084982a3..89a71abc71e0 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> @@ -90,7 +90,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
>>>               struct dma_fence **fence);
>>>     int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
>>> -bool amdgpu_ttm_is_bound(struct ttm_tt *ttm);
>>>   int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct 
>>> ttm_mem_reg *bo_mem);
>>>   int amdgpu_ttm_recover_gart(struct amdgpu_device *adev);
>>
>

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

Re: [PATCH 2/5] drm/amdgpu: nuke amdgpu_ttm_is_bound()

[Christian König]

> Why I prefer to keep is_bound for naming is we only can get gpu offset 
> after gtt is bound not check if node is allocated.
No, it is just the other way around.

When the TTM is bound it doesn't mean we can get the GPU offset because 
it can be that there wasn't any GART space assigned.

But if the GART space was allocated, we can be sure that we also have an 
GPU offset to use.

Regards,
Christian.

Am 23.10.2017 um 10:34 schrieb Chunming Zhou:
> Yes, I see the checking in backend_bind:
>
>         if (!amdgpu_gtt_mgr_is_allocated(bo_mem))
>                 return 0;
>
> OK, then my only concern is your ’instead‘ assumes 
> amdgpu_gtt_mgr_is_allocated means gart is bound, how about:
>
> bool amdgpu_gtt_is_bound(struct ttm_tt *ttm)
> {
>     return amdgpu_gtt_mgr_is_allocated(bo_mem);
> }
>
> Why I prefer to keep is_bound for naming is we only can get gpu offset 
> after gtt is bound not check if node is allocated.
>
> Regards,
> David Zhou
> On 2017年10月23日 16:11, Christian König wrote:
>> Am 23.10.2017 um 09:33 schrieb Chunming Zhou:
>>> using ttm->state == tt_bound seems make more sense.
>>
>> That won't work. We set ttm->state = tt_bound even when we haven't 
>> allocated GART space.
>>
>> That's also a reason why I wanted to remove it, the name is confusing.
>>
>> Christian.
>>
>>>
>>>
>>> On 2017年10月20日 17:20, Christian König wrote:
>>>> From: Christian König <christian.koenig@amd.com>
>>>>
>>>> use amdgpu_gtt_mgr_is_allocated() instead.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  2 +-
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +-
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 15 +++++----------
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  1 -
>>>>   4 files changed, 7 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> index 8b4ed8a98a18..71a0c1a477ae 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> @@ -980,7 +980,7 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
>>>>   {
>>>>       WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_SYSTEM);
>>>>       WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_TT &&
>>>> -             !amdgpu_ttm_is_bound(bo->tbo.ttm));
>>>> + !amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem));
>>>> WARN_ON_ONCE(!ww_mutex_is_locked(&bo->tbo.resv->lock) &&
>>>>                !bo->pin_count);
>>>>       WARN_ON_ONCE(bo->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>> index 428aae048f4b..3affcc497be6 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>> @@ -187,7 +187,7 @@ static inline u64 amdgpu_bo_mmap_offset(struct 
>>>> amdgpu_bo *bo)
>>>>   static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo)
>>>>   {
>>>>       switch (bo->tbo.mem.mem_type) {
>>>> -    case TTM_PL_TT: return amdgpu_ttm_is_bound(bo->tbo.ttm);
>>>> +    case TTM_PL_TT: return amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem);
>>>>       case TTM_PL_VRAM: return true;
>>>>       default: return false;
>>>>       }
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> index 32c822f3db11..1c9faa1a53ce 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> @@ -861,8 +861,10 @@ static int amdgpu_ttm_backend_bind(struct 
>>>> ttm_tt *ttm,
>>>>           bo_mem->mem_type == AMDGPU_PL_OA)
>>>>           return -EINVAL;
>>>>   -    if (!amdgpu_gtt_mgr_is_allocated(bo_mem))
>>>> +    if (!amdgpu_gtt_mgr_is_allocated(bo_mem)) {
>>>> +        gtt->offset = AMDGPU_BO_INVALID_OFFSET;
>>>>           return 0;
>>>> +    }
>>>>         spin_lock(&gtt->adev->gtt_list_lock);
>>>>       flags = amdgpu_ttm_tt_pte_flags(gtt->adev, ttm, bo_mem);
>>>> @@ -882,13 +884,6 @@ static int amdgpu_ttm_backend_bind(struct 
>>>> ttm_tt *ttm,
>>>>       return r;
>>>>   }
>>>>   -bool amdgpu_ttm_is_bound(struct ttm_tt *ttm)
>>>> -{
>>>> -    struct amdgpu_ttm_tt *gtt = (void *)ttm;
>>>> -
>>>> -    return gtt && !list_empty(&gtt->list);
>>>> -}
>>>> -
>>>>   int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct 
>>>> ttm_mem_reg *bo_mem)
>>>>   {
>>>>       struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
>>>> @@ -899,7 +894,7 @@ int amdgpu_ttm_bind(struct ttm_buffer_object 
>>>> *bo, struct ttm_mem_reg *bo_mem)
>>>>       struct ttm_place placements;
>>>>       int r;
>>>>   -    if (!ttm || amdgpu_ttm_is_bound(ttm))
>>>> +    if (!ttm || amdgpu_gtt_mgr_is_allocated(bo_mem))
>>>>           return 0;
>>>>         tmp = bo->mem;
>>>> @@ -960,7 +955,7 @@ static int amdgpu_ttm_backend_unbind(struct 
>>>> ttm_tt *ttm)
>>>>       if (gtt->userptr)
>>>>           amdgpu_ttm_tt_unpin_userptr(ttm);
>>>>   -    if (!amdgpu_ttm_is_bound(ttm))
>>>> +    if (gtt->offset == AMDGPU_BO_INVALID_OFFSET)
>>>>           return 0;
>>>>         /* unbind shouldn't be done for GDS/GWS/OA in 
>>>> ttm_bo_clean_mm */
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>> index abd4084982a3..89a71abc71e0 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>> @@ -90,7 +90,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
>>>>               struct dma_fence **fence);
>>>>     int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
>>>> -bool amdgpu_ttm_is_bound(struct ttm_tt *ttm);
>>>>   int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct 
>>>> ttm_mem_reg *bo_mem);
>>>>   int amdgpu_ttm_recover_gart(struct amdgpu_device *adev);
>>>
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

Re: [PATCH 2/5] drm/amdgpu: nuke amdgpu_ttm_is_bound()

[Chunming Zhou]

On 2017年10月23日 22:26, Christian König wrote:
>> Why I prefer to keep is_bound for naming is we only can get gpu 
>> offset after gtt is bound not check if node is allocated.
> No, it is just the other way around.
>
> When the TTM is bound it doesn't mean we can get the GPU offset 
> because it can be that there wasn't any GART space assigned.
Sorry, I didn't say clear, I'm not saying TTM bound but GART bound, we 
only can get GPU offset after calling amdgpu_gart_bind() not node 
allocated, so I thought amdgpu_gtt_is_bound is more sense than old 
amdgpu_ttm_is_bound.
>
> But if the GART space was allocated, we can be sure that we also have 
> an GPU offset to use.
I know it, in current all use case, we always do gart_bind when GART 
node is allocated, so I'd like use amdgpu_gtt_is_bound() to wrap 
amdgpu_gtt_mgr_is_allocated().

Regards,
David Zhou
>
> Regards,
> Christian.
>
> Am 23.10.2017 um 10:34 schrieb Chunming Zhou:
>> Yes, I see the checking in backend_bind:
>>
>>         if (!amdgpu_gtt_mgr_is_allocated(bo_mem))
>>                 return 0;
>>
>> OK, then my only concern is your ’instead‘ assumes 
>> amdgpu_gtt_mgr_is_allocated means gart is bound, how about:
>>
>> bool amdgpu_gtt_is_bound(struct ttm_tt *ttm)
>> {
>>     return amdgpu_gtt_mgr_is_allocated(bo_mem);
>> }
>>
>> Why I prefer to keep is_bound for naming is we only can get gpu 
>> offset after gtt is bound not check if node is allocated.
>>
>> Regards,
>> David Zhou
>> On 2017年10月23日 16:11, Christian König wrote:
>>> Am 23.10.2017 um 09:33 schrieb Chunming Zhou:
>>>> using ttm->state == tt_bound seems make more sense.
>>>
>>> That won't work. We set ttm->state = tt_bound even when we haven't 
>>> allocated GART space.
>>>
>>> That's also a reason why I wanted to remove it, the name is confusing.
>>>
>>> Christian.
>>>
>>>>
>>>>
>>>> On 2017年10月20日 17:20, Christian König wrote:
>>>>> From: Christian König <christian.koenig@amd.com>
>>>>>
>>>>> use amdgpu_gtt_mgr_is_allocated() instead.
>>>>>
>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  2 +-
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +-
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 15 +++++----------
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  1 -
>>>>>   4 files changed, 7 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> index 8b4ed8a98a18..71a0c1a477ae 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> @@ -980,7 +980,7 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
>>>>>   {
>>>>>       WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_SYSTEM);
>>>>>       WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_TT &&
>>>>> -             !amdgpu_ttm_is_bound(bo->tbo.ttm));
>>>>> + !amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem));
>>>>> WARN_ON_ONCE(!ww_mutex_is_locked(&bo->tbo.resv->lock) &&
>>>>>                !bo->pin_count);
>>>>>       WARN_ON_ONCE(bo->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET);
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>>> index 428aae048f4b..3affcc497be6 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>>> @@ -187,7 +187,7 @@ static inline u64 amdgpu_bo_mmap_offset(struct 
>>>>> amdgpu_bo *bo)
>>>>>   static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo)
>>>>>   {
>>>>>       switch (bo->tbo.mem.mem_type) {
>>>>> -    case TTM_PL_TT: return amdgpu_ttm_is_bound(bo->tbo.ttm);
>>>>> +    case TTM_PL_TT: return 
>>>>> amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem);
>>>>>       case TTM_PL_VRAM: return true;
>>>>>       default: return false;
>>>>>       }
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> index 32c822f3db11..1c9faa1a53ce 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> @@ -861,8 +861,10 @@ static int amdgpu_ttm_backend_bind(struct 
>>>>> ttm_tt *ttm,
>>>>>           bo_mem->mem_type == AMDGPU_PL_OA)
>>>>>           return -EINVAL;
>>>>>   -    if (!amdgpu_gtt_mgr_is_allocated(bo_mem))
>>>>> +    if (!amdgpu_gtt_mgr_is_allocated(bo_mem)) {
>>>>> +        gtt->offset = AMDGPU_BO_INVALID_OFFSET;
>>>>>           return 0;
>>>>> +    }
>>>>>         spin_lock(&gtt->adev->gtt_list_lock);
>>>>>       flags = amdgpu_ttm_tt_pte_flags(gtt->adev, ttm, bo_mem);
>>>>> @@ -882,13 +884,6 @@ static int amdgpu_ttm_backend_bind(struct 
>>>>> ttm_tt *ttm,
>>>>>       return r;
>>>>>   }
>>>>>   -bool amdgpu_ttm_is_bound(struct ttm_tt *ttm)
>>>>> -{
>>>>> -    struct amdgpu_ttm_tt *gtt = (void *)ttm;
>>>>> -
>>>>> -    return gtt && !list_empty(&gtt->list);
>>>>> -}
>>>>> -
>>>>>   int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct 
>>>>> ttm_mem_reg *bo_mem)
>>>>>   {
>>>>>       struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
>>>>> @@ -899,7 +894,7 @@ int amdgpu_ttm_bind(struct ttm_buffer_object 
>>>>> *bo, struct ttm_mem_reg *bo_mem)
>>>>>       struct ttm_place placements;
>>>>>       int r;
>>>>>   -    if (!ttm || amdgpu_ttm_is_bound(ttm))
>>>>> +    if (!ttm || amdgpu_gtt_mgr_is_allocated(bo_mem))
>>>>>           return 0;
>>>>>         tmp = bo->mem;
>>>>> @@ -960,7 +955,7 @@ static int amdgpu_ttm_backend_unbind(struct 
>>>>> ttm_tt *ttm)
>>>>>       if (gtt->userptr)
>>>>>           amdgpu_ttm_tt_unpin_userptr(ttm);
>>>>>   -    if (!amdgpu_ttm_is_bound(ttm))
>>>>> +    if (gtt->offset == AMDGPU_BO_INVALID_OFFSET)
>>>>>           return 0;
>>>>>         /* unbind shouldn't be done for GDS/GWS/OA in 
>>>>> ttm_bo_clean_mm */
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> index abd4084982a3..89a71abc71e0 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> @@ -90,7 +90,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
>>>>>               struct dma_fence **fence);
>>>>>     int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
>>>>> -bool amdgpu_ttm_is_bound(struct ttm_tt *ttm);
>>>>>   int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct 
>>>>> ttm_mem_reg *bo_mem);
>>>>>   int amdgpu_ttm_recover_gart(struct amdgpu_device *adev);
>>>>
>>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>

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

Re: [PATCH 2/5] drm/amdgpu: nuke amdgpu_ttm_is_bound()

[Christian König]

Am 24.10.2017 um 03:41 schrieb Chunming Zhou:
>
>
> On 2017年10月23日 22:26, Christian König wrote:
>>> Why I prefer to keep is_bound for naming is we only can get gpu 
>>> offset after gtt is bound not check if node is allocated.
>> No, it is just the other way around.
>>
>> When the TTM is bound it doesn't mean we can get the GPU offset 
>> because it can be that there wasn't any GART space assigned.
> Sorry, I didn't say clear, I'm not saying TTM bound but GART bound, we 
> only can get GPU offset after calling amdgpu_gart_bind() not node 
> allocated, so I thought amdgpu_gtt_is_bound is more sense than old 
> amdgpu_ttm_is_bound.

And exactly that is incorrect. You can get the offset as soon as the 
GART space is allocated. We don't have to call amdgpu_gart_bind() for that.

The GART binding is needed for the stuff to work in the end, but it can 
in theory can come later on.

>>
>> But if the GART space was allocated, we can be sure that we also have 
>> an GPU offset to use.
> I know it, in current all use case, we always do gart_bind when GART 
> node is allocated, so I'd like use amdgpu_gtt_is_bound() to wrap 
> amdgpu_gtt_mgr_is_allocated().

Yeah, I think I can live with that as well.

Regards,
Christian.

>
> Regards,
> David Zhou
>>
>> Regards,
>> Christian.
>>
>> Am 23.10.2017 um 10:34 schrieb Chunming Zhou:
>>> Yes, I see the checking in backend_bind:
>>>
>>>         if (!amdgpu_gtt_mgr_is_allocated(bo_mem))
>>>                 return 0;
>>>
>>> OK, then my only concern is your ’instead‘ assumes 
>>> amdgpu_gtt_mgr_is_allocated means gart is bound, how about:
>>>
>>> bool amdgpu_gtt_is_bound(struct ttm_tt *ttm)
>>> {
>>>     return amdgpu_gtt_mgr_is_allocated(bo_mem);
>>> }
>>>
>>> Why I prefer to keep is_bound for naming is we only can get gpu 
>>> offset after gtt is bound not check if node is allocated.
>>>
>>> Regards,
>>> David Zhou
>>> On 2017年10月23日 16:11, Christian König wrote:
>>>> Am 23.10.2017 um 09:33 schrieb Chunming Zhou:
>>>>> using ttm->state == tt_bound seems make more sense.
>>>>
>>>> That won't work. We set ttm->state = tt_bound even when we haven't 
>>>> allocated GART space.
>>>>
>>>> That's also a reason why I wanted to remove it, the name is confusing.
>>>>
>>>> Christian.
>>>>
>>>>>
>>>>>
>>>>> On 2017年10月20日 17:20, Christian König wrote:
>>>>>> From: Christian König <christian.koenig@amd.com>
>>>>>>
>>>>>> use amdgpu_gtt_mgr_is_allocated() instead.
>>>>>>
>>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  2 +-
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +-
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 15 +++++----------
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  1 -
>>>>>>   4 files changed, 7 insertions(+), 13 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>>> index 8b4ed8a98a18..71a0c1a477ae 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>>> @@ -980,7 +980,7 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
>>>>>>   {
>>>>>>       WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_SYSTEM);
>>>>>>       WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_TT &&
>>>>>> -             !amdgpu_ttm_is_bound(bo->tbo.ttm));
>>>>>> + !amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem));
>>>>>> WARN_ON_ONCE(!ww_mutex_is_locked(&bo->tbo.resv->lock) &&
>>>>>>                !bo->pin_count);
>>>>>>       WARN_ON_ONCE(bo->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET);
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>>>> index 428aae048f4b..3affcc497be6 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>>>> @@ -187,7 +187,7 @@ static inline u64 
>>>>>> amdgpu_bo_mmap_offset(struct amdgpu_bo *bo)
>>>>>>   static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo)
>>>>>>   {
>>>>>>       switch (bo->tbo.mem.mem_type) {
>>>>>> -    case TTM_PL_TT: return amdgpu_ttm_is_bound(bo->tbo.ttm);
>>>>>> +    case TTM_PL_TT: return 
>>>>>> amdgpu_gtt_mgr_is_allocated(&bo->tbo.mem);
>>>>>>       case TTM_PL_VRAM: return true;
>>>>>>       default: return false;
>>>>>>       }
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>>> index 32c822f3db11..1c9faa1a53ce 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>>> @@ -861,8 +861,10 @@ static int amdgpu_ttm_backend_bind(struct 
>>>>>> ttm_tt *ttm,
>>>>>>           bo_mem->mem_type == AMDGPU_PL_OA)
>>>>>>           return -EINVAL;
>>>>>>   -    if (!amdgpu_gtt_mgr_is_allocated(bo_mem))
>>>>>> +    if (!amdgpu_gtt_mgr_is_allocated(bo_mem)) {
>>>>>> +        gtt->offset = AMDGPU_BO_INVALID_OFFSET;
>>>>>>           return 0;
>>>>>> +    }
>>>>>>         spin_lock(&gtt->adev->gtt_list_lock);
>>>>>>       flags = amdgpu_ttm_tt_pte_flags(gtt->adev, ttm, bo_mem);
>>>>>> @@ -882,13 +884,6 @@ static int amdgpu_ttm_backend_bind(struct 
>>>>>> ttm_tt *ttm,
>>>>>>       return r;
>>>>>>   }
>>>>>>   -bool amdgpu_ttm_is_bound(struct ttm_tt *ttm)
>>>>>> -{
>>>>>> -    struct amdgpu_ttm_tt *gtt = (void *)ttm;
>>>>>> -
>>>>>> -    return gtt && !list_empty(&gtt->list);
>>>>>> -}
>>>>>> -
>>>>>>   int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct 
>>>>>> ttm_mem_reg *bo_mem)
>>>>>>   {
>>>>>>       struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
>>>>>> @@ -899,7 +894,7 @@ int amdgpu_ttm_bind(struct ttm_buffer_object 
>>>>>> *bo, struct ttm_mem_reg *bo_mem)
>>>>>>       struct ttm_place placements;
>>>>>>       int r;
>>>>>>   -    if (!ttm || amdgpu_ttm_is_bound(ttm))
>>>>>> +    if (!ttm || amdgpu_gtt_mgr_is_allocated(bo_mem))
>>>>>>           return 0;
>>>>>>         tmp = bo->mem;
>>>>>> @@ -960,7 +955,7 @@ static int amdgpu_ttm_backend_unbind(struct 
>>>>>> ttm_tt *ttm)
>>>>>>       if (gtt->userptr)
>>>>>>           amdgpu_ttm_tt_unpin_userptr(ttm);
>>>>>>   -    if (!amdgpu_ttm_is_bound(ttm))
>>>>>> +    if (gtt->offset == AMDGPU_BO_INVALID_OFFSET)
>>>>>>           return 0;
>>>>>>         /* unbind shouldn't be done for GDS/GWS/OA in 
>>>>>> ttm_bo_clean_mm */
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>>> index abd4084982a3..89a71abc71e0 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>>> @@ -90,7 +90,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
>>>>>>               struct dma_fence **fence);
>>>>>>     int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
>>>>>> -bool amdgpu_ttm_is_bound(struct ttm_tt *ttm);
>>>>>>   int amdgpu_ttm_bind(struct ttm_buffer_object *bo, struct 
>>>>>> ttm_mem_reg *bo_mem);
>>>>>>   int amdgpu_ttm_recover_gart(struct amdgpu_device *adev);
>>>>>
>>>>
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>>
>

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