[PATCH] drm/ttm: partial revert "cleanup ttm_tt_(unbind|destroy)" v2

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Christian König
> Sent: Friday, July 22, 2016 9:10 AM
> To: amd-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org
> Cc: Lin, Amber; Kuehling, Felix; funfunctor at folklore1984.net
> Subject: [PATCH] drm/ttm: partial revert "cleanup ttm_tt_(unbind|destroy)"
> v2
> 
> From: Christian König 
> 
> We still need to unbind explicitely during a move.
> 
> This partial reverts commit ff20caa0bcbfef9f7686f8d1868a3b990921afd6.
> 
> v2: remove unnecessary check and unused variable
> 
> Signed-off-by: Christian König 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/ttm/ttm_bo_util.c |  1 +
>  drivers/gpu/drm/ttm/ttm_tt.c  | 19 ---
>  include/drm/ttm/ttm_bo_driver.h   |  9 +
>  3 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 0c389a5..6c412c3 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -53,6 +53,7 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
>   int ret;
> 
>   if (old_mem->mem_type != TTM_PL_SYSTEM) {
> + ttm_tt_unbind(ttm);
>   ttm_bo_free_old_node(bo);
>   ttm_flag_masked(_mem->placement,
> TTM_PL_FLAG_SYSTEM,
>   TTM_PL_MASK_MEM);
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index d28d4333..526e5a7 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -166,16 +166,10 @@ EXPORT_SYMBOL(ttm_tt_set_placement_caching);
> 
>  void ttm_tt_destroy(struct ttm_tt *ttm)
>  {
> - int ret;
> -
>   if (ttm == NULL)
>   return;
> 
> - if (ttm->state == tt_bound) {
> - ret = ttm->func->unbind(ttm);
> - BUG_ON(ret);
> - ttm->state = tt_unbound;
> - }
> + ttm_tt_unbind(ttm);
> 
>   if (ttm->state == tt_unbound)
>   ttm_tt_unpopulate(ttm);
> @@ -255,6 +249,17 @@ void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
>  }
>  EXPORT_SYMBOL(ttm_dma_tt_fini);
> 
> +void ttm_tt_unbind(struct ttm_tt *ttm)
> +{
> + int ret;
> +
> + if (ttm->state == tt_bound) {
> + ret = ttm->func->unbind(ttm);
> + BUG_ON(ret);
> + ttm->state = tt_unbound;
> + }
> +}
> +
>  int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem)
>  {
>   int ret = 0;
> diff --git a/include/drm/ttm/ttm_bo_driver.h
> b/include/drm/ttm/ttm_bo_driver.h
> index 141cefb..e7d81a8 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -628,6 +628,15 @@ extern int ttm_tt_bind(struct ttm_tt *ttm, struct
> ttm_mem_reg *bo_mem);
>  extern void ttm_tt_destroy(struct ttm_tt *ttm);
> 
>  /**
> + * ttm_ttm_unbind:
> + *
> + * @ttm: The struct ttm_tt.
> + *
> + * Unbind a struct ttm_tt.
> + */
> +extern void ttm_tt_unbind(struct ttm_tt *ttm);
> +
> +/**
>   * ttm_tt_swapin:
>   *
>   * @ttm: The struct ttm_tt.
> --
> 2.5.0
> 
> ___
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/ttm: partial revert "cleanup ttm_tt_(unbind|destroy)" v2

[Edward O'Callaghan]

Reviewed-by: Edward O'Callaghan 

On 07/22/2016 11:10 PM, Christian König wrote:
> From: Christian König 
> 
> We still need to unbind explicitely during a move.
> 
> This partial reverts commit ff20caa0bcbfef9f7686f8d1868a3b990921afd6.
> 
> v2: remove unnecessary check and unused variable
> 
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/ttm/ttm_bo_util.c |  1 +
>  drivers/gpu/drm/ttm/ttm_tt.c  | 19 ---
>  include/drm/ttm/ttm_bo_driver.h   |  9 +
>  3 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 0c389a5..6c412c3 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -53,6 +53,7 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
>   int ret;
>  
>   if (old_mem->mem_type != TTM_PL_SYSTEM) {
> + ttm_tt_unbind(ttm);
>   ttm_bo_free_old_node(bo);
>   ttm_flag_masked(_mem->placement, TTM_PL_FLAG_SYSTEM,
>   TTM_PL_MASK_MEM);
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index d28d4333..526e5a7 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -166,16 +166,10 @@ EXPORT_SYMBOL(ttm_tt_set_placement_caching);
>  
>  void ttm_tt_destroy(struct ttm_tt *ttm)
>  {
> - int ret;
> -
>   if (ttm == NULL)
>   return;
>  
> - if (ttm->state == tt_bound) {
> - ret = ttm->func->unbind(ttm);
> - BUG_ON(ret);
> - ttm->state = tt_unbound;
> - }
> + ttm_tt_unbind(ttm);
>  
>   if (ttm->state == tt_unbound)
>   ttm_tt_unpopulate(ttm);
> @@ -255,6 +249,17 @@ void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
>  }
>  EXPORT_SYMBOL(ttm_dma_tt_fini);
>  
> +void ttm_tt_unbind(struct ttm_tt *ttm)
> +{
> + int ret;
> +
> + if (ttm->state == tt_bound) {
> + ret = ttm->func->unbind(ttm);
> + BUG_ON(ret);
> + ttm->state = tt_unbound;
> + }
> +}
> +
>  int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem)
>  {
>   int ret = 0;
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 141cefb..e7d81a8 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -628,6 +628,15 @@ extern int ttm_tt_bind(struct ttm_tt *ttm, struct 
> ttm_mem_reg *bo_mem);
>  extern void ttm_tt_destroy(struct ttm_tt *ttm);
>  
>  /**
> + * ttm_ttm_unbind:
> + *
> + * @ttm: The struct ttm_tt.
> + *
> + * Unbind a struct ttm_tt.
> + */
> +extern void ttm_tt_unbind(struct ttm_tt *ttm);
> +
> +/**
>   * ttm_tt_swapin:
>   *
>   * @ttm: The struct ttm_tt.
> 

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: 

[PATCH] drm/ttm: partial revert "cleanup ttm_tt_(unbind|destroy)" v2

[Christian König]

From: Christian König 

We still need to unbind explicitely during a move.

This partial reverts commit ff20caa0bcbfef9f7686f8d1868a3b990921afd6.

v2: remove unnecessary check and unused variable

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo_util.c |  1 +
 drivers/gpu/drm/ttm/ttm_tt.c  | 19 ---
 include/drm/ttm/ttm_bo_driver.h   |  9 +
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 0c389a5..6c412c3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -53,6 +53,7 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
int ret;

if (old_mem->mem_type != TTM_PL_SYSTEM) {
+   ttm_tt_unbind(ttm);
ttm_bo_free_old_node(bo);
ttm_flag_masked(_mem->placement, TTM_PL_FLAG_SYSTEM,
TTM_PL_MASK_MEM);
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index d28d4333..526e5a7 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -166,16 +166,10 @@ EXPORT_SYMBOL(ttm_tt_set_placement_caching);

 void ttm_tt_destroy(struct ttm_tt *ttm)
 {
-   int ret;
-
if (ttm == NULL)
return;

-   if (ttm->state == tt_bound) {
-   ret = ttm->func->unbind(ttm);
-   BUG_ON(ret);
-   ttm->state = tt_unbound;
-   }
+   ttm_tt_unbind(ttm);

if (ttm->state == tt_unbound)
ttm_tt_unpopulate(ttm);
@@ -255,6 +249,17 @@ void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
 }
 EXPORT_SYMBOL(ttm_dma_tt_fini);

+void ttm_tt_unbind(struct ttm_tt *ttm)
+{
+   int ret;
+
+   if (ttm->state == tt_bound) {
+   ret = ttm->func->unbind(ttm);
+   BUG_ON(ret);
+   ttm->state = tt_unbound;
+   }
+}
+
 int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem)
 {
int ret = 0;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 141cefb..e7d81a8 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -628,6 +628,15 @@ extern int ttm_tt_bind(struct ttm_tt *ttm, struct 
ttm_mem_reg *bo_mem);
 extern void ttm_tt_destroy(struct ttm_tt *ttm);

 /**
+ * ttm_ttm_unbind:
+ *
+ * @ttm: The struct ttm_tt.
+ *
+ * Unbind a struct ttm_tt.
+ */
+extern void ttm_tt_unbind(struct ttm_tt *ttm);
+
+/**
  * ttm_tt_swapin:
  *
  * @ttm: The struct ttm_tt.
-- 
2.5.0

[PATCH] drm/ttm: partial revert "cleanup ttm_tt_(unbind|destroy)" v2

[Sean Paul]

On Fri, Jul 22, 2016 at 9:10 AM, Christian König
 wrote:
> From: Christian König 
>
> We still need to unbind explicitely during a move.

Sorry for the drive-by nit, but:

s/explicitely/explicitly/


>
> This partial reverts commit ff20caa0bcbfef9f7686f8d1868a3b990921afd6.
>
> v2: remove unnecessary check and unused variable
>
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/ttm/ttm_bo_util.c |  1 +
>  drivers/gpu/drm/ttm/ttm_tt.c  | 19 ---
>  include/drm/ttm/ttm_bo_driver.h   |  9 +
>  3 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 0c389a5..6c412c3 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -53,6 +53,7 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
> int ret;
>
> if (old_mem->mem_type != TTM_PL_SYSTEM) {
> +   ttm_tt_unbind(ttm);
> ttm_bo_free_old_node(bo);
> ttm_flag_masked(_mem->placement, TTM_PL_FLAG_SYSTEM,
> TTM_PL_MASK_MEM);
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index d28d4333..526e5a7 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -166,16 +166,10 @@ EXPORT_SYMBOL(ttm_tt_set_placement_caching);
>
>  void ttm_tt_destroy(struct ttm_tt *ttm)
>  {
> -   int ret;
> -
> if (ttm == NULL)
> return;
>
> -   if (ttm->state == tt_bound) {
> -   ret = ttm->func->unbind(ttm);
> -   BUG_ON(ret);
> -   ttm->state = tt_unbound;
> -   }
> +   ttm_tt_unbind(ttm);
>
> if (ttm->state == tt_unbound)
> ttm_tt_unpopulate(ttm);
> @@ -255,6 +249,17 @@ void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
>  }
>  EXPORT_SYMBOL(ttm_dma_tt_fini);
>
> +void ttm_tt_unbind(struct ttm_tt *ttm)
> +{
> +   int ret;
> +
> +   if (ttm->state == tt_bound) {
> +   ret = ttm->func->unbind(ttm);
> +   BUG_ON(ret);
> +   ttm->state = tt_unbound;
> +   }
> +}
> +
>  int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem)
>  {
> int ret = 0;
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 141cefb..e7d81a8 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -628,6 +628,15 @@ extern int ttm_tt_bind(struct ttm_tt *ttm, struct 
> ttm_mem_reg *bo_mem);
>  extern void ttm_tt_destroy(struct ttm_tt *ttm);
>
>  /**
> + * ttm_ttm_unbind:
> + *
> + * @ttm: The struct ttm_tt.
> + *
> + * Unbind a struct ttm_tt.
> + */
> +extern void ttm_tt_unbind(struct ttm_tt *ttm);
> +
> +/**
>   * ttm_tt_swapin:
>   *
>   * @ttm: The struct ttm_tt.
> --
> 2.5.0
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/ttm: partial revert "cleanup ttm_tt_(unbind|destroy)"

[Lin, Amber]

Hi Christian,

May I know in which branch I can cherry-pick your patch below? Attached is the 
patch I'll submit to our development branch following your patch. I'll add you 
to the code review since this change will eventually merge to amdgpu branch.

Thanks.

Amber

-Original Message-
From: Christian König [mailto:deathsim...@vodafone.de] 
Sent: Thursday, July 21, 2016 6:21 AM
To: dri-devel at lists.freedesktop.org
Cc: Lin, Amber; Kuehling, Felix
Subject: [PATCH] drm/ttm: partial revert "cleanup ttm_tt_(unbind|destroy)"

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

We still need to unbind explicitely during a move.

This partial reverts commit ff20caa0bcbfef9f7686f8d1868a3b990921afd6.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo_util.c |  1 +
 drivers/gpu/drm/ttm/ttm_tt.c  | 15 ---
 include/drm/ttm/ttm_bo_driver.h   |  9 +
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 0c389a5..6c412c3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -53,6 +53,7 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
int ret;

if (old_mem->mem_type != TTM_PL_SYSTEM) {
+   ttm_tt_unbind(ttm);
ttm_bo_free_old_node(bo);
ttm_flag_masked(_mem->placement, TTM_PL_FLAG_SYSTEM,
TTM_PL_MASK_MEM);
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 
d28d4333..fa42dc3 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -172,9 +172,7 @@ void ttm_tt_destroy(struct ttm_tt *ttm)
return;

if (ttm->state == tt_bound) {
-   ret = ttm->func->unbind(ttm);
-   BUG_ON(ret);
-   ttm->state = tt_unbound;
+   ttm_tt_unbind(ttm);
}

if (ttm->state == tt_unbound)
@@ -255,6 +253,17 @@ void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)  }  
EXPORT_SYMBOL(ttm_dma_tt_fini);

+void ttm_tt_unbind(struct ttm_tt *ttm)
+{
+   int ret;
+
+   if (ttm->state == tt_bound) {
+   ret = ttm->func->unbind(ttm);
+   BUG_ON(ret);
+   ttm->state = tt_unbound;
+   }
+}
+
 int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem)  {
int ret = 0;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h 
index 141cefb..e7d81a8 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -628,6 +628,15 @@ extern int ttm_tt_bind(struct ttm_tt *ttm, struct 
ttm_mem_reg *bo_mem);  extern void ttm_tt_destroy(struct ttm_tt *ttm);

 /**
+ * ttm_ttm_unbind:
+ *
+ * @ttm: The struct ttm_tt.
+ *
+ * Unbind a struct ttm_tt.
+ */
+extern void ttm_tt_unbind(struct ttm_tt *ttm);
+
+/**
  * ttm_tt_swapin:
  *
  * @ttm: The struct ttm_tt.
--
2.5.0

-- next part --
A non-text attachment was scrubbed...
Name: 0002-drm-amdgpu-Get-user-pages-in-non-current-task.patch
Type: application/octet-stream
Size: 1416 bytes
Desc: 0002-drm-amdgpu-Get-user-pages-in-non-current-task.patch
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160721/4bc252f8/attachment.obj>

[PATCH] drm/ttm: partial revert "cleanup ttm_tt_(unbind|destroy)"

[Christian König]

From: Christian König 

We still need to unbind explicitely during a move.

This partial reverts commit ff20caa0bcbfef9f7686f8d1868a3b990921afd6.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo_util.c |  1 +
 drivers/gpu/drm/ttm/ttm_tt.c  | 15 ---
 include/drm/ttm/ttm_bo_driver.h   |  9 +
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 0c389a5..6c412c3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -53,6 +53,7 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo,
int ret;

if (old_mem->mem_type != TTM_PL_SYSTEM) {
+   ttm_tt_unbind(ttm);
ttm_bo_free_old_node(bo);
ttm_flag_masked(_mem->placement, TTM_PL_FLAG_SYSTEM,
TTM_PL_MASK_MEM);
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index d28d4333..fa42dc3 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -172,9 +172,7 @@ void ttm_tt_destroy(struct ttm_tt *ttm)
return;

if (ttm->state == tt_bound) {
-   ret = ttm->func->unbind(ttm);
-   BUG_ON(ret);
-   ttm->state = tt_unbound;
+   ttm_tt_unbind(ttm);
}

if (ttm->state == tt_unbound)
@@ -255,6 +253,17 @@ void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
 }
 EXPORT_SYMBOL(ttm_dma_tt_fini);

+void ttm_tt_unbind(struct ttm_tt *ttm)
+{
+   int ret;
+
+   if (ttm->state == tt_bound) {
+   ret = ttm->func->unbind(ttm);
+   BUG_ON(ret);
+   ttm->state = tt_unbound;
+   }
+}
+
 int ttm_tt_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem)
 {
int ret = 0;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 141cefb..e7d81a8 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -628,6 +628,15 @@ extern int ttm_tt_bind(struct ttm_tt *ttm, struct 
ttm_mem_reg *bo_mem);
 extern void ttm_tt_destroy(struct ttm_tt *ttm);

 /**
+ * ttm_ttm_unbind:
+ *
+ * @ttm: The struct ttm_tt.
+ *
+ * Unbind a struct ttm_tt.
+ */
+extern void ttm_tt_unbind(struct ttm_tt *ttm);
+
+/**
  * ttm_tt_swapin:
  *
  * @ttm: The struct ttm_tt.
-- 
2.5.0