[PATCH 00/45] TTM move refactoring

[PATCH 00/45] TTM move refactoring

[Dave Airlie]

This refactors how TTM moves get called between core and drivers.

1) puts the driver in charge of all moves, and enforces
drivers have a move callback.
2) moves move_notify actions around moves to the driver side
3) moves binding/unbinding completely into move and driver side
4) add a new invalidate callback to replace the last use of move_notify
5) add some helpers to cleanup the resulting move code

There's a bit of internal churn to try and make each patch logical, so
don't get too caught up in each patches changes unless the end result
is a problem.

Dave.


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

[PATCH 01/45] drm/qxl: drop unused code

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 2c35ca4270c6..5738be300078 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -100,17 +100,12 @@ int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
  */
 struct qxl_ttm_tt {
 	struct ttm_tt		        ttm;
-	struct qxl_device		*qdev;
-	u64				offset;
 };
 
 static int qxl_ttm_backend_bind(struct ttm_bo_device *bdev,
 				struct ttm_tt *ttm,
 				struct ttm_resource *bo_mem)
 {
-	struct qxl_ttm_tt *gtt = (void *)ttm;
-
-	gtt->offset = (unsigned long)(bo_mem->start << PAGE_SHIFT);
 	if (!ttm->num_pages) {
 		WARN(1, "nothing to bind %lu pages for mreg %p back %p!\n",
 		     ttm->num_pages, bo_mem, ttm);
@@ -138,14 +133,11 @@ static void qxl_ttm_backend_destroy(struct ttm_bo_device *bdev,
 static struct ttm_tt *qxl_ttm_tt_create(struct ttm_buffer_object *bo,
 					uint32_t page_flags)
 {
-	struct qxl_device *qdev;
 	struct qxl_ttm_tt *gtt;
 
-	qdev = qxl_get_qdev(bo->bdev);
 	gtt = kzalloc(sizeof(struct qxl_ttm_tt), GFP_KERNEL);
 	if (gtt == NULL)
 		return NULL;
-	gtt->qdev = qdev;
 	if (ttm_tt_init(&gtt->ttm, bo, page_flags)) {
 		kfree(gtt);
 		return NULL;
-- 
2.27.0

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

Re: [PATCH 01/45] drm/qxl: drop unused code

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>   drivers/gpu/drm/qxl/qxl_ttm.c | 8 --------
>   1 file changed, 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index 2c35ca4270c6..5738be300078 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -100,17 +100,12 @@ int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
>    */
>   struct qxl_ttm_tt {
>   	struct ttm_tt		        ttm;
> -	struct qxl_device		*qdev;
> -	u64				offset;
>   };

Any reason we keep the qxl_ttm_tt structure around when it only contains 
the ttm_tt field?

Christian.

>   
>   static int qxl_ttm_backend_bind(struct ttm_bo_device *bdev,
>   				struct ttm_tt *ttm,
>   				struct ttm_resource *bo_mem)
>   {
> -	struct qxl_ttm_tt *gtt = (void *)ttm;
> -
> -	gtt->offset = (unsigned long)(bo_mem->start << PAGE_SHIFT);
>   	if (!ttm->num_pages) {
>   		WARN(1, "nothing to bind %lu pages for mreg %p back %p!\n",
>   		     ttm->num_pages, bo_mem, ttm);
> @@ -138,14 +133,11 @@ static void qxl_ttm_backend_destroy(struct ttm_bo_device *bdev,
>   static struct ttm_tt *qxl_ttm_tt_create(struct ttm_buffer_object *bo,
>   					uint32_t page_flags)
>   {
> -	struct qxl_device *qdev;
>   	struct qxl_ttm_tt *gtt;
>   
> -	qdev = qxl_get_qdev(bo->bdev);
>   	gtt = kzalloc(sizeof(struct qxl_ttm_tt), GFP_KERNEL);
>   	if (gtt == NULL)
>   		return NULL;
> -	gtt->qdev = qdev;
>   	if (ttm_tt_init(&gtt->ttm, bo, page_flags)) {
>   		kfree(gtt);
>   		return NULL;

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

[PATCH 02/45] drm/ttm: handle the SYSTEM->TT path in same place as others.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This just consolidates the code making the flow easier to understand
and also helps when moving move to the driver side.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index c342bfc2b4c1..6d1520255fc1 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -265,20 +265,18 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 			if (ret)
 				goto out_err;
 		}
-
-		if (bo->mem.mem_type == TTM_PL_SYSTEM) {
-			if (bdev->driver->move_notify)
-				bdev->driver->move_notify(bo, evict, mem);
-			bo->mem = *mem;
-			goto moved;
-		}
 	}
 
 	if (bdev->driver->move_notify)
 		bdev->driver->move_notify(bo, evict, mem);
 
-	if (old_man->use_tt && new_man->use_tt)
-		ret = ttm_bo_move_ttm(bo, ctx, mem);
+	if (old_man->use_tt && new_man->use_tt) {
+		if (bo->mem.mem_type == TTM_PL_SYSTEM) {
+			ttm_bo_assign_mem(bo, mem);
+			ret = 0;
+		} else
+			ret = ttm_bo_move_ttm(bo, ctx, mem);
+	}
 	else if (bdev->driver->move)
 		ret = bdev->driver->move(bo, evict, ctx, mem);
 	else
@@ -294,7 +292,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 		goto out_err;
 	}
 
-moved:
 	ctx->bytes_moved += bo->num_pages << PAGE_SHIFT;
 	return 0;
 
-- 
2.27.0

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

Re: [PATCH 02/45] drm/ttm: handle the SYSTEM->TT path in same place as others.

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> This just consolidates the code making the flow easier to understand
> and also helps when moving move to the driver side.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>   drivers/gpu/drm/ttm/ttm_bo.c | 17 +++++++----------
>   1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index c342bfc2b4c1..6d1520255fc1 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -265,20 +265,18 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>   			if (ret)
>   				goto out_err;
>   		}
> -
> -		if (bo->mem.mem_type == TTM_PL_SYSTEM) {
> -			if (bdev->driver->move_notify)
> -				bdev->driver->move_notify(bo, evict, mem);
> -			bo->mem = *mem;
> -			goto moved;
> -		}
>   	}
>   
>   	if (bdev->driver->move_notify)
>   		bdev->driver->move_notify(bo, evict, mem);
>   
> -	if (old_man->use_tt && new_man->use_tt)
> -		ret = ttm_bo_move_ttm(bo, ctx, mem);
> +	if (old_man->use_tt && new_man->use_tt) {
> +		if (bo->mem.mem_type == TTM_PL_SYSTEM) {
> +			ttm_bo_assign_mem(bo, mem);
> +			ret = 0;
> +		} else
> +			ret = ttm_bo_move_ttm(bo, ctx, mem);
> +	}
>   	else if (bdev->driver->move)

This should then use "} else if (...) {", apart from that the patch is 
Reviewed-by: Christian König <christian.koenig@amd.com>.

Christian.

>   		ret = bdev->driver->move(bo, evict, ctx, mem);
>   	else
> @@ -294,7 +292,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>   		goto out_err;
>   	}
>   
> -moved:
>   	ctx->bytes_moved += bo->num_pages << PAGE_SHIFT;
>   	return 0;
>   

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

[PATCH 03/45] drm/amdgpu/ttm: handle tt moves properly.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

The core move code currently handles use_tt moves, for amdgpu
this was being handled also in the driver, but not using the same
paths.

If moving between TT/SYSTEM (all the use_tt paths on amdgpu) use
the core move function.

Eventually the core will be flipped over to calling the driver.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index db5f761f37ec..d3bd2fd448be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -671,14 +671,16 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
 		ttm_bo_move_null(bo, new_mem);
 		return 0;
 	}
-	if ((old_mem->mem_type == TTM_PL_TT &&
-	     new_mem->mem_type == TTM_PL_SYSTEM) ||
-	    (old_mem->mem_type == TTM_PL_SYSTEM &&
-	     new_mem->mem_type == TTM_PL_TT)) {
-		/* bind is enough */
+	if (old_mem->mem_type == TTM_PL_SYSTEM &&
+	    new_mem->mem_type == TTM_PL_TT) {
 		ttm_bo_move_null(bo, new_mem);
 		return 0;
 	}
+
+	if (old_mem->mem_type == TTM_PL_TT &&
+	    new_mem->mem_type == TTM_PL_SYSTEM)
+		return ttm_bo_move_ttm(bo, ctx, new_mem);
+
 	if (old_mem->mem_type == AMDGPU_PL_GDS ||
 	    old_mem->mem_type == AMDGPU_PL_GWS ||
 	    old_mem->mem_type == AMDGPU_PL_OA ||
-- 
2.27.0

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

Re: [PATCH 03/45] drm/amdgpu/ttm: handle tt moves properly.

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> The core move code currently handles use_tt moves, for amdgpu
> this was being handled also in the driver, but not using the same
> paths.
>
> If moving between TT/SYSTEM (all the use_tt paths on amdgpu) use
> the core move function.
>
> Eventually the core will be flipped over to calling the driver.
>
> 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 | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index db5f761f37ec..d3bd2fd448be 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -671,14 +671,16 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
>   		ttm_bo_move_null(bo, new_mem);
>   		return 0;
>   	}
> -	if ((old_mem->mem_type == TTM_PL_TT &&
> -	     new_mem->mem_type == TTM_PL_SYSTEM) ||
> -	    (old_mem->mem_type == TTM_PL_SYSTEM &&
> -	     new_mem->mem_type == TTM_PL_TT)) {
> -		/* bind is enough */
> +	if (old_mem->mem_type == TTM_PL_SYSTEM &&
> +	    new_mem->mem_type == TTM_PL_TT) {
>   		ttm_bo_move_null(bo, new_mem);
>   		return 0;
>   	}
> +
> +	if (old_mem->mem_type == TTM_PL_TT &&
> +	    new_mem->mem_type == TTM_PL_SYSTEM)
> +		return ttm_bo_move_ttm(bo, ctx, new_mem);
> +
>   	if (old_mem->mem_type == AMDGPU_PL_GDS ||
>   	    old_mem->mem_type == AMDGPU_PL_GWS ||
>   	    old_mem->mem_type == AMDGPU_PL_OA ||

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

[PATCH 04/45] drm/radeon/ttm: handle ttm moves properly

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

The core move code currently handles use_tt moves, for radeon
this was being handled also in the driver, but not using the same
paths.

If moving between TT/SYSTEM (all the use_tt paths on radeon) use
the core move function.

Eventually the core will be flipped over to calling the driver.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index ea9ffa6198da..df5cedb2b632 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -316,14 +316,16 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
 		ttm_bo_move_null(bo, new_mem);
 		return 0;
 	}
-	if ((old_mem->mem_type == TTM_PL_TT &&
-	     new_mem->mem_type == TTM_PL_SYSTEM) ||
-	    (old_mem->mem_type == TTM_PL_SYSTEM &&
-	     new_mem->mem_type == TTM_PL_TT)) {
-		/* bind is enough */
+	if (old_mem->mem_type == TTM_PL_SYSTEM &&
+	    new_mem->mem_type == TTM_PL_TT) {
 		ttm_bo_move_null(bo, new_mem);
 		return 0;
 	}
+
+	if (old_mem->mem_type == TTM_PL_TT &&
+	    new_mem->mem_type == TTM_PL_SYSTEM)
+		return ttm_bo_move_ttm(bo, ctx, new_mem);
+
 	if (!rdev->ring[radeon_copy_ring_index(rdev)].ready ||
 	    rdev->asic->copy.copy == NULL) {
 		/* use memcpy */
-- 
2.27.0

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

Re: [PATCH 04/45] drm/radeon/ttm: handle ttm moves properly

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> The core move code currently handles use_tt moves, for radeon
> this was being handled also in the driver, but not using the same
> paths.
>
> If moving between TT/SYSTEM (all the use_tt paths on radeon) use
> the core move function.
>
> Eventually the core will be flipped over to calling the driver.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

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

> ---
>   drivers/gpu/drm/radeon/radeon_ttm.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index ea9ffa6198da..df5cedb2b632 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -316,14 +316,16 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
>   		ttm_bo_move_null(bo, new_mem);
>   		return 0;
>   	}
> -	if ((old_mem->mem_type == TTM_PL_TT &&
> -	     new_mem->mem_type == TTM_PL_SYSTEM) ||
> -	    (old_mem->mem_type == TTM_PL_SYSTEM &&
> -	     new_mem->mem_type == TTM_PL_TT)) {
> -		/* bind is enough */
> +	if (old_mem->mem_type == TTM_PL_SYSTEM &&
> +	    new_mem->mem_type == TTM_PL_TT) {
>   		ttm_bo_move_null(bo, new_mem);
>   		return 0;
>   	}
> +
> +	if (old_mem->mem_type == TTM_PL_TT &&
> +	    new_mem->mem_type == TTM_PL_SYSTEM)
> +		return ttm_bo_move_ttm(bo, ctx, new_mem);
> +
>   	if (!rdev->ring[radeon_copy_ring_index(rdev)].ready ||
>   	    rdev->asic->copy.copy == NULL) {
>   		/* use memcpy */

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

[PATCH 05/45] drm/nouveau/ttm: handle ttm moves properly.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

The idea is to flip the core over to calling the driver always,
so add support for moves here.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 8d51cfca07c8..2c10a84b2cc0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1057,6 +1057,18 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
 		goto out;
 	}
 
+	if (old_reg->mem_type == TTM_PL_SYSTEM &&
+	    new_reg->mem_type == TTM_PL_TT) {
+		ttm_bo_move_null(bo, new_reg);
+		goto out;
+	}
+
+	if (old_reg->mem_type == TTM_PL_TT &&
+	    new_reg->mem_type == TTM_PL_SYSTEM) {
+		ret = ttm_bo_move_ttm(bo, ctx, new_reg);
+		goto out;
+	}
+
 	/* Hardware assisted copy. */
 	if (drm->ttm.move) {
 		if (new_reg->mem_type == TTM_PL_SYSTEM)
-- 
2.27.0

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

Re: [PATCH 05/45] drm/nouveau/ttm: handle ttm moves properly.

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> The idea is to flip the core over to calling the driver always,
> so add support for moves here.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

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

> ---
>   drivers/gpu/drm/nouveau/nouveau_bo.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 8d51cfca07c8..2c10a84b2cc0 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -1057,6 +1057,18 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
>   		goto out;
>   	}
>   
> +	if (old_reg->mem_type == TTM_PL_SYSTEM &&
> +	    new_reg->mem_type == TTM_PL_TT) {
> +		ttm_bo_move_null(bo, new_reg);
> +		goto out;
> +	}
> +
> +	if (old_reg->mem_type == TTM_PL_TT &&
> +	    new_reg->mem_type == TTM_PL_SYSTEM) {
> +		ret = ttm_bo_move_ttm(bo, ctx, new_reg);
> +		goto out;
> +	}
> +
>   	/* Hardware assisted copy. */
>   	if (drm->ttm.move) {
>   		if (new_reg->mem_type == TTM_PL_SYSTEM)

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

[PATCH 06/45] drm/vmwgfx: move null mem checks outside move notifies

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Both fns checked mem == NULL, just move the check outside.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c         | 3 ---
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   | 2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 ++
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index a1f675c5f471..b09f4f064ae4 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -1191,9 +1191,6 @@ void vmw_bo_move_notify(struct ttm_buffer_object *bo,
 {
 	struct vmw_buffer_object *vbo;
 
-	if (mem == NULL)
-		return;
-
 	/* Make sure @bo is embedded in a struct vmw_buffer_object? */
 	if (bo->destroy != vmw_bo_bo_free &&
 	    bo->destroy != vmw_user_bo_destroy)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index 5e922d9d5f2c..00b535831a7a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -867,7 +867,7 @@ void vmw_query_move_notify(struct ttm_buffer_object *bo,
 	mutex_lock(&dev_priv->binding_mutex);
 
 	dx_query_mob = container_of(bo, struct vmw_buffer_object, base);
-	if (mem == NULL || !dx_query_mob || !dx_query_mob->dx_query_ctx) {
+	if (!dx_query_mob || !dx_query_mob->dx_query_ctx) {
 		mutex_unlock(&dev_priv->binding_mutex);
 		return;
 	}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index fc68f54df46a..2f88d2d79f9a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -707,6 +707,8 @@ static void vmw_move_notify(struct ttm_buffer_object *bo,
 			    bool evict,
 			    struct ttm_resource *mem)
 {
+	if (!mem)
+		return;
 	vmw_bo_move_notify(bo, mem);
 	vmw_query_move_notify(bo, mem);
 }
-- 
2.27.0

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

Re: [PATCH 06/45] drm/vmwgfx: move null mem checks outside move notifies

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> Both fns checked mem == NULL, just move the check outside.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

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

> ---
>   drivers/gpu/drm/vmwgfx/vmwgfx_bo.c         | 3 ---
>   drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   | 2 +-
>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 ++
>   3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index a1f675c5f471..b09f4f064ae4 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -1191,9 +1191,6 @@ void vmw_bo_move_notify(struct ttm_buffer_object *bo,
>   {
>   	struct vmw_buffer_object *vbo;
>   
> -	if (mem == NULL)
> -		return;
> -
>   	/* Make sure @bo is embedded in a struct vmw_buffer_object? */
>   	if (bo->destroy != vmw_bo_bo_free &&
>   	    bo->destroy != vmw_user_bo_destroy)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> index 5e922d9d5f2c..00b535831a7a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> @@ -867,7 +867,7 @@ void vmw_query_move_notify(struct ttm_buffer_object *bo,
>   	mutex_lock(&dev_priv->binding_mutex);
>   
>   	dx_query_mob = container_of(bo, struct vmw_buffer_object, base);
> -	if (mem == NULL || !dx_query_mob || !dx_query_mob->dx_query_ctx) {
> +	if (!dx_query_mob || !dx_query_mob->dx_query_ctx) {
>   		mutex_unlock(&dev_priv->binding_mutex);
>   		return;
>   	}
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index fc68f54df46a..2f88d2d79f9a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -707,6 +707,8 @@ static void vmw_move_notify(struct ttm_buffer_object *bo,
>   			    bool evict,
>   			    struct ttm_resource *mem)
>   {
> +	if (!mem)
> +		return;
>   	vmw_bo_move_notify(bo, mem);
>   	vmw_query_move_notify(bo, mem);
>   }

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

[PATCH 07/45] drm/vmwgfx: add a move callback.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This just copies the fallback to vmwgfx, I'm going to iterate on this
a bit until it's not the same as the fallback path.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 2f88d2d79f9a..6e36fc932aeb 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -725,6 +725,23 @@ static void vmw_swap_notify(struct ttm_buffer_object *bo)
 	(void) ttm_bo_wait(bo, false, false);
 }
 
+static int vmw_move(struct ttm_buffer_object *bo,
+		     bool evict,
+		     struct ttm_operation_ctx *ctx,
+		     struct ttm_resource *new_mem)
+{
+	struct ttm_resource_manager *old_man = ttm_manager_type(bo->bdev, bo->mem.mem_type);
+	struct ttm_resource_manager *new_man = ttm_manager_type(bo->bdev, new_mem->mem_type);
+
+	if (old_man->use_tt && new_man->use_tt) {
+		if (bo->mem.mem_type == TTM_PL_SYSTEM) {
+			ttm_bo_assign_mem(bo, new_mem);
+			return 0;
+		}
+		return ttm_bo_move_ttm(bo, ctx, new_mem);
+	} else
+		return ttm_bo_move_memcpy(bo, ctx, new_mem);
+}
 
 struct ttm_bo_driver vmw_bo_driver = {
 	.ttm_tt_create = &vmw_ttm_tt_create,
@@ -735,7 +752,7 @@ struct ttm_bo_driver vmw_bo_driver = {
 	.ttm_tt_destroy = &vmw_ttm_destroy,
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = vmw_evict_flags,
-	.move = NULL,
+	.move = vmw_move,
 	.verify_access = vmw_verify_access,
 	.move_notify = vmw_move_notify,
 	.swap_notify = vmw_swap_notify,
-- 
2.27.0

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

Re: [PATCH 07/45] drm/vmwgfx: add a move callback.

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> This just copies the fallback to vmwgfx, I'm going to iterate on this
> a bit until it's not the same as the fallback path.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 19 ++++++++++++++++++-
>   1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index 2f88d2d79f9a..6e36fc932aeb 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -725,6 +725,23 @@ static void vmw_swap_notify(struct ttm_buffer_object *bo)
>   	(void) ttm_bo_wait(bo, false, false);
>   }
>   
> +static int vmw_move(struct ttm_buffer_object *bo,
> +		     bool evict,
> +		     struct ttm_operation_ctx *ctx,
> +		     struct ttm_resource *new_mem)
> +{
> +	struct ttm_resource_manager *old_man = ttm_manager_type(bo->bdev, bo->mem.mem_type);
> +	struct ttm_resource_manager *new_man = ttm_manager_type(bo->bdev, new_mem->mem_type);
> +
> +	if (old_man->use_tt && new_man->use_tt) {
> +		if (bo->mem.mem_type == TTM_PL_SYSTEM) {
> +			ttm_bo_assign_mem(bo, new_mem);
> +			return 0;
> +		}
> +		return ttm_bo_move_ttm(bo, ctx, new_mem);
> +	} else
> +		return ttm_bo_move_memcpy(bo, ctx, new_mem);

This should be "} else {", apart from that Reviewed-by: Christian König 
<christian.koenig@amd.com>

> +}
>   
>   struct ttm_bo_driver vmw_bo_driver = {
>   	.ttm_tt_create = &vmw_ttm_tt_create,
> @@ -735,7 +752,7 @@ struct ttm_bo_driver vmw_bo_driver = {
>   	.ttm_tt_destroy = &vmw_ttm_destroy,
>   	.eviction_valuable = ttm_bo_eviction_valuable,
>   	.evict_flags = vmw_evict_flags,
> -	.move = NULL,
> +	.move = vmw_move,
>   	.verify_access = vmw_verify_access,
>   	.move_notify = vmw_move_notify,
>   	.swap_notify = vmw_swap_notify,

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

[PATCH 08/45] drm/vram_helper: implement a ttm move callback.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This will always do memcpy moves.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 171ea57b0311..9fd80a3643f6 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -600,6 +600,14 @@ static void drm_gem_vram_bo_driver_move_notify(struct drm_gem_vram_object *gbo,
 	kmap->virtual = NULL;
 }
 
+static int drm_gem_vram_bo_driver_move(struct drm_gem_vram_object *gbo,
+				       bool evict,
+				       struct ttm_operation_ctx *ctx,
+				       struct ttm_resource *new_mem)
+{
+	return ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
+}
+
 /*
  * Helpers for struct drm_gem_object_funcs
  */
@@ -962,6 +970,18 @@ static void bo_driver_move_notify(struct ttm_buffer_object *bo,
 	drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
 }
 
+static int bo_driver_move(struct ttm_buffer_object *bo,
+			  bool evict,
+			  struct ttm_operation_ctx *ctx,
+			  struct ttm_resource *new_mem)
+{
+	struct drm_gem_vram_object *gbo;
+
+	gbo = drm_gem_vram_of_bo(bo);
+
+	return drm_gem_vram_bo_driver_move(gbo, evict, ctx, new_mem);
+}
+
 static int bo_driver_io_mem_reserve(struct ttm_bo_device *bdev,
 				    struct ttm_resource *mem)
 {
@@ -986,6 +1006,7 @@ static struct ttm_bo_driver bo_driver = {
 	.ttm_tt_destroy = bo_driver_ttm_tt_destroy,
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = bo_driver_evict_flags,
+	.move = bo_driver_move,
 	.move_notify = bo_driver_move_notify,
 	.io_mem_reserve = bo_driver_io_mem_reserve,
 };
-- 
2.27.0

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

Re: [PATCH 08/45] drm/vram_helper: implement a ttm move callback.

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> This will always do memcpy moves.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

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

> ---
>   drivers/gpu/drm/drm_gem_vram_helper.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 171ea57b0311..9fd80a3643f6 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -600,6 +600,14 @@ static void drm_gem_vram_bo_driver_move_notify(struct drm_gem_vram_object *gbo,
>   	kmap->virtual = NULL;
>   }
>   
> +static int drm_gem_vram_bo_driver_move(struct drm_gem_vram_object *gbo,
> +				       bool evict,
> +				       struct ttm_operation_ctx *ctx,
> +				       struct ttm_resource *new_mem)
> +{
> +	return ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
> +}
> +
>   /*
>    * Helpers for struct drm_gem_object_funcs
>    */
> @@ -962,6 +970,18 @@ static void bo_driver_move_notify(struct ttm_buffer_object *bo,
>   	drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
>   }
>   
> +static int bo_driver_move(struct ttm_buffer_object *bo,
> +			  bool evict,
> +			  struct ttm_operation_ctx *ctx,
> +			  struct ttm_resource *new_mem)
> +{
> +	struct drm_gem_vram_object *gbo;
> +
> +	gbo = drm_gem_vram_of_bo(bo);
> +
> +	return drm_gem_vram_bo_driver_move(gbo, evict, ctx, new_mem);
> +}
> +
>   static int bo_driver_io_mem_reserve(struct ttm_bo_device *bdev,
>   				    struct ttm_resource *mem)
>   {
> @@ -986,6 +1006,7 @@ static struct ttm_bo_driver bo_driver = {
>   	.ttm_tt_destroy = bo_driver_ttm_tt_destroy,
>   	.eviction_valuable = ttm_bo_eviction_valuable,
>   	.evict_flags = bo_driver_evict_flags,
> +	.move = bo_driver_move,
>   	.move_notify = bo_driver_move_notify,
>   	.io_mem_reserve = bo_driver_io_mem_reserve,
>   };

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

[PATCH 09/45] drm/ttm: make move callback compulstory

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

All drivers should have a move callback now so make it compulsory.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 6d1520255fc1..6a7f4c028801 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -270,18 +270,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 	if (bdev->driver->move_notify)
 		bdev->driver->move_notify(bo, evict, mem);
 
-	if (old_man->use_tt && new_man->use_tt) {
-		if (bo->mem.mem_type == TTM_PL_SYSTEM) {
-			ttm_bo_assign_mem(bo, mem);
-			ret = 0;
-		} else
-			ret = ttm_bo_move_ttm(bo, ctx, mem);
-	}
-	else if (bdev->driver->move)
-		ret = bdev->driver->move(bo, evict, ctx, mem);
-	else
-		ret = ttm_bo_move_memcpy(bo, ctx, mem);
-
+	ret = bdev->driver->move(bo, evict, ctx, mem);
 	if (ret) {
 		if (bdev->driver->move_notify) {
 			swap(*mem, bo->mem);
-- 
2.27.0

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

Re: [PATCH 09/45] drm/ttm: make move callback compulstory

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> All drivers should have a move callback now so make it compulsory.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

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

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c | 13 +------------
>   1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 6d1520255fc1..6a7f4c028801 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -270,18 +270,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>   	if (bdev->driver->move_notify)
>   		bdev->driver->move_notify(bo, evict, mem);
>   
> -	if (old_man->use_tt && new_man->use_tt) {
> -		if (bo->mem.mem_type == TTM_PL_SYSTEM) {
> -			ttm_bo_assign_mem(bo, mem);
> -			ret = 0;
> -		} else
> -			ret = ttm_bo_move_ttm(bo, ctx, mem);
> -	}
> -	else if (bdev->driver->move)
> -		ret = bdev->driver->move(bo, evict, ctx, mem);
> -	else
> -		ret = ttm_bo_move_memcpy(bo, ctx, mem);
> -
> +	ret = bdev->driver->move(bo, evict, ctx, mem);
>   	if (ret) {
>   		if (bdev->driver->move_notify) {
>   			swap(*mem, bo->mem);

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

[PATCH 10/45] drm/ttm: refactor out common code to setup a new tt backed resource

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This factors out the code to set the new caching and for non-system
tt populate and bind things.

The same code was used twice in the move paths.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c      | 12 +---------
 drivers/gpu/drm/ttm/ttm_bo_util.c | 40 +++++++++++++++++++------------
 include/drm/ttm/ttm_bo_driver.h   |  3 +++
 3 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 6a7f4c028801..c8dffc8b40fc 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -252,19 +252,9 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 		if (ret)
 			goto out_err;
 
-		ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement);
+		ret = ttm_bo_move_to_new_tt_mem(bo, ctx, mem);
 		if (ret)
 			goto out_err;
-
-		if (mem->mem_type != TTM_PL_SYSTEM) {
-			ret = ttm_tt_populate(bdev, bo->ttm, ctx);
-			if (ret)
-				goto out_err;
-
-			ret = ttm_bo_tt_bind(bo, mem);
-			if (ret)
-				goto out_err;
-		}
 	}
 
 	if (bdev->driver->move_notify)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index bdee4df1f3f2..aecdb2d92a54 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -50,11 +50,33 @@ void ttm_bo_free_old_node(struct ttm_buffer_object *bo)
 	ttm_resource_free(bo, &bo->mem);
 }
 
+int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
+			      struct ttm_operation_ctx *ctx,
+			      struct ttm_resource *new_mem)
+{
+	struct ttm_tt *ttm = bo->ttm;
+	int ret;
+
+	ret = ttm_tt_set_placement_caching(ttm, new_mem->placement);
+	if (unlikely(ret != 0))
+		return ret;
+
+	if (new_mem->mem_type != TTM_PL_SYSTEM) {
+		ret = ttm_tt_populate(bo->bdev, ttm, ctx);
+		if (unlikely(ret != 0))
+			return ret;
+
+		ret = ttm_bo_tt_bind(bo, new_mem);
+		if (unlikely(ret != 0))
+			return ret;
+	}
+	return 0;
+}
+
 int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
 		   struct ttm_operation_ctx *ctx,
 		    struct ttm_resource *new_mem)
 {
-	struct ttm_tt *ttm = bo->ttm;
 	struct ttm_resource *old_mem = &bo->mem;
 	int ret;
 
@@ -72,21 +94,9 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
 		old_mem->mem_type = TTM_PL_SYSTEM;
 	}
 
-	ret = ttm_tt_set_placement_caching(ttm, new_mem->placement);
-	if (unlikely(ret != 0))
+	ret = ttm_bo_move_to_new_tt_mem(bo, ctx, new_mem);
+	if (ret)
 		return ret;
-
-	if (new_mem->mem_type != TTM_PL_SYSTEM) {
-
-		ret = ttm_tt_populate(bo->bdev, ttm, ctx);
-		if (unlikely(ret != 0))
-			return ret;
-
-		ret = ttm_bo_tt_bind(bo, new_mem);
-		if (unlikely(ret != 0))
-			return ret;
-	}
-
 	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 864afa8f6f18..20e6839e9b73 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -605,6 +605,9 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
 		    struct ttm_operation_ctx *ctx,
 		    struct ttm_resource *new_mem);
 
+int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
+			      struct ttm_operation_ctx *ctx,
+			      struct ttm_resource *new_mem);
 /**
  * ttm_bo_move_memcpy
  *
-- 
2.27.0

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

Re: [PATCH 10/45] drm/ttm: refactor out common code to setup a new tt backed resource

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> This factors out the code to set the new caching and for non-system
> tt populate and bind things.
>
> The same code was used twice in the move paths.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

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

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c      | 12 +---------
>   drivers/gpu/drm/ttm/ttm_bo_util.c | 40 +++++++++++++++++++------------
>   include/drm/ttm/ttm_bo_driver.h   |  3 +++
>   3 files changed, 29 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 6a7f4c028801..c8dffc8b40fc 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -252,19 +252,9 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>   		if (ret)
>   			goto out_err;
>   
> -		ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement);
> +		ret = ttm_bo_move_to_new_tt_mem(bo, ctx, mem);
>   		if (ret)
>   			goto out_err;
> -
> -		if (mem->mem_type != TTM_PL_SYSTEM) {
> -			ret = ttm_tt_populate(bdev, bo->ttm, ctx);
> -			if (ret)
> -				goto out_err;
> -
> -			ret = ttm_bo_tt_bind(bo, mem);
> -			if (ret)
> -				goto out_err;
> -		}
>   	}
>   
>   	if (bdev->driver->move_notify)
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index bdee4df1f3f2..aecdb2d92a54 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -50,11 +50,33 @@ void ttm_bo_free_old_node(struct ttm_buffer_object *bo)
>   	ttm_resource_free(bo, &bo->mem);
>   }
>   
> +int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
> +			      struct ttm_operation_ctx *ctx,
> +			      struct ttm_resource *new_mem)
> +{
> +	struct ttm_tt *ttm = bo->ttm;
> +	int ret;
> +
> +	ret = ttm_tt_set_placement_caching(ttm, new_mem->placement);
> +	if (unlikely(ret != 0))
> +		return ret;
> +
> +	if (new_mem->mem_type != TTM_PL_SYSTEM) {
> +		ret = ttm_tt_populate(bo->bdev, ttm, ctx);
> +		if (unlikely(ret != 0))
> +			return ret;
> +
> +		ret = ttm_bo_tt_bind(bo, new_mem);
> +		if (unlikely(ret != 0))
> +			return ret;
> +	}
> +	return 0;
> +}
> +
>   int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
>   		   struct ttm_operation_ctx *ctx,
>   		    struct ttm_resource *new_mem)
>   {
> -	struct ttm_tt *ttm = bo->ttm;
>   	struct ttm_resource *old_mem = &bo->mem;
>   	int ret;
>   
> @@ -72,21 +94,9 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
>   		old_mem->mem_type = TTM_PL_SYSTEM;
>   	}
>   
> -	ret = ttm_tt_set_placement_caching(ttm, new_mem->placement);
> -	if (unlikely(ret != 0))
> +	ret = ttm_bo_move_to_new_tt_mem(bo, ctx, new_mem);
> +	if (ret)
>   		return ret;
> -
> -	if (new_mem->mem_type != TTM_PL_SYSTEM) {
> -
> -		ret = ttm_tt_populate(bo->bdev, ttm, ctx);
> -		if (unlikely(ret != 0))
> -			return ret;
> -
> -		ret = ttm_bo_tt_bind(bo, new_mem);
> -		if (unlikely(ret != 0))
> -			return ret;
> -	}
> -
>   	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 864afa8f6f18..20e6839e9b73 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -605,6 +605,9 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
>   		    struct ttm_operation_ctx *ctx,
>   		    struct ttm_resource *new_mem);
>   
> +int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
> +			      struct ttm_operation_ctx *ctx,
> +			      struct ttm_resource *new_mem);
>   /**
>    * ttm_bo_move_memcpy
>    *

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

[PATCH 11/45] drm/ttm: split out the move to system from move ttm code

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

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

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index aecdb2d92a54..0ad02e27865d 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -73,27 +73,38 @@ int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
 	return 0;
 }
 
-int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
-		   struct ttm_operation_ctx *ctx,
-		    struct ttm_resource *new_mem)
+static int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
+				     struct ttm_operation_ctx *ctx)
 {
 	struct ttm_resource *old_mem = &bo->mem;
 	int ret;
 
-	if (old_mem->mem_type != TTM_PL_SYSTEM) {
-		ret = ttm_bo_wait_ctx(bo, ctx);
-
-		if (unlikely(ret != 0)) {
-			if (ret != -ERESTARTSYS)
-				pr_err("Failed to expire sync object before unbinding TTM\n");
-			return ret;
-		}
+	if (old_mem->mem_type == TTM_PL_SYSTEM)
+		return 0;
 
-		ttm_bo_tt_unbind(bo);
-		ttm_bo_free_old_node(bo);
-		old_mem->mem_type = TTM_PL_SYSTEM;
+	ret = ttm_bo_wait_ctx(bo, ctx);
+	if (unlikely(ret != 0)) {
+		if (ret != -ERESTARTSYS)
+			pr_err("Failed to expire sync object before unbinding TTM\n");
+		return ret;
 	}
 
+	ttm_bo_tt_unbind(bo);
+	ttm_bo_free_old_node(bo);
+	old_mem->mem_type = TTM_PL_SYSTEM;
+	return 0;
+}
+
+int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
+		   struct ttm_operation_ctx *ctx,
+		    struct ttm_resource *new_mem)
+{
+	int ret;
+
+	ret = ttm_bo_move_old_to_system(bo, ctx);
+	if (ret)
+		return ret;
+
 	ret = ttm_bo_move_to_new_tt_mem(bo, ctx, new_mem);
 	if (ret)
 		return ret;
-- 
2.27.0

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

Re: [PATCH 11/45] drm/ttm: split out the move to system from move ttm code

[Christian König]

What should that be good for?

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>   drivers/gpu/drm/ttm/ttm_bo_util.c | 39 ++++++++++++++++++++-----------
>   1 file changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index aecdb2d92a54..0ad02e27865d 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -73,27 +73,38 @@ int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
>   	return 0;
>   }
>   
> -int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
> -		   struct ttm_operation_ctx *ctx,
> -		    struct ttm_resource *new_mem)
> +static int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
> +				     struct ttm_operation_ctx *ctx)
>   {
>   	struct ttm_resource *old_mem = &bo->mem;
>   	int ret;
>   
> -	if (old_mem->mem_type != TTM_PL_SYSTEM) {
> -		ret = ttm_bo_wait_ctx(bo, ctx);
> -
> -		if (unlikely(ret != 0)) {
> -			if (ret != -ERESTARTSYS)
> -				pr_err("Failed to expire sync object before unbinding TTM\n");
> -			return ret;
> -		}
> +	if (old_mem->mem_type == TTM_PL_SYSTEM)
> +		return 0;
>   
> -		ttm_bo_tt_unbind(bo);
> -		ttm_bo_free_old_node(bo);
> -		old_mem->mem_type = TTM_PL_SYSTEM;
> +	ret = ttm_bo_wait_ctx(bo, ctx);
> +	if (unlikely(ret != 0)) {
> +		if (ret != -ERESTARTSYS)
> +			pr_err("Failed to expire sync object before unbinding TTM\n");
> +		return ret;
>   	}
>   
> +	ttm_bo_tt_unbind(bo);
> +	ttm_bo_free_old_node(bo);
> +	old_mem->mem_type = TTM_PL_SYSTEM;
> +	return 0;
> +}
> +
> +int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
> +		   struct ttm_operation_ctx *ctx,
> +		    struct ttm_resource *new_mem)
> +{
> +	int ret;
> +
> +	ret = ttm_bo_move_old_to_system(bo, ctx);
> +	if (ret)
> +		return ret;
> +
>   	ret = ttm_bo_move_to_new_tt_mem(bo, ctx, new_mem);
>   	if (ret)
>   		return ret;

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

[PATCH 12/45] drm/ttm: drop free old node wrapper.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This isn't really used anymore, if drivers needs it later,
just add back an inline wrapper.

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

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 0ad02e27865d..daf9a91857f8 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -45,11 +45,6 @@ struct ttm_transfer_obj {
 	struct ttm_buffer_object *bo;
 };
 
-void ttm_bo_free_old_node(struct ttm_buffer_object *bo)
-{
-	ttm_resource_free(bo, &bo->mem);
-}
-
 int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
 			      struct ttm_operation_ctx *ctx,
 			      struct ttm_resource *new_mem)
@@ -90,7 +85,7 @@ static int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
 	}
 
 	ttm_bo_tt_unbind(bo);
-	ttm_bo_free_old_node(bo);
+	ttm_resource_free(bo, &bo->mem);
 	old_mem->mem_type = TTM_PL_SYSTEM;
 	return 0;
 }
@@ -557,7 +552,7 @@ static int ttm_bo_wait_free_node(struct ttm_buffer_object *bo,
 
 	if (!dst_use_tt)
 		ttm_bo_tt_destroy(bo);
-	ttm_bo_free_old_node(bo);
+	ttm_resource_free(bo, &bo->mem);
 	return 0;
 }
 
@@ -618,7 +613,7 @@ static void ttm_bo_move_pipeline_evict(struct ttm_buffer_object *bo,
 	}
 	spin_unlock(&from->move_lock);
 
-	ttm_bo_free_old_node(bo);
+	ttm_resource_free(bo, &bo->mem);
 
 	dma_fence_put(bo->moving);
 	bo->moving = dma_fence_get(fence);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 20e6839e9b73..6690ec5d90ec 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -630,15 +630,6 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
 		       struct ttm_operation_ctx *ctx,
 		       struct ttm_resource *new_mem);
 
-/**
- * ttm_bo_free_old_node
- *
- * @bo: A pointer to a struct ttm_buffer_object.
- *
- * Utility function to free an old placement after a successful move.
- */
-void ttm_bo_free_old_node(struct ttm_buffer_object *bo);
-
 /**
  * ttm_bo_move_accel_cleanup.
  *
-- 
2.27.0

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

Re: [PATCH 12/45] drm/ttm: drop free old node wrapper.

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> This isn't really used anymore, if drivers needs it later,
> just add back an inline wrapper.
>
> 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 | 11 +++--------
>   include/drm/ttm/ttm_bo_driver.h   |  9 ---------
>   2 files changed, 3 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 0ad02e27865d..daf9a91857f8 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -45,11 +45,6 @@ struct ttm_transfer_obj {
>   	struct ttm_buffer_object *bo;
>   };
>   
> -void ttm_bo_free_old_node(struct ttm_buffer_object *bo)
> -{
> -	ttm_resource_free(bo, &bo->mem);
> -}
> -
>   int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
>   			      struct ttm_operation_ctx *ctx,
>   			      struct ttm_resource *new_mem)
> @@ -90,7 +85,7 @@ static int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
>   	}
>   
>   	ttm_bo_tt_unbind(bo);
> -	ttm_bo_free_old_node(bo);
> +	ttm_resource_free(bo, &bo->mem);
>   	old_mem->mem_type = TTM_PL_SYSTEM;
>   	return 0;
>   }
> @@ -557,7 +552,7 @@ static int ttm_bo_wait_free_node(struct ttm_buffer_object *bo,
>   
>   	if (!dst_use_tt)
>   		ttm_bo_tt_destroy(bo);
> -	ttm_bo_free_old_node(bo);
> +	ttm_resource_free(bo, &bo->mem);
>   	return 0;
>   }
>   
> @@ -618,7 +613,7 @@ static void ttm_bo_move_pipeline_evict(struct ttm_buffer_object *bo,
>   	}
>   	spin_unlock(&from->move_lock);
>   
> -	ttm_bo_free_old_node(bo);
> +	ttm_resource_free(bo, &bo->mem);
>   
>   	dma_fence_put(bo->moving);
>   	bo->moving = dma_fence_get(fence);
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 20e6839e9b73..6690ec5d90ec 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -630,15 +630,6 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>   		       struct ttm_operation_ctx *ctx,
>   		       struct ttm_resource *new_mem);
>   
> -/**
> - * ttm_bo_free_old_node
> - *
> - * @bo: A pointer to a struct ttm_buffer_object.
> - *
> - * Utility function to free an old placement after a successful move.
> - */
> -void ttm_bo_free_old_node(struct ttm_buffer_object *bo);
> -
>   /**
>    * ttm_bo_move_accel_cleanup.
>    *

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

[PATCH 13/45] drm/ttm: use new move interface for known system->ttm moves

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

In all 3 drivers there is a case where the driver knows the
bo is in SYSTEM so don't call the api that checks that.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index d3bd2fd448be..960a99d6793a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -607,11 +607,11 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo, bool evict,
 	}
 
 	/* move/bind old memory to GTT space */
-	r = ttm_bo_move_ttm(bo, ctx, &tmp_mem);
+	r = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_mem);
 	if (unlikely(r)) {
 		goto out_cleanup;
 	}
-
+	ttm_bo_assign_mem(bo, &tmp_mem);
 	/* copy to VRAM */
 	r = amdgpu_move_blit(bo, evict, new_mem, old_mem);
 	if (unlikely(r)) {
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 2c10a84b2cc0..2cb61eea9481 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -945,10 +945,11 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict,
 	if (ret)
 		return ret;
 
-	ret = ttm_bo_move_ttm(bo, ctx, &tmp_reg);
+	ret = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_reg);
 	if (ret)
 		goto out;
 
+	ttm_bo_assign_mem(bo, &tmp_reg);
 	ret = nouveau_bo_move_m2mf(bo, true, ctx, new_reg);
 	if (ret)
 		goto out;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index df5cedb2b632..7b778fc74f7b 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -280,10 +280,11 @@ static int radeon_move_ram_vram(struct ttm_buffer_object *bo,
 	if (unlikely(r)) {
 		return r;
 	}
-	r = ttm_bo_move_ttm(bo, ctx, &tmp_mem);
+	r = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_mem);
 	if (unlikely(r)) {
 		goto out_cleanup;
 	}
+	ttm_bo_assign_mem(bo, &tmp_mem);
 	r = radeon_move_blit(bo, true, new_mem, old_mem);
 	if (unlikely(r)) {
 		goto out_cleanup;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index daf9a91857f8..e76883836e6e 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -67,6 +67,7 @@ int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
 	}
 	return 0;
 }
+EXPORT_SYMBOL(ttm_bo_move_to_new_tt_mem);
 
 static int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
 				     struct ttm_operation_ctx *ctx)
-- 
2.27.0

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

Re: [PATCH 13/45] drm/ttm: use new move interface for known system->ttm moves

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> In all 3 drivers there is a case where the driver knows the
> bo is in SYSTEM so don't call the api that checks that.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

In the long term I completely want to get rid of this nonsense because 
it means that we call back into TTM which then calls us again.

But for now the patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++--
>   drivers/gpu/drm/nouveau/nouveau_bo.c    | 3 ++-
>   drivers/gpu/drm/radeon/radeon_ttm.c     | 3 ++-
>   drivers/gpu/drm/ttm/ttm_bo_util.c       | 1 +
>   4 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index d3bd2fd448be..960a99d6793a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -607,11 +607,11 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo, bool evict,
>   	}
>   
>   	/* move/bind old memory to GTT space */
> -	r = ttm_bo_move_ttm(bo, ctx, &tmp_mem);
> +	r = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_mem);
>   	if (unlikely(r)) {
>   		goto out_cleanup;
>   	}
> -
> +	ttm_bo_assign_mem(bo, &tmp_mem);
>   	/* copy to VRAM */
>   	r = amdgpu_move_blit(bo, evict, new_mem, old_mem);
>   	if (unlikely(r)) {
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 2c10a84b2cc0..2cb61eea9481 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -945,10 +945,11 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict,
>   	if (ret)
>   		return ret;
>   
> -	ret = ttm_bo_move_ttm(bo, ctx, &tmp_reg);
> +	ret = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_reg);
>   	if (ret)
>   		goto out;
>   
> +	ttm_bo_assign_mem(bo, &tmp_reg);
>   	ret = nouveau_bo_move_m2mf(bo, true, ctx, new_reg);
>   	if (ret)
>   		goto out;
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index df5cedb2b632..7b778fc74f7b 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -280,10 +280,11 @@ static int radeon_move_ram_vram(struct ttm_buffer_object *bo,
>   	if (unlikely(r)) {
>   		return r;
>   	}
> -	r = ttm_bo_move_ttm(bo, ctx, &tmp_mem);
> +	r = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_mem);
>   	if (unlikely(r)) {
>   		goto out_cleanup;
>   	}
> +	ttm_bo_assign_mem(bo, &tmp_mem);
>   	r = radeon_move_blit(bo, true, new_mem, old_mem);
>   	if (unlikely(r)) {
>   		goto out_cleanup;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index daf9a91857f8..e76883836e6e 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -67,6 +67,7 @@ int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
>   	}
>   	return 0;
>   }
> +EXPORT_SYMBOL(ttm_bo_move_to_new_tt_mem);
>   
>   static int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
>   				     struct ttm_operation_ctx *ctx)

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

[PATCH 14/45] drm/ttm: add move old to system to drivers.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Uninline ttm_bo_move_ttm. Eventually want to unhook the unbind out.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  9 ++++++++-
 drivers/gpu/drm/nouveau/nouveau_bo.c    |  9 ++++++++-
 drivers/gpu/drm/radeon/radeon_ttm.c     | 10 +++++++++-
 drivers/gpu/drm/ttm/ttm_bo_util.c       |  5 +++--
 include/drm/ttm/ttm_bo_driver.h         |  2 ++
 5 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 960a99d6793a..e20ce380f627 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -568,7 +568,14 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict,
 	}
 
 	/* move BO (in tmp_mem) to new_mem */
-	r = ttm_bo_move_ttm(bo, ctx, new_mem);
+	r = ttm_bo_move_old_to_system(bo, ctx);
+	if (unlikely(r))
+		goto out_cleanup;
+
+	r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
+	if (unlikely(r))
+		goto out_cleanup;
+	ttm_bo_assign_mem(bo, new_mem);
 out_cleanup:
 	ttm_resource_free(bo, &tmp_mem);
 	return r;
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 2cb61eea9481..a95d208c76a1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -915,7 +915,14 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict,
 	if (ret)
 		goto out;
 
-	ret = ttm_bo_move_ttm(bo, ctx, new_reg);
+	ret = ttm_bo_move_old_to_system(bo, ctx);
+	if (ret)
+		goto out;
+
+	ret = ttm_tt_set_placement_caching(bo->ttm, new_reg->placement);
+	if (ret)
+		goto out;
+	ttm_bo_assign_mem(bo, new_reg);
 out:
 	ttm_resource_free(bo, &tmp_reg);
 	return ret;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 7b778fc74f7b..89455f2d3bb6 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -249,7 +249,15 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
 	if (unlikely(r)) {
 		goto out_cleanup;
 	}
-	r = ttm_bo_move_ttm(bo, ctx, new_mem);
+	r = ttm_bo_move_old_to_system(bo, ctx);
+	if (unlikely(r))
+		goto out_cleanup;
+
+	r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
+	if (unlikely(r))
+		goto out_cleanup;
+	ttm_bo_assign_mem(bo, new_mem);
+
 out_cleanup:
 	ttm_resource_free(bo, &tmp_mem);
 	return r;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index e76883836e6e..1e701dd192d3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -69,8 +69,8 @@ int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
 }
 EXPORT_SYMBOL(ttm_bo_move_to_new_tt_mem);
 
-static int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
-				     struct ttm_operation_ctx *ctx)
+int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
+			      struct ttm_operation_ctx *ctx)
 {
 	struct ttm_resource *old_mem = &bo->mem;
 	int ret;
@@ -90,6 +90,7 @@ static int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
 	old_mem->mem_type = TTM_PL_SYSTEM;
 	return 0;
 }
+EXPORT_SYMBOL(ttm_bo_move_old_to_system);
 
 int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
 		   struct ttm_operation_ctx *ctx,
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 6690ec5d90ec..65cf86b3ba0b 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -605,6 +605,8 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
 		    struct ttm_operation_ctx *ctx,
 		    struct ttm_resource *new_mem);
 
+int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
+			      struct ttm_operation_ctx *ctx);
 int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
 			      struct ttm_operation_ctx *ctx,
 			      struct ttm_resource *new_mem);
-- 
2.27.0

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

Re: [PATCH 14/45] drm/ttm: add move old to system to drivers.

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> Uninline ttm_bo_move_ttm. Eventually want to unhook the unbind out.
>
> 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 |  9 ++++++++-
>   drivers/gpu/drm/nouveau/nouveau_bo.c    |  9 ++++++++-
>   drivers/gpu/drm/radeon/radeon_ttm.c     | 10 +++++++++-
>   drivers/gpu/drm/ttm/ttm_bo_util.c       |  5 +++--
>   include/drm/ttm/ttm_bo_driver.h         |  2 ++
>   5 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 960a99d6793a..e20ce380f627 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -568,7 +568,14 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict,
>   	}
>   
>   	/* move BO (in tmp_mem) to new_mem */
> -	r = ttm_bo_move_ttm(bo, ctx, new_mem);
> +	r = ttm_bo_move_old_to_system(bo, ctx);
> +	if (unlikely(r))
> +		goto out_cleanup;
> +
> +	r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
> +	if (unlikely(r))
> +		goto out_cleanup;
> +	ttm_bo_assign_mem(bo, new_mem);
>   out_cleanup:
>   	ttm_resource_free(bo, &tmp_mem);
>   	return r;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 2cb61eea9481..a95d208c76a1 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -915,7 +915,14 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict,
>   	if (ret)
>   		goto out;
>   
> -	ret = ttm_bo_move_ttm(bo, ctx, new_reg);
> +	ret = ttm_bo_move_old_to_system(bo, ctx);
> +	if (ret)
> +		goto out;
> +
> +	ret = ttm_tt_set_placement_caching(bo->ttm, new_reg->placement);
> +	if (ret)
> +		goto out;
> +	ttm_bo_assign_mem(bo, new_reg);
>   out:
>   	ttm_resource_free(bo, &tmp_reg);
>   	return ret;
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 7b778fc74f7b..89455f2d3bb6 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -249,7 +249,15 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
>   	if (unlikely(r)) {
>   		goto out_cleanup;
>   	}
> -	r = ttm_bo_move_ttm(bo, ctx, new_mem);
> +	r = ttm_bo_move_old_to_system(bo, ctx);
> +	if (unlikely(r))
> +		goto out_cleanup;
> +
> +	r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
> +	if (unlikely(r))
> +		goto out_cleanup;
> +	ttm_bo_assign_mem(bo, new_mem);
> +
>   out_cleanup:
>   	ttm_resource_free(bo, &tmp_mem);
>   	return r;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index e76883836e6e..1e701dd192d3 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -69,8 +69,8 @@ int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
>   }
>   EXPORT_SYMBOL(ttm_bo_move_to_new_tt_mem);
>   
> -static int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
> -				     struct ttm_operation_ctx *ctx)
> +int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
> +			      struct ttm_operation_ctx *ctx)
>   {
>   	struct ttm_resource *old_mem = &bo->mem;
>   	int ret;
> @@ -90,6 +90,7 @@ static int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
>   	old_mem->mem_type = TTM_PL_SYSTEM;
>   	return 0;
>   }
> +EXPORT_SYMBOL(ttm_bo_move_old_to_system);
>   
>   int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
>   		   struct ttm_operation_ctx *ctx,
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 6690ec5d90ec..65cf86b3ba0b 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -605,6 +605,8 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
>   		    struct ttm_operation_ctx *ctx,
>   		    struct ttm_resource *new_mem);
>   
> +int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
> +			      struct ttm_operation_ctx *ctx);
>   int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
>   			      struct ttm_operation_ctx *ctx,
>   			      struct ttm_resource *new_mem);

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

[PATCH 15/45] drm/ttm: push copy unbind into drivers.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This uninlines ttm_bo_move_old_to_system into 3 places

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e20ce380f627..d165edacc347 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -66,6 +66,8 @@
 static int amdgpu_ttm_backend_bind(struct ttm_bo_device *bdev,
 				   struct ttm_tt *ttm,
 				   struct ttm_resource *bo_mem);
+static void amdgpu_ttm_backend_unbind(struct ttm_bo_device *bdev,
+				      struct ttm_tt *ttm);
 
 static int amdgpu_ttm_init_on_chip(struct amdgpu_device *adev,
 				    unsigned int type,
@@ -568,10 +570,13 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict,
 	}
 
 	/* move BO (in tmp_mem) to new_mem */
-	r = ttm_bo_move_old_to_system(bo, ctx);
+	r = ttm_bo_wait_ctx(bo, ctx);
 	if (unlikely(r))
 		goto out_cleanup;
 
+	amdgpu_ttm_backend_unbind(bo->bdev, bo->ttm);
+	ttm_resource_free(bo, &bo->mem);
+
 	r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
 	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 a95d208c76a1..1e6c2561d692 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -46,7 +46,7 @@
 
 static int nouveau_ttm_tt_bind(struct ttm_bo_device *bdev, struct ttm_tt *ttm,
 			       struct ttm_resource *reg);
-
+static void nouveau_ttm_tt_unbind(struct ttm_bo_device *bdev, struct ttm_tt *ttm);
 /*
  * NV10-NV40 tiling helpers
  */
@@ -915,10 +915,13 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict,
 	if (ret)
 		goto out;
 
-	ret = ttm_bo_move_old_to_system(bo, ctx);
+	ret = ttm_bo_wait_ctx(bo, ctx);
 	if (ret)
 		goto out;
 
+	nouveau_ttm_tt_unbind(bo->bdev, bo->ttm);
+	ttm_resource_free(bo, &bo->mem);
+
 	ret = ttm_tt_set_placement_caching(bo->ttm, new_reg->placement);
 	if (ret)
 		goto out;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 89455f2d3bb6..10d25d3b83f2 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -59,6 +59,8 @@ 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);
+static void radeon_ttm_tt_unbind(struct ttm_bo_device *bdev,
+				 struct ttm_tt *ttm);
 
 struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
 {
@@ -249,10 +251,13 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
 	if (unlikely(r)) {
 		goto out_cleanup;
 	}
-	r = ttm_bo_move_old_to_system(bo, ctx);
+	r = ttm_bo_wait_ctx(bo, ctx);
 	if (unlikely(r))
 		goto out_cleanup;
 
+	radeon_ttm_tt_unbind(bo->bdev, bo->ttm);
+	ttm_resource_free(bo, &bo->mem);
+
 	r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
 	if (unlikely(r))
 		goto out_cleanup;
-- 
2.27.0

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

Re: [PATCH 15/45] drm/ttm: push copy unbind into drivers.

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> This uninlines ttm_bo_move_old_to_system into 3 places
>
> 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 | 7 ++++++-
>   drivers/gpu/drm/nouveau/nouveau_bo.c    | 7 +++++--
>   drivers/gpu/drm/radeon/radeon_ttm.c     | 7 ++++++-
>   3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index e20ce380f627..d165edacc347 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -66,6 +66,8 @@
>   static int amdgpu_ttm_backend_bind(struct ttm_bo_device *bdev,
>   				   struct ttm_tt *ttm,
>   				   struct ttm_resource *bo_mem);
> +static void amdgpu_ttm_backend_unbind(struct ttm_bo_device *bdev,
> +				      struct ttm_tt *ttm);
>   
>   static int amdgpu_ttm_init_on_chip(struct amdgpu_device *adev,
>   				    unsigned int type,
> @@ -568,10 +570,13 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict,
>   	}
>   
>   	/* move BO (in tmp_mem) to new_mem */
> -	r = ttm_bo_move_old_to_system(bo, ctx);
> +	r = ttm_bo_wait_ctx(bo, ctx);
>   	if (unlikely(r))
>   		goto out_cleanup;
>   
> +	amdgpu_ttm_backend_unbind(bo->bdev, bo->ttm);
> +	ttm_resource_free(bo, &bo->mem);
> +
>   	r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
>   	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 a95d208c76a1..1e6c2561d692 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -46,7 +46,7 @@
>   
>   static int nouveau_ttm_tt_bind(struct ttm_bo_device *bdev, struct ttm_tt *ttm,
>   			       struct ttm_resource *reg);
> -
> +static void nouveau_ttm_tt_unbind(struct ttm_bo_device *bdev, struct ttm_tt *ttm);
>   /*
>    * NV10-NV40 tiling helpers
>    */
> @@ -915,10 +915,13 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict,
>   	if (ret)
>   		goto out;
>   
> -	ret = ttm_bo_move_old_to_system(bo, ctx);
> +	ret = ttm_bo_wait_ctx(bo, ctx);
>   	if (ret)
>   		goto out;
>   
> +	nouveau_ttm_tt_unbind(bo->bdev, bo->ttm);
> +	ttm_resource_free(bo, &bo->mem);
> +
>   	ret = ttm_tt_set_placement_caching(bo->ttm, new_reg->placement);
>   	if (ret)
>   		goto out;
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 89455f2d3bb6..10d25d3b83f2 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -59,6 +59,8 @@ 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);
> +static void radeon_ttm_tt_unbind(struct ttm_bo_device *bdev,
> +				 struct ttm_tt *ttm);
>   
>   struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
>   {
> @@ -249,10 +251,13 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
>   	if (unlikely(r)) {
>   		goto out_cleanup;
>   	}
> -	r = ttm_bo_move_old_to_system(bo, ctx);
> +	r = ttm_bo_wait_ctx(bo, ctx);
>   	if (unlikely(r))
>   		goto out_cleanup;
>   
> +	radeon_ttm_tt_unbind(bo->bdev, bo->ttm);
> +	ttm_resource_free(bo, &bo->mem);
> +
>   	r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
>   	if (unlikely(r))
>   		goto out_cleanup;

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

[PATCH 16/45] drm/radeon/ttm: do move notify actions inside move.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

move notify can be gotten done inside moves instead of a separate
callback for radeon. move notify is now only called from one other
place where new_mem == NULL, so handle that properly.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_object.c | 28 +++++++++++++++++---------
 drivers/gpu/drm/radeon/radeon_object.h |  4 ++++
 drivers/gpu/drm/radeon/radeon_ttm.c    |  8 +++++++-
 3 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 689426dd8480..36a16d7a24b2 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -754,6 +754,21 @@ int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved,
 	return radeon_bo_get_surface_reg(bo);
 }
 
+
+void radeon_bo_invalidate(struct radeon_bo *rbo)
+{
+	radeon_bo_check_tiling(rbo, 0, 1);
+	radeon_vm_bo_invalidate(rbo->rdev, rbo);
+}
+
+void radeon_bo_memory_usage(struct radeon_bo *rbo,
+			    uint32_t old_mem_type,
+			    uint32_t new_mem_type)
+{
+	radeon_update_memory_usage(rbo, old_mem_type, -1);
+	radeon_update_memory_usage(rbo, new_mem_type, 1);
+}
+
 void radeon_bo_move_notify(struct ttm_buffer_object *bo,
 			   bool evict,
 			   struct ttm_resource *new_mem)
@@ -763,16 +778,11 @@ void radeon_bo_move_notify(struct ttm_buffer_object *bo,
 	if (!radeon_ttm_bo_is_radeon_bo(bo))
 		return;
 
-	rbo = container_of(bo, struct radeon_bo, tbo);
-	radeon_bo_check_tiling(rbo, 0, 1);
-	radeon_vm_bo_invalidate(rbo->rdev, rbo);
-
-	/* update statistics */
-	if (!new_mem)
+	/* the new_mem path is handled via the move callback now */
+	if (new_mem)
 		return;
-
-	radeon_update_memory_usage(rbo, bo->mem.mem_type, -1);
-	radeon_update_memory_usage(rbo, new_mem->mem_type, 1);
+	rbo = container_of(bo, struct radeon_bo, tbo);
+	radeon_bo_invalidate(rbo);
 }
 
 int radeon_bo_fault_reserve_notify(struct ttm_buffer_object *bo)
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index 27cfb64057fe..6f886e2ffaf3 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -160,6 +160,10 @@ extern void radeon_bo_get_tiling_flags(struct radeon_bo *bo,
 				u32 *tiling_flags, u32 *pitch);
 extern int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved,
 				bool force_drop);
+void radeon_bo_memory_usage(struct radeon_bo *rbo,
+			    uint32_t old_mem_type,
+			    uint32_t new_mem_type);
+void radeon_bo_invalidate(struct radeon_bo *rbo);
 extern void radeon_bo_move_notify(struct ttm_buffer_object *bo,
 				  bool evict,
 				  struct ttm_resource *new_mem);
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 10d25d3b83f2..e814b11187b3 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -316,12 +316,16 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
 	struct ttm_resource *old_mem = &bo->mem;
 	int r;
 
+	rbo = container_of(bo, struct radeon_bo, tbo);
+
+	radeon_bo_invalidate(rbo);
+	radeon_bo_memory_usage(rbo, bo->mem.mem_type, new_mem->mem_type);
+
 	r = ttm_bo_wait_ctx(bo, ctx);
 	if (r)
 		return r;
 
 	/* Can't move a pinned BO */
-	rbo = container_of(bo, struct radeon_bo, tbo);
 	if (WARN_ON_ONCE(rbo->tbo.pin_count > 0))
 		return -EINVAL;
 
@@ -361,6 +365,8 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
 memcpy:
 		r = ttm_bo_move_memcpy(bo, ctx, new_mem);
 		if (r) {
+			radeon_bo_invalidate(rbo);
+			radeon_bo_memory_usage(rbo, new_mem->mem_type, old_mem->mem_type);
 			return r;
 		}
 	}
-- 
2.27.0

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

[PATCH 17/45] drm/amdgpu/ttm: handle invalidation in move callback.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Leave the delete path alone (new_mem == NULL), but otherwise
do all the invalidate and accounting in the move callback.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 35 ++++++++++------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 13 ++++++--
 3 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 63e9c5793c30..42d530e2351a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1246,6 +1246,19 @@ int amdgpu_bo_get_metadata(struct amdgpu_bo *bo, void *buffer,
 	return 0;
 }
 
+void amdgpu_bo_move_invalidate(struct amdgpu_bo *abo,
+			       bool evict)
+{
+	struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
+
+	amdgpu_vm_bo_invalidate(adev, abo, evict);
+	amdgpu_bo_kunmap(abo);
+
+	if (abo->tbo.base.dma_buf && !abo->tbo.base.import_attach &&
+	    abo->tbo.mem.mem_type != TTM_PL_SYSTEM)
+		dma_buf_move_notify(abo->tbo.base.dma_buf);
+
+}
 /**
  * amdgpu_bo_move_notify - notification about a memory move
  * @bo: pointer to a buffer object
@@ -1260,32 +1273,14 @@ void amdgpu_bo_move_notify(struct ttm_buffer_object *bo,
 			   bool evict,
 			   struct ttm_resource *new_mem)
 {
-	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
 	struct amdgpu_bo *abo;
-	struct ttm_resource *old_mem = &bo->mem;
 
 	if (!amdgpu_bo_is_amdgpu_bo(bo))
 		return;
 
-	abo = ttm_to_amdgpu_bo(bo);
-	amdgpu_vm_bo_invalidate(adev, abo, evict);
-
-	amdgpu_bo_kunmap(abo);
-
-	if (abo->tbo.base.dma_buf && !abo->tbo.base.import_attach &&
-	    bo->mem.mem_type != TTM_PL_SYSTEM)
-		dma_buf_move_notify(abo->tbo.base.dma_buf);
-
-	/* remember the eviction */
-	if (evict)
-		atomic64_inc(&adev->num_evictions);
-
-	/* update statistics */
+	/* new_mem path is handled in move */
 	if (!new_mem)
-		return;
-
-	/* move_notify is called before move happens */
-	trace_amdgpu_bo_move(abo, new_mem->mem_type, old_mem->mem_type);
+		amdgpu_bo_move_invalidate(abo, false);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index e91750e43448..53d980661410 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -283,6 +283,8 @@ int amdgpu_bo_get_metadata(struct amdgpu_bo *bo, void *buffer,
 void amdgpu_bo_move_notify(struct ttm_buffer_object *bo,
 			   bool evict,
 			   struct ttm_resource *new_mem);
+void amdgpu_bo_move_invalidate(struct amdgpu_bo *abo,
+			       bool evict);
 void amdgpu_bo_release_notify(struct ttm_buffer_object *bo);
 int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo);
 void amdgpu_bo_fence(struct amdgpu_bo *bo, struct dma_fence *fence,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index d165edacc347..38ddced2775d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -679,6 +679,12 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
 
 	adev = amdgpu_ttm_adev(bo->bdev);
 
+	amdgpu_bo_move_invalidate(abo, evict);
+	/* remember the eviction */
+	if (evict)
+		atomic64_inc(&adev->num_evictions);
+	trace_amdgpu_bo_move(abo, new_mem->mem_type, bo->mem.mem_type);
+
 	if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) {
 		ttm_bo_move_null(bo, new_mem);
 		return 0;
@@ -726,12 +732,12 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
 		if (!amdgpu_mem_visible(adev, old_mem) ||
 		    !amdgpu_mem_visible(adev, new_mem)) {
 			pr_err("Move buffer fallback to memcpy unavailable\n");
-			return r;
+			goto out_invalidate;
 		}
 
 		r = ttm_bo_move_memcpy(bo, ctx, new_mem);
 		if (r)
-			return r;
+			goto out_invalidate;
 	}
 
 	if (bo->type == ttm_bo_type_device &&
@@ -746,6 +752,9 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
 	/* update statistics */
 	atomic64_add((u64)bo->num_pages << PAGE_SHIFT, &adev->num_bytes_moved);
 	return 0;
+out_invalidate:
+	amdgpu_bo_move_invalidate(abo, evict);
+	return r;
 }
 
 /**
-- 
2.27.0

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

[PATCH 18/45] drm/nouveau: handle move notify inside move callback.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Don't use explicit move notify for moves just do it in the driver side.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 62 ++++++++++++++++++++--------
 1 file changed, 44 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 1e6c2561d692..144b82db16ac 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -970,38 +970,42 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict,
 }
 
 static void
-nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool evict,
-		     struct ttm_resource *new_reg)
+nouveau_bo_vma_map_update(struct nouveau_bo *nvbo,
+			  uint32_t mem_type,
+			  struct nouveau_mem *mem)
 {
-	struct nouveau_mem *mem = new_reg ? nouveau_mem(new_reg) : NULL;
-	struct nouveau_bo *nvbo = nouveau_bo(bo);
 	struct nouveau_vma *vma;
 
-	/* ttm can now (stupidly) pass the driver bos it didn't create... */
-	if (bo->destroy != nouveau_bo_del_ttm)
-		return;
-
-	nouveau_bo_del_io_reserve_lru(bo);
-
-	if (mem && new_reg->mem_type != TTM_PL_SYSTEM &&
+	if (mem && mem_type != TTM_PL_SYSTEM &&
 	    mem->mem.page == nvbo->page) {
 		list_for_each_entry(vma, &nvbo->vma_list, head) {
 			nouveau_vma_map(vma, mem);
 		}
 	} else {
 		list_for_each_entry(vma, &nvbo->vma_list, head) {
-			WARN_ON(ttm_bo_wait(bo, false, false));
+			WARN_ON(ttm_bo_wait(&nvbo->bo, false, false));
 			nouveau_vma_unmap(vma);
 		}
 	}
+}
 
-	if (new_reg) {
-		if (new_reg->mm_node)
-			nvbo->offset = (new_reg->start << PAGE_SHIFT);
-		else
-			nvbo->offset = 0;
-	}
+static void
+nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool evict,
+		     struct ttm_resource *new_reg)
+{
+	struct nouveau_bo *nvbo = nouveau_bo(bo);
+
+	/* ttm can now (stupidly) pass the driver bos it didn't create... */
+	if (bo->destroy != nouveau_bo_del_ttm)
+		return;
+
+	/* handle new_reg path in move */
+	if (new_reg)
+		return;
+
+	nouveau_bo_del_io_reserve_lru(bo);
 
+	nouveau_bo_vma_map_update(nvbo, 0, NULL);
 }
 
 static int
@@ -1038,6 +1042,20 @@ nouveau_bo_vm_cleanup(struct ttm_buffer_object *bo,
 	*old_tile = new_tile;
 }
 
+
+static void
+nouveau_bo_update_mem(struct nouveau_bo *nvbo,
+		      struct ttm_resource *new_reg)
+{
+	nouveau_bo_vma_map_update(nvbo, new_reg->mem_type, nouveau_mem(new_reg));
+	if (new_reg) {
+		if (new_reg->mm_node)
+			nvbo->offset = (new_reg->start << PAGE_SHIFT);
+		else
+			nvbo->offset = 0;
+	}
+}
+
 static int
 nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
 		struct ttm_operation_ctx *ctx,
@@ -1053,6 +1071,9 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
 	if (ret)
 		return ret;
 
+	nouveau_bo_del_io_reserve_lru(bo);
+	nouveau_bo_update_mem(nvbo, new_reg);
+
 	if (nvbo->bo.pin_count)
 		NV_WARN(drm, "Moving pinned object %p!\n", nvbo);
 
@@ -1108,6 +1129,11 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
 			nouveau_bo_vm_cleanup(bo, new_tile, &nvbo->tile);
 	}
 
+	if (ret) {
+		nouveau_bo_del_io_reserve_lru(bo);
+		nouveau_bo_update_mem(nvbo, &bo->mem);
+	}
+
 	return ret;
 }
 
-- 
2.27.0

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

Re: [PATCH 18/45] drm/nouveau: handle move notify inside move callback.

[Ben Skeggs]

On Thu, 24 Sep 2020 at 15:20, Dave Airlie <airlied@gmail.com> wrote:
>
> From: Dave Airlie <airlied@redhat.com>
>
> Don't use explicit move notify for moves just do it in the driver side.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_bo.c | 62 ++++++++++++++++++++--------
>  1 file changed, 44 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 1e6c2561d692..144b82db16ac 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -970,38 +970,42 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict,
>  }
>
>  static void
> -nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool evict,
> -                    struct ttm_resource *new_reg)
> +nouveau_bo_vma_map_update(struct nouveau_bo *nvbo,
> +                         uint32_t mem_type,
> +                         struct nouveau_mem *mem)
>  {
> -       struct nouveau_mem *mem = new_reg ? nouveau_mem(new_reg) : NULL;
> -       struct nouveau_bo *nvbo = nouveau_bo(bo);
>         struct nouveau_vma *vma;
>
> -       /* ttm can now (stupidly) pass the driver bos it didn't create... */
> -       if (bo->destroy != nouveau_bo_del_ttm)
> -               return;
> -
> -       nouveau_bo_del_io_reserve_lru(bo);
> -
> -       if (mem && new_reg->mem_type != TTM_PL_SYSTEM &&
> +       if (mem && mem_type != TTM_PL_SYSTEM &&
>             mem->mem.page == nvbo->page) {
>                 list_for_each_entry(vma, &nvbo->vma_list, head) {
>                         nouveau_vma_map(vma, mem);
>                 }
>         } else {
>                 list_for_each_entry(vma, &nvbo->vma_list, head) {
> -                       WARN_ON(ttm_bo_wait(bo, false, false));
> +                       WARN_ON(ttm_bo_wait(&nvbo->bo, false, false));
I can look at this more closely myself a bit down the track, as I need
to do a lot in this area in the near future anyway, but it'd be nice
to pass the failure back here where possible to do so.  The new
invalidate_notify() hook still can't fail, but the other uses can and
it'd be nice to do the right thing where it's possible.

Ben.

>                         nouveau_vma_unmap(vma);
>                 }
>         }
> +}
>
> -       if (new_reg) {
> -               if (new_reg->mm_node)
> -                       nvbo->offset = (new_reg->start << PAGE_SHIFT);
> -               else
> -                       nvbo->offset = 0;
> -       }
> +static void
> +nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool evict,
> +                    struct ttm_resource *new_reg)
> +{
> +       struct nouveau_bo *nvbo = nouveau_bo(bo);
> +
> +       /* ttm can now (stupidly) pass the driver bos it didn't create... */
> +       if (bo->destroy != nouveau_bo_del_ttm)
> +               return;
> +
> +       /* handle new_reg path in move */
> +       if (new_reg)
> +               return;
> +
> +       nouveau_bo_del_io_reserve_lru(bo);
>
> +       nouveau_bo_vma_map_update(nvbo, 0, NULL);
>  }
>
>  static int
> @@ -1038,6 +1042,20 @@ nouveau_bo_vm_cleanup(struct ttm_buffer_object *bo,
>         *old_tile = new_tile;
>  }
>
> +
> +static void
> +nouveau_bo_update_mem(struct nouveau_bo *nvbo,
> +                     struct ttm_resource *new_reg)
> +{
> +       nouveau_bo_vma_map_update(nvbo, new_reg->mem_type, nouveau_mem(new_reg));
> +       if (new_reg) {
> +               if (new_reg->mm_node)
> +                       nvbo->offset = (new_reg->start << PAGE_SHIFT);
> +               else
> +                       nvbo->offset = 0;
> +       }
> +}
> +
>  static int
>  nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
>                 struct ttm_operation_ctx *ctx,
> @@ -1053,6 +1071,9 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
>         if (ret)
>                 return ret;
>
> +       nouveau_bo_del_io_reserve_lru(bo);
> +       nouveau_bo_update_mem(nvbo, new_reg);
> +
>         if (nvbo->bo.pin_count)
>                 NV_WARN(drm, "Moving pinned object %p!\n", nvbo);
>
> @@ -1108,6 +1129,11 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
>                         nouveau_bo_vm_cleanup(bo, new_tile, &nvbo->tile);
>         }
>
> +       if (ret) {
> +               nouveau_bo_del_io_reserve_lru(bo);
> +               nouveau_bo_update_mem(nvbo, &bo->mem);
> +       }
> +
>         return ret;
>  }
>
> --
> 2.27.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 19/45] drm/qxl/ttm: handle move notify inside move callback

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 5738be300078..378b6827b7a3 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -151,6 +151,11 @@ static int qxl_bo_move(struct ttm_buffer_object *bo, bool evict,
 {
 	struct ttm_resource *old_mem = &bo->mem;
 	int ret;
+	struct qxl_bo *qbo = to_qxl_bo(bo);
+	struct qxl_device *qdev = to_qxl(qbo->tbo.base.dev);
+
+	if (bo->mem.mem_type == TTM_PL_PRIV && qbo->surface_id)
+		qxl_surface_evict(qdev, qbo, true);
 
 	ret = ttm_bo_wait_ctx(bo, ctx);
 	if (ret)
@@ -175,8 +180,8 @@ static void qxl_bo_move_notify(struct ttm_buffer_object *bo,
 	qbo = to_qxl_bo(bo);
 	qdev = to_qxl(qbo->tbo.base.dev);
 
-	if (bo->mem.mem_type == TTM_PL_PRIV && qbo->surface_id)
-		qxl_surface_evict(qdev, qbo, new_mem ? true : false);
+	if (!new_mem && bo->mem.mem_type == TTM_PL_PRIV && qbo->surface_id)
+		qxl_surface_evict(qdev, qbo, false);
 }
 
 static struct ttm_bo_driver qxl_bo_driver = {
-- 
2.27.0

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

[PATCH 20/45] drm/vmwgfx/ttm: handle move notify inside move.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This means move notify isn't used for the cleanup path, since
mem would be NULL, so the callback can be removed

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 6e36fc932aeb..d3262e07e76d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -704,11 +704,8 @@ static int vmw_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_resourc
  * (currently only resources).
  */
 static void vmw_move_notify(struct ttm_buffer_object *bo,
-			    bool evict,
 			    struct ttm_resource *mem)
 {
-	if (!mem)
-		return;
 	vmw_bo_move_notify(bo, mem);
 	vmw_query_move_notify(bo, mem);
 }
@@ -732,15 +729,21 @@ static int vmw_move(struct ttm_buffer_object *bo,
 {
 	struct ttm_resource_manager *old_man = ttm_manager_type(bo->bdev, bo->mem.mem_type);
 	struct ttm_resource_manager *new_man = ttm_manager_type(bo->bdev, new_mem->mem_type);
+	int ret;
 
+	vmw_move_notify(bo, new_mem);
 	if (old_man->use_tt && new_man->use_tt) {
 		if (bo->mem.mem_type == TTM_PL_SYSTEM) {
 			ttm_bo_assign_mem(bo, new_mem);
 			return 0;
 		}
-		return ttm_bo_move_ttm(bo, ctx, new_mem);
+		ret = ttm_bo_move_ttm(bo, ctx, new_mem);
 	} else
-		return ttm_bo_move_memcpy(bo, ctx, new_mem);
+		ret = ttm_bo_move_memcpy(bo, ctx, new_mem);
+
+	if (ret)
+		vmw_move_notify(bo, &bo->mem);
+	return ret;
 }
 
 struct ttm_bo_driver vmw_bo_driver = {
@@ -754,7 +757,6 @@ struct ttm_bo_driver vmw_bo_driver = {
 	.evict_flags = vmw_evict_flags,
 	.move = vmw_move,
 	.verify_access = vmw_verify_access,
-	.move_notify = vmw_move_notify,
 	.swap_notify = vmw_swap_notify,
 	.io_mem_reserve = &vmw_ttm_io_mem_reserve,
 };
-- 
2.27.0

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

[PATCH 21/45] drm/vram_helper: call move notify from the move callback.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 9fd80a3643f6..5d4182f5c22f 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -605,6 +605,7 @@ static int drm_gem_vram_bo_driver_move(struct drm_gem_vram_object *gbo,
 				       struct ttm_operation_ctx *ctx,
 				       struct ttm_resource *new_mem)
 {
+	drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
 	return ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
 }
 
-- 
2.27.0

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

[PATCH 22/45] drm/ttm: don't call move notify around move

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Drivers should be handling this internally.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index c8dffc8b40fc..3b07db525417 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -257,19 +257,9 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 			goto out_err;
 	}
 
-	if (bdev->driver->move_notify)
-		bdev->driver->move_notify(bo, evict, mem);
-
 	ret = bdev->driver->move(bo, evict, ctx, mem);
-	if (ret) {
-		if (bdev->driver->move_notify) {
-			swap(*mem, bo->mem);
-			bdev->driver->move_notify(bo, false, mem);
-			swap(*mem, bo->mem);
-		}
-
+	if (ret)
 		goto out_err;
-	}
 
 	ctx->bytes_moved += bo->num_pages << PAGE_SHIFT;
 	return 0;
-- 
2.27.0

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

[PATCH 23/45] drm/ttm: move bind out of move_to_new_tt_mem

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Move the bind out of this for now.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 38ddced2775d..805730481c3f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -623,6 +623,10 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo, bool evict,
 	if (unlikely(r)) {
 		goto out_cleanup;
 	}
+	r = amdgpu_ttm_backend_bind(bo->bdev, bo->ttm, &tmp_mem);
+	if (unlikely(r))
+		goto out_cleanup;
+
 	ttm_bo_assign_mem(bo, &tmp_mem);
 	/* copy to VRAM */
 	r = amdgpu_move_blit(bo, evict, new_mem, old_mem);
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 144b82db16ac..dde7d59f1168 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -958,7 +958,9 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict,
 	ret = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_reg);
 	if (ret)
 		goto out;
-
+	ret = nouveau_ttm_tt_bind(bo->bdev, bo->ttm, &tmp_reg);
+	if (ret)
+		goto out;
 	ttm_bo_assign_mem(bo, &tmp_reg);
 	ret = nouveau_bo_move_m2mf(bo, true, ctx, new_reg);
 	if (ret)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index e814b11187b3..2c7c11cd65d8 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -297,6 +297,10 @@ static int radeon_move_ram_vram(struct ttm_buffer_object *bo,
 	if (unlikely(r)) {
 		goto out_cleanup;
 	}
+	r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, &tmp_mem);
+	if (unlikely(r))
+		goto out_cleanup;
+
 	ttm_bo_assign_mem(bo, &tmp_mem);
 	r = radeon_move_blit(bo, true, new_mem, old_mem);
 	if (unlikely(r)) {
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 3b07db525417..82ea0abb5070 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -255,6 +255,13 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 		ret = ttm_bo_move_to_new_tt_mem(bo, ctx, mem);
 		if (ret)
 			goto out_err;
+
+		if (mem->mem_type != TTM_PL_SYSTEM) {
+			ret = ttm_bo_tt_bind(bo, mem);
+			if (ret)
+				goto out_err;
+		}
+
 	}
 
 	ret = bdev->driver->move(bo, evict, ctx, mem);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 1e701dd192d3..2cf3c89e7944 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -60,11 +60,8 @@ int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
 		ret = ttm_tt_populate(bo->bdev, ttm, ctx);
 		if (unlikely(ret != 0))
 			return ret;
-
-		ret = ttm_bo_tt_bind(bo, new_mem);
-		if (unlikely(ret != 0))
-			return ret;
 	}
+
 	return 0;
 }
 EXPORT_SYMBOL(ttm_bo_move_to_new_tt_mem);
@@ -105,6 +102,13 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
 	ret = ttm_bo_move_to_new_tt_mem(bo, ctx, new_mem);
 	if (ret)
 		return ret;
+
+	if (new_mem->mem_type != TTM_PL_SYSTEM) {
+		ret = ttm_bo_tt_bind(bo, new_mem);
+		if (unlikely(ret != 0))
+			return ret;
+	}
+
 	ttm_bo_assign_mem(bo, new_mem);
 	return 0;
 }
-- 
2.27.0

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

[PATCH 24/45] drm/ttm: handle binding in move callback

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 805730481c3f..18b7d28a0c94 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -683,6 +683,14 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
 
 	adev = amdgpu_ttm_adev(bo->bdev);
 
+	if ((new_mem->mem_type == TTM_PL_TT ||
+	     new_mem->mem_type == TTM_PL_SYSTEM) &&
+	    old_mem->mem_type == TTM_PL_VRAM) {
+		r = amdgpu_ttm_backend_bind(bo->bdev, bo->ttm, new_mem);
+		if (r)
+			return r;
+	}
+
 	amdgpu_bo_move_invalidate(abo, evict);
 	/* remember the eviction */
 	if (evict)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index dde7d59f1168..1ce13223939b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1069,6 +1069,14 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
 	struct nouveau_drm_tile *new_tile = NULL;
 	int ret = 0;
 
+	if ((new_reg->mem_type == TTM_PL_TT ||
+	     new_reg->mem_type == TTM_PL_SYSTEM) &&
+	    old_reg->mem_type == TTM_PL_VRAM) {
+		ret = nouveau_ttm_tt_bind(bo->bdev, bo->ttm, new_reg);
+		if (ret)
+			return ret;
+	}
+
 	ret = ttm_bo_wait_ctx(bo, ctx);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 2c7c11cd65d8..435dc37eea34 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -322,6 +322,14 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
 
 	rbo = container_of(bo, struct radeon_bo, tbo);
 
+	if ((new_mem->mem_type == TTM_PL_TT ||
+	     new_mem->mem_type == TTM_PL_SYSTEM) &&
+	    old_mem->mem_type == TTM_PL_VRAM) {
+		r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, new_mem);
+		if (r)
+			return r;
+	}
+
 	radeon_bo_invalidate(rbo);
 	radeon_bo_memory_usage(rbo, bo->mem.mem_type, new_mem->mem_type);
 
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 2cf3c89e7944..ac921d6456b7 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -103,12 +103,6 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
 	if (ret)
 		return ret;
 
-	if (new_mem->mem_type != TTM_PL_SYSTEM) {
-		ret = ttm_bo_tt_bind(bo, new_mem);
-		if (unlikely(ret != 0))
-			return ret;
-	}
-
 	ttm_bo_assign_mem(bo, new_mem);
 	return 0;
 }
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index d3262e07e76d..2a99c24abbdf 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -731,6 +731,11 @@ static int vmw_move(struct ttm_buffer_object *bo,
 	struct ttm_resource_manager *new_man = ttm_manager_type(bo->bdev, new_mem->mem_type);
 	int ret;
 
+	if (new_man->use_tt && bo->mem.mem_type != TTM_PL_SYSTEM) {
+		ret = vmw_ttm_bind(bo->bdev, bo->ttm, new_mem);
+		if (ret)
+			return ret;
+	}
 	vmw_move_notify(bo, new_mem);
 	if (old_man->use_tt && new_man->use_tt) {
 		if (bo->mem.mem_type == TTM_PL_SYSTEM) {
-- 
2.27.0

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

[PATCH 25/45] drm/ttm: don't call ttm_bo_move_ttm from drivers

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Drop the interface completely

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 12 ++++++++++--
 drivers/gpu/drm/nouveau/nouveau_bo.c       | 10 ++++++++--
 drivers/gpu/drm/radeon/radeon_ttm.c        | 12 ++++++++++--
 drivers/gpu/drm/ttm/ttm_bo_util.c          | 19 -------------------
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 10 +++++++++-
 include/drm/ttm/ttm_bo_driver.h            | 21 ---------------------
 6 files changed, 37 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 18b7d28a0c94..50362f56d2d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -708,8 +708,16 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
 	}
 
 	if (old_mem->mem_type == TTM_PL_TT &&
-	    new_mem->mem_type == TTM_PL_SYSTEM)
-		return ttm_bo_move_ttm(bo, ctx, new_mem);
+	    new_mem->mem_type == TTM_PL_SYSTEM) {
+		r = ttm_bo_move_old_to_system(bo, ctx);
+		if (r)
+			return r;
+		r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
+		if (r)
+			return r;
+		ttm_bo_assign_mem(bo, new_mem);
+		return 0;
+	}
 
 	if (old_mem->mem_type == AMDGPU_PL_GDS ||
 	    old_mem->mem_type == AMDGPU_PL_GWS ||
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 1ce13223939b..fc0f9e9232db 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1107,8 +1107,14 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
 
 	if (old_reg->mem_type == TTM_PL_TT &&
 	    new_reg->mem_type == TTM_PL_SYSTEM) {
-		ret = ttm_bo_move_ttm(bo, ctx, new_reg);
-		goto out;
+		ret = ttm_bo_move_old_to_system(bo, ctx);
+		if (ret)
+			goto out;
+		ret = ttm_tt_set_placement_caching(bo->ttm, new_reg->placement);
+		if (ret)
+			goto out;
+		ttm_bo_assign_mem(bo, new_reg);
+		return 0;
 	}
 
 	/* Hardware assisted copy. */
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 435dc37eea34..0ea20dc15cb2 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -353,8 +353,16 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
 	}
 
 	if (old_mem->mem_type == TTM_PL_TT &&
-	    new_mem->mem_type == TTM_PL_SYSTEM)
-		return ttm_bo_move_ttm(bo, ctx, new_mem);
+	    new_mem->mem_type == TTM_PL_SYSTEM) {
+		r = ttm_bo_move_old_to_system(bo, ctx);
+		if (r)
+			return r;
+		r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
+		if (r)
+			return r;
+		ttm_bo_assign_mem(bo, new_mem);
+		return 0;
+	}
 
 	if (!rdev->ring[radeon_copy_ring_index(rdev)].ready ||
 	    rdev->asic->copy.copy == NULL) {
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index ac921d6456b7..4ceef9f7dce6 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -89,25 +89,6 @@ int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
 }
 EXPORT_SYMBOL(ttm_bo_move_old_to_system);
 
-int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
-		   struct ttm_operation_ctx *ctx,
-		    struct ttm_resource *new_mem)
-{
-	int ret;
-
-	ret = ttm_bo_move_old_to_system(bo, ctx);
-	if (ret)
-		return ret;
-
-	ret = ttm_bo_move_to_new_tt_mem(bo, ctx, new_mem);
-	if (ret)
-		return ret;
-
-	ttm_bo_assign_mem(bo, new_mem);
-	return 0;
-}
-EXPORT_SYMBOL(ttm_bo_move_ttm);
-
 int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
 		       struct ttm_resource *mem)
 {
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 2a99c24abbdf..0b8d5655e416 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -742,7 +742,15 @@ static int vmw_move(struct ttm_buffer_object *bo,
 			ttm_bo_assign_mem(bo, new_mem);
 			return 0;
 		}
-		ret = ttm_bo_move_ttm(bo, ctx, new_mem);
+		ret = ttm_bo_move_old_to_system(bo, ctx);
+		if (ret)
+			return ret;
+
+		ret = ttm_bo_move_to_new_tt_mem(bo, ctx, new_mem);
+		if (ret)
+			return ret;
+
+		ttm_bo_assign_mem(bo, new_mem);
 	} else
 		ret = ttm_bo_move_memcpy(bo, ctx, new_mem);
 
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 65cf86b3ba0b..58d2d3a5ed20 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -583,27 +583,6 @@ int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
 		       struct ttm_resource *mem);
 void ttm_mem_io_free(struct ttm_bo_device *bdev,
 		     struct ttm_resource *mem);
-/**
- * ttm_bo_move_ttm
- *
- * @bo: A pointer to a struct ttm_buffer_object.
- * @interruptible: Sleep interruptible if waiting.
- * @no_wait_gpu: Return immediately if the GPU is busy.
- * @new_mem: struct ttm_resource indicating where to move.
- *
- * Optimized move function for a buffer object with both old and
- * new placement backed by a TTM. The function will, if successful,
- * free any old aperture space, and set (@new_mem)->mm_node to NULL,
- * and update the (@bo)->mem placement flags. If unsuccessful, the old
- * data remains untouched, and it's up to the caller to free the
- * memory space indicated by @new_mem.
- * Returns:
- * !0: Failure.
- */
-
-int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
-		    struct ttm_operation_ctx *ctx,
-		    struct ttm_resource *new_mem);
 
 int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
 			      struct ttm_operation_ctx *ctx);
-- 
2.27.0

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

[PATCH 26/45] drm/ttm: remove bind/unbind callbacks.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

If a driver wants to bind/unbind then it should implement a move
callback.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 +++--
 drivers/gpu/drm/nouveau/nouveau_bo.c       |  8 ++---
 drivers/gpu/drm/qxl/qxl_ttm.c              | 20 -----------
 drivers/gpu/drm/radeon/radeon_ttm.c        |  8 ++---
 drivers/gpu/drm/ttm/ttm_bo.c               | 17 ----------
 drivers/gpu/drm/ttm/ttm_bo_util.c          |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  8 +++--
 include/drm/ttm/ttm_bo_driver.h            | 39 ----------------------
 8 files changed, 16 insertions(+), 93 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 50362f56d2d0..a729bdcdd017 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -701,6 +701,7 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
 		ttm_bo_move_null(bo, new_mem);
 		return 0;
 	}
+
 	if (old_mem->mem_type == TTM_PL_SYSTEM &&
 	    new_mem->mem_type == TTM_PL_TT) {
 		ttm_bo_move_null(bo, new_mem);
@@ -709,9 +710,12 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
 
 	if (old_mem->mem_type == TTM_PL_TT &&
 	    new_mem->mem_type == TTM_PL_SYSTEM) {
-		r = ttm_bo_move_old_to_system(bo, ctx);
+		r = ttm_bo_wait_ctx(bo, ctx);
 		if (r)
 			return r;
+		amdgpu_ttm_backend_unbind(bo->bdev, bo->ttm);
+		ttm_resource_free(bo, &bo->mem);
+
 		r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
 		if (r)
 			return r;
@@ -1740,8 +1744,6 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
 	.ttm_tt_create = &amdgpu_ttm_tt_create,
 	.ttm_tt_populate = &amdgpu_ttm_tt_populate,
 	.ttm_tt_unpopulate = &amdgpu_ttm_tt_unpopulate,
-	.ttm_tt_bind = &amdgpu_ttm_backend_bind,
-	.ttm_tt_unbind = &amdgpu_ttm_backend_unbind,
 	.ttm_tt_destroy = &amdgpu_ttm_backend_destroy,
 	.eviction_valuable = amdgpu_ttm_bo_eviction_valuable,
 	.evict_flags = &amdgpu_evict_flags,
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index fc0f9e9232db..cb2a0f1bf7ff 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1107,9 +1107,9 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
 
 	if (old_reg->mem_type == TTM_PL_TT &&
 	    new_reg->mem_type == TTM_PL_SYSTEM) {
-		ret = ttm_bo_move_old_to_system(bo, ctx);
-		if (ret)
-			goto out;
+		nouveau_ttm_tt_unbind(bo->bdev, bo->ttm);
+		ttm_resource_free(bo, &bo->mem);
+
 		ret = ttm_tt_set_placement_caching(bo->ttm, new_reg->placement);
 		if (ret)
 			goto out;
@@ -1438,8 +1438,6 @@ struct ttm_bo_driver nouveau_bo_driver = {
 	.ttm_tt_create = &nouveau_ttm_tt_create,
 	.ttm_tt_populate = &nouveau_ttm_tt_populate,
 	.ttm_tt_unpopulate = &nouveau_ttm_tt_unpopulate,
-	.ttm_tt_bind = &nouveau_ttm_tt_bind,
-	.ttm_tt_unbind = &nouveau_ttm_tt_unbind,
 	.ttm_tt_destroy = &nouveau_ttm_tt_destroy,
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = nouveau_bo_evict_flags,
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 378b6827b7a3..3bca5f8d8ac5 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -102,24 +102,6 @@ struct qxl_ttm_tt {
 	struct ttm_tt		        ttm;
 };
 
-static int qxl_ttm_backend_bind(struct ttm_bo_device *bdev,
-				struct ttm_tt *ttm,
-				struct ttm_resource *bo_mem)
-{
-	if (!ttm->num_pages) {
-		WARN(1, "nothing to bind %lu pages for mreg %p back %p!\n",
-		     ttm->num_pages, bo_mem, ttm);
-	}
-	/* Not implemented */
-	return -1;
-}
-
-static void qxl_ttm_backend_unbind(struct ttm_bo_device *bdev,
-				   struct ttm_tt *ttm)
-{
-	/* Not implemented */
-}
-
 static void qxl_ttm_backend_destroy(struct ttm_bo_device *bdev,
 				    struct ttm_tt *ttm)
 {
@@ -186,9 +168,7 @@ static void qxl_bo_move_notify(struct ttm_buffer_object *bo,
 
 static struct ttm_bo_driver qxl_bo_driver = {
 	.ttm_tt_create = &qxl_ttm_tt_create,
-	.ttm_tt_bind = &qxl_ttm_backend_bind,
 	.ttm_tt_destroy = &qxl_ttm_backend_destroy,
-	.ttm_tt_unbind = &qxl_ttm_backend_unbind,
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = &qxl_evict_flags,
 	.move = &qxl_bo_move,
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 0ea20dc15cb2..496e7b1e14ad 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -354,9 +354,9 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
 
 	if (old_mem->mem_type == TTM_PL_TT &&
 	    new_mem->mem_type == TTM_PL_SYSTEM) {
-		r = ttm_bo_move_old_to_system(bo, ctx);
-		if (r)
-			return r;
+		radeon_ttm_tt_unbind(bo->bdev, bo->ttm);
+		ttm_resource_free(bo, &bo->mem);
+
 		r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
 		if (r)
 			return r;
@@ -835,8 +835,6 @@ static struct ttm_bo_driver radeon_bo_driver = {
 	.ttm_tt_create = &radeon_ttm_tt_create,
 	.ttm_tt_populate = &radeon_ttm_tt_populate,
 	.ttm_tt_unpopulate = &radeon_ttm_tt_unpopulate,
-	.ttm_tt_bind = &radeon_ttm_tt_bind,
-	.ttm_tt_unbind = &radeon_ttm_tt_unbind,
 	.ttm_tt_destroy = &radeon_ttm_tt_destroy,
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = &radeon_evict_flags,
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 82ea0abb5070..a2a61a8d1394 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -255,13 +255,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 		ret = ttm_bo_move_to_new_tt_mem(bo, ctx, mem);
 		if (ret)
 			goto out_err;
-
-		if (mem->mem_type != TTM_PL_SYSTEM) {
-			ret = ttm_bo_tt_bind(bo, mem);
-			if (ret)
-				goto out_err;
-		}
-
 	}
 
 	ret = bdev->driver->move(bo, evict, ctx, mem);
@@ -1548,13 +1541,3 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
 	ttm_tt_destroy(bo->bdev, bo->ttm);
 	bo->ttm = NULL;
 }
-
-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);
-}
-
-void ttm_bo_tt_unbind(struct ttm_buffer_object *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 4ceef9f7dce6..05768decb24c 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -82,7 +82,6 @@ int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
 		return ret;
 	}
 
-	ttm_bo_tt_unbind(bo);
 	ttm_resource_free(bo, &bo->mem);
 	old_mem->mem_type = TTM_PL_SYSTEM;
 	return 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 0b8d5655e416..0ff8feac6ba4 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -742,10 +742,14 @@ static int vmw_move(struct ttm_buffer_object *bo,
 			ttm_bo_assign_mem(bo, new_mem);
 			return 0;
 		}
-		ret = ttm_bo_move_old_to_system(bo, ctx);
+
+		ret = ttm_bo_wait_ctx(bo, ctx);
 		if (ret)
 			return ret;
 
+		vmw_ttm_unbind(bo->bdev, bo->ttm);
+		ttm_resource_free(bo, &bo->mem);
+
 		ret = ttm_bo_move_to_new_tt_mem(bo, ctx, new_mem);
 		if (ret)
 			return ret;
@@ -763,8 +767,6 @@ struct ttm_bo_driver vmw_bo_driver = {
 	.ttm_tt_create = &vmw_ttm_tt_create,
 	.ttm_tt_populate = &vmw_ttm_populate,
 	.ttm_tt_unpopulate = &vmw_ttm_unpopulate,
-	.ttm_tt_bind = &vmw_ttm_bind,
-	.ttm_tt_unbind = &vmw_ttm_unbind,
 	.ttm_tt_destroy = &vmw_ttm_destroy,
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = vmw_evict_flags,
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 58d2d3a5ed20..e400dbd2a143 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -90,31 +90,6 @@ struct ttm_bo_driver {
 	 */
 	void (*ttm_tt_unpopulate)(struct ttm_bo_device *bdev, struct ttm_tt *ttm);
 
-	/**
-	 * ttm_tt_bind
-	 *
-	 * @bdev: Pointer to a ttm device
-	 * @ttm: Pointer to a struct ttm_tt.
-	 * @bo_mem: Pointer to a struct ttm_resource describing the
-	 * memory type and location for binding.
-	 *
-	 * Bind the backend pages into the aperture in the location
-	 * indicated by @bo_mem. This function should be able to handle
-	 * differences between aperture and system page sizes.
-	 */
-	int (*ttm_tt_bind)(struct ttm_bo_device *bdev, struct ttm_tt *ttm, struct ttm_resource *bo_mem);
-
-	/**
-	 * ttm_tt_unbind
-	 *
-	 * @bdev: Pointer to a ttm device
-	 * @ttm: Pointer to a struct ttm_tt.
-	 *
-	 * Unbind previously bound backend pages. This function should be
-	 * able to handle differences between aperture and system page sizes.
-	 */
-	void (*ttm_tt_unbind)(struct ttm_bo_device *bdev, struct ttm_tt *ttm);
-
 	/**
 	 * ttm_tt_destroy
 	 *
@@ -652,20 +627,6 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo);
  */
 pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp);
 
-/**
- * ttm_bo_tt_bind
- *
- * Bind the object tt to a memory resource.
- */
-int ttm_bo_tt_bind(struct ttm_buffer_object *bo, struct ttm_resource *mem);
-
-/**
- * ttm_bo_tt_bind
- *
- * Unbind the object tt from a memory resource.
- */
-void ttm_bo_tt_unbind(struct ttm_buffer_object *bo);
-
 /**
  * ttm_bo_tt_destroy.
  */
-- 
2.27.0

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

Re: [PATCH 26/45] drm/ttm: remove bind/unbind callbacks.

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> If a driver wants to bind/unbind then it should implement a move
> callback.
>
> 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    |  8 +++--
>   drivers/gpu/drm/nouveau/nouveau_bo.c       |  8 ++---
>   drivers/gpu/drm/qxl/qxl_ttm.c              | 20 -----------
>   drivers/gpu/drm/radeon/radeon_ttm.c        |  8 ++---
>   drivers/gpu/drm/ttm/ttm_bo.c               | 17 ----------
>   drivers/gpu/drm/ttm/ttm_bo_util.c          |  1 -
>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  8 +++--
>   include/drm/ttm/ttm_bo_driver.h            | 39 ----------------------
>   8 files changed, 16 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 50362f56d2d0..a729bdcdd017 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -701,6 +701,7 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
>   		ttm_bo_move_null(bo, new_mem);
>   		return 0;
>   	}
> +
>   	if (old_mem->mem_type == TTM_PL_SYSTEM &&
>   	    new_mem->mem_type == TTM_PL_TT) {
>   		ttm_bo_move_null(bo, new_mem);
> @@ -709,9 +710,12 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
>   
>   	if (old_mem->mem_type == TTM_PL_TT &&
>   	    new_mem->mem_type == TTM_PL_SYSTEM) {
> -		r = ttm_bo_move_old_to_system(bo, ctx);
> +		r = ttm_bo_wait_ctx(bo, ctx);
>   		if (r)
>   			return r;
> +		amdgpu_ttm_backend_unbind(bo->bdev, bo->ttm);
> +		ttm_resource_free(bo, &bo->mem);
> +
>   		r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
>   		if (r)
>   			return r;
> @@ -1740,8 +1744,6 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
>   	.ttm_tt_create = &amdgpu_ttm_tt_create,
>   	.ttm_tt_populate = &amdgpu_ttm_tt_populate,
>   	.ttm_tt_unpopulate = &amdgpu_ttm_tt_unpopulate,
> -	.ttm_tt_bind = &amdgpu_ttm_backend_bind,
> -	.ttm_tt_unbind = &amdgpu_ttm_backend_unbind,
>   	.ttm_tt_destroy = &amdgpu_ttm_backend_destroy,
>   	.eviction_valuable = amdgpu_ttm_bo_eviction_valuable,
>   	.evict_flags = &amdgpu_evict_flags,
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index fc0f9e9232db..cb2a0f1bf7ff 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -1107,9 +1107,9 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
>   
>   	if (old_reg->mem_type == TTM_PL_TT &&
>   	    new_reg->mem_type == TTM_PL_SYSTEM) {
> -		ret = ttm_bo_move_old_to_system(bo, ctx);
> -		if (ret)
> -			goto out;
> +		nouveau_ttm_tt_unbind(bo->bdev, bo->ttm);
> +		ttm_resource_free(bo, &bo->mem);
> +
>   		ret = ttm_tt_set_placement_caching(bo->ttm, new_reg->placement);
>   		if (ret)
>   			goto out;
> @@ -1438,8 +1438,6 @@ struct ttm_bo_driver nouveau_bo_driver = {
>   	.ttm_tt_create = &nouveau_ttm_tt_create,
>   	.ttm_tt_populate = &nouveau_ttm_tt_populate,
>   	.ttm_tt_unpopulate = &nouveau_ttm_tt_unpopulate,
> -	.ttm_tt_bind = &nouveau_ttm_tt_bind,
> -	.ttm_tt_unbind = &nouveau_ttm_tt_unbind,
>   	.ttm_tt_destroy = &nouveau_ttm_tt_destroy,
>   	.eviction_valuable = ttm_bo_eviction_valuable,
>   	.evict_flags = nouveau_bo_evict_flags,
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index 378b6827b7a3..3bca5f8d8ac5 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -102,24 +102,6 @@ struct qxl_ttm_tt {
>   	struct ttm_tt		        ttm;
>   };
>   
> -static int qxl_ttm_backend_bind(struct ttm_bo_device *bdev,
> -				struct ttm_tt *ttm,
> -				struct ttm_resource *bo_mem)
> -{
> -	if (!ttm->num_pages) {
> -		WARN(1, "nothing to bind %lu pages for mreg %p back %p!\n",
> -		     ttm->num_pages, bo_mem, ttm);
> -	}
> -	/* Not implemented */
> -	return -1;
> -}
> -
> -static void qxl_ttm_backend_unbind(struct ttm_bo_device *bdev,
> -				   struct ttm_tt *ttm)
> -{
> -	/* Not implemented */
> -}
> -
>   static void qxl_ttm_backend_destroy(struct ttm_bo_device *bdev,
>   				    struct ttm_tt *ttm)
>   {
> @@ -186,9 +168,7 @@ static void qxl_bo_move_notify(struct ttm_buffer_object *bo,
>   
>   static struct ttm_bo_driver qxl_bo_driver = {
>   	.ttm_tt_create = &qxl_ttm_tt_create,
> -	.ttm_tt_bind = &qxl_ttm_backend_bind,
>   	.ttm_tt_destroy = &qxl_ttm_backend_destroy,
> -	.ttm_tt_unbind = &qxl_ttm_backend_unbind,
>   	.eviction_valuable = ttm_bo_eviction_valuable,
>   	.evict_flags = &qxl_evict_flags,
>   	.move = &qxl_bo_move,
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 0ea20dc15cb2..496e7b1e14ad 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -354,9 +354,9 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
>   
>   	if (old_mem->mem_type == TTM_PL_TT &&
>   	    new_mem->mem_type == TTM_PL_SYSTEM) {
> -		r = ttm_bo_move_old_to_system(bo, ctx);
> -		if (r)
> -			return r;
> +		radeon_ttm_tt_unbind(bo->bdev, bo->ttm);
> +		ttm_resource_free(bo, &bo->mem);
> +
>   		r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
>   		if (r)
>   			return r;
> @@ -835,8 +835,6 @@ static struct ttm_bo_driver radeon_bo_driver = {
>   	.ttm_tt_create = &radeon_ttm_tt_create,
>   	.ttm_tt_populate = &radeon_ttm_tt_populate,
>   	.ttm_tt_unpopulate = &radeon_ttm_tt_unpopulate,
> -	.ttm_tt_bind = &radeon_ttm_tt_bind,
> -	.ttm_tt_unbind = &radeon_ttm_tt_unbind,
>   	.ttm_tt_destroy = &radeon_ttm_tt_destroy,
>   	.eviction_valuable = ttm_bo_eviction_valuable,
>   	.evict_flags = &radeon_evict_flags,
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 82ea0abb5070..a2a61a8d1394 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -255,13 +255,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>   		ret = ttm_bo_move_to_new_tt_mem(bo, ctx, mem);
>   		if (ret)
>   			goto out_err;
> -
> -		if (mem->mem_type != TTM_PL_SYSTEM) {
> -			ret = ttm_bo_tt_bind(bo, mem);
> -			if (ret)
> -				goto out_err;
> -		}
> -
>   	}
>   
>   	ret = bdev->driver->move(bo, evict, ctx, mem);
> @@ -1548,13 +1541,3 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
>   	ttm_tt_destroy(bo->bdev, bo->ttm);
>   	bo->ttm = NULL;
>   }
> -
> -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);
> -}
> -
> -void ttm_bo_tt_unbind(struct ttm_buffer_object *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 4ceef9f7dce6..05768decb24c 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -82,7 +82,6 @@ int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
>   		return ret;
>   	}
>   
> -	ttm_bo_tt_unbind(bo);
>   	ttm_resource_free(bo, &bo->mem);
>   	old_mem->mem_type = TTM_PL_SYSTEM;
>   	return 0;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index 0b8d5655e416..0ff8feac6ba4 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -742,10 +742,14 @@ static int vmw_move(struct ttm_buffer_object *bo,
>   			ttm_bo_assign_mem(bo, new_mem);
>   			return 0;
>   		}
> -		ret = ttm_bo_move_old_to_system(bo, ctx);
> +
> +		ret = ttm_bo_wait_ctx(bo, ctx);
>   		if (ret)
>   			return ret;
>   
> +		vmw_ttm_unbind(bo->bdev, bo->ttm);
> +		ttm_resource_free(bo, &bo->mem);
> +
>   		ret = ttm_bo_move_to_new_tt_mem(bo, ctx, new_mem);
>   		if (ret)
>   			return ret;
> @@ -763,8 +767,6 @@ struct ttm_bo_driver vmw_bo_driver = {
>   	.ttm_tt_create = &vmw_ttm_tt_create,
>   	.ttm_tt_populate = &vmw_ttm_populate,
>   	.ttm_tt_unpopulate = &vmw_ttm_unpopulate,
> -	.ttm_tt_bind = &vmw_ttm_bind,
> -	.ttm_tt_unbind = &vmw_ttm_unbind,
>   	.ttm_tt_destroy = &vmw_ttm_destroy,
>   	.eviction_valuable = ttm_bo_eviction_valuable,
>   	.evict_flags = vmw_evict_flags,
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 58d2d3a5ed20..e400dbd2a143 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -90,31 +90,6 @@ struct ttm_bo_driver {
>   	 */
>   	void (*ttm_tt_unpopulate)(struct ttm_bo_device *bdev, struct ttm_tt *ttm);
>   
> -	/**
> -	 * ttm_tt_bind
> -	 *
> -	 * @bdev: Pointer to a ttm device
> -	 * @ttm: Pointer to a struct ttm_tt.
> -	 * @bo_mem: Pointer to a struct ttm_resource describing the
> -	 * memory type and location for binding.
> -	 *
> -	 * Bind the backend pages into the aperture in the location
> -	 * indicated by @bo_mem. This function should be able to handle
> -	 * differences between aperture and system page sizes.
> -	 */
> -	int (*ttm_tt_bind)(struct ttm_bo_device *bdev, struct ttm_tt *ttm, struct ttm_resource *bo_mem);
> -
> -	/**
> -	 * ttm_tt_unbind
> -	 *
> -	 * @bdev: Pointer to a ttm device
> -	 * @ttm: Pointer to a struct ttm_tt.
> -	 *
> -	 * Unbind previously bound backend pages. This function should be
> -	 * able to handle differences between aperture and system page sizes.
> -	 */
> -	void (*ttm_tt_unbind)(struct ttm_bo_device *bdev, struct ttm_tt *ttm);
> -
>   	/**
>   	 * ttm_tt_destroy
>   	 *
> @@ -652,20 +627,6 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo);
>    */
>   pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp);
>   
> -/**
> - * ttm_bo_tt_bind
> - *
> - * Bind the object tt to a memory resource.
> - */
> -int ttm_bo_tt_bind(struct ttm_buffer_object *bo, struct ttm_resource *mem);
> -
> -/**
> - * ttm_bo_tt_bind
> - *
> - * Unbind the object tt from a memory resource.
> - */
> -void ttm_bo_tt_unbind(struct ttm_buffer_object *bo);
> -
>   /**
>    * ttm_bo_tt_destroy.
>    */

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

[PATCH 27/45] drm/radeon/ttm: cleanup move exit paths

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 496e7b1e14ad..490c7355c9cc 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -342,15 +342,12 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
 		return -EINVAL;
 
 	rdev = radeon_get_rdev(bo->bdev);
-	if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) {
-		ttm_bo_move_null(bo, new_mem);
-		return 0;
-	}
+	if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL)
+		goto out_assign;
+
 	if (old_mem->mem_type == TTM_PL_SYSTEM &&
-	    new_mem->mem_type == TTM_PL_TT) {
-		ttm_bo_move_null(bo, new_mem);
-		return 0;
-	}
+	    new_mem->mem_type == TTM_PL_TT)
+		goto out_assign;
 
 	if (old_mem->mem_type == TTM_PL_TT &&
 	    new_mem->mem_type == TTM_PL_SYSTEM) {
@@ -360,8 +357,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
 		r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
 		if (r)
 			return r;
-		ttm_bo_assign_mem(bo, new_mem);
-		return 0;
+		goto out_assign;
 	}
 
 	if (!rdev->ring[radeon_copy_ring_index(rdev)].ready ||
@@ -394,6 +390,9 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
 	/* update statistics */
 	atomic64_add((u64)bo->num_pages << PAGE_SHIFT, &rdev->num_bytes_moved);
 	return 0;
+out_assign:
+	ttm_bo_assign_mem(bo, new_mem);
+	return 0;
 }
 
 static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_resource *mem)
-- 
2.27.0

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

[PATCH 28/45] drm/nouveau/ttm: memcpy waits for bo already

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

no need for driver to wait here.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index cb2a0f1bf7ff..9e3fbe09a6d8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1133,9 +1133,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
 	}
 
 	/* Fallback to software copy. */
-	ret = ttm_bo_wait_ctx(bo, ctx);
-	if (ret == 0)
-		ret = ttm_bo_move_memcpy(bo, ctx, new_reg);
+	ret = ttm_bo_move_memcpy(bo, ctx, new_reg);
 
 out:
 	if (drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA) {
-- 
2.27.0

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

[PATCH 29/45] drm/ttm: don't expose move old to system helper to drivers.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This is just used internally by fallback paths.

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

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 05768decb24c..e9a10f5280b6 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -66,28 +66,6 @@ int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
 }
 EXPORT_SYMBOL(ttm_bo_move_to_new_tt_mem);
 
-int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
-			      struct ttm_operation_ctx *ctx)
-{
-	struct ttm_resource *old_mem = &bo->mem;
-	int ret;
-
-	if (old_mem->mem_type == TTM_PL_SYSTEM)
-		return 0;
-
-	ret = ttm_bo_wait_ctx(bo, ctx);
-	if (unlikely(ret != 0)) {
-		if (ret != -ERESTARTSYS)
-			pr_err("Failed to expire sync object before unbinding TTM\n");
-		return ret;
-	}
-
-	ttm_resource_free(bo, &bo->mem);
-	old_mem->mem_type = TTM_PL_SYSTEM;
-	return 0;
-}
-EXPORT_SYMBOL(ttm_bo_move_old_to_system);
-
 int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
 		       struct ttm_resource *mem)
 {
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index e400dbd2a143..cfb151dbb2d0 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -559,8 +559,6 @@ int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
 void ttm_mem_io_free(struct ttm_bo_device *bdev,
 		     struct ttm_resource *mem);
 
-int ttm_bo_move_old_to_system(struct ttm_buffer_object *bo,
-			      struct ttm_operation_ctx *ctx);
 int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
 			      struct ttm_operation_ctx *ctx,
 			      struct ttm_resource *new_mem);
-- 
2.27.0

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

[PATCH 30/45] drm/ttm: add a new invalidate notify callback.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c    | 4 +++-
 include/drm/ttm/ttm_bo_driver.h | 7 +++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index a2a61a8d1394..ba69c682e53b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -282,7 +282,9 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 
 static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
 {
-	if (bo->bdev->driver->move_notify)
+	if (bo->bdev->driver->invalidate_notify)
+		bo->bdev->driver->invalidate_notify(bo);
+	else if (bo->bdev->driver->move_notify)
 		bo->bdev->driver->move_notify(bo, false, NULL);
 
 	ttm_bo_tt_destroy(bo);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index cfb151dbb2d0..da4afe669664 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -165,6 +165,13 @@ struct ttm_bo_driver {
 	void (*move_notify)(struct ttm_buffer_object *bo,
 			    bool evict,
 			    struct ttm_resource *new_mem);
+
+	/**
+	 * Hook to notify driver about a bo being torn down.
+	 * can be used for invalidation instead of move_notify.
+	 */
+	void (*invalidate_notify)(struct ttm_buffer_object *bo);
+
 	/* notify the driver we are taking a fault on this BO
 	 * and have reserved it */
 	int (*fault_reserve_notify)(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 30/45] drm/ttm: add a new invalidate notify callback.

[Daniel Vetter]

On Thu, Sep 24, 2020 at 03:18:30PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>

A bikeshed, but why not just call this ->invalidate? ->move_notify needed
the _notify because we already had the ->move callback, but there's not
invalidate that also needs a invalidate_notify. And a callback kinda
implies that the driver gets notified about some stuff, but doesn't really
add anything about what it should do now. Plus if we go with notify, I
guess it should be called ->delete_notify, and use that to do it's
invalidation.

</bikeshed>

Otherwise I think this entire series is a solid demidlayer of all the move
stuff here.

Cheers, Daniel
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c    | 4 +++-
>  include/drm/ttm/ttm_bo_driver.h | 7 +++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index a2a61a8d1394..ba69c682e53b 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -282,7 +282,9 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>  
>  static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
>  {
> -	if (bo->bdev->driver->move_notify)
> +	if (bo->bdev->driver->invalidate_notify)
> +		bo->bdev->driver->invalidate_notify(bo);
> +	else if (bo->bdev->driver->move_notify)
>  		bo->bdev->driver->move_notify(bo, false, NULL);
>  
>  	ttm_bo_tt_destroy(bo);
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index cfb151dbb2d0..da4afe669664 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -165,6 +165,13 @@ struct ttm_bo_driver {
>  	void (*move_notify)(struct ttm_buffer_object *bo,
>  			    bool evict,
>  			    struct ttm_resource *new_mem);
> +
> +	/**
> +	 * Hook to notify driver about a bo being torn down.
> +	 * can be used for invalidation instead of move_notify.
> +	 */
> +	void (*invalidate_notify)(struct ttm_buffer_object *bo);
> +
>  	/* notify the driver we are taking a fault on this BO
>  	 * and have reserved it */
>  	int (*fault_reserve_notify)(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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 30/45] drm/ttm: add a new invalidate notify callback.

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

NAK, completely unnecessary.

We should rather do the remaining accounting in the already existing 
release_notify() callback.

That makes much more sense and if I'm not completely mistaken could 
actually fix a bug in amdgpu.

Christian.

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c    | 4 +++-
>   include/drm/ttm/ttm_bo_driver.h | 7 +++++++
>   2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index a2a61a8d1394..ba69c682e53b 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -282,7 +282,9 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>   
>   static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
>   {
> -	if (bo->bdev->driver->move_notify)
> +	if (bo->bdev->driver->invalidate_notify)
> +		bo->bdev->driver->invalidate_notify(bo);
> +	else if (bo->bdev->driver->move_notify)
>   		bo->bdev->driver->move_notify(bo, false, NULL);
>   
>   	ttm_bo_tt_destroy(bo);
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index cfb151dbb2d0..da4afe669664 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -165,6 +165,13 @@ struct ttm_bo_driver {
>   	void (*move_notify)(struct ttm_buffer_object *bo,
>   			    bool evict,
>   			    struct ttm_resource *new_mem);
> +
> +	/**
> +	 * Hook to notify driver about a bo being torn down.
> +	 * can be used for invalidation instead of move_notify.
> +	 */
> +	void (*invalidate_notify)(struct ttm_buffer_object *bo);
> +
>   	/* notify the driver we are taking a fault on this BO
>   	 * and have reserved it */
>   	int (*fault_reserve_notify)(struct ttm_buffer_object *bo);

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

Re: [PATCH 30/45] drm/ttm: add a new invalidate notify callback.

[Dave Airlie]

On Thu, 24 Sep 2020 at 22:25, Christian König <christian.koenig@amd.com> wrote:
>
> Am 24.09.20 um 07:18 schrieb Dave Airlie:
> > From: Dave Airlie <airlied@redhat.com>
> >
> > Signed-off-by: Dave Airlie <airlied@redhat.com>
>
> NAK, completely unnecessary.
>
> We should rather do the remaining accounting in the already existing
> release_notify() callback.
>
> That makes much more sense and if I'm not completely mistaken could
> actually fix a bug in amdgpu.

release_notify is called from one path, but there is a path in
eviction where if it gets pass 0 placements
it tears down the memory, and allocates a tt.

I'm considering whether it should be acceptable to call evict with no
placements (though maybe that just means evict to system).

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

Re: [PATCH 30/45] drm/ttm: add a new invalidate notify callback.

[Christian König]

Am 29.09.20 um 05:23 schrieb Dave Airlie:
> On Thu, 24 Sep 2020 at 22:25, Christian König <christian.koenig@amd.com> wrote:
>> Am 24.09.20 um 07:18 schrieb Dave Airlie:
>>> From: Dave Airlie <airlied@redhat.com>
>>>
>>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>> NAK, completely unnecessary.
>>
>> We should rather do the remaining accounting in the already existing
>> release_notify() callback.
>>
>> That makes much more sense and if I'm not completely mistaken could
>> actually fix a bug in amdgpu.
> release_notify is called from one path, but there is a path in
> eviction where if it gets pass 0 placements
> it tears down the memory, and allocates a tt.

You mean for the pipelined gutting? Mhm, I see. Probably better to call 
the move callback for those cases as well.

Ok, go ahead with that approach for now. But we really need to clean 
that up further.

Christian.

>
> I'm considering whether it should be acceptable to call evict with no
> placements (though maybe that just means evict to system).
>
> Dave.

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

[PATCH 31/45] drm/radeon: switch to invalidate notify callback

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_object.c | 7 +------
 drivers/gpu/drm/radeon/radeon_object.h | 4 +---
 drivers/gpu/drm/radeon/radeon_ttm.c    | 2 +-
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 36a16d7a24b2..44e2e7475dfe 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -769,18 +769,13 @@ void radeon_bo_memory_usage(struct radeon_bo *rbo,
 	radeon_update_memory_usage(rbo, new_mem_type, 1);
 }
 
-void radeon_bo_move_notify(struct ttm_buffer_object *bo,
-			   bool evict,
-			   struct ttm_resource *new_mem)
+void radeon_bo_invalidate_notify(struct ttm_buffer_object *bo)
 {
 	struct radeon_bo *rbo;
 
 	if (!radeon_ttm_bo_is_radeon_bo(bo))
 		return;
 
-	/* the new_mem path is handled via the move callback now */
-	if (new_mem)
-		return;
 	rbo = container_of(bo, struct radeon_bo, tbo);
 	radeon_bo_invalidate(rbo);
 }
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index 6f886e2ffaf3..a4ecff031f8f 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -164,9 +164,7 @@ void radeon_bo_memory_usage(struct radeon_bo *rbo,
 			    uint32_t old_mem_type,
 			    uint32_t new_mem_type);
 void radeon_bo_invalidate(struct radeon_bo *rbo);
-extern void radeon_bo_move_notify(struct ttm_buffer_object *bo,
-				  bool evict,
-				  struct ttm_resource *new_mem);
+void radeon_bo_invalidate_notify(struct ttm_buffer_object *bo);
 extern int radeon_bo_fault_reserve_notify(struct ttm_buffer_object *bo);
 extern int radeon_bo_get_surface_reg(struct radeon_bo *bo);
 extern void radeon_bo_fence(struct radeon_bo *bo, struct radeon_fence *fence,
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 490c7355c9cc..b38bc688f665 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -839,7 +839,7 @@ static struct ttm_bo_driver radeon_bo_driver = {
 	.evict_flags = &radeon_evict_flags,
 	.move = &radeon_bo_move,
 	.verify_access = &radeon_verify_access,
-	.move_notify = &radeon_bo_move_notify,
+	.invalidate_notify = &radeon_bo_invalidate_notify,
 	.fault_reserve_notify = &radeon_bo_fault_reserve_notify,
 	.io_mem_reserve = &radeon_ttm_io_mem_reserve,
 };
-- 
2.27.0

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

[PATCH 32/45] drm/amdgpu/ttm: switch to invalidate notify callback

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 8 ++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 4 +---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 2 +-
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 42d530e2351a..8b224a8ac727 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1269,18 +1269,14 @@ void amdgpu_bo_move_invalidate(struct amdgpu_bo *abo,
  * bookkeeping.
  * TTM driver callback which is called when ttm moves a buffer.
  */
-void amdgpu_bo_move_notify(struct ttm_buffer_object *bo,
-			   bool evict,
-			   struct ttm_resource *new_mem)
+void amdgpu_bo_invalidate_notify(struct ttm_buffer_object *bo)
 {
 	struct amdgpu_bo *abo;
 
 	if (!amdgpu_bo_is_amdgpu_bo(bo))
 		return;
 
-	/* new_mem path is handled in move */
-	if (!new_mem)
-		amdgpu_bo_move_invalidate(abo, false);
+	amdgpu_bo_move_invalidate(abo, false);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 53d980661410..571cdaeb68df 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -280,9 +280,7 @@ int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void *metadata,
 int amdgpu_bo_get_metadata(struct amdgpu_bo *bo, void *buffer,
 			   size_t buffer_size, uint32_t *metadata_size,
 			   uint64_t *flags);
-void amdgpu_bo_move_notify(struct ttm_buffer_object *bo,
-			   bool evict,
-			   struct ttm_resource *new_mem);
+void amdgpu_bo_invalidate_notify(struct ttm_buffer_object *bo);
 void amdgpu_bo_move_invalidate(struct amdgpu_bo *abo,
 			       bool evict);
 void amdgpu_bo_release_notify(struct ttm_buffer_object *bo);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index a729bdcdd017..e1133acb8536 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1749,7 +1749,7 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
 	.evict_flags = &amdgpu_evict_flags,
 	.move = &amdgpu_bo_move,
 	.verify_access = &amdgpu_verify_access,
-	.move_notify = &amdgpu_bo_move_notify,
+	.invalidate_notify = &amdgpu_bo_invalidate_notify,
 	.release_notify = &amdgpu_bo_release_notify,
 	.fault_reserve_notify = &amdgpu_bo_fault_reserve_notify,
 	.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
-- 
2.27.0

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

[PATCH 33/45] drm/nouveau/ttm: switch to invalidate notify callback

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 9e3fbe09a6d8..a032fdacf5f8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -992,8 +992,7 @@ nouveau_bo_vma_map_update(struct nouveau_bo *nvbo,
 }
 
 static void
-nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool evict,
-		     struct ttm_resource *new_reg)
+nouveau_bo_invalidate_ntfy(struct ttm_buffer_object *bo)
 {
 	struct nouveau_bo *nvbo = nouveau_bo(bo);
 
@@ -1001,10 +1000,6 @@ nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool evict,
 	if (bo->destroy != nouveau_bo_del_ttm)
 		return;
 
-	/* handle new_reg path in move */
-	if (new_reg)
-		return;
-
 	nouveau_bo_del_io_reserve_lru(bo);
 
 	nouveau_bo_vma_map_update(nvbo, 0, NULL);
@@ -1439,7 +1434,7 @@ struct ttm_bo_driver nouveau_bo_driver = {
 	.ttm_tt_destroy = &nouveau_ttm_tt_destroy,
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = nouveau_bo_evict_flags,
-	.move_notify = nouveau_bo_move_ntfy,
+	.invalidate_notify = nouveau_bo_invalidate_ntfy,
 	.move = nouveau_bo_move,
 	.verify_access = nouveau_bo_verify_access,
 	.fault_reserve_notify = &nouveau_ttm_fault_reserve_notify,
-- 
2.27.0

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

[PATCH 34/45] drm/qxl/ttm: move to invalidate notify callback

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 3bca5f8d8ac5..be012b2c8516 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -150,9 +150,7 @@ static int qxl_bo_move(struct ttm_buffer_object *bo, bool evict,
 	return ttm_bo_move_memcpy(bo, ctx, new_mem);
 }
 
-static void qxl_bo_move_notify(struct ttm_buffer_object *bo,
-			       bool evict,
-			       struct ttm_resource *new_mem)
+static void qxl_bo_invalidate_notify(struct ttm_buffer_object *bo)
 {
 	struct qxl_bo *qbo;
 	struct qxl_device *qdev;
@@ -162,7 +160,7 @@ static void qxl_bo_move_notify(struct ttm_buffer_object *bo,
 	qbo = to_qxl_bo(bo);
 	qdev = to_qxl(qbo->tbo.base.dev);
 
-	if (!new_mem && bo->mem.mem_type == TTM_PL_PRIV && qbo->surface_id)
+	if (bo->mem.mem_type == TTM_PL_PRIV && qbo->surface_id)
 		qxl_surface_evict(qdev, qbo, false);
 }
 
@@ -173,7 +171,7 @@ static struct ttm_bo_driver qxl_bo_driver = {
 	.evict_flags = &qxl_evict_flags,
 	.move = &qxl_bo_move,
 	.io_mem_reserve = &qxl_ttm_io_mem_reserve,
-	.move_notify = &qxl_bo_move_notify,
+	.invalidate_notify = &qxl_bo_invalidate_notify,
 };
 
 static int qxl_ttm_init_mem_type(struct qxl_device *qdev,
-- 
2.27.0

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

[PATCH 35/45] drm/vram-helper: move to invalidate callback.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 5d4182f5c22f..9d4100071e1d 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -433,7 +433,7 @@ static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
 	 * Permanently mapping and unmapping buffers adds overhead from
 	 * updating the page tables and creates debugging output. Therefore,
 	 * we delay the actual unmap operation until the BO gets evicted
-	 * from memory. See drm_gem_vram_bo_driver_move_notify().
+	 * from memory. See drm_gem_vram_bo_driver_invalidate_notify().
 	 */
 }
 
@@ -585,9 +585,7 @@ static void drm_gem_vram_bo_driver_evict_flags(struct drm_gem_vram_object *gbo,
 	*pl = gbo->placement;
 }
 
-static void drm_gem_vram_bo_driver_move_notify(struct drm_gem_vram_object *gbo,
-					       bool evict,
-					       struct ttm_resource *new_mem)
+static void drm_gem_vram_bo_driver_invalidate_notify(struct drm_gem_vram_object *gbo)
 {
 	struct ttm_bo_kmap_obj *kmap = &gbo->kmap;
 
@@ -605,7 +603,7 @@ static int drm_gem_vram_bo_driver_move(struct drm_gem_vram_object *gbo,
 				       struct ttm_operation_ctx *ctx,
 				       struct ttm_resource *new_mem)
 {
-	drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
+	drm_gem_vram_bo_driver_invalidate_notify(gbo);
 	return ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
 }
 
@@ -956,9 +954,7 @@ static void bo_driver_evict_flags(struct ttm_buffer_object *bo,
 	drm_gem_vram_bo_driver_evict_flags(gbo, placement);
 }
 
-static void bo_driver_move_notify(struct ttm_buffer_object *bo,
-				  bool evict,
-				  struct ttm_resource *new_mem)
+static void bo_driver_invalidate_notify(struct ttm_buffer_object *bo)
 {
 	struct drm_gem_vram_object *gbo;
 
@@ -968,7 +964,7 @@ static void bo_driver_move_notify(struct ttm_buffer_object *bo,
 
 	gbo = drm_gem_vram_of_bo(bo);
 
-	drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
+	drm_gem_vram_bo_driver_invalidate_notify(gbo);
 }
 
 static int bo_driver_move(struct ttm_buffer_object *bo,
@@ -1008,7 +1004,7 @@ static struct ttm_bo_driver bo_driver = {
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = bo_driver_evict_flags,
 	.move = bo_driver_move,
-	.move_notify = bo_driver_move_notify,
+	.invalidate_notify = bo_driver_invalidate_notify,
 	.io_mem_reserve = bo_driver_io_mem_reserve,
 };
 
-- 
2.27.0

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

Re: [PATCH 35/45] drm/vram-helper: move to invalidate callback.

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 3313 bytes --]

Hi

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/drm_gem_vram_helper.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 5d4182f5c22f..9d4100071e1d 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -433,7 +433,7 @@ static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
>  	 * Permanently mapping and unmapping buffers adds overhead from
>  	 * updating the page tables and creates debugging output. Therefore,
>  	 * we delay the actual unmap operation until the BO gets evicted
> -	 * from memory. See drm_gem_vram_bo_driver_move_notify().
> +	 * from memory. See drm_gem_vram_bo_driver_invalidate_notify().
>  	 */
>  }
>  
> @@ -585,9 +585,7 @@ static void drm_gem_vram_bo_driver_evict_flags(struct drm_gem_vram_object *gbo,
>  	*pl = gbo->placement;
>  }
>  
> -static void drm_gem_vram_bo_driver_move_notify(struct drm_gem_vram_object *gbo,
> -					       bool evict,
> -					       struct ttm_resource *new_mem)
> +static void drm_gem_vram_bo_driver_invalidate_notify(struct drm_gem_vram_object *gbo)
>  {
>  	struct ttm_bo_kmap_obj *kmap = &gbo->kmap;
>  
> @@ -605,7 +603,7 @@ static int drm_gem_vram_bo_driver_move(struct drm_gem_vram_object *gbo,
>  				       struct ttm_operation_ctx *ctx,
>  				       struct ttm_resource *new_mem)
>  {
> -	drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
> +	drm_gem_vram_bo_driver_invalidate_notify(gbo);
>  	return ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
>  }

I don't fully understand TTM's order of operation, so this might be a
dumb question: why is invalidate_notify() called from within the move()
callback? I'd expect that the invalidate_notify() callback is called by
TTM before moving the BO?

>  
> @@ -956,9 +954,7 @@ static void bo_driver_evict_flags(struct ttm_buffer_object *bo,
>  	drm_gem_vram_bo_driver_evict_flags(gbo, placement);
>  }
>  
> -static void bo_driver_move_notify(struct ttm_buffer_object *bo,
> -				  bool evict,
> -				  struct ttm_resource *new_mem)
> +static void bo_driver_invalidate_notify(struct ttm_buffer_object *bo)
>  {
>  	struct drm_gem_vram_object *gbo;
>  
> @@ -968,7 +964,7 @@ static void bo_driver_move_notify(struct ttm_buffer_object *bo,
>  
>  	gbo = drm_gem_vram_of_bo(bo);
>  
> -	drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
> +	drm_gem_vram_bo_driver_invalidate_notify(gbo);
>  }
>  
>  static int bo_driver_move(struct ttm_buffer_object *bo,
> @@ -1008,7 +1004,7 @@ static struct ttm_bo_driver bo_driver = {
>  	.eviction_valuable = ttm_bo_eviction_valuable,
>  	.evict_flags = bo_driver_evict_flags,
>  	.move = bo_driver_move,
> -	.move_notify = bo_driver_move_notify,
> +	.invalidate_notify = bo_driver_invalidate_notify,
>  	.io_mem_reserve = bo_driver_io_mem_reserve,
>  };
>  
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCH 36/45] drm/ttm: drop move_notify callback.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This isn't used anymore.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c    |  2 --
 include/drm/ttm/ttm_bo_driver.h | 11 -----------
 2 files changed, 13 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index ba69c682e53b..eb76002aa53d 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -284,8 +284,6 @@ static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
 {
 	if (bo->bdev->driver->invalidate_notify)
 		bo->bdev->driver->invalidate_notify(bo);
-	else if (bo->bdev->driver->move_notify)
-		bo->bdev->driver->move_notify(bo, false, NULL);
 
 	ttm_bo_tt_destroy(bo);
 	ttm_resource_free(bo, &bo->mem);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index da4afe669664..4a63fec24e90 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -155,17 +155,6 @@ struct ttm_bo_driver {
 	int (*verify_access)(struct ttm_buffer_object *bo,
 			     struct file *filp);
 
-	/**
-	 * Hook to notify driver about a driver move so it
-	 * can do tiling things and book-keeping.
-	 *
-	 * @evict: whether this move is evicting the buffer from the graphics
-	 * address space
-	 */
-	void (*move_notify)(struct ttm_buffer_object *bo,
-			    bool evict,
-			    struct ttm_resource *new_mem);
-
 	/**
 	 * Hook to notify driver about a bo being torn down.
 	 * can be used for invalidation instead of move_notify.
-- 
2.27.0

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

[PATCH 37/45] drm/ttm: add a helper to allocate a temp tt for copies.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

All the accel moves do the same pattern here, provide a helper

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c    | 28 ++++++++++++++++++++++++++++
 include/drm/ttm/ttm_bo_driver.h |  5 +++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index eb76002aa53d..358d1580dc16 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1541,3 +1541,31 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
 	ttm_tt_destroy(bo->bdev, bo->ttm);
 	bo->ttm = NULL;
 }
+
+int ttm_bo_create_tt_tmp(struct ttm_buffer_object *bo,
+			 struct ttm_operation_ctx *ctx,
+			 struct ttm_resource *new_mem,
+			 struct ttm_resource *new_temp)
+{
+	struct ttm_place placement_memtype = {
+		.fpfn = 0,
+		.lpfn = 0,
+		.mem_type = TTM_PL_TT,
+		.flags = TTM_PL_MASK_CACHING
+	};
+	struct ttm_placement placement;
+	int ret;
+
+	placement.num_placement = placement.num_busy_placement = 1;
+	placement.placement = placement.busy_placement = &placement_memtype;
+
+	*new_temp = *new_mem;
+	new_temp->mm_node = NULL;
+
+	ret = ttm_bo_mem_space(bo, &placement, new_temp, ctx);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL(ttm_bo_create_tt_tmp);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 4a63fec24e90..a7507dfaa89d 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -558,6 +558,11 @@ void ttm_mem_io_free(struct ttm_bo_device *bdev,
 int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
 			      struct ttm_operation_ctx *ctx,
 			      struct ttm_resource *new_mem);
+
+int ttm_bo_create_tt_tmp(struct ttm_buffer_object *bo,
+			 struct ttm_operation_ctx *ctx,
+			 struct ttm_resource *new_mem,
+			 struct ttm_resource *new_temp);
 /**
  * ttm_bo_move_memcpy
  *
-- 
2.27.0

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

Re: [PATCH 37/45] drm/ttm: add a helper to allocate a temp tt for copies.

[Christian König]

Am 24.09.20 um 07:18 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> All the accel moves do the same pattern here, provide a helper

And exactly that pattern I want to get away from.

See what happens if we (for example) have a VRAM -> SYSTEM move is the 
following:

1. TTM allocates a new ttm_resource object in the SYSTEM domain.
2. We call the driver to move from VRAM to SYSTEM.
3. Driver finds that it can't do this and calls TTM  to allocate GTT.
4. Since we are maybe out of GTT TTM evicts a different BO from GTT to 
SYSTEM and call driver again.

This is a horrible ping/pong between driver/TTM/driver/TTM/driver and we 
should stop that immediately.

My suggestion is that we rewrite how drivers call the ttm_bo_validate() 
function so that we can guarantee that this never happens.

What do you think?

Thanks,
Christian.

>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>   drivers/gpu/drm/ttm/ttm_bo.c    | 28 ++++++++++++++++++++++++++++
>   include/drm/ttm/ttm_bo_driver.h |  5 +++++
>   2 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index eb76002aa53d..358d1580dc16 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1541,3 +1541,31 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
>   	ttm_tt_destroy(bo->bdev, bo->ttm);
>   	bo->ttm = NULL;
>   }
> +
> +int ttm_bo_create_tt_tmp(struct ttm_buffer_object *bo,
> +			 struct ttm_operation_ctx *ctx,
> +			 struct ttm_resource *new_mem,
> +			 struct ttm_resource *new_temp)
> +{
> +	struct ttm_place placement_memtype = {
> +		.fpfn = 0,
> +		.lpfn = 0,
> +		.mem_type = TTM_PL_TT,
> +		.flags = TTM_PL_MASK_CACHING
> +	};
> +	struct ttm_placement placement;
> +	int ret;
> +
> +	placement.num_placement = placement.num_busy_placement = 1;
> +	placement.placement = placement.busy_placement = &placement_memtype;
> +
> +	*new_temp = *new_mem;
> +	new_temp->mm_node = NULL;
> +
> +	ret = ttm_bo_mem_space(bo, &placement, new_temp, ctx);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ttm_bo_create_tt_tmp);
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 4a63fec24e90..a7507dfaa89d 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -558,6 +558,11 @@ void ttm_mem_io_free(struct ttm_bo_device *bdev,
>   int ttm_bo_move_to_new_tt_mem(struct ttm_buffer_object *bo,
>   			      struct ttm_operation_ctx *ctx,
>   			      struct ttm_resource *new_mem);
> +
> +int ttm_bo_create_tt_tmp(struct ttm_buffer_object *bo,
> +			 struct ttm_operation_ctx *ctx,
> +			 struct ttm_resource *new_mem,
> +			 struct ttm_resource *new_temp);
>   /**
>    * ttm_bo_move_memcpy
>    *

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

Re: [PATCH 37/45] drm/ttm: add a helper to allocate a temp tt for copies.

[Dave Airlie]

On Thu, 24 Sep 2020 at 22:42, Christian König <christian.koenig@amd.com> wrote:
>
> Am 24.09.20 um 07:18 schrieb Dave Airlie:
> > From: Dave Airlie <airlied@redhat.com>
> >
> > All the accel moves do the same pattern here, provide a helper
>
> And exactly that pattern I want to get away from.

Currently this is just refactoring out the helper code in each driver, but I see
since it calls bo_mem_space we are probably moving a bit in the wrong direction.

> See what happens if we (for example) have a VRAM -> SYSTEM move is the
> following:
>
> 1. TTM allocates a new ttm_resource object in the SYSTEM domain.
> 2. We call the driver to move from VRAM to SYSTEM.
> 3. Driver finds that it can't do this and calls TTM  to allocate GTT.
> 4. Since we are maybe out of GTT TTM evicts a different BO from GTT to
> SYSTEM and call driver again.
>
> This is a horrible ping/pong between driver/TTM/driver/TTM/driver and we
> should stop that immediately.
>
> My suggestion is that we rewrite how drivers call the ttm_bo_validate()
> function so that we can guarantee that this never happens.
>
> What do you think?

I think that is likely the next step I'd like to take after this
refactor, it's a lot bigger, and I'm not sure how it will look yet.

Do we envision the driver calling validate in a loop but when it can't
find space it tells the driver and the driver does eviction and
recalls validate?

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

Re: [PATCH 37/45] drm/ttm: add a helper to allocate a temp tt for copies.

[Christian König]

Am 25.09.20 um 01:14 schrieb Dave Airlie:
> On Thu, 24 Sep 2020 at 22:42, Christian König <christian.koenig@amd.com> wrote:
>> Am 24.09.20 um 07:18 schrieb Dave Airlie:
>>> From: Dave Airlie <airlied@redhat.com>
>>>
>>> All the accel moves do the same pattern here, provide a helper
>> And exactly that pattern I want to get away from.
> Currently this is just refactoring out the helper code in each driver, but I see
> since it calls bo_mem_space we are probably moving a bit in the wrong direction.

Exactly that's why I'm noting this.

>
>> See what happens if we (for example) have a VRAM -> SYSTEM move is the
>> following:
>>
>> 1. TTM allocates a new ttm_resource object in the SYSTEM domain.
>> 2. We call the driver to move from VRAM to SYSTEM.
>> 3. Driver finds that it can't do this and calls TTM  to allocate GTT.
>> 4. Since we are maybe out of GTT TTM evicts a different BO from GTT to
>> SYSTEM and call driver again.
>>
>> This is a horrible ping/pong between driver/TTM/driver/TTM/driver and we
>> should stop that immediately.
>>
>> My suggestion is that we rewrite how drivers call the ttm_bo_validate()
>> function so that we can guarantee that this never happens.
>>
>> What do you think?
> I think that is likely the next step I'd like to take after this
> refactor, it's a lot bigger, and I'm not sure how it will look yet.

Agree, yes. I have some ideas in mind for that, but not fully baked either.

> Do we envision the driver calling validate in a loop but when it can't
> find space it tells the driver and the driver does eviction and
> recalls validate?

Not in a loop, but more like in a chain.

My plan is something like this:
Instead of having "normal" and "busy" placement we have a flag in the 
context if evictions are allowed or not.
The call to ttm_bo_validate are then replaced with two calls, first 
without evictions and if that didn't worked one with evictions.

Then the normal validate sequence should look like this:
1. If a BO is in the SYSTEM (or SWAP domain) we validate it to GTT first 
with evictions=true.
2. If a BO should be in VRAM we then validate it to VRAM. If evictions 
are only allowed if the GEM flags say that GTT is not desired.

For special BOs, like amdgpus GDS, GWS and OA domain or VMWGFX special 
domains that will obviously look a bit different.

Christian.

>
> Dave.

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

Re: [PATCH 37/45] drm/ttm: add a helper to allocate a temp tt for copies.

[Daniel Vetter]

On Fri, Sep 25, 2020 at 9:39 AM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 25.09.20 um 01:14 schrieb Dave Airlie:
> > On Thu, 24 Sep 2020 at 22:42, Christian König <christian.koenig@amd.com> wrote:
> >> Am 24.09.20 um 07:18 schrieb Dave Airlie:
> >>> From: Dave Airlie <airlied@redhat.com>
> >>>
> >>> All the accel moves do the same pattern here, provide a helper
> >> And exactly that pattern I want to get away from.
> > Currently this is just refactoring out the helper code in each driver, but I see
> > since it calls bo_mem_space we are probably moving a bit in the wrong direction.
>
> Exactly that's why I'm noting this.
>
> >
> >> See what happens if we (for example) have a VRAM -> SYSTEM move is the
> >> following:
> >>
> >> 1. TTM allocates a new ttm_resource object in the SYSTEM domain.
> >> 2. We call the driver to move from VRAM to SYSTEM.
> >> 3. Driver finds that it can't do this and calls TTM  to allocate GTT.
> >> 4. Since we are maybe out of GTT TTM evicts a different BO from GTT to
> >> SYSTEM and call driver again.
> >>
> >> This is a horrible ping/pong between driver/TTM/driver/TTM/driver and we
> >> should stop that immediately.
> >>
> >> My suggestion is that we rewrite how drivers call the ttm_bo_validate()
> >> function so that we can guarantee that this never happens.
> >>
> >> What do you think?
> > I think that is likely the next step I'd like to take after this
> > refactor, it's a lot bigger, and I'm not sure how it will look yet.
>
> Agree, yes. I have some ideas in mind for that, but not fully baked either.
>
> > Do we envision the driver calling validate in a loop but when it can't
> > find space it tells the driver and the driver does eviction and
> > recalls validate?
>
> Not in a loop, but more like in a chain.
>
> My plan is something like this:
> Instead of having "normal" and "busy" placement we have a flag in the
> context if evictions are allowed or not.
> The call to ttm_bo_validate are then replaced with two calls, first
> without evictions and if that didn't worked one with evictions.
>
> Then the normal validate sequence should look like this:
> 1. If a BO is in the SYSTEM (or SWAP domain) we validate it to GTT first
> with evictions=true.
> 2. If a BO should be in VRAM we then validate it to VRAM. If evictions
> are only allowed if the GEM flags say that GTT is not desired.

That solves the trouble when you move a bo into vram as part of
validate. But I'm not seeing how this solves the "need gtt mapping to
move something out of vram" problem.

Or should we instead move the entire eviction logic out from ttm into
drivers, building it up from helpers? Then drivers which need gtt for
moving stuff out of vram can do that right away. Also, this would
allow us to implement very fancy eviction algorithms like all the
nonsense we're doing in i915 for gtt handling on gen2/3 (but I really
hope that never ever becomes a thing again in future gpus, so this is
maybe more a what-if kind of thing). Not sure how that would look
like, maybe a special validate function which takes a ttm_resource the
driver already found (through evicting stuff or whatever) and then ttm
just does the move and book-keeping and everything. And drivers would
at first only call validate without allowing any eviction. Ofc anyone
without special needs could use the standard eviction function that
validate already has.
-Daniel

> For special BOs, like amdgpus GDS, GWS and OA domain or VMWGFX special
> domains that will obviously look a bit different.
>
> Christian.
>
> >
> > Dave.
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 37/45] drm/ttm: add a helper to allocate a temp tt for copies.

[Daniel Vetter]

On Fri, Sep 25, 2020 at 10:16 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Fri, Sep 25, 2020 at 9:39 AM Christian König
> <christian.koenig@amd.com> wrote:
> >
> > Am 25.09.20 um 01:14 schrieb Dave Airlie:
> > > On Thu, 24 Sep 2020 at 22:42, Christian König <christian.koenig@amd.com> wrote:
> > >> Am 24.09.20 um 07:18 schrieb Dave Airlie:
> > >>> From: Dave Airlie <airlied@redhat.com>
> > >>>
> > >>> All the accel moves do the same pattern here, provide a helper
> > >> And exactly that pattern I want to get away from.
> > > Currently this is just refactoring out the helper code in each driver, but I see
> > > since it calls bo_mem_space we are probably moving a bit in the wrong direction.
> >
> > Exactly that's why I'm noting this.
> >
> > >
> > >> See what happens if we (for example) have a VRAM -> SYSTEM move is the
> > >> following:
> > >>
> > >> 1. TTM allocates a new ttm_resource object in the SYSTEM domain.
> > >> 2. We call the driver to move from VRAM to SYSTEM.
> > >> 3. Driver finds that it can't do this and calls TTM  to allocate GTT.
> > >> 4. Since we are maybe out of GTT TTM evicts a different BO from GTT to
> > >> SYSTEM and call driver again.
> > >>
> > >> This is a horrible ping/pong between driver/TTM/driver/TTM/driver and we
> > >> should stop that immediately.
> > >>
> > >> My suggestion is that we rewrite how drivers call the ttm_bo_validate()
> > >> function so that we can guarantee that this never happens.
> > >>
> > >> What do you think?
> > > I think that is likely the next step I'd like to take after this
> > > refactor, it's a lot bigger, and I'm not sure how it will look yet.
> >
> > Agree, yes. I have some ideas in mind for that, but not fully baked either.
> >
> > > Do we envision the driver calling validate in a loop but when it can't
> > > find space it tells the driver and the driver does eviction and
> > > recalls validate?
> >
> > Not in a loop, but more like in a chain.
> >
> > My plan is something like this:
> > Instead of having "normal" and "busy" placement we have a flag in the
> > context if evictions are allowed or not.
> > The call to ttm_bo_validate are then replaced with two calls, first
> > without evictions and if that didn't worked one with evictions.
> >
> > Then the normal validate sequence should look like this:
> > 1. If a BO is in the SYSTEM (or SWAP domain) we validate it to GTT first
> > with evictions=true.
> > 2. If a BO should be in VRAM we then validate it to VRAM. If evictions
> > are only allowed if the GEM flags say that GTT is not desired.
>
> That solves the trouble when you move a bo into vram as part of
> validate. But I'm not seeing how this solves the "need gtt mapping to
> move something out of vram" problem.
>
> Or should we instead move the entire eviction logic out from ttm into
> drivers, building it up from helpers? Then drivers which need gtt for
> moving stuff out of vram can do that right away. Also, this would
> allow us to implement very fancy eviction algorithms like all the
> nonsense we're doing in i915 for gtt handling on gen2/3 (but I really
> hope that never ever becomes a thing again in future gpus, so this is
> maybe more a what-if kind of thing). Not sure how that would look
> like, maybe a special validate function which takes a ttm_resource the
> driver already found (through evicting stuff or whatever) and then ttm
> just does the move and book-keeping and everything. And drivers would
> at first only call validate without allowing any eviction. Ofc anyone
> without special needs could use the standard eviction function that
> validate already has.

Spinning this a bit more, we could have different default eviction
functions with this, e.g. so all the drivers that need gtt mapping for
moving stuff around can share that code, but with specific&flat
control flow instead of lots of ping-ping. And drivers that don't need
gtt mapping (like i915, we just need dma_map_sg which we assume works
always, or something from the ttm dma page pool, which really always
works) can then use something simpler that's completely flat.
-Daniel

>
> > For special BOs, like amdgpus GDS, GWS and OA domain or VMWGFX special
> > domains that will obviously look a bit different.
> >
> > Christian.
> >
> > >
> > > Dave.
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 37/45] drm/ttm: add a helper to allocate a temp tt for copies.

[Christian König]

Am 25.09.20 um 10:18 schrieb Daniel Vetter:
> On Fri, Sep 25, 2020 at 10:16 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Fri, Sep 25, 2020 at 9:39 AM Christian König
>> <christian.koenig@amd.com> wrote:
>>> Am 25.09.20 um 01:14 schrieb Dave Airlie:
>>>> On Thu, 24 Sep 2020 at 22:42, Christian König <christian.koenig@amd.com> wrote:
>>>>> Am 24.09.20 um 07:18 schrieb Dave Airlie:
>>>>>> From: Dave Airlie <airlied@redhat.com>
>>>>>>
>>>>>> All the accel moves do the same pattern here, provide a helper
>>>>> And exactly that pattern I want to get away from.
>>>> Currently this is just refactoring out the helper code in each driver, but I see
>>>> since it calls bo_mem_space we are probably moving a bit in the wrong direction.
>>> Exactly that's why I'm noting this.
>>>
>>>>> See what happens if we (for example) have a VRAM -> SYSTEM move is the
>>>>> following:
>>>>>
>>>>> 1. TTM allocates a new ttm_resource object in the SYSTEM domain.
>>>>> 2. We call the driver to move from VRAM to SYSTEM.
>>>>> 3. Driver finds that it can't do this and calls TTM  to allocate GTT.
>>>>> 4. Since we are maybe out of GTT TTM evicts a different BO from GTT to
>>>>> SYSTEM and call driver again.
>>>>>
>>>>> This is a horrible ping/pong between driver/TTM/driver/TTM/driver and we
>>>>> should stop that immediately.
>>>>>
>>>>> My suggestion is that we rewrite how drivers call the ttm_bo_validate()
>>>>> function so that we can guarantee that this never happens.
>>>>>
>>>>> What do you think?
>>>> I think that is likely the next step I'd like to take after this
>>>> refactor, it's a lot bigger, and I'm not sure how it will look yet.
>>> Agree, yes. I have some ideas in mind for that, but not fully baked either.
>>>
>>>> Do we envision the driver calling validate in a loop but when it can't
>>>> find space it tells the driver and the driver does eviction and
>>>> recalls validate?
>>> Not in a loop, but more like in a chain.
>>>
>>> My plan is something like this:
>>> Instead of having "normal" and "busy" placement we have a flag in the
>>> context if evictions are allowed or not.
>>> The call to ttm_bo_validate are then replaced with two calls, first
>>> without evictions and if that didn't worked one with evictions.
>>>
>>> Then the normal validate sequence should look like this:
>>> 1. If a BO is in the SYSTEM (or SWAP domain) we validate it to GTT first
>>> with evictions=true.
>>> 2. If a BO should be in VRAM we then validate it to VRAM. If evictions
>>> are only allowed if the GEM flags say that GTT is not desired.
>> That solves the trouble when you move a bo into vram as part of
>> validate. But I'm not seeing how this solves the "need gtt mapping to
>> move something out of vram" problem.

Eviction is not a problem because the driver gets asked where to put an 
evicted BO and then TTM does all the moving.

>>
>> Or should we instead move the entire eviction logic out from ttm into
>> drivers, building it up from helpers?

I've been playing with that thought for a while as well, but then 
decided against it.

The main problem I see is that we sometimes need to evict things from 
other drivers.

E.g. when we overcommitted system memory and move things to swap for 
example.

>> Then drivers which need gtt for
>> moving stuff out of vram can do that right away. Also, this would
>> allow us to implement very fancy eviction algorithms like all the
>> nonsense we're doing in i915 for gtt handling on gen2/3 (but I really
>> hope that never ever becomes a thing again in future gpus, so this is
>> maybe more a what-if kind of thing). Not sure how that would look
>> like, maybe a special validate function which takes a ttm_resource the
>> driver already found (through evicting stuff or whatever) and then ttm
>> just does the move and book-keeping and everything. And drivers would
>> at first only call validate without allowing any eviction. Ofc anyone
>> without special needs could use the standard eviction function that
>> validate already has.
> Spinning this a bit more, we could have different default eviction
> functions with this, e.g. so all the drivers that need gtt mapping for
> moving stuff around can share that code, but with specific&flat
> control flow instead of lots of ping-ping. And drivers that don't need
> gtt mapping (like i915, we just need dma_map_sg which we assume works
> always, or something from the ttm dma page pool, which really always
> works) can then use something simpler that's completely flat.

Ok you need to explain a bit more what exactly the problem with the GTT 
eviction is here :)

Christian.

> -Daniel
>
>>> For special BOs, like amdgpus GDS, GWS and OA domain or VMWGFX special
>>> domains that will obviously look a bit different.
>>>
>>> Christian.
>>>
>>>> Dave.
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7C66b5c2bfed8541cd986208d8612b8e1d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637366187002213152&amp;sdata=OPdXMe2rG1Uy%2BZD%2BjYKHfkLc9xVAQ1AL23QbEu3drnE%3D&amp;reserved=0
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ffwll.ch%2F&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7C66b5c2bfed8541cd986208d8612b8e1d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637366187002223136&amp;sdata=7OMV01n5pzZq%2F1C9tdrB81E0KsuXXaGk0raFAyQXiYg%3D&amp;reserved=0
>
>

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

Re: [PATCH 37/45] drm/ttm: add a helper to allocate a temp tt for copies.

[Daniel Vetter]

On Fri, Sep 25, 2020 at 11:34 AM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 25.09.20 um 10:18 schrieb Daniel Vetter:
> > On Fri, Sep 25, 2020 at 10:16 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> >> On Fri, Sep 25, 2020 at 9:39 AM Christian König
> >> <christian.koenig@amd.com> wrote:
> >>> Am 25.09.20 um 01:14 schrieb Dave Airlie:
> >>>> On Thu, 24 Sep 2020 at 22:42, Christian König <christian.koenig@amd.com> wrote:
> >>>>> Am 24.09.20 um 07:18 schrieb Dave Airlie:
> >>>>>> From: Dave Airlie <airlied@redhat.com>
> >>>>>>
> >>>>>> All the accel moves do the same pattern here, provide a helper
> >>>>> And exactly that pattern I want to get away from.
> >>>> Currently this is just refactoring out the helper code in each driver, but I see
> >>>> since it calls bo_mem_space we are probably moving a bit in the wrong direction.
> >>> Exactly that's why I'm noting this.
> >>>
> >>>>> See what happens if we (for example) have a VRAM -> SYSTEM move is the
> >>>>> following:
> >>>>>
> >>>>> 1. TTM allocates a new ttm_resource object in the SYSTEM domain.
> >>>>> 2. We call the driver to move from VRAM to SYSTEM.
> >>>>> 3. Driver finds that it can't do this and calls TTM  to allocate GTT.
> >>>>> 4. Since we are maybe out of GTT TTM evicts a different BO from GTT to
> >>>>> SYSTEM and call driver again.
> >>>>>
> >>>>> This is a horrible ping/pong between driver/TTM/driver/TTM/driver and we
> >>>>> should stop that immediately.
> >>>>>
> >>>>> My suggestion is that we rewrite how drivers call the ttm_bo_validate()
> >>>>> function so that we can guarantee that this never happens.
> >>>>>
> >>>>> What do you think?
> >>>> I think that is likely the next step I'd like to take after this
> >>>> refactor, it's a lot bigger, and I'm not sure how it will look yet.
> >>> Agree, yes. I have some ideas in mind for that, but not fully baked either.
> >>>
> >>>> Do we envision the driver calling validate in a loop but when it can't
> >>>> find space it tells the driver and the driver does eviction and
> >>>> recalls validate?
> >>> Not in a loop, but more like in a chain.
> >>>
> >>> My plan is something like this:
> >>> Instead of having "normal" and "busy" placement we have a flag in the
> >>> context if evictions are allowed or not.
> >>> The call to ttm_bo_validate are then replaced with two calls, first
> >>> without evictions and if that didn't worked one with evictions.
> >>>
> >>> Then the normal validate sequence should look like this:
> >>> 1. If a BO is in the SYSTEM (or SWAP domain) we validate it to GTT first
> >>> with evictions=true.
> >>> 2. If a BO should be in VRAM we then validate it to VRAM. If evictions
> >>> are only allowed if the GEM flags say that GTT is not desired.
> >> That solves the trouble when you move a bo into vram as part of
> >> validate. But I'm not seeing how this solves the "need gtt mapping to
> >> move something out of vram" problem.
>
> Eviction is not a problem because the driver gets asked where to put an
> evicted BO and then TTM does all the moving.

Hm I guess then I don't quite get where you see the ping-pong
happening, I thought that only happens for evicting stuff. But hey not
much actual working experience with ttm over here, I'm just reading
:-) I thought the issue is that ttm wants to evict from $something to
SYSTEM, and to do that the driver first needs to set a GTT mapping for
the SYSTEM ttm_resource allocation, so that it can use the
blitter/sdma engine or whatever to move the data over. But for
swap-in/validation I'm confused how you can end up with the "wrong"
placement, that feels like a driver bug.

How exactly can you get into a situation with validation where ttm
gives you SYSTEM, but not GTT and the driver has to fix that up? I'm
not really following I think, I guess there's something obvious I'm
missing.

> >> Or should we instead move the entire eviction logic out from ttm into
> >> drivers, building it up from helpers?
>
> I've been playing with that thought for a while as well, but then
> decided against it.
>
> The main problem I see is that we sometimes need to evict things from
> other drivers.
>
> E.g. when we overcommitted system memory and move things to swap for
> example.

Hm yeah ttm has that limit to avoid stepping into the shrinker,
directly calling into another driver to keep within the limit while
ignoring that there's other memory users and caches out there still
feels wrong, it's kinda a parallel world vs shrinker callbacks. And
there's nothing stopping you from doing the SYSTEM->SWAP movement from
a shrinker callback with the locking rules we've established around
dma_resv (just needs to be a trylock).

So feels a bit backwards if we design ttm eviction around this part of it ...

> >> Then drivers which need gtt for
> >> moving stuff out of vram can do that right away. Also, this would
> >> allow us to implement very fancy eviction algorithms like all the
> >> nonsense we're doing in i915 for gtt handling on gen2/3 (but I really
> >> hope that never ever becomes a thing again in future gpus, so this is
> >> maybe more a what-if kind of thing). Not sure how that would look
> >> like, maybe a special validate function which takes a ttm_resource the
> >> driver already found (through evicting stuff or whatever) and then ttm
> >> just does the move and book-keeping and everything. And drivers would
> >> at first only call validate without allowing any eviction. Ofc anyone
> >> without special needs could use the standard eviction function that
> >> validate already has.
> > Spinning this a bit more, we could have different default eviction
> > functions with this, e.g. so all the drivers that need gtt mapping for
> > moving stuff around can share that code, but with specific&flat
> > control flow instead of lots of ping-ping. And drivers that don't need
> > gtt mapping (like i915, we just need dma_map_sg which we assume works
> > always, or something from the ttm dma page pool, which really always
> > works) can then use something simpler that's completely flat.
>
> Ok you need to explain a bit more what exactly the problem with the GTT
> eviction is here :)

So the full set of limitations are
- range limits
- power-of-two alignement of start
- some other (smaller) power of two alignment for size (lol)
- "color", i.e. different caching modes needs at least one page of
empty space in-between

Stuffing all that into a generic eviction logic is imo silly. On top
of that we have the eviction collector where we scan the entire thing
until we've built up a sufficiently big hole, then evict just these
buffers. If we don't do this then pretty much any big buffer with
constraints results in the entire GTT getting evicted. Again something
that's only worth it if you have ridiculous placement constraints like
these old intel chips. gen2/3 in i915.ko is maybe a bit extreme, but
having the driver in control of the eviction code feels like a much
better design than ttm inflicting a one-size-fits-all on everyone. Ofc
with defaults and building blocks and all that.
-Daniel

> Christian.
>
> > -Daniel
> >
> >>> For special BOs, like amdgpus GDS, GWS and OA domain or VMWGFX special
> >>> domains that will obviously look a bit different.
> >>>
> >>> Christian.
> >>>
> >>>> Dave.
> >>> _______________________________________________
> >>> dri-devel mailing list
> >>> dri-devel@lists.freedesktop.org
> >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7C66b5c2bfed8541cd986208d8612b8e1d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637366187002213152&amp;sdata=OPdXMe2rG1Uy%2BZD%2BjYKHfkLc9xVAQ1AL23QbEu3drnE%3D&amp;reserved=0
> >>
> >>
> >> --
> >> Daniel Vetter
> >> Software Engineer, Intel Corporation
> >> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ffwll.ch%2F&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7C66b5c2bfed8541cd986208d8612b8e1d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637366187002223136&amp;sdata=7OMV01n5pzZq%2F1C9tdrB81E0KsuXXaGk0raFAyQXiYg%3D&amp;reserved=0
> >
> >
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 37/45] drm/ttm: add a helper to allocate a temp tt for copies.

[Christian König]

Am 25.09.20 um 15:17 schrieb Daniel Vetter:
> [SNIP]
>> Eviction is not a problem because the driver gets asked where to put an
>> evicted BO and then TTM does all the moving.
> Hm I guess then I don't quite get where you see the ping-pong
> happening, I thought that only happens for evicting stuff.

No, eviction is the nice case. One step after another.

E.g. from VRAM to GTT, then from GTT to SYSTEM and then eventually 
swapped out.

> But hey not much actual working experience with ttm over here, I'm just reading
> :-) I thought the issue is that ttm wants to evict from $something to
> SYSTEM, and to do that the driver first needs to set a GTT mapping for
> the SYSTEM ttm_resource allocation, so that it can use the
> blitter/sdma engine or whatever to move the data over. But for
> swap-in/validation I'm confused how you can end up with the "wrong"
> placement, that feels like a driver bug.

The problem happens in the other direction and always directly triggered 
by the driver.

> How exactly can you get into a situation with validation where ttm
> gives you SYSTEM, but not GTT and the driver has to fix that up? I'm
> not really following I think, I guess there's something obvious I'm
> missing.

For example a buffer which was evicted to SYSTEM needs to be moved back 
directly to VRAM.

Or we want to evacuate all buffers from VRAM to SYSTEM because of 
hibernation.

etc....

>>>> Or should we instead move the entire eviction logic out from ttm into
>>>> drivers, building it up from helpers?
>> I've been playing with that thought for a while as well, but then
>> decided against it.
>>
>> The main problem I see is that we sometimes need to evict things from
>> other drivers.
>>
>> E.g. when we overcommitted system memory and move things to swap for
>> example.
> Hm yeah ttm has that limit to avoid stepping into the shrinker,
> directly calling into another driver to keep within the limit while
> ignoring that there's other memory users and caches out there still
> feels wrong, it's kinda a parallel world vs shrinker callbacks. And
> there's nothing stopping you from doing the SYSTEM->SWAP movement from
> a shrinker callback with the locking rules we've established around
> dma_resv (just needs to be a trylock).
>
> So feels a bit backwards if we design ttm eviction around this part of it ...

Ok, that's a good point. Need to think about that a bit more then.

>>>> Then drivers which need gtt for
>>>> moving stuff out of vram can do that right away. Also, this would
>>>> allow us to implement very fancy eviction algorithms like all the
>>>> nonsense we're doing in i915 for gtt handling on gen2/3 (but I really
>>>> hope that never ever becomes a thing again in future gpus, so this is
>>>> maybe more a what-if kind of thing). Not sure how that would look
>>>> like, maybe a special validate function which takes a ttm_resource the
>>>> driver already found (through evicting stuff or whatever) and then ttm
>>>> just does the move and book-keeping and everything. And drivers would
>>>> at first only call validate without allowing any eviction. Ofc anyone
>>>> without special needs could use the standard eviction function that
>>>> validate already has.
>>> Spinning this a bit more, we could have different default eviction
>>> functions with this, e.g. so all the drivers that need gtt mapping for
>>> moving stuff around can share that code, but with specific&flat
>>> control flow instead of lots of ping-ping. And drivers that don't need
>>> gtt mapping (like i915, we just need dma_map_sg which we assume works
>>> always, or something from the ttm dma page pool, which really always
>>> works) can then use something simpler that's completely flat.
>> Ok you need to explain a bit more what exactly the problem with the GTT
>> eviction is here :)
> So the full set of limitations are
> - range limits
> - power-of-two alignement of start
> - some other (smaller) power of two alignment for size (lol)
> - "color", i.e. different caching modes needs at least one page of
> empty space in-between
>
> Stuffing all that into a generic eviction logic is imo silly. On top
> of that we have the eviction collector where we scan the entire thing
> until we've built up a sufficiently big hole, then evict just these
> buffers. If we don't do this then pretty much any big buffer with
> constraints results in the entire GTT getting evicted. Again something
> that's only worth it if you have ridiculous placement constraints like
> these old intel chips. gen2/3 in i915.ko is maybe a bit extreme, but
> having the driver in control of the eviction code feels like a much
> better design than ttm inflicting a one-size-fits-all on everyone. Ofc
> with defaults and building blocks and all that.

Yeah, that makes a lot of sense.

Christian.

> -Daniel
>

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

Re: [PATCH 37/45] drm/ttm: add a helper to allocate a temp tt for copies.

[Daniel Vetter]

On Fri, Sep 25, 2020 at 3:40 PM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 25.09.20 um 15:17 schrieb Daniel Vetter:
> > [SNIP]
> >> Eviction is not a problem because the driver gets asked where to put an
> >> evicted BO and then TTM does all the moving.
> > Hm I guess then I don't quite get where you see the ping-pong
> > happening, I thought that only happens for evicting stuff.
>
> No, eviction is the nice case. One step after another.
>
> E.g. from VRAM to GTT, then from GTT to SYSTEM and then eventually
> swapped out.
>
> > But hey not much actual working experience with ttm over here, I'm just reading
> > :-) I thought the issue is that ttm wants to evict from $something to
> > SYSTEM, and to do that the driver first needs to set a GTT mapping for
> > the SYSTEM ttm_resource allocation, so that it can use the
> > blitter/sdma engine or whatever to move the data over. But for
> > swap-in/validation I'm confused how you can end up with the "wrong"
> > placement, that feels like a driver bug.
>
> The problem happens in the other direction and always directly triggered
> by the driver.
>
> > How exactly can you get into a situation with validation where ttm
> > gives you SYSTEM, but not GTT and the driver has to fix that up? I'm
> > not really following I think, I guess there's something obvious I'm
> > missing.
>
> For example a buffer which was evicted to SYSTEM needs to be moved back
> directly to VRAM.
>
> Or we want to evacuate all buffers from VRAM to SYSTEM because of
> hibernation.
>
> etc....

Ok, I think I get it. Eviction always goes one step only in the chain
(but maybe multiple times as part of an overall eu validate for all
the buffers). But swap in can go the entire length in one go. So yeah
demidlayering eviction isn't quite the right thing here, since it's
not the problem.
-Daniel

>
> >>>> Or should we instead move the entire eviction logic out from ttm into
> >>>> drivers, building it up from helpers?
> >> I've been playing with that thought for a while as well, but then
> >> decided against it.
> >>
> >> The main problem I see is that we sometimes need to evict things from
> >> other drivers.
> >>
> >> E.g. when we overcommitted system memory and move things to swap for
> >> example.
> > Hm yeah ttm has that limit to avoid stepping into the shrinker,
> > directly calling into another driver to keep within the limit while
> > ignoring that there's other memory users and caches out there still
> > feels wrong, it's kinda a parallel world vs shrinker callbacks. And
> > there's nothing stopping you from doing the SYSTEM->SWAP movement from
> > a shrinker callback with the locking rules we've established around
> > dma_resv (just needs to be a trylock).
> >
> > So feels a bit backwards if we design ttm eviction around this part of it ...
>
> Ok, that's a good point. Need to think about that a bit more then.
>
> >>>> Then drivers which need gtt for
> >>>> moving stuff out of vram can do that right away. Also, this would
> >>>> allow us to implement very fancy eviction algorithms like all the
> >>>> nonsense we're doing in i915 for gtt handling on gen2/3 (but I really
> >>>> hope that never ever becomes a thing again in future gpus, so this is
> >>>> maybe more a what-if kind of thing). Not sure how that would look
> >>>> like, maybe a special validate function which takes a ttm_resource the
> >>>> driver already found (through evicting stuff or whatever) and then ttm
> >>>> just does the move and book-keeping and everything. And drivers would
> >>>> at first only call validate without allowing any eviction. Ofc anyone
> >>>> without special needs could use the standard eviction function that
> >>>> validate already has.
> >>> Spinning this a bit more, we could have different default eviction
> >>> functions with this, e.g. so all the drivers that need gtt mapping for
> >>> moving stuff around can share that code, but with specific&flat
> >>> control flow instead of lots of ping-ping. And drivers that don't need
> >>> gtt mapping (like i915, we just need dma_map_sg which we assume works
> >>> always, or something from the ttm dma page pool, which really always
> >>> works) can then use something simpler that's completely flat.
> >> Ok you need to explain a bit more what exactly the problem with the GTT
> >> eviction is here :)
> > So the full set of limitations are
> > - range limits
> > - power-of-two alignement of start
> > - some other (smaller) power of two alignment for size (lol)
> > - "color", i.e. different caching modes needs at least one page of
> > empty space in-between
> >
> > Stuffing all that into a generic eviction logic is imo silly. On top
> > of that we have the eviction collector where we scan the entire thing
> > until we've built up a sufficiently big hole, then evict just these
> > buffers. If we don't do this then pretty much any big buffer with
> > constraints results in the entire GTT getting evicted. Again something
> > that's only worth it if you have ridiculous placement constraints like
> > these old intel chips. gen2/3 in i915.ko is maybe a bit extreme, but
> > having the driver in control of the eviction code feels like a much
> > better design than ttm inflicting a one-size-fits-all on everyone. Ofc
> > with defaults and building blocks and all that.
>
> Yeah, that makes a lot of sense.
>
> Christian.
>
> > -Daniel
> >
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 38/45] drm/nouveau/ttm: use helper to allocate tt temp

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 30 +++-------------------------
 1 file changed, 3 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index a032fdacf5f8..93f24b828ede 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -884,28 +884,16 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict,
 		      struct ttm_operation_ctx *ctx,
 		      struct ttm_resource *new_reg)
 {
-	struct ttm_place placement_memtype = {
-		.fpfn = 0,
-		.lpfn = 0,
-		.mem_type = TTM_PL_TT,
-		.flags = TTM_PL_MASK_CACHING
-	};
-	struct ttm_placement placement;
 	struct ttm_resource tmp_reg;
 	int ret;
 
-	placement.num_placement = placement.num_busy_placement = 1;
-	placement.placement = placement.busy_placement = &placement_memtype;
-
-	tmp_reg = *new_reg;
-	tmp_reg.mm_node = NULL;
-	ret = ttm_bo_mem_space(bo, &placement, &tmp_reg, ctx);
+	ret = ttm_bo_create_tt_tmp(bo, ctx, new_reg, &tmp_reg);
 	if (ret)
 		return ret;
 
 	ret = ttm_tt_populate(bo->bdev, bo->ttm, ctx);
 	if (ret)
-		goto out;
+		return ret;
 
 	ret = nouveau_ttm_tt_bind(bo->bdev, bo->ttm, &tmp_reg);
 	if (ret)
@@ -936,22 +924,10 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict,
 		      struct ttm_operation_ctx *ctx,
 		      struct ttm_resource *new_reg)
 {
-	struct ttm_place placement_memtype = {
-		.fpfn = 0,
-		.lpfn = 0,
-		.mem_type = TTM_PL_TT,
-		.flags = TTM_PL_MASK_CACHING
-	};
-	struct ttm_placement placement;
 	struct ttm_resource tmp_reg;
 	int ret;
 
-	placement.num_placement = placement.num_busy_placement = 1;
-	placement.placement = placement.busy_placement = &placement_memtype;
-
-	tmp_reg = *new_reg;
-	tmp_reg.mm_node = NULL;
-	ret = ttm_bo_mem_space(bo, &placement, &tmp_reg, ctx);
+	ret = ttm_bo_create_tt_tmp(bo, ctx, new_reg, &tmp_reg);
 	if (ret)
 		return ret;
 
-- 
2.27.0

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

[PATCH 39/45] drm/radeon/ttm: use new helper to create tmp tt

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 28 ++--------------------------
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index b38bc688f665..b004857f536b 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -214,21 +214,9 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
 {
 	struct ttm_resource *old_mem = &bo->mem;
 	struct ttm_resource tmp_mem;
-	struct ttm_place placements;
-	struct ttm_placement placement;
 	int r;
 
-	tmp_mem = *new_mem;
-	tmp_mem.mm_node = NULL;
-	placement.num_placement = 1;
-	placement.placement = &placements;
-	placement.num_busy_placement = 1;
-	placement.busy_placement = &placements;
-	placements.fpfn = 0;
-	placements.lpfn = 0;
-	placements.mem_type = TTM_PL_TT;
-	placements.flags = TTM_PL_MASK_CACHING;
-	r = ttm_bo_mem_space(bo, &placement, &tmp_mem, ctx);
+	r = ttm_bo_create_tt_tmp(bo, ctx, new_mem, &tmp_mem);
 	if (unlikely(r)) {
 		return r;
 	}
@@ -275,21 +263,9 @@ static int radeon_move_ram_vram(struct ttm_buffer_object *bo,
 {
 	struct ttm_resource *old_mem = &bo->mem;
 	struct ttm_resource tmp_mem;
-	struct ttm_placement placement;
-	struct ttm_place placements;
 	int r;
 
-	tmp_mem = *new_mem;
-	tmp_mem.mm_node = NULL;
-	placement.num_placement = 1;
-	placement.placement = &placements;
-	placement.num_busy_placement = 1;
-	placement.busy_placement = &placements;
-	placements.fpfn = 0;
-	placements.lpfn = 0;
-	placements.mem_type = TTM_PL_TT;
-	placements.flags = TTM_PL_MASK_CACHING;
-	r = ttm_bo_mem_space(bo, &placement, &tmp_mem, ctx);
+	r = ttm_bo_create_tt_tmp(bo, ctx, new_mem, &tmp_mem);
 	if (unlikely(r)) {
 		return r;
 	}
-- 
2.27.0

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

[PATCH 40/45] drm/amdgpu/ttm: use new ttm helper to create temp tt

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 29 ++-----------------------
 1 file changed, 2 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e1133acb8536..1fbcb16a3c24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -526,22 +526,9 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict,
 {
 	struct ttm_resource *old_mem = &bo->mem;
 	struct ttm_resource tmp_mem;
-	struct ttm_place placements;
-	struct ttm_placement placement;
 	int r;
 
-	/* create space/pages for new_mem in GTT space */
-	tmp_mem = *new_mem;
-	tmp_mem.mm_node = NULL;
-	placement.num_placement = 1;
-	placement.placement = &placements;
-	placement.num_busy_placement = 1;
-	placement.busy_placement = &placements;
-	placements.fpfn = 0;
-	placements.lpfn = 0;
-	placements.mem_type = TTM_PL_TT;
-	placements.flags = TTM_PL_MASK_CACHING;
-	r = ttm_bo_mem_space(bo, &placement, &tmp_mem, ctx);
+	r = ttm_bo_create_tt_tmp(bo, ctx, new_mem, &tmp_mem);
 	if (unlikely(r)) {
 		pr_err("Failed to find GTT space for blit from VRAM\n");
 		return r;
@@ -597,22 +584,10 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo, bool evict,
 {
 	struct ttm_resource *old_mem = &bo->mem;
 	struct ttm_resource tmp_mem;
-	struct ttm_placement placement;
-	struct ttm_place placements;
 	int r;
 
 	/* make space in GTT for old_mem buffer */
-	tmp_mem = *new_mem;
-	tmp_mem.mm_node = NULL;
-	placement.num_placement = 1;
-	placement.placement = &placements;
-	placement.num_busy_placement = 1;
-	placement.busy_placement = &placements;
-	placements.fpfn = 0;
-	placements.lpfn = 0;
-	placements.mem_type = TTM_PL_TT;
-	placements.flags = TTM_PL_MASK_CACHING;
-	r = ttm_bo_mem_space(bo, &placement, &tmp_mem, ctx);
+	r = ttm_bo_create_tt_tmp(bo, ctx, new_mem, &tmp_mem);
 	if (unlikely(r)) {
 		pr_err("Failed to find GTT space for blit to VRAM\n");
 		return r;
-- 
2.27.0

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

[PATCH 41/45] drm/amdgpu/ttm: use helper function for caching/populate

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 1fbcb16a3c24..099752bc42e9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -535,12 +535,7 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict,
 	}
 
 	/* set caching flags */
-	r = ttm_tt_set_placement_caching(bo->ttm, tmp_mem.placement);
-	if (unlikely(r)) {
-		goto out_cleanup;
-	}
-
-	r = ttm_tt_populate(bo->bdev, bo->ttm, ctx);
+	r = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_mem);
 	if (unlikely(r))
 		goto out_cleanup;
 
-- 
2.27.0

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

[PATCH 42/45] drm/radeon/ttm: use helper function for caching/populate

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index b004857f536b..ccd588bd4ea5 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -220,13 +220,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
 	if (unlikely(r)) {
 		return r;
 	}
-
-	r = ttm_tt_set_placement_caching(bo->ttm, tmp_mem.placement);
-	if (unlikely(r)) {
-		goto out_cleanup;
-	}
-
-	r = ttm_tt_populate(bo->bdev, bo->ttm, ctx);
+	r = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_mem);
 	if (unlikely(r)) {
 		goto out_cleanup;
 	}
-- 
2.27.0

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

[PATCH 43/45] drm/nouveau/ttm: use helper for placement + populaate

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 93f24b828ede..1786c92bf3fc 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -891,7 +891,7 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict,
 	if (ret)
 		return ret;
 
-	ret = ttm_tt_populate(bo->bdev, bo->ttm, ctx);
+	ret = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_reg);
 	if (ret)
 		return ret;
 
-- 
2.27.0

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

[PATCH 44/45] drm/ttm: move more functionality into helper function.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 099752bc42e9..efb74ddc1877 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -534,11 +534,6 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict,
 		return r;
 	}
 
-	/* set caching flags */
-	r = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_mem);
-	if (unlikely(r))
-		goto out_cleanup;
-
 	/* Bind the memory to the GTT space */
 	r = amdgpu_ttm_backend_bind(bo->bdev, bo->ttm, &tmp_mem);
 	if (unlikely(r)) {
@@ -588,11 +583,6 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo, bool evict,
 		return r;
 	}
 
-	/* move/bind old memory to GTT space */
-	r = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_mem);
-	if (unlikely(r)) {
-		goto out_cleanup;
-	}
 	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 1786c92bf3fc..08beb92b0571 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -891,10 +891,6 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict,
 	if (ret)
 		return ret;
 
-	ret = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_reg);
-	if (ret)
-		return ret;
-
 	ret = nouveau_ttm_tt_bind(bo->bdev, bo->ttm, &tmp_reg);
 	if (ret)
 		goto out;
@@ -931,9 +927,6 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict,
 	if (ret)
 		return ret;
 
-	ret = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_reg);
-	if (ret)
-		goto out;
 	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 ccd588bd4ea5..6698e288cfbc 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -220,10 +220,6 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
 	if (unlikely(r)) {
 		return r;
 	}
-	r = ttm_bo_move_to_new_tt_mem(bo, ctx, &tmp_mem);
-	if (unlikely(r)) {
-		goto out_cleanup;
-	}
 
 	r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, &tmp_mem);
 	if (unlikely(r)) {
@@ -260,10 +256,6 @@ static int radeon_move_ram_vram(struct ttm_buffer_object *bo,
 	int r;
 
 	r = ttm_bo_create_tt_tmp(bo, ctx, new_mem, &tmp_mem);
-	if (unlikely(r)) {
-		return r;
-	}
-	r = ttm_bo_move_to_new_tt_mem(bo, ctx, &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 358d1580dc16..7ea7ed85e289 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1566,6 +1566,10 @@ int ttm_bo_create_tt_tmp(struct ttm_buffer_object *bo,
 	if (ret)
 		return ret;
 
+	ret  = ttm_bo_move_to_new_tt_mem(bo, ctx, new_temp);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 EXPORT_SYMBOL(ttm_bo_create_tt_tmp);
-- 
2.27.0

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

[PATCH 45/45] drm/ttm: add a new helper for a cleaning up after a ram move

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This is a pretty common pattern in the drivers.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 13 ++-----------
 drivers/gpu/drm/nouveau/nouveau_bo.c    | 14 +++-----------
 drivers/gpu/drm/radeon/radeon_ttm.c     | 13 ++-----------
 drivers/gpu/drm/ttm/ttm_bo.c            | 15 +++++++++++++++
 include/drm/ttm/ttm_bo_driver.h         |  3 +++
 5 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index efb74ddc1877..72a1c06ef15c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -552,12 +552,8 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict,
 		goto out_cleanup;
 
 	amdgpu_ttm_backend_unbind(bo->bdev, bo->ttm);
-	ttm_resource_free(bo, &bo->mem);
 
-	r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
-	if (unlikely(r))
-		goto out_cleanup;
-	ttm_bo_assign_mem(bo, new_mem);
+	r = ttm_bo_cleanup_ram_move(bo, new_mem);
 out_cleanup:
 	ttm_resource_free(bo, &tmp_mem);
 	return r;
@@ -674,13 +670,8 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
 		if (r)
 			return r;
 		amdgpu_ttm_backend_unbind(bo->bdev, bo->ttm);
-		ttm_resource_free(bo, &bo->mem);
 
-		r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
-		if (r)
-			return r;
-		ttm_bo_assign_mem(bo, new_mem);
-		return 0;
+		return ttm_bo_cleanup_ram_move(bo, new_mem);
 	}
 
 	if (old_mem->mem_type == AMDGPU_PL_GDS ||
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 08beb92b0571..9b5acb7dae50 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -904,12 +904,8 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict,
 		goto out;
 
 	nouveau_ttm_tt_unbind(bo->bdev, bo->ttm);
-	ttm_resource_free(bo, &bo->mem);
 
-	ret = ttm_tt_set_placement_caching(bo->ttm, new_reg->placement);
-	if (ret)
-		goto out;
-	ttm_bo_assign_mem(bo, new_reg);
+	ret = ttm_bo_cleanup_ram_move(bo, new_reg);
 out:
 	ttm_resource_free(bo, &tmp_reg);
 	return ret;
@@ -1072,13 +1068,9 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
 	if (old_reg->mem_type == TTM_PL_TT &&
 	    new_reg->mem_type == TTM_PL_SYSTEM) {
 		nouveau_ttm_tt_unbind(bo->bdev, bo->ttm);
-		ttm_resource_free(bo, &bo->mem);
 
-		ret = ttm_tt_set_placement_caching(bo->ttm, new_reg->placement);
-		if (ret)
-			goto out;
-		ttm_bo_assign_mem(bo, new_reg);
-		return 0;
+		ret = ttm_bo_cleanup_ram_move(bo, new_reg);
+		goto out;
 	}
 
 	/* Hardware assisted copy. */
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 6698e288cfbc..83ca0a519752 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -234,13 +234,8 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
 		goto out_cleanup;
 
 	radeon_ttm_tt_unbind(bo->bdev, bo->ttm);
-	ttm_resource_free(bo, &bo->mem);
-
-	r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
-	if (unlikely(r))
-		goto out_cleanup;
-	ttm_bo_assign_mem(bo, new_mem);
 
+	r = ttm_bo_cleanup_ram_move(bo, new_mem);
 out_cleanup:
 	ttm_resource_free(bo, &tmp_mem);
 	return r;
@@ -314,12 +309,8 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
 	if (old_mem->mem_type == TTM_PL_TT &&
 	    new_mem->mem_type == TTM_PL_SYSTEM) {
 		radeon_ttm_tt_unbind(bo->bdev, bo->ttm);
-		ttm_resource_free(bo, &bo->mem);
 
-		r = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
-		if (r)
-			return r;
-		goto out_assign;
+		return ttm_bo_cleanup_ram_move(bo, new_mem);
 	}
 
 	if (!rdev->ring[radeon_copy_ring_index(rdev)].ready ||
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 7ea7ed85e289..a3955fde448b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1573,3 +1573,18 @@ int ttm_bo_create_tt_tmp(struct ttm_buffer_object *bo,
 	return 0;
 }
 EXPORT_SYMBOL(ttm_bo_create_tt_tmp);
+
+int ttm_bo_cleanup_ram_move(struct ttm_buffer_object *bo,
+			    struct ttm_resource *new_mem)
+{
+	int ret;
+
+	ttm_resource_free(bo, &bo->mem);
+
+	ret = ttm_tt_set_placement_caching(bo->ttm, new_mem->placement);
+	if (ret)
+		return ret;
+	ttm_bo_assign_mem(bo, new_mem);
+	return 0;
+}
+EXPORT_SYMBOL(ttm_bo_cleanup_ram_move);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index a7507dfaa89d..a89149b440b0 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -563,6 +563,9 @@ int ttm_bo_create_tt_tmp(struct ttm_buffer_object *bo,
 			 struct ttm_operation_ctx *ctx,
 			 struct ttm_resource *new_mem,
 			 struct ttm_resource *new_temp);
+
+int ttm_bo_cleanup_ram_move(struct ttm_buffer_object *bo,
+			    struct ttm_resource *new_mem);
 /**
  * ttm_bo_move_memcpy
  *
-- 
2.27.0

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

Re: [PATCH 00/45] TTM move refactoring

[Dave Airlie]

On Thu, 24 Sep 2020 at 15:19, Dave Airlie <airlied@gmail.com> wrote:
>
> This refactors how TTM moves get called between core and drivers.
>

Full series is also:

https://github.com/airlied/linux/tree/ttm-no-more-bind

(it's based on top of Christian's last series).

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

Re: [PATCH 00/45] TTM move refactoring

[Ben Skeggs]

On Thu, 24 Sep 2020 at 15:19, Dave Airlie <airlied@gmail.com> wrote:
>
> This refactors how TTM moves get called between core and drivers.
>
> 1) puts the driver in charge of all moves, and enforces
> drivers have a move callback.
> 2) moves move_notify actions around moves to the driver side
> 3) moves binding/unbinding completely into move and driver side
> 4) add a new invalidate callback to replace the last use of move_notify
> 5) add some helpers to cleanup the resulting move code
>
> There's a bit of internal churn to try and make each patch logical, so
> don't get too caught up in each patches changes unless the end result
> is a problem.
I've looked over the nouveau-specific patches, and the ttm ones
(mostly ignoring the changes to other drivers beyond a quick glance
over).  For all but 37/45 and the patches that depend on it:

Reviewed-by: Ben Skeggs <bskeggs@redhat.com>

I'll add some more specific comments to one of the patches, but it's
also fine as-is.

Ben.

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