[PATCH 0/7] drm/ttm: bind/unbind + cleanups

[PATCH 0/7] drm/ttm: bind/unbind + cleanups

[Dave Airlie]

I've pulled a few of the cleaner bits of rework out for review.

The first thing is to move all the tt bind tracking into the driver
so the core doesn't do it all anymore. Once that is done it's possible
to flip over how destroy works so the drivers call some common code
instead of vice-versa.

With those two done the unbind can be completely moved into drivers,
so it happens before the unpopulate in the common code. This removes
a lot of unbinds from the core code (only really one left).

Then a patch to flip some drivers over to direct calling their
bind functions instead of indirect (just a trivial cleanup really)
(bind needs a lot more work).

Then I just refactored out some common patters in the move code,
which should make it easier to pinpoint the codeflow for future
bind changes.

Dave.


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/7] drm/ttm: protect against reentrant bind in the drivers

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This moves the generic tracking into the drivers and protects
against reentrancy in the drivers. It fixes up radeon and agp
to be able to query the bound status as that is required.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 12 +++++++++
 drivers/gpu/drm/nouveau/nouveau_bo.c       |  5 +++-
 drivers/gpu/drm/nouveau/nouveau_sgdma.c    |  8 +++++-
 drivers/gpu/drm/radeon/radeon.h            |  1 +
 drivers/gpu/drm/radeon/radeon_mn.c         |  2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c        | 31 ++++++++++++++++++++++
 drivers/gpu/drm/ttm/ttm_agp_backend.c      | 14 ++++++++++
 drivers/gpu/drm/ttm/ttm_bo.c               | 20 ++------------
 drivers/gpu/drm/ttm/ttm_bo_util.c          |  5 ----
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 22 ++++++++++++---
 include/drm/ttm/ttm_bo_api.h               |  1 -
 include/drm/ttm/ttm_bo_driver.h            | 14 ----------
 include/drm/ttm/ttm_tt.h                   |  1 +
 13 files changed, 91 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e86f8f6371c4..2851cf30091a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -813,6 +813,7 @@ struct amdgpu_ttm_tt {
 	uint64_t		userptr;
 	struct task_struct	*usertask;
 	uint32_t		userflags;
+	bool			bound;
 #if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
 	struct hmm_range	*range;
 #endif
@@ -1110,6 +1111,12 @@ static int amdgpu_ttm_backend_bind(struct ttm_bo_device *bdev,
 	uint64_t flags;
 	int r = 0;
 
+	if (!bo_mem)
+		return -EINVAL;
+
+	if (gtt->bound)
+		return 0;
+
 	if (gtt->userptr) {
 		r = amdgpu_ttm_tt_pin_userptr(bdev, ttm);
 		if (r) {
@@ -1143,6 +1150,7 @@ static int amdgpu_ttm_backend_bind(struct ttm_bo_device *bdev,
 	if (r)
 		DRM_ERROR("failed to bind %lu pages at 0x%08llX\n",
 			  ttm->num_pages, gtt->offset);
+	gtt->bound = true;
 	return r;
 }
 
@@ -1236,6 +1244,9 @@ static void amdgpu_ttm_backend_unbind(struct ttm_bo_device *bdev,
 	struct amdgpu_ttm_tt *gtt = (void *)ttm;
 	int r;
 
+	if (!gtt->bound)
+		return;
+
 	/* if the pages have userptr pinning then clear that first */
 	if (gtt->userptr)
 		amdgpu_ttm_tt_unpin_userptr(bdev, ttm);
@@ -1248,6 +1259,7 @@ static void amdgpu_ttm_backend_unbind(struct ttm_bo_device *bdev,
 	if (r)
 		DRM_ERROR("failed to unbind %lu pages at 0x%08llX\n",
 			  gtt->ttm.ttm.num_pages, gtt->offset);
+	gtt->bound = false;
 }
 
 static void amdgpu_ttm_backend_destroy(struct ttm_bo_device *bdev,
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index aea201d9c513..616d8844ba97 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -718,7 +718,10 @@ nouveau_ttm_tt_bind(struct ttm_bo_device *bdev, struct ttm_tt *ttm,
 {
 #if IS_ENABLED(CONFIG_AGP)
 	struct nouveau_drm *drm = nouveau_bdev(bdev);
-
+#endif
+	if (!reg)
+		return -EINVAL;
+#if IS_ENABLED(CONFIG_AGP)
 	if (drm->agp.bridge)
 		return ttm_agp_bind(ttm, reg);
 #endif
diff --git a/drivers/gpu/drm/nouveau/nouveau_sgdma.c b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
index 05e542254e1f..21fb92770ea2 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sgdma.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
@@ -33,6 +33,9 @@ nouveau_sgdma_bind(struct ttm_bo_device *bdev, struct ttm_tt *ttm, struct ttm_re
 	struct nouveau_mem *mem = nouveau_mem(reg);
 	int ret;
 
+	if (nvbe->mem)
+		return 0;
+
 	ret = nouveau_mem_host(reg, &nvbe->ttm);
 	if (ret)
 		return ret;
@@ -53,7 +56,10 @@ void
 nouveau_sgdma_unbind(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
 {
 	struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)ttm;
-	nouveau_mem_fini(nvbe->mem);
+	if (nvbe->mem) {
+		nouveau_mem_fini(nvbe->mem);
+		nvbe->mem = NULL;
+	}
 }
 
 struct ttm_tt *
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index df6f0b49836b..a6d8de01194a 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2820,6 +2820,7 @@ extern int radeon_ttm_tt_set_userptr(struct radeon_device *rdev,
 				     uint32_t flags);
 extern bool radeon_ttm_tt_has_userptr(struct radeon_device *rdev, struct ttm_tt *ttm);
 extern bool radeon_ttm_tt_is_readonly(struct radeon_device *rdev, struct ttm_tt *ttm);
+bool radeon_ttm_tt_is_bound(struct ttm_bo_device *bdev, struct ttm_tt *ttm);
 extern void radeon_vram_location(struct radeon_device *rdev, struct radeon_mc *mc, u64 base);
 extern void radeon_gtt_location(struct radeon_device *rdev, struct radeon_mc *mc);
 extern int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon);
diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/radeon_mn.c
index eb46d2220236..97b9b6dd6dd3 100644
--- a/drivers/gpu/drm/radeon/radeon_mn.c
+++ b/drivers/gpu/drm/radeon/radeon_mn.c
@@ -53,7 +53,7 @@ static bool radeon_mn_invalidate(struct mmu_interval_notifier *mn,
 	struct ttm_operation_ctx ctx = { false, false };
 	long r;
 
-	if (!bo->tbo.ttm || !ttm_bo_tt_is_bound(&bo->tbo))
+	if (!bo->tbo.ttm || !radeon_ttm_tt_is_bound(bo->tbo.bdev, bo->tbo.ttm))
 		return true;
 
 	if (!mmu_notifier_range_blockable(range))
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index c6c1008fefd2..fc8bbca28b34 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -420,6 +420,7 @@ struct radeon_ttm_tt {
 	uint64_t			userptr;
 	struct mm_struct		*usermm;
 	uint32_t			userflags;
+	bool bound;
 };
 
 /* prepare the sg table with the user pages */
@@ -513,6 +514,13 @@ static void radeon_ttm_tt_unpin_userptr(struct ttm_bo_device *bdev, struct ttm_t
 	sg_free_table(ttm->sg);
 }
 
+static bool radeon_ttm_backend_is_bound(struct ttm_tt *ttm)
+{
+	struct radeon_ttm_tt *gtt = (void*)ttm;
+
+	return (gtt->bound);
+}
+
 static int radeon_ttm_backend_bind(struct ttm_bo_device *bdev,
 				   struct ttm_tt *ttm,
 				   struct ttm_resource *bo_mem)
@@ -523,6 +531,9 @@ static int radeon_ttm_backend_bind(struct ttm_bo_device *bdev,
 		RADEON_GART_PAGE_WRITE;
 	int r;
 
+	if (gtt->bound)
+		return 0;
+
 	if (gtt->userptr) {
 		radeon_ttm_tt_pin_userptr(bdev, ttm);
 		flags &= ~RADEON_GART_PAGE_WRITE;
@@ -542,6 +553,7 @@ static int radeon_ttm_backend_bind(struct ttm_bo_device *bdev,
 			  ttm->num_pages, (unsigned)gtt->offset);
 		return r;
 	}
+	gtt->bound = true;
 	return 0;
 }
 
@@ -550,10 +562,14 @@ static void radeon_ttm_backend_unbind(struct ttm_bo_device *bdev, struct ttm_tt
 	struct radeon_ttm_tt *gtt = (void *)ttm;
 	struct radeon_device *rdev = radeon_get_rdev(bdev);
 
+	if (!gtt->bound)
+		return;
+
 	radeon_gart_unbind(rdev, gtt->offset, ttm->num_pages);
 
 	if (gtt->userptr)
 		radeon_ttm_tt_unpin_userptr(bdev, ttm);
+	gtt->bound = false;
 }
 
 static void radeon_ttm_backend_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
@@ -689,12 +705,27 @@ int radeon_ttm_tt_set_userptr(struct radeon_device *rdev,
 	return 0;
 }
 
+bool radeon_ttm_tt_is_bound(struct ttm_bo_device *bdev,
+			    struct ttm_tt *ttm)
+{
+#if IS_ENABLED(CONFIG_AGP)
+	struct radeon_device *rdev = radeon_get_rdev(bdev);
+	if (rdev->flags & RADEON_IS_AGP)
+		return ttm_agp_is_bound(ttm);
+#endif
+	return radeon_ttm_backend_is_bound(ttm);
+}
+
 static int radeon_ttm_tt_bind(struct ttm_bo_device *bdev,
 			      struct ttm_tt *ttm,
 			      struct ttm_resource *bo_mem)
 {
+#if IS_ENABLED(CONFIG_AGP)
 	struct radeon_device *rdev = radeon_get_rdev(bdev);
+#endif
 
+	if (!bo_mem)
+		return -EINVAL;
 #if IS_ENABLED(CONFIG_AGP)
 	if (rdev->flags & RADEON_IS_AGP)
 		return ttm_agp_bind(ttm, bo_mem);
diff --git a/drivers/gpu/drm/ttm/ttm_agp_backend.c b/drivers/gpu/drm/ttm/ttm_agp_backend.c
index 7b36fdaab766..a98fd795b752 100644
--- a/drivers/gpu/drm/ttm/ttm_agp_backend.c
+++ b/drivers/gpu/drm/ttm/ttm_agp_backend.c
@@ -57,6 +57,9 @@ int ttm_agp_bind(struct ttm_tt *ttm, struct ttm_resource *bo_mem)
 	int ret, cached = (bo_mem->placement & TTM_PL_FLAG_CACHED);
 	unsigned i;
 
+	if (agp_be->mem)
+		return 0;
+
 	mem = agp_allocate_memory(agp_be->bridge, ttm->num_pages, AGP_USER_MEMORY);
 	if (unlikely(mem == NULL))
 		return -ENOMEM;
@@ -98,6 +101,17 @@ void ttm_agp_unbind(struct ttm_tt *ttm)
 }
 EXPORT_SYMBOL(ttm_agp_unbind);
 
+bool ttm_agp_is_bound(struct ttm_tt *ttm)
+{
+	struct ttm_agp_backend *agp_be = container_of(ttm, struct ttm_agp_backend, ttm);
+
+	if (!ttm)
+		return false;
+
+	return (agp_be->mem != NULL);
+}
+EXPORT_SYMBOL(ttm_agp_is_bound);
+
 void ttm_agp_destroy(struct ttm_tt *ttm)
 {
 	struct ttm_agp_backend *agp_be = container_of(ttm, struct ttm_agp_backend, ttm);
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 17010e7d0ea9..649a7d0a0766 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1627,27 +1627,11 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
 
 int ttm_bo_tt_bind(struct ttm_buffer_object *bo, struct ttm_resource *mem)
 {
-	int ret;
-
-	if (!bo->ttm)
-		return -EINVAL;
-
-	if (ttm_bo_tt_is_bound(bo))
-		return 0;
-
-	ret = bo->bdev->driver->ttm_tt_bind(bo->bdev, bo->ttm, mem);
-	if (unlikely(ret != 0))
-		return ret;
-
-	ttm_bo_tt_set_bound(bo);
-	return 0;
+	return bo->bdev->driver->ttm_tt_bind(bo->bdev, bo->ttm, mem);
 }
 EXPORT_SYMBOL(ttm_bo_tt_bind);
 
 void ttm_bo_tt_unbind(struct ttm_buffer_object *bo)
 {
-	if (ttm_bo_tt_is_bound(bo)) {
-		bo->bdev->driver->ttm_tt_unbind(bo->bdev, bo->ttm);
-		ttm_bo_tt_set_unbound(bo);
-	}
+	bo->bdev->driver->ttm_tt_unbind(bo->bdev, bo->ttm);
 }
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 980368049d68..919489f6a5a3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -574,10 +574,8 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 
 		if (man->use_tt) {
 			ghost_obj->ttm = NULL;
-			ttm_bo_tt_set_unbound(ghost_obj);
 		} else {
 			bo->ttm = NULL;
-			ttm_bo_tt_set_unbound(bo);
 		}
 
 		dma_resv_unlock(&ghost_obj->base._resv);
@@ -633,10 +631,8 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
 
 		if (to->use_tt) {
 			ghost_obj->ttm = NULL;
-			ttm_bo_tt_set_unbound(ghost_obj);
 		} else {
 			bo->ttm = NULL;
-			ttm_bo_tt_set_unbound(bo);
 		}
 
 		dma_resv_unlock(&ghost_obj->base._resv);
@@ -701,7 +697,6 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
 	memset(&bo->mem, 0, sizeof(bo->mem));
 	bo->mem.mem_type = TTM_PL_SYSTEM;
 	bo->ttm = NULL;
-	ttm_bo_tt_set_unbound(bo);
 
 	dma_resv_unlock(&ghost->base._resv);
 	ttm_bo_put(ghost);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 3458c5c3531d..01146b27c9a1 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -267,6 +267,7 @@ struct vmw_ttm_tt {
 	struct vmw_sg_table vsgt;
 	uint64_t sg_alloc_size;
 	bool mapped;
+	bool bound;
 };
 
 const size_t vmw_tt_size = sizeof(struct vmw_ttm_tt);
@@ -565,7 +566,13 @@ static int vmw_ttm_bind(struct ttm_bo_device *bdev,
 {
 	struct vmw_ttm_tt *vmw_be =
 		container_of(ttm, struct vmw_ttm_tt, dma_ttm.ttm);
-	int ret;
+	int ret = 0;
+
+	if (!bo_mem)
+		return -EINVAL;
+
+	if (vmw_be->bound)
+		return 0;
 
 	ret = vmw_ttm_map_dma(vmw_be);
 	if (unlikely(ret != 0))
@@ -576,8 +583,9 @@ static int vmw_ttm_bind(struct ttm_bo_device *bdev,
 
 	switch (bo_mem->mem_type) {
 	case VMW_PL_GMR:
-		return vmw_gmr_bind(vmw_be->dev_priv, &vmw_be->vsgt,
+		ret = vmw_gmr_bind(vmw_be->dev_priv, &vmw_be->vsgt,
 				    ttm->num_pages, vmw_be->gmr_id);
+		break;
 	case VMW_PL_MOB:
 		if (unlikely(vmw_be->mob == NULL)) {
 			vmw_be->mob =
@@ -586,13 +594,15 @@ static int vmw_ttm_bind(struct ttm_bo_device *bdev,
 				return -ENOMEM;
 		}
 
-		return vmw_mob_bind(vmw_be->dev_priv, vmw_be->mob,
+		ret = vmw_mob_bind(vmw_be->dev_priv, vmw_be->mob,
 				    &vmw_be->vsgt, ttm->num_pages,
 				    vmw_be->gmr_id);
+		break;
 	default:
 		BUG();
 	}
-	return 0;
+	vmw_be->bound = true;
+	return ret;
 }
 
 static void vmw_ttm_unbind(struct ttm_bo_device *bdev,
@@ -601,6 +611,9 @@ static void vmw_ttm_unbind(struct ttm_bo_device *bdev,
 	struct vmw_ttm_tt *vmw_be =
 		container_of(ttm, struct vmw_ttm_tt, dma_ttm.ttm);
 
+	if (!vmw_be->bound)
+		return;
+
 	switch (vmw_be->mem_type) {
 	case VMW_PL_GMR:
 		vmw_gmr_unbind(vmw_be->dev_priv, vmw_be->gmr_id);
@@ -614,6 +627,7 @@ static void vmw_ttm_unbind(struct ttm_bo_device *bdev,
 
 	if (vmw_be->dev_priv->map_mode == vmw_dma_map_bind)
 		vmw_ttm_unmap_dma(vmw_be);
+	vmw_be->bound = false;
 }
 
 
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 1d20a7f15a7a..36ff64e2736c 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -141,7 +141,6 @@ struct ttm_buffer_object {
 	struct ttm_resource mem;
 	struct file *persistent_swap_storage;
 	struct ttm_tt *ttm;
-	bool ttm_bound;
 	bool evicted;
 	bool deleted;
 
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index e66672f703a3..7846dfa507f7 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -698,20 +698,6 @@ int ttm_bo_tt_bind(struct ttm_buffer_object *bo, struct ttm_resource *mem);
  */
 void ttm_bo_tt_unbind(struct ttm_buffer_object *bo);
 
-static inline bool ttm_bo_tt_is_bound(struct ttm_buffer_object *bo)
-{
-	return bo->ttm_bound;
-}
-
-static inline void ttm_bo_tt_set_unbound(struct ttm_buffer_object *bo)
-{
-	bo->ttm_bound = false;
-}
-
-static inline void ttm_bo_tt_set_bound(struct ttm_buffer_object *bo)
-{
-	bo->ttm_bound = true;
-}
 /**
  * ttm_bo_tt_destroy.
  */
diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
index c777b72063db..4e906e32d08c 100644
--- a/include/drm/ttm/ttm_tt.h
+++ b/include/drm/ttm/ttm_tt.h
@@ -219,6 +219,7 @@ struct ttm_tt *ttm_agp_tt_create(struct ttm_buffer_object *bo,
 int ttm_agp_bind(struct ttm_tt *ttm, struct ttm_resource *bo_mem);
 void ttm_agp_unbind(struct ttm_tt *ttm);
 void ttm_agp_destroy(struct ttm_tt *ttm);
+bool ttm_agp_is_bound(struct ttm_tt *ttm);
 #endif
 
 #endif
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/7] drm/ttm: protect against reentrant bind in the drivers

[Christian König]

Am 17.09.20 um 06:30 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> This moves the generic tracking into the drivers and protects
> against reentrancy in the drivers. It fixes up radeon and agp
> to be able to query the bound status as that is required.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

If would prefer splitting it up with one patch per driver, but of hand 
that looks like it should work.

Patch is Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 12 +++++++++
>   drivers/gpu/drm/nouveau/nouveau_bo.c       |  5 +++-
>   drivers/gpu/drm/nouveau/nouveau_sgdma.c    |  8 +++++-
>   drivers/gpu/drm/radeon/radeon.h            |  1 +
>   drivers/gpu/drm/radeon/radeon_mn.c         |  2 +-
>   drivers/gpu/drm/radeon/radeon_ttm.c        | 31 ++++++++++++++++++++++
>   drivers/gpu/drm/ttm/ttm_agp_backend.c      | 14 ++++++++++
>   drivers/gpu/drm/ttm/ttm_bo.c               | 20 ++------------
>   drivers/gpu/drm/ttm/ttm_bo_util.c          |  5 ----
>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 22 ++++++++++++---
>   include/drm/ttm/ttm_bo_api.h               |  1 -
>   include/drm/ttm/ttm_bo_driver.h            | 14 ----------
>   include/drm/ttm/ttm_tt.h                   |  1 +
>   13 files changed, 91 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index e86f8f6371c4..2851cf30091a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -813,6 +813,7 @@ struct amdgpu_ttm_tt {
>   	uint64_t		userptr;
>   	struct task_struct	*usertask;
>   	uint32_t		userflags;
> +	bool			bound;
>   #if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
>   	struct hmm_range	*range;
>   #endif
> @@ -1110,6 +1111,12 @@ static int amdgpu_ttm_backend_bind(struct ttm_bo_device *bdev,
>   	uint64_t flags;
>   	int r = 0;
>   
> +	if (!bo_mem)
> +		return -EINVAL;
> +
> +	if (gtt->bound)
> +		return 0;
> +
>   	if (gtt->userptr) {
>   		r = amdgpu_ttm_tt_pin_userptr(bdev, ttm);
>   		if (r) {
> @@ -1143,6 +1150,7 @@ static int amdgpu_ttm_backend_bind(struct ttm_bo_device *bdev,
>   	if (r)
>   		DRM_ERROR("failed to bind %lu pages at 0x%08llX\n",
>   			  ttm->num_pages, gtt->offset);
> +	gtt->bound = true;
>   	return r;
>   }
>   
> @@ -1236,6 +1244,9 @@ static void amdgpu_ttm_backend_unbind(struct ttm_bo_device *bdev,
>   	struct amdgpu_ttm_tt *gtt = (void *)ttm;
>   	int r;
>   
> +	if (!gtt->bound)
> +		return;
> +
>   	/* if the pages have userptr pinning then clear that first */
>   	if (gtt->userptr)
>   		amdgpu_ttm_tt_unpin_userptr(bdev, ttm);
> @@ -1248,6 +1259,7 @@ static void amdgpu_ttm_backend_unbind(struct ttm_bo_device *bdev,
>   	if (r)
>   		DRM_ERROR("failed to unbind %lu pages at 0x%08llX\n",
>   			  gtt->ttm.ttm.num_pages, gtt->offset);
> +	gtt->bound = false;
>   }
>   
>   static void amdgpu_ttm_backend_destroy(struct ttm_bo_device *bdev,
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index aea201d9c513..616d8844ba97 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -718,7 +718,10 @@ nouveau_ttm_tt_bind(struct ttm_bo_device *bdev, struct ttm_tt *ttm,
>   {
>   #if IS_ENABLED(CONFIG_AGP)
>   	struct nouveau_drm *drm = nouveau_bdev(bdev);
> -
> +#endif
> +	if (!reg)
> +		return -EINVAL;
> +#if IS_ENABLED(CONFIG_AGP)
>   	if (drm->agp.bridge)
>   		return ttm_agp_bind(ttm, reg);
>   #endif
> diff --git a/drivers/gpu/drm/nouveau/nouveau_sgdma.c b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
> index 05e542254e1f..21fb92770ea2 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_sgdma.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
> @@ -33,6 +33,9 @@ nouveau_sgdma_bind(struct ttm_bo_device *bdev, struct ttm_tt *ttm, struct ttm_re
>   	struct nouveau_mem *mem = nouveau_mem(reg);
>   	int ret;
>   
> +	if (nvbe->mem)
> +		return 0;
> +
>   	ret = nouveau_mem_host(reg, &nvbe->ttm);
>   	if (ret)
>   		return ret;
> @@ -53,7 +56,10 @@ void
>   nouveau_sgdma_unbind(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
>   {
>   	struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)ttm;
> -	nouveau_mem_fini(nvbe->mem);
> +	if (nvbe->mem) {
> +		nouveau_mem_fini(nvbe->mem);
> +		nvbe->mem = NULL;
> +	}
>   }
>   
>   struct ttm_tt *
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index df6f0b49836b..a6d8de01194a 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2820,6 +2820,7 @@ extern int radeon_ttm_tt_set_userptr(struct radeon_device *rdev,
>   				     uint32_t flags);
>   extern bool radeon_ttm_tt_has_userptr(struct radeon_device *rdev, struct ttm_tt *ttm);
>   extern bool radeon_ttm_tt_is_readonly(struct radeon_device *rdev, struct ttm_tt *ttm);
> +bool radeon_ttm_tt_is_bound(struct ttm_bo_device *bdev, struct ttm_tt *ttm);
>   extern void radeon_vram_location(struct radeon_device *rdev, struct radeon_mc *mc, u64 base);
>   extern void radeon_gtt_location(struct radeon_device *rdev, struct radeon_mc *mc);
>   extern int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon);
> diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/radeon_mn.c
> index eb46d2220236..97b9b6dd6dd3 100644
> --- a/drivers/gpu/drm/radeon/radeon_mn.c
> +++ b/drivers/gpu/drm/radeon/radeon_mn.c
> @@ -53,7 +53,7 @@ static bool radeon_mn_invalidate(struct mmu_interval_notifier *mn,
>   	struct ttm_operation_ctx ctx = { false, false };
>   	long r;
>   
> -	if (!bo->tbo.ttm || !ttm_bo_tt_is_bound(&bo->tbo))
> +	if (!bo->tbo.ttm || !radeon_ttm_tt_is_bound(bo->tbo.bdev, bo->tbo.ttm))
>   		return true;
>   
>   	if (!mmu_notifier_range_blockable(range))
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index c6c1008fefd2..fc8bbca28b34 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -420,6 +420,7 @@ struct radeon_ttm_tt {
>   	uint64_t			userptr;
>   	struct mm_struct		*usermm;
>   	uint32_t			userflags;
> +	bool bound;
>   };
>   
>   /* prepare the sg table with the user pages */
> @@ -513,6 +514,13 @@ static void radeon_ttm_tt_unpin_userptr(struct ttm_bo_device *bdev, struct ttm_t
>   	sg_free_table(ttm->sg);
>   }
>   
> +static bool radeon_ttm_backend_is_bound(struct ttm_tt *ttm)
> +{
> +	struct radeon_ttm_tt *gtt = (void*)ttm;
> +
> +	return (gtt->bound);
> +}
> +
>   static int radeon_ttm_backend_bind(struct ttm_bo_device *bdev,
>   				   struct ttm_tt *ttm,
>   				   struct ttm_resource *bo_mem)
> @@ -523,6 +531,9 @@ static int radeon_ttm_backend_bind(struct ttm_bo_device *bdev,
>   		RADEON_GART_PAGE_WRITE;
>   	int r;
>   
> +	if (gtt->bound)
> +		return 0;
> +
>   	if (gtt->userptr) {
>   		radeon_ttm_tt_pin_userptr(bdev, ttm);
>   		flags &= ~RADEON_GART_PAGE_WRITE;
> @@ -542,6 +553,7 @@ static int radeon_ttm_backend_bind(struct ttm_bo_device *bdev,
>   			  ttm->num_pages, (unsigned)gtt->offset);
>   		return r;
>   	}
> +	gtt->bound = true;
>   	return 0;
>   }
>   
> @@ -550,10 +562,14 @@ static void radeon_ttm_backend_unbind(struct ttm_bo_device *bdev, struct ttm_tt
>   	struct radeon_ttm_tt *gtt = (void *)ttm;
>   	struct radeon_device *rdev = radeon_get_rdev(bdev);
>   
> +	if (!gtt->bound)
> +		return;
> +
>   	radeon_gart_unbind(rdev, gtt->offset, ttm->num_pages);
>   
>   	if (gtt->userptr)
>   		radeon_ttm_tt_unpin_userptr(bdev, ttm);
> +	gtt->bound = false;
>   }
>   
>   static void radeon_ttm_backend_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
> @@ -689,12 +705,27 @@ int radeon_ttm_tt_set_userptr(struct radeon_device *rdev,
>   	return 0;
>   }
>   
> +bool radeon_ttm_tt_is_bound(struct ttm_bo_device *bdev,
> +			    struct ttm_tt *ttm)
> +{
> +#if IS_ENABLED(CONFIG_AGP)
> +	struct radeon_device *rdev = radeon_get_rdev(bdev);
> +	if (rdev->flags & RADEON_IS_AGP)
> +		return ttm_agp_is_bound(ttm);
> +#endif
> +	return radeon_ttm_backend_is_bound(ttm);
> +}
> +
>   static int radeon_ttm_tt_bind(struct ttm_bo_device *bdev,
>   			      struct ttm_tt *ttm,
>   			      struct ttm_resource *bo_mem)
>   {
> +#if IS_ENABLED(CONFIG_AGP)
>   	struct radeon_device *rdev = radeon_get_rdev(bdev);
> +#endif
>   
> +	if (!bo_mem)
> +		return -EINVAL;
>   #if IS_ENABLED(CONFIG_AGP)
>   	if (rdev->flags & RADEON_IS_AGP)
>   		return ttm_agp_bind(ttm, bo_mem);
> diff --git a/drivers/gpu/drm/ttm/ttm_agp_backend.c b/drivers/gpu/drm/ttm/ttm_agp_backend.c
> index 7b36fdaab766..a98fd795b752 100644
> --- a/drivers/gpu/drm/ttm/ttm_agp_backend.c
> +++ b/drivers/gpu/drm/ttm/ttm_agp_backend.c
> @@ -57,6 +57,9 @@ int ttm_agp_bind(struct ttm_tt *ttm, struct ttm_resource *bo_mem)
>   	int ret, cached = (bo_mem->placement & TTM_PL_FLAG_CACHED);
>   	unsigned i;
>   
> +	if (agp_be->mem)
> +		return 0;
> +
>   	mem = agp_allocate_memory(agp_be->bridge, ttm->num_pages, AGP_USER_MEMORY);
>   	if (unlikely(mem == NULL))
>   		return -ENOMEM;
> @@ -98,6 +101,17 @@ void ttm_agp_unbind(struct ttm_tt *ttm)
>   }
>   EXPORT_SYMBOL(ttm_agp_unbind);
>   
> +bool ttm_agp_is_bound(struct ttm_tt *ttm)
> +{
> +	struct ttm_agp_backend *agp_be = container_of(ttm, struct ttm_agp_backend, ttm);
> +
> +	if (!ttm)
> +		return false;
> +
> +	return (agp_be->mem != NULL);
> +}
> +EXPORT_SYMBOL(ttm_agp_is_bound);
> +
>   void ttm_agp_destroy(struct ttm_tt *ttm)
>   {
>   	struct ttm_agp_backend *agp_be = container_of(ttm, struct ttm_agp_backend, ttm);
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 17010e7d0ea9..649a7d0a0766 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1627,27 +1627,11 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
>   
>   int ttm_bo_tt_bind(struct ttm_buffer_object *bo, struct ttm_resource *mem)
>   {
> -	int ret;
> -
> -	if (!bo->ttm)
> -		return -EINVAL;
> -
> -	if (ttm_bo_tt_is_bound(bo))
> -		return 0;
> -
> -	ret = bo->bdev->driver->ttm_tt_bind(bo->bdev, bo->ttm, mem);
> -	if (unlikely(ret != 0))
> -		return ret;
> -
> -	ttm_bo_tt_set_bound(bo);
> -	return 0;
> +	return bo->bdev->driver->ttm_tt_bind(bo->bdev, bo->ttm, mem);
>   }
>   EXPORT_SYMBOL(ttm_bo_tt_bind);
>   
>   void ttm_bo_tt_unbind(struct ttm_buffer_object *bo)
>   {
> -	if (ttm_bo_tt_is_bound(bo)) {
> -		bo->bdev->driver->ttm_tt_unbind(bo->bdev, bo->ttm);
> -		ttm_bo_tt_set_unbound(bo);
> -	}
> +	bo->bdev->driver->ttm_tt_unbind(bo->bdev, bo->ttm);
>   }
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 980368049d68..919489f6a5a3 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -574,10 +574,8 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>   
>   		if (man->use_tt) {
>   			ghost_obj->ttm = NULL;
> -			ttm_bo_tt_set_unbound(ghost_obj);
>   		} else {
>   			bo->ttm = NULL;
> -			ttm_bo_tt_set_unbound(bo);
>   		}
>   
>   		dma_resv_unlock(&ghost_obj->base._resv);
> @@ -633,10 +631,8 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>   
>   		if (to->use_tt) {
>   			ghost_obj->ttm = NULL;
> -			ttm_bo_tt_set_unbound(ghost_obj);
>   		} else {
>   			bo->ttm = NULL;
> -			ttm_bo_tt_set_unbound(bo);
>   		}
>   
>   		dma_resv_unlock(&ghost_obj->base._resv);
> @@ -701,7 +697,6 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
>   	memset(&bo->mem, 0, sizeof(bo->mem));
>   	bo->mem.mem_type = TTM_PL_SYSTEM;
>   	bo->ttm = NULL;
> -	ttm_bo_tt_set_unbound(bo);
>   
>   	dma_resv_unlock(&ghost->base._resv);
>   	ttm_bo_put(ghost);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index 3458c5c3531d..01146b27c9a1 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -267,6 +267,7 @@ struct vmw_ttm_tt {
>   	struct vmw_sg_table vsgt;
>   	uint64_t sg_alloc_size;
>   	bool mapped;
> +	bool bound;
>   };
>   
>   const size_t vmw_tt_size = sizeof(struct vmw_ttm_tt);
> @@ -565,7 +566,13 @@ static int vmw_ttm_bind(struct ttm_bo_device *bdev,
>   {
>   	struct vmw_ttm_tt *vmw_be =
>   		container_of(ttm, struct vmw_ttm_tt, dma_ttm.ttm);
> -	int ret;
> +	int ret = 0;
> +
> +	if (!bo_mem)
> +		return -EINVAL;
> +
> +	if (vmw_be->bound)
> +		return 0;
>   
>   	ret = vmw_ttm_map_dma(vmw_be);
>   	if (unlikely(ret != 0))
> @@ -576,8 +583,9 @@ static int vmw_ttm_bind(struct ttm_bo_device *bdev,
>   
>   	switch (bo_mem->mem_type) {
>   	case VMW_PL_GMR:
> -		return vmw_gmr_bind(vmw_be->dev_priv, &vmw_be->vsgt,
> +		ret = vmw_gmr_bind(vmw_be->dev_priv, &vmw_be->vsgt,
>   				    ttm->num_pages, vmw_be->gmr_id);
> +		break;
>   	case VMW_PL_MOB:
>   		if (unlikely(vmw_be->mob == NULL)) {
>   			vmw_be->mob =
> @@ -586,13 +594,15 @@ static int vmw_ttm_bind(struct ttm_bo_device *bdev,
>   				return -ENOMEM;
>   		}
>   
> -		return vmw_mob_bind(vmw_be->dev_priv, vmw_be->mob,
> +		ret = vmw_mob_bind(vmw_be->dev_priv, vmw_be->mob,
>   				    &vmw_be->vsgt, ttm->num_pages,
>   				    vmw_be->gmr_id);
> +		break;
>   	default:
>   		BUG();
>   	}
> -	return 0;
> +	vmw_be->bound = true;
> +	return ret;
>   }
>   
>   static void vmw_ttm_unbind(struct ttm_bo_device *bdev,
> @@ -601,6 +611,9 @@ static void vmw_ttm_unbind(struct ttm_bo_device *bdev,
>   	struct vmw_ttm_tt *vmw_be =
>   		container_of(ttm, struct vmw_ttm_tt, dma_ttm.ttm);
>   
> +	if (!vmw_be->bound)
> +		return;
> +
>   	switch (vmw_be->mem_type) {
>   	case VMW_PL_GMR:
>   		vmw_gmr_unbind(vmw_be->dev_priv, vmw_be->gmr_id);
> @@ -614,6 +627,7 @@ static void vmw_ttm_unbind(struct ttm_bo_device *bdev,
>   
>   	if (vmw_be->dev_priv->map_mode == vmw_dma_map_bind)
>   		vmw_ttm_unmap_dma(vmw_be);
> +	vmw_be->bound = false;
>   }
>   
>   
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 1d20a7f15a7a..36ff64e2736c 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -141,7 +141,6 @@ struct ttm_buffer_object {
>   	struct ttm_resource mem;
>   	struct file *persistent_swap_storage;
>   	struct ttm_tt *ttm;
> -	bool ttm_bound;
>   	bool evicted;
>   	bool deleted;
>   
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index e66672f703a3..7846dfa507f7 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -698,20 +698,6 @@ int ttm_bo_tt_bind(struct ttm_buffer_object *bo, struct ttm_resource *mem);
>    */
>   void ttm_bo_tt_unbind(struct ttm_buffer_object *bo);
>   
> -static inline bool ttm_bo_tt_is_bound(struct ttm_buffer_object *bo)
> -{
> -	return bo->ttm_bound;
> -}
> -
> -static inline void ttm_bo_tt_set_unbound(struct ttm_buffer_object *bo)
> -{
> -	bo->ttm_bound = false;
> -}
> -
> -static inline void ttm_bo_tt_set_bound(struct ttm_buffer_object *bo)
> -{
> -	bo->ttm_bound = true;
> -}
>   /**
>    * ttm_bo_tt_destroy.
>    */
> diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
> index c777b72063db..4e906e32d08c 100644
> --- a/include/drm/ttm/ttm_tt.h
> +++ b/include/drm/ttm/ttm_tt.h
> @@ -219,6 +219,7 @@ struct ttm_tt *ttm_agp_tt_create(struct ttm_buffer_object *bo,
>   int ttm_agp_bind(struct ttm_tt *ttm, struct ttm_resource *bo_mem);
>   void ttm_agp_unbind(struct ttm_tt *ttm);
>   void ttm_agp_destroy(struct ttm_tt *ttm);
> +bool ttm_agp_is_bound(struct ttm_tt *ttm);
>   #endif
>   
>   #endif

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/7] drm/ttm: flip tt destroy ordering.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Call the driver first and have it call the common code cleanup.

This is useful later to fix unbind.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 1 +
 drivers/gpu/drm/drm_gem_vram_helper.c      | 1 +
 drivers/gpu/drm/nouveau/nouveau_bo.c       | 1 +
 drivers/gpu/drm/nouveau/nouveau_sgdma.c    | 1 +
 drivers/gpu/drm/qxl/qxl_ttm.c              | 1 +
 drivers/gpu/drm/radeon/radeon_ttm.c        | 3 +++
 drivers/gpu/drm/ttm/ttm_tt.c               | 7 ++++++-
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 1 +
 include/drm/ttm/ttm_tt.h                   | 7 +++++++
 9 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 2851cf30091a..dbe3f90a0cf6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1267,6 +1267,7 @@ static void amdgpu_ttm_backend_destroy(struct ttm_bo_device *bdev,
 {
 	struct amdgpu_ttm_tt *gtt = (void *)ttm;
 
+	ttm_tt_destroy_common(bdev, ttm);
 	if (gtt->usertask)
 		put_task_struct(gtt->usertask);
 
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index f96321509d7e..50cad0e4a92e 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -922,6 +922,7 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs = {
 
 static void bo_driver_ttm_tt_destroy(struct ttm_bo_device *bdev, struct ttm_tt *tt)
 {
+	ttm_tt_destroy_common(bdev, tt);
 	ttm_tt_fini(tt);
 	kfree(tt);
 }
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 616d8844ba97..b239381498c3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1372,6 +1372,7 @@ nouveau_ttm_tt_destroy(struct ttm_bo_device *bdev,
 #if IS_ENABLED(CONFIG_AGP)
 	struct nouveau_drm *drm = nouveau_bdev(bdev);
 	if (drm->agp.bridge) {
+		ttm_tt_destroy_common(bdev, ttm);
 		ttm_agp_destroy(ttm);
 		return;
 	}
diff --git a/drivers/gpu/drm/nouveau/nouveau_sgdma.c b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
index 21fb92770ea2..0dfaa6fb536e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sgdma.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
@@ -20,6 +20,7 @@ nouveau_sgdma_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
 	struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)ttm;
 
 	if (ttm) {
+		ttm_tt_destroy_common(bdev, ttm);
 		ttm_dma_tt_fini(&nvbe->ttm);
 		kfree(nvbe);
 	}
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index bf3b091d0e2a..fd691fff8394 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -130,6 +130,7 @@ static void qxl_ttm_backend_destroy(struct ttm_bo_device *bdev,
 {
 	struct qxl_ttm_tt *gtt = (void *)ttm;
 
+	ttm_tt_destroy_common(bdev, ttm);
 	ttm_tt_fini(&gtt->ttm);
 	kfree(gtt);
 }
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index fc8bbca28b34..76e24c6058ff 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -576,6 +576,8 @@ static void radeon_ttm_backend_destroy(struct ttm_bo_device *bdev, struct ttm_tt
 {
 	struct radeon_ttm_tt *gtt = (void *)ttm;
 
+	ttm_tt_destroy_common(bdev, ttm);
+
 	ttm_dma_tt_fini(&gtt->ttm);
 	kfree(gtt);
 }
@@ -755,6 +757,7 @@ static void radeon_ttm_tt_destroy(struct ttm_bo_device *bdev,
 	struct radeon_device *rdev = radeon_get_rdev(bdev);
 
 	if (rdev->flags & RADEON_IS_AGP) {
+		ttm_tt_destroy_common(bdev, ttm);
 		ttm_agp_destroy(ttm);
 		return;
 	}
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index a4f0296effac..f43fa69a1e65 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -207,7 +207,7 @@ int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement)
 }
 EXPORT_SYMBOL(ttm_tt_set_placement_caching);
 
-void ttm_tt_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
+void ttm_tt_destroy_common(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
 {
 	ttm_tt_unpopulate(bdev, ttm);
 
@@ -216,6 +216,11 @@ void ttm_tt_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
 		fput(ttm->swap_storage);
 
 	ttm->swap_storage = NULL;
+}
+EXPORT_SYMBOL(ttm_tt_destroy_common);
+
+void ttm_tt_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
+{
 	bdev->driver->ttm_tt_destroy(bdev, ttm);
 }
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 01146b27c9a1..d46426164577 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -636,6 +636,7 @@ static void vmw_ttm_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
 	struct vmw_ttm_tt *vmw_be =
 		container_of(ttm, struct vmw_ttm_tt, dma_ttm.ttm);
 
+	ttm_tt_destroy_common(bdev, ttm);
 	vmw_ttm_unmap_dma(vmw_be);
 	if (vmw_be->dev_priv->map_mode == vmw_dma_alloc_coherent)
 		ttm_dma_tt_fini(&vmw_be->dma_ttm);
diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
index 4e906e32d08c..75208c0a0cac 100644
--- a/include/drm/ttm/ttm_tt.h
+++ b/include/drm/ttm/ttm_tt.h
@@ -155,6 +155,13 @@ void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma);
  */
 void ttm_tt_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm);
 
+/**
+ * ttm_tt_destroy_common:
+ *
+ * Called from driver to destroy common path.
+ */
+void ttm_tt_destroy_common(struct ttm_bo_device *bdev, struct ttm_tt *ttm);
+
 /**
  * ttm_tt_swapin:
  *
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/7] drm/ttm: flip tt destroy ordering.

[Christian König]

Am 17.09.20 um 06:30 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> Call the driver first and have it call the common code cleanup.
>
> This is useful later to fix unbind.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 1 +
>   drivers/gpu/drm/drm_gem_vram_helper.c      | 1 +
>   drivers/gpu/drm/nouveau/nouveau_bo.c       | 1 +
>   drivers/gpu/drm/nouveau/nouveau_sgdma.c    | 1 +
>   drivers/gpu/drm/qxl/qxl_ttm.c              | 1 +
>   drivers/gpu/drm/radeon/radeon_ttm.c        | 3 +++
>   drivers/gpu/drm/ttm/ttm_tt.c               | 7 ++++++-
>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 1 +
>   include/drm/ttm/ttm_tt.h                   | 7 +++++++
>   9 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 2851cf30091a..dbe3f90a0cf6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1267,6 +1267,7 @@ static void amdgpu_ttm_backend_destroy(struct ttm_bo_device *bdev,
>   {
>   	struct amdgpu_ttm_tt *gtt = (void *)ttm;
>   
> +	ttm_tt_destroy_common(bdev, ttm);
>   	if (gtt->usertask)
>   		put_task_struct(gtt->usertask);
>   
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index f96321509d7e..50cad0e4a92e 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -922,6 +922,7 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs = {
>   
>   static void bo_driver_ttm_tt_destroy(struct ttm_bo_device *bdev, struct ttm_tt *tt)
>   {
> +	ttm_tt_destroy_common(bdev, tt);
>   	ttm_tt_fini(tt);
>   	kfree(tt);
>   }
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 616d8844ba97..b239381498c3 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -1372,6 +1372,7 @@ nouveau_ttm_tt_destroy(struct ttm_bo_device *bdev,
>   #if IS_ENABLED(CONFIG_AGP)
>   	struct nouveau_drm *drm = nouveau_bdev(bdev);
>   	if (drm->agp.bridge) {
> +		ttm_tt_destroy_common(bdev, ttm);
>   		ttm_agp_destroy(ttm);
>   		return;
>   	}
> diff --git a/drivers/gpu/drm/nouveau/nouveau_sgdma.c b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
> index 21fb92770ea2..0dfaa6fb536e 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_sgdma.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
> @@ -20,6 +20,7 @@ nouveau_sgdma_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
>   	struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)ttm;
>   
>   	if (ttm) {
> +		ttm_tt_destroy_common(bdev, ttm);
>   		ttm_dma_tt_fini(&nvbe->ttm);
>   		kfree(nvbe);
>   	}
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index bf3b091d0e2a..fd691fff8394 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -130,6 +130,7 @@ static void qxl_ttm_backend_destroy(struct ttm_bo_device *bdev,
>   {
>   	struct qxl_ttm_tt *gtt = (void *)ttm;
>   
> +	ttm_tt_destroy_common(bdev, ttm);
>   	ttm_tt_fini(&gtt->ttm);
>   	kfree(gtt);
>   }
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index fc8bbca28b34..76e24c6058ff 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -576,6 +576,8 @@ static void radeon_ttm_backend_destroy(struct ttm_bo_device *bdev, struct ttm_tt
>   {
>   	struct radeon_ttm_tt *gtt = (void *)ttm;
>   
> +	ttm_tt_destroy_common(bdev, ttm);
> +
>   	ttm_dma_tt_fini(&gtt->ttm);
>   	kfree(gtt);
>   }
> @@ -755,6 +757,7 @@ static void radeon_ttm_tt_destroy(struct ttm_bo_device *bdev,
>   	struct radeon_device *rdev = radeon_get_rdev(bdev);
>   
>   	if (rdev->flags & RADEON_IS_AGP) {
> +		ttm_tt_destroy_common(bdev, ttm);
>   		ttm_agp_destroy(ttm);
>   		return;
>   	}
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index a4f0296effac..f43fa69a1e65 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -207,7 +207,7 @@ int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement)
>   }
>   EXPORT_SYMBOL(ttm_tt_set_placement_caching);
>   
> -void ttm_tt_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
> +void ttm_tt_destroy_common(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
>   {
>   	ttm_tt_unpopulate(bdev, ttm);
>   
> @@ -216,6 +216,11 @@ void ttm_tt_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
>   		fput(ttm->swap_storage);
>   
>   	ttm->swap_storage = NULL;
> +}
> +EXPORT_SYMBOL(ttm_tt_destroy_common);
> +
> +void ttm_tt_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
> +{
>   	bdev->driver->ttm_tt_destroy(bdev, ttm);
>   }
>   
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index 01146b27c9a1..d46426164577 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -636,6 +636,7 @@ static void vmw_ttm_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
>   	struct vmw_ttm_tt *vmw_be =
>   		container_of(ttm, struct vmw_ttm_tt, dma_ttm.ttm);
>   
> +	ttm_tt_destroy_common(bdev, ttm);
>   	vmw_ttm_unmap_dma(vmw_be);
>   	if (vmw_be->dev_priv->map_mode == vmw_dma_alloc_coherent)
>   		ttm_dma_tt_fini(&vmw_be->dma_ttm);
> diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
> index 4e906e32d08c..75208c0a0cac 100644
> --- a/include/drm/ttm/ttm_tt.h
> +++ b/include/drm/ttm/ttm_tt.h
> @@ -155,6 +155,13 @@ void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma);
>    */
>   void ttm_tt_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm);
>   
> +/**
> + * ttm_tt_destroy_common:
> + *
> + * Called from driver to destroy common path.
> + */
> +void ttm_tt_destroy_common(struct ttm_bo_device *bdev, struct ttm_tt *ttm);
> +
>   /**
>    * ttm_tt_swapin:
>    *

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/7] drm/ttm: move unbind into the tt destroy.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This moves unbind into the driver side on destroy paths.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 1 +
 drivers/gpu/drm/nouveau/nouveau_bo.c       | 1 +
 drivers/gpu/drm/nouveau/nouveau_sgdma.c    | 1 +
 drivers/gpu/drm/radeon/radeon_ttm.c        | 2 ++
 drivers/gpu/drm/ttm/ttm_bo.c               | 1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 1 +
 6 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index dbe3f90a0cf6..2028b9e4c25c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1267,6 +1267,7 @@ static void amdgpu_ttm_backend_destroy(struct ttm_bo_device *bdev,
 {
 	struct amdgpu_ttm_tt *gtt = (void *)ttm;
 
+	amdgpu_ttm_backend_unbind(bdev, ttm);
 	ttm_tt_destroy_common(bdev, ttm);
 	if (gtt->usertask)
 		put_task_struct(gtt->usertask);
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index b239381498c3..4a020a3da3dd 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1372,6 +1372,7 @@ nouveau_ttm_tt_destroy(struct ttm_bo_device *bdev,
 #if IS_ENABLED(CONFIG_AGP)
 	struct nouveau_drm *drm = nouveau_bdev(bdev);
 	if (drm->agp.bridge) {
+		ttm_agp_unbind(ttm);
 		ttm_tt_destroy_common(bdev, ttm);
 		ttm_agp_destroy(ttm);
 		return;
diff --git a/drivers/gpu/drm/nouveau/nouveau_sgdma.c b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
index 0dfaa6fb536e..806d9ec310f5 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sgdma.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
@@ -20,6 +20,7 @@ nouveau_sgdma_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
 	struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)ttm;
 
 	if (ttm) {
+		nouveau_sgdma_unbind(bdev, ttm);
 		ttm_tt_destroy_common(bdev, ttm);
 		ttm_dma_tt_fini(&nvbe->ttm);
 		kfree(nvbe);
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 76e24c6058ff..03e5ae4e3bf6 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -576,6 +576,7 @@ static void radeon_ttm_backend_destroy(struct ttm_bo_device *bdev, struct ttm_tt
 {
 	struct radeon_ttm_tt *gtt = (void *)ttm;
 
+	radeon_ttm_backend_unbind(bdev, ttm);
 	ttm_tt_destroy_common(bdev, ttm);
 
 	ttm_dma_tt_fini(&gtt->ttm);
@@ -757,6 +758,7 @@ static void radeon_ttm_tt_destroy(struct ttm_bo_device *bdev,
 	struct radeon_device *rdev = radeon_get_rdev(bdev);
 
 	if (rdev->flags & RADEON_IS_AGP) {
+		ttm_agp_unbind(ttm);
 		ttm_tt_destroy_common(bdev, ttm);
 		ttm_agp_destroy(ttm);
 		return;
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 649a7d0a0766..2c527cd20dfa 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1620,7 +1620,6 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
 	if (bo->ttm == NULL)
 		return;
 
-	ttm_bo_tt_unbind(bo);
 	ttm_tt_destroy(bo->bdev, bo->ttm);
 	bo->ttm = NULL;
 }
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index d46426164577..7454f797d37b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -636,6 +636,7 @@ static void vmw_ttm_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
 	struct vmw_ttm_tt *vmw_be =
 		container_of(ttm, struct vmw_ttm_tt, dma_ttm.ttm);
 
+	vmw_ttm_unbind(bdev, ttm);
 	ttm_tt_destroy_common(bdev, ttm);
 	vmw_ttm_unmap_dma(vmw_be);
 	if (vmw_be->dev_priv->map_mode == vmw_dma_alloc_coherent)
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/7] drm/ttm: move unbind into the tt destroy.

[Christian König]

Am 17.09.20 um 06:30 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> This moves unbind into the driver side on destroy paths.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 1 +
>   drivers/gpu/drm/nouveau/nouveau_bo.c       | 1 +
>   drivers/gpu/drm/nouveau/nouveau_sgdma.c    | 1 +
>   drivers/gpu/drm/radeon/radeon_ttm.c        | 2 ++
>   drivers/gpu/drm/ttm/ttm_bo.c               | 1 -
>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 1 +
>   6 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index dbe3f90a0cf6..2028b9e4c25c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1267,6 +1267,7 @@ static void amdgpu_ttm_backend_destroy(struct ttm_bo_device *bdev,
>   {
>   	struct amdgpu_ttm_tt *gtt = (void *)ttm;
>   
> +	amdgpu_ttm_backend_unbind(bdev, ttm);
>   	ttm_tt_destroy_common(bdev, ttm);
>   	if (gtt->usertask)
>   		put_task_struct(gtt->usertask);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index b239381498c3..4a020a3da3dd 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -1372,6 +1372,7 @@ nouveau_ttm_tt_destroy(struct ttm_bo_device *bdev,
>   #if IS_ENABLED(CONFIG_AGP)
>   	struct nouveau_drm *drm = nouveau_bdev(bdev);
>   	if (drm->agp.bridge) {
> +		ttm_agp_unbind(ttm);
>   		ttm_tt_destroy_common(bdev, ttm);
>   		ttm_agp_destroy(ttm);
>   		return;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_sgdma.c b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
> index 0dfaa6fb536e..806d9ec310f5 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_sgdma.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
> @@ -20,6 +20,7 @@ nouveau_sgdma_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
>   	struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)ttm;
>   
>   	if (ttm) {
> +		nouveau_sgdma_unbind(bdev, ttm);
>   		ttm_tt_destroy_common(bdev, ttm);
>   		ttm_dma_tt_fini(&nvbe->ttm);
>   		kfree(nvbe);
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 76e24c6058ff..03e5ae4e3bf6 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -576,6 +576,7 @@ static void radeon_ttm_backend_destroy(struct ttm_bo_device *bdev, struct ttm_tt
>   {
>   	struct radeon_ttm_tt *gtt = (void *)ttm;
>   
> +	radeon_ttm_backend_unbind(bdev, ttm);
>   	ttm_tt_destroy_common(bdev, ttm);
>   
>   	ttm_dma_tt_fini(&gtt->ttm);
> @@ -757,6 +758,7 @@ static void radeon_ttm_tt_destroy(struct ttm_bo_device *bdev,
>   	struct radeon_device *rdev = radeon_get_rdev(bdev);
>   
>   	if (rdev->flags & RADEON_IS_AGP) {
> +		ttm_agp_unbind(ttm);
>   		ttm_tt_destroy_common(bdev, ttm);
>   		ttm_agp_destroy(ttm);
>   		return;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 649a7d0a0766..2c527cd20dfa 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1620,7 +1620,6 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
>   	if (bo->ttm == NULL)
>   		return;
>   
> -	ttm_bo_tt_unbind(bo);
>   	ttm_tt_destroy(bo->bdev, bo->ttm);
>   	bo->ttm = NULL;
>   }
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index d46426164577..7454f797d37b 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -636,6 +636,7 @@ static void vmw_ttm_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
>   	struct vmw_ttm_tt *vmw_be =
>   		container_of(ttm, struct vmw_ttm_tt, dma_ttm.ttm);
>   
> +	vmw_ttm_unbind(bdev, ttm);
>   	ttm_tt_destroy_common(bdev, ttm);
>   	vmw_ttm_unmap_dma(vmw_be);
>   	if (vmw_be->dev_priv->map_mode == vmw_dma_alloc_coherent)

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/7] drm/ttm/drivers: call the bind function directly.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Now the bind functions have all the protection explicitly the
drivers can just call them directly, and the api can be unexported

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 +++++-
 drivers/gpu/drm/nouveau/nouveau_bo.c    | 5 ++++-
 drivers/gpu/drm/radeon/radeon_ttm.c     | 6 +++++-
 drivers/gpu/drm/ttm/ttm_bo.c            | 1 -
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 2028b9e4c25c..a57aaf666340 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -63,6 +63,10 @@
 
 #define AMDGPU_TTM_VRAM_MAX_DW_READ	(size_t)128
 
+static int amdgpu_ttm_backend_bind(struct ttm_bo_device *bdev,
+				   struct ttm_tt *ttm,
+				   struct ttm_resource *bo_mem);
+
 static int amdgpu_ttm_init_on_chip(struct amdgpu_device *adev,
 				    unsigned int type,
 				    uint64_t size)
@@ -552,7 +556,7 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict,
 		goto out_cleanup;
 
 	/* Bind the memory to the GTT space */
-	r = ttm_bo_tt_bind(bo, &tmp_mem);
+	r = amdgpu_ttm_backend_bind(bo->bdev, bo->ttm, &tmp_mem);
 	if (unlikely(r)) {
 		goto out_cleanup;
 	}
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 4a020a3da3dd..56f974c28eb5 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -44,6 +44,9 @@
 #include <nvif/if500b.h>
 #include <nvif/if900b.h>
 
+static int nouveau_ttm_tt_bind(struct ttm_bo_device *bdev, struct ttm_tt *ttm,
+			       struct ttm_resource *reg);
+
 /*
  * NV10-NV40 tiling helpers
  */
@@ -927,7 +930,7 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict, bool intr,
 	if (ret)
 		goto out;
 
-	ret = ttm_bo_tt_bind(bo, &tmp_reg);
+	ret = nouveau_ttm_tt_bind(bo->bdev, bo->ttm, &tmp_reg);
 	if (ret)
 		goto out;
 
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 03e5ae4e3bf6..99d9ca1087b7 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -56,6 +56,10 @@
 static int radeon_ttm_debugfs_init(struct radeon_device *rdev);
 static void radeon_ttm_debugfs_fini(struct radeon_device *rdev);
 
+static int radeon_ttm_tt_bind(struct ttm_bo_device *bdev,
+			      struct ttm_tt *ttm,
+			      struct ttm_resource *bo_mem);
+
 struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
 {
 	struct radeon_mman *mman;
@@ -238,7 +242,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
 		goto out_cleanup;
 	}
 
-	r = ttm_bo_tt_bind(bo, &tmp_mem);
+	r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, &tmp_mem);
 	if (unlikely(r)) {
 		goto out_cleanup;
 	}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 2c527cd20dfa..279a0e44a5ed 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1628,7 +1628,6 @@ int ttm_bo_tt_bind(struct ttm_buffer_object *bo, struct ttm_resource *mem)
 {
 	return bo->bdev->driver->ttm_tt_bind(bo->bdev, bo->ttm, mem);
 }
-EXPORT_SYMBOL(ttm_bo_tt_bind);
 
 void ttm_bo_tt_unbind(struct ttm_buffer_object *bo)
 {
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/7] drm/ttm/drivers: call the bind function directly.

[Christian König]

Am 17.09.20 um 06:30 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> Now the bind functions have all the protection explicitly the
> drivers can just call them directly, and the api can be unexported
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 +++++-
>   drivers/gpu/drm/nouveau/nouveau_bo.c    | 5 ++++-
>   drivers/gpu/drm/radeon/radeon_ttm.c     | 6 +++++-
>   drivers/gpu/drm/ttm/ttm_bo.c            | 1 -
>   4 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 2028b9e4c25c..a57aaf666340 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -63,6 +63,10 @@
>   
>   #define AMDGPU_TTM_VRAM_MAX_DW_READ	(size_t)128
>   
> +static int amdgpu_ttm_backend_bind(struct ttm_bo_device *bdev,
> +				   struct ttm_tt *ttm,
> +				   struct ttm_resource *bo_mem);
> +
>   static int amdgpu_ttm_init_on_chip(struct amdgpu_device *adev,
>   				    unsigned int type,
>   				    uint64_t size)
> @@ -552,7 +556,7 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict,
>   		goto out_cleanup;
>   
>   	/* Bind the memory to the GTT space */
> -	r = ttm_bo_tt_bind(bo, &tmp_mem);
> +	r = amdgpu_ttm_backend_bind(bo->bdev, bo->ttm, &tmp_mem);
>   	if (unlikely(r)) {
>   		goto out_cleanup;
>   	}
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 4a020a3da3dd..56f974c28eb5 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -44,6 +44,9 @@
>   #include <nvif/if500b.h>
>   #include <nvif/if900b.h>
>   
> +static int nouveau_ttm_tt_bind(struct ttm_bo_device *bdev, struct ttm_tt *ttm,
> +			       struct ttm_resource *reg);
> +
>   /*
>    * NV10-NV40 tiling helpers
>    */
> @@ -927,7 +930,7 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict, bool intr,
>   	if (ret)
>   		goto out;
>   
> -	ret = ttm_bo_tt_bind(bo, &tmp_reg);
> +	ret = nouveau_ttm_tt_bind(bo->bdev, bo->ttm, &tmp_reg);
>   	if (ret)
>   		goto out;
>   
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 03e5ae4e3bf6..99d9ca1087b7 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -56,6 +56,10 @@
>   static int radeon_ttm_debugfs_init(struct radeon_device *rdev);
>   static void radeon_ttm_debugfs_fini(struct radeon_device *rdev);
>   
> +static int radeon_ttm_tt_bind(struct ttm_bo_device *bdev,
> +			      struct ttm_tt *ttm,
> +			      struct ttm_resource *bo_mem);
> +
>   struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
>   {
>   	struct radeon_mman *mman;
> @@ -238,7 +242,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
>   		goto out_cleanup;
>   	}
>   
> -	r = ttm_bo_tt_bind(bo, &tmp_mem);
> +	r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, &tmp_mem);
>   	if (unlikely(r)) {
>   		goto out_cleanup;
>   	}
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 2c527cd20dfa..279a0e44a5ed 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1628,7 +1628,6 @@ int ttm_bo_tt_bind(struct ttm_buffer_object *bo, struct ttm_resource *mem)
>   {
>   	return bo->bdev->driver->ttm_tt_bind(bo->bdev, bo->ttm, mem);
>   }
> -EXPORT_SYMBOL(ttm_bo_tt_bind);
>   
>   void ttm_bo_tt_unbind(struct ttm_buffer_object *bo)
>   {

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/7] drm/ttm: add a simple assign mem to bo wrapper

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This pattern is called in a few places, just clean it up.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 18 ++++++------------
 include/drm/ttm/ttm_bo_driver.h   | 10 ++++++++--
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 919489f6a5a3..5a8d77ef504f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -87,9 +87,7 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
 			return ret;
 	}
 
-	*old_mem = *new_mem;
-	new_mem->mm_node = NULL;
-
+	ttm_bo_assign_mem(bo, new_mem);
 	return 0;
 }
 EXPORT_SYMBOL(ttm_bo_move_ttm);
@@ -299,8 +297,8 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
 	mb();
 out2:
 	old_copy = *old_mem;
-	*old_mem = *new_mem;
-	new_mem->mm_node = NULL;
+
+	ttm_bo_assign_mem(bo, new_mem);
 
 	if (!man->use_tt)
 		ttm_bo_tt_destroy(bo);
@@ -535,7 +533,6 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 {
 	struct ttm_bo_device *bdev = bo->bdev;
 	struct ttm_resource_manager *man = ttm_manager_type(bdev, new_mem->mem_type);
-	struct ttm_resource *old_mem = &bo->mem;
 	int ret;
 	struct ttm_buffer_object *ghost_obj;
 
@@ -582,8 +579,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 		ttm_bo_put(ghost_obj);
 	}
 
-	*old_mem = *new_mem;
-	new_mem->mm_node = NULL;
+	ttm_bo_assign_mem(bo, new_mem);
 
 	return 0;
 }
@@ -594,9 +590,8 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
 			 struct ttm_resource *new_mem)
 {
 	struct ttm_bo_device *bdev = bo->bdev;
-	struct ttm_resource *old_mem = &bo->mem;
 
-	struct ttm_resource_manager *from = ttm_manager_type(bdev, old_mem->mem_type);
+	struct ttm_resource_manager *from = ttm_manager_type(bdev, bo->mem.mem_type);
 	struct ttm_resource_manager *to = ttm_manager_type(bdev, new_mem->mem_type);
 
 	int ret;
@@ -673,8 +668,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
 		ttm_bo_free_old_node(bo);
 	}
 
-	*old_mem = *new_mem;
-	new_mem->mm_node = NULL;
+	ttm_bo_assign_mem(bo, new_mem);
 
 	return 0;
 }
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 7846dfa507f7..25cc932d63f1 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -539,6 +539,13 @@ static inline void ttm_bo_move_to_lru_tail_unlocked(struct ttm_buffer_object *bo
 	spin_unlock(&ttm_bo_glob.lru_lock);
 }
 
+static inline void ttm_bo_assign_mem(struct ttm_buffer_object *bo,
+				     struct ttm_resource *new_mem)
+{
+	bo->mem = *new_mem;
+	new_mem->mm_node = NULL;
+}
+
 /**
  * ttm_bo_move_null = assign memory for a buffer object.
  * @bo: The bo to assign the memory to
@@ -552,8 +559,7 @@ static inline void ttm_bo_move_null(struct ttm_buffer_object *bo,
 	struct ttm_resource *old_mem = &bo->mem;
 
 	WARN_ON(old_mem->mm_node != NULL);
-	*old_mem = *new_mem;
-	new_mem->mm_node = NULL;
+	ttm_bo_assign_mem(bo, new_mem);
 }
 
 /**
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/7] drm/ttm: add a simple assign mem to bo wrapper

[Christian König]

Am 17.09.20 um 06:30 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> This pattern is called in a few places, just clean it up.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/ttm/ttm_bo_util.c | 18 ++++++------------
>   include/drm/ttm/ttm_bo_driver.h   | 10 ++++++++--
>   2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 919489f6a5a3..5a8d77ef504f 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -87,9 +87,7 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
>   			return ret;
>   	}
>   
> -	*old_mem = *new_mem;
> -	new_mem->mm_node = NULL;
> -
> +	ttm_bo_assign_mem(bo, new_mem);
>   	return 0;
>   }
>   EXPORT_SYMBOL(ttm_bo_move_ttm);
> @@ -299,8 +297,8 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>   	mb();
>   out2:
>   	old_copy = *old_mem;
> -	*old_mem = *new_mem;
> -	new_mem->mm_node = NULL;
> +
> +	ttm_bo_assign_mem(bo, new_mem);
>   
>   	if (!man->use_tt)
>   		ttm_bo_tt_destroy(bo);
> @@ -535,7 +533,6 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>   {
>   	struct ttm_bo_device *bdev = bo->bdev;
>   	struct ttm_resource_manager *man = ttm_manager_type(bdev, new_mem->mem_type);
> -	struct ttm_resource *old_mem = &bo->mem;
>   	int ret;
>   	struct ttm_buffer_object *ghost_obj;
>   
> @@ -582,8 +579,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>   		ttm_bo_put(ghost_obj);
>   	}
>   
> -	*old_mem = *new_mem;
> -	new_mem->mm_node = NULL;
> +	ttm_bo_assign_mem(bo, new_mem);
>   
>   	return 0;
>   }
> @@ -594,9 +590,8 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>   			 struct ttm_resource *new_mem)
>   {
>   	struct ttm_bo_device *bdev = bo->bdev;
> -	struct ttm_resource *old_mem = &bo->mem;
>   
> -	struct ttm_resource_manager *from = ttm_manager_type(bdev, old_mem->mem_type);
> +	struct ttm_resource_manager *from = ttm_manager_type(bdev, bo->mem.mem_type);
>   	struct ttm_resource_manager *to = ttm_manager_type(bdev, new_mem->mem_type);
>   
>   	int ret;
> @@ -673,8 +668,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>   		ttm_bo_free_old_node(bo);
>   	}
>   
> -	*old_mem = *new_mem;
> -	new_mem->mm_node = NULL;
> +	ttm_bo_assign_mem(bo, new_mem);
>   
>   	return 0;
>   }
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 7846dfa507f7..25cc932d63f1 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -539,6 +539,13 @@ static inline void ttm_bo_move_to_lru_tail_unlocked(struct ttm_buffer_object *bo
>   	spin_unlock(&ttm_bo_glob.lru_lock);
>   }
>   
> +static inline void ttm_bo_assign_mem(struct ttm_buffer_object *bo,
> +				     struct ttm_resource *new_mem)
> +{
> +	bo->mem = *new_mem;
> +	new_mem->mm_node = NULL;
> +}
> +
>   /**
>    * ttm_bo_move_null = assign memory for a buffer object.
>    * @bo: The bo to assign the memory to
> @@ -552,8 +559,7 @@ static inline void ttm_bo_move_null(struct ttm_buffer_object *bo,
>   	struct ttm_resource *old_mem = &bo->mem;
>   
>   	WARN_ON(old_mem->mm_node != NULL);
> -	*old_mem = *new_mem;
> -	new_mem->mm_node = NULL;
> +	ttm_bo_assign_mem(bo, new_mem);
>   }
>   
>   /**

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/7] drm/ttm: move ghost object creation to a common function

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Both accel cleanup and pipeline move had the same code, make
a single function for it.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 105 ++++++++++++------------------
 1 file changed, 43 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 5a8d77ef504f..3c16e303d2e2 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -526,6 +526,47 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
 }
 EXPORT_SYMBOL(ttm_bo_kunmap);
 
+static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo,
+				struct dma_fence *fence,
+				bool dst_use_tt)
+{
+	struct ttm_buffer_object *ghost_obj;
+	int ret;
+
+	/**
+	 * This should help pipeline ordinary buffer moves.
+	 *
+	 * Hang old buffer memory on a new buffer object,
+	 * and leave it to be released when the GPU
+	 * 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;
+
+	dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
+
+	/**
+	 * If we're not moving to fixed memory, the TTM object
+	 * needs to stay alive. Otherwhise hang it on the ghost
+	 * bo to be unbound and destroyed.
+	 */
+
+	if (dst_use_tt) {
+		ghost_obj->ttm = NULL;
+	} else {
+		bo->ttm = NULL;
+	}
+
+	dma_resv_unlock(&ghost_obj->base._resv);
+	ttm_bo_put(ghost_obj);
+	return 0;
+}
+
 int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 			      struct dma_fence *fence,
 			      bool evict,
@@ -534,7 +575,6 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 	struct ttm_bo_device *bdev = bo->bdev;
 	struct ttm_resource_manager *man = ttm_manager_type(bdev, new_mem->mem_type);
 	int ret;
-	struct ttm_buffer_object *ghost_obj;
 
 	dma_resv_add_excl_fence(bo->base.resv, fence);
 	if (evict) {
@@ -546,37 +586,9 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 			ttm_bo_tt_destroy(bo);
 		ttm_bo_free_old_node(bo);
 	} else {
-		/**
-		 * This should help pipeline ordinary buffer moves.
-		 *
-		 * Hang old buffer memory on a new buffer object,
-		 * and leave it to be released when the GPU
-		 * operation has completed.
-		 */
-
-		dma_fence_put(bo->moving);
-		bo->moving = dma_fence_get(fence);
-
-		ret = ttm_buffer_object_transfer(bo, &ghost_obj);
+		ret = ttm_bo_move_to_ghost(bo, fence, man->use_tt);
 		if (ret)
 			return ret;
-
-		dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
-
-		/**
-		 * If we're not moving to fixed memory, the TTM object
-		 * needs to stay alive. Otherwhise hang it on the ghost
-		 * bo to be unbound and destroyed.
-		 */
-
-		if (man->use_tt) {
-			ghost_obj->ttm = NULL;
-		} else {
-			bo->ttm = NULL;
-		}
-
-		dma_resv_unlock(&ghost_obj->base._resv);
-		ttm_bo_put(ghost_obj);
 	}
 
 	ttm_bo_assign_mem(bo, new_mem);
@@ -599,40 +611,9 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
 	dma_resv_add_excl_fence(bo->base.resv, fence);
 
 	if (!evict) {
-		struct ttm_buffer_object *ghost_obj;
-
-		/**
-		 * This should help pipeline ordinary buffer moves.
-		 *
-		 * Hang old buffer memory on a new buffer object,
-		 * and leave it to be released when the GPU
-		 * operation has completed.
-		 */
-
-		dma_fence_put(bo->moving);
-		bo->moving = dma_fence_get(fence);
-
-		ret = ttm_buffer_object_transfer(bo, &ghost_obj);
+		ret = ttm_bo_move_to_ghost(bo, fence, to->use_tt);
 		if (ret)
 			return ret;
-
-		dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
-
-		/**
-		 * If we're not moving to fixed memory, the TTM object
-		 * needs to stay alive. Otherwhise hang it on the ghost
-		 * bo to be unbound and destroyed.
-		 */
-
-		if (to->use_tt) {
-			ghost_obj->ttm = NULL;
-		} else {
-			bo->ttm = NULL;
-		}
-
-		dma_resv_unlock(&ghost_obj->base._resv);
-		ttm_bo_put(ghost_obj);
-
 	} else if (!from->use_tt) {
 
 		/**
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/7] drm/ttm: move ghost object creation to a common function

[Christian König]

Am 17.09.20 um 06:30 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> Both accel cleanup and pipeline move had the same code, make
> a single function for it.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>   drivers/gpu/drm/ttm/ttm_bo_util.c | 105 ++++++++++++------------------
>   1 file changed, 43 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 5a8d77ef504f..3c16e303d2e2 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -526,6 +526,47 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
>   }
>   EXPORT_SYMBOL(ttm_bo_kunmap);
>   
> +static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo,
> +				struct dma_fence *fence,
> +				bool dst_use_tt)
> +{
> +	struct ttm_buffer_object *ghost_obj;
> +	int ret;
> +
> +	/**
> +	 * This should help pipeline ordinary buffer moves.
> +	 *
> +	 * Hang old buffer memory on a new buffer object,
> +	 * and leave it to be released when the GPU
> +	 * 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;
> +
> +	dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
> +
> +	/**
> +	 * If we're not moving to fixed memory, the TTM object
> +	 * needs to stay alive. Otherwhise hang it on the ghost
> +	 * bo to be unbound and destroyed.
> +	 */
> +
> +	if (dst_use_tt) {
> +		ghost_obj->ttm = NULL;
> +	} else {
> +		bo->ttm = NULL;
> +	}

The {} can be dropped here, apart from that the patch is Reviewed-by: 
Christian König <christian.koenig@amd.com>

> +
> +	dma_resv_unlock(&ghost_obj->base._resv);
> +	ttm_bo_put(ghost_obj);
> +	return 0;
> +}
> +
>   int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>   			      struct dma_fence *fence,
>   			      bool evict,
> @@ -534,7 +575,6 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>   	struct ttm_bo_device *bdev = bo->bdev;
>   	struct ttm_resource_manager *man = ttm_manager_type(bdev, new_mem->mem_type);
>   	int ret;
> -	struct ttm_buffer_object *ghost_obj;
>   
>   	dma_resv_add_excl_fence(bo->base.resv, fence);
>   	if (evict) {
> @@ -546,37 +586,9 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>   			ttm_bo_tt_destroy(bo);
>   		ttm_bo_free_old_node(bo);
>   	} else {
> -		/**
> -		 * This should help pipeline ordinary buffer moves.
> -		 *
> -		 * Hang old buffer memory on a new buffer object,
> -		 * and leave it to be released when the GPU
> -		 * operation has completed.
> -		 */
> -
> -		dma_fence_put(bo->moving);
> -		bo->moving = dma_fence_get(fence);
> -
> -		ret = ttm_buffer_object_transfer(bo, &ghost_obj);
> +		ret = ttm_bo_move_to_ghost(bo, fence, man->use_tt);
>   		if (ret)
>   			return ret;
> -
> -		dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
> -
> -		/**
> -		 * If we're not moving to fixed memory, the TTM object
> -		 * needs to stay alive. Otherwhise hang it on the ghost
> -		 * bo to be unbound and destroyed.
> -		 */
> -
> -		if (man->use_tt) {
> -			ghost_obj->ttm = NULL;
> -		} else {
> -			bo->ttm = NULL;
> -		}
> -
> -		dma_resv_unlock(&ghost_obj->base._resv);
> -		ttm_bo_put(ghost_obj);
>   	}
>   
>   	ttm_bo_assign_mem(bo, new_mem);
> @@ -599,40 +611,9 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>   	dma_resv_add_excl_fence(bo->base.resv, fence);
>   
>   	if (!evict) {
> -		struct ttm_buffer_object *ghost_obj;
> -
> -		/**
> -		 * This should help pipeline ordinary buffer moves.
> -		 *
> -		 * Hang old buffer memory on a new buffer object,
> -		 * and leave it to be released when the GPU
> -		 * operation has completed.
> -		 */
> -
> -		dma_fence_put(bo->moving);
> -		bo->moving = dma_fence_get(fence);
> -
> -		ret = ttm_buffer_object_transfer(bo, &ghost_obj);
> +		ret = ttm_bo_move_to_ghost(bo, fence, to->use_tt);
>   		if (ret)
>   			return ret;
> -
> -		dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
> -
> -		/**
> -		 * If we're not moving to fixed memory, the TTM object
> -		 * needs to stay alive. Otherwhise hang it on the ghost
> -		 * bo to be unbound and destroyed.
> -		 */
> -
> -		if (to->use_tt) {
> -			ghost_obj->ttm = NULL;
> -		} else {
> -			bo->ttm = NULL;
> -		}
> -
> -		dma_resv_unlock(&ghost_obj->base._resv);
> -		ttm_bo_put(ghost_obj);
> -
>   	} else if (!from->use_tt) {
>   
>   		/**

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 7/7] drm/ttm: make common function for wait/free node path.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

The pipeline and accel cleansups has similiar paths here.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 38 ++++++++++++++++---------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 3c16e303d2e2..502d334786d2 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -526,6 +526,20 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
 }
 EXPORT_SYMBOL(ttm_bo_kunmap);
 
+static int ttm_bo_wait_free_node(struct ttm_buffer_object *bo,
+				 bool dst_use_tt)
+{
+	int ret;
+	ret = ttm_bo_wait(bo, false, false);
+	if (ret)
+		return ret;
+
+	if (!dst_use_tt)
+		ttm_bo_tt_destroy(bo);
+	ttm_bo_free_old_node(bo);
+	return 0;
+}
+
 static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo,
 				struct dma_fence *fence,
 				bool dst_use_tt)
@@ -577,19 +591,12 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 	int ret;
 
 	dma_resv_add_excl_fence(bo->base.resv, fence);
-	if (evict) {
-		ret = ttm_bo_wait(bo, false, false);
-		if (ret)
-			return ret;
-
-		if (!man->use_tt)
-			ttm_bo_tt_destroy(bo);
-		ttm_bo_free_old_node(bo);
-	} else {
+	if (evict)
+		ret = ttm_bo_wait_free_node(bo, man->use_tt);
+	else
 		ret = ttm_bo_move_to_ghost(bo, fence, man->use_tt);
-		if (ret)
-			return ret;
-	}
+	if (ret)
+		return ret;
 
 	ttm_bo_assign_mem(bo, new_mem);
 
@@ -639,14 +646,9 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
 		 *
 		 * Should never happen in pratice.
 		 */
-
-		ret = ttm_bo_wait(bo, false, false);
+		ret = ttm_bo_wait_free_node(bo, to->use_tt);
 		if (ret)
 			return ret;
-
-		if (!to->use_tt)
-			ttm_bo_tt_destroy(bo);
-		ttm_bo_free_old_node(bo);
 	}
 
 	ttm_bo_assign_mem(bo, new_mem);
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 7/7] drm/ttm: make common function for wait/free node path.

[Christian König]

Am 17.09.20 um 06:30 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> The pipeline and accel cleansups has similiar paths here.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/ttm/ttm_bo_util.c | 38 ++++++++++++++++---------------
>   1 file changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 3c16e303d2e2..502d334786d2 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -526,6 +526,20 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
>   }
>   EXPORT_SYMBOL(ttm_bo_kunmap);
>   
> +static int ttm_bo_wait_free_node(struct ttm_buffer_object *bo,
> +				 bool dst_use_tt)
> +{
> +	int ret;
> +	ret = ttm_bo_wait(bo, false, false);
> +	if (ret)
> +		return ret;
> +
> +	if (!dst_use_tt)
> +		ttm_bo_tt_destroy(bo);
> +	ttm_bo_free_old_node(bo);
> +	return 0;
> +}
> +
>   static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo,
>   				struct dma_fence *fence,
>   				bool dst_use_tt)
> @@ -577,19 +591,12 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>   	int ret;
>   
>   	dma_resv_add_excl_fence(bo->base.resv, fence);
> -	if (evict) {
> -		ret = ttm_bo_wait(bo, false, false);
> -		if (ret)
> -			return ret;
> -
> -		if (!man->use_tt)
> -			ttm_bo_tt_destroy(bo);
> -		ttm_bo_free_old_node(bo);
> -	} else {
> +	if (evict)
> +		ret = ttm_bo_wait_free_node(bo, man->use_tt);
> +	else
>   		ret = ttm_bo_move_to_ghost(bo, fence, man->use_tt);
> -		if (ret)
> -			return ret;
> -	}
> +	if (ret)
> +		return ret;
>   
>   	ttm_bo_assign_mem(bo, new_mem);
>   
> @@ -639,14 +646,9 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>   		 *
>   		 * Should never happen in pratice.
>   		 */
> -
> -		ret = ttm_bo_wait(bo, false, false);
> +		ret = ttm_bo_wait_free_node(bo, to->use_tt);
>   		if (ret)
>   			return ret;
> -
> -		if (!to->use_tt)
> -			ttm_bo_tt_destroy(bo);
> -		ttm_bo_free_old_node(bo);
>   	}
>   
>   	ttm_bo_assign_mem(bo, new_mem);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel