All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback
@ 2021-06-14 19:26 Nirmoy Das
  2021-06-14 19:26 ` [PATCH 2/2] drm/amdgpu: move shadow_list to amdgpu_bo_vm Nirmoy Das
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Nirmoy Das @ 2021-06-14 19:26 UTC (permalink / raw)
  To: amd-gfx; +Cc: Nirmoy Das, Christian.Koenig

Make provision to pass different ttm BO destroy callback
while creating a amdgpu_bo.

v2: pass destroy callback using amdgpu_bo_param.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 52 +++++++++++++++++-----
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
 2 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 9092ac12a270..f4f57a73d095 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -12,7 +12,7 @@
  *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * FITNEsS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
  * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
  * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
  * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
@@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct amdgpu_bo *bo)
 	}
 }

-static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)
 {
-	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
 	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
-	struct amdgpu_bo_user *ubo;

 	if (bo->tbo.pin_count > 0)
 		amdgpu_bo_subtract_pin_size(bo);
@@ -87,18 +85,40 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
 	if (bo->tbo.base.import_attach)
 		drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
 	drm_gem_object_release(&bo->tbo.base);
+	amdgpu_bo_unref(&bo->parent);
+}
+
+static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+{
+	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+
+	amdgpu_bo_destroy_base(tbo);
+	kvfree(bo);
+}
+
+static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
+{
+	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+	struct amdgpu_bo_user *ubo;
+
+	amdgpu_bo_destroy_base(tbo);
+	ubo = to_amdgpu_bo_user(bo);
+	kfree(ubo->metadata);
+	kvfree(bo);
+}
+
+static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
+{
+	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
+	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+
+	amdgpu_bo_destroy_base(tbo);
 	/* in case amdgpu_device_recover_vram got NULL of bo->parent */
 	if (!list_empty(&bo->shadow_list)) {
 		mutex_lock(&adev->shadow_list_lock);
 		list_del_init(&bo->shadow_list);
 		mutex_unlock(&adev->shadow_list_lock);
 	}
-	amdgpu_bo_unref(&bo->parent);
-
-	if (bo->tbo.type != ttm_bo_type_kernel) {
-		ubo = to_amdgpu_bo_user(bo);
-		kfree(ubo->metadata);
-	}

 	kvfree(bo);
 }
@@ -115,8 +135,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
  */
 bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
 {
-	if (bo->destroy == &amdgpu_bo_destroy)
+	if (bo->destroy == &amdgpu_bo_destroy ||
+	    bo->destroy == &amdgpu_bo_user_destroy ||
+	    bo->destroy == &amdgpu_bo_vm_destroy)
 		return true;
+
 	return false;
 }

@@ -592,9 +615,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 	if (bp->type == ttm_bo_type_kernel)
 		bo->tbo.priority = 1;

+	if (!bp->destroy)
+		bp->destroy = &amdgpu_bo_destroy;
+
 	r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type,
 				 &bo->placement, page_align, &ctx,  NULL,
-				 bp->resv, &amdgpu_bo_destroy);
+				 bp->resv, bp->destroy);
 	if (unlikely(r != 0))
 		return r;

@@ -658,6 +684,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
 	int r;

 	bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
+	bp->destroy = &amdgpu_bo_user_destroy;
 	r = amdgpu_bo_create(adev, bp, &bo_ptr);
 	if (r)
 		return r;
@@ -689,6 +716,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
 	 * num of amdgpu_vm_pt entries.
 	 */
 	BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
+	bp->destroy = &amdgpu_bo_vm_destroy;
 	r = amdgpu_bo_create(adev, bp, &bo_ptr);
 	if (r)
 		return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index e36b84516b4e..a8c702634e1b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -55,7 +55,8 @@ struct amdgpu_bo_param {
 	u64				flags;
 	enum ttm_bo_type		type;
 	bool				no_wait_gpu;
-	struct dma_resv	*resv;
+	struct dma_resv			*resv;
+	void				(*destroy)(struct ttm_buffer_object *bo);
 };

 /* bo virtual addresses in a vm */
--
2.31.1

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 2/2] drm/amdgpu: move shadow_list to amdgpu_bo_vm
  2021-06-14 19:26 [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback Nirmoy Das
@ 2021-06-14 19:26 ` Nirmoy Das
  2021-06-15  6:55   ` Christian König
  2021-06-14 20:00 ` [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback Alex Deucher
  2021-06-15  6:53 ` Christian König
  2 siblings, 1 reply; 10+ messages in thread
From: Nirmoy Das @ 2021-06-14 19:26 UTC (permalink / raw)
  To: amd-gfx; +Cc: Nirmoy Das, Christian.Koenig

Move shadow_list to struct amdgpu_bo_vm as shadow BOs
are part of PT/PD BOs.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  5 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 13 +++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  6 ++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     |  2 +-
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f2636f411a25..3f51b142fc83 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4063,6 +4063,7 @@ static int amdgpu_device_recover_vram(struct amdgpu_device *adev)
 {
 	struct dma_fence *fence = NULL, *next = NULL;
 	struct amdgpu_bo *shadow;
+	struct amdgpu_bo_vm *vmbo;
 	long r = 1, tmo;
 
 	if (amdgpu_sriov_runtime(adev))
@@ -4072,8 +4073,8 @@ static int amdgpu_device_recover_vram(struct amdgpu_device *adev)
 
 	dev_info(adev->dev, "recover vram bo from shadow start\n");
 	mutex_lock(&adev->shadow_list_lock);
-	list_for_each_entry(shadow, &adev->shadow_list, shadow_list) {
-
+	list_for_each_entry(vmbo, &adev->shadow_list, shadow_list) {
+		shadow = &vmbo->bo;
 		/* No need to recover an evicted BO */
 		if (shadow->tbo.mem.mem_type != TTM_PL_TT ||
 		    shadow->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET ||
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index f4f57a73d095..9f1e6bd8601b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -111,12 +111,13 @@ static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
 {
 	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
 	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+	struct amdgpu_bo_vm *vmbo = to_amdgpu_bo_vm(bo);
 
 	amdgpu_bo_destroy_base(tbo);
 	/* in case amdgpu_device_recover_vram got NULL of bo->parent */
-	if (!list_empty(&bo->shadow_list)) {
+	if (!list_empty(&vmbo->shadow_list)) {
 		mutex_lock(&adev->shadow_list_lock);
-		list_del_init(&bo->shadow_list);
+		list_del_init(&vmbo->shadow_list);
 		mutex_unlock(&adev->shadow_list_lock);
 	}
 
@@ -592,7 +593,6 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 	if (bo == NULL)
 		return -ENOMEM;
 	drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
-	INIT_LIST_HEAD(&bo->shadow_list);
 	bo->vm_bo = NULL;
 	bo->preferred_domains = bp->preferred_domain ? bp->preferred_domain :
 		bp->domain;
@@ -722,6 +722,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
 		return r;
 
 	*vmbo_ptr = to_amdgpu_bo_vm(bo_ptr);
+	INIT_LIST_HEAD(&(*vmbo_ptr)->shadow_list);
 	return r;
 }
 
@@ -766,12 +767,12 @@ int amdgpu_bo_validate(struct amdgpu_bo *bo)
  *
  * Insert a BO to the shadow list.
  */
-void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo)
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo)
 {
-	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+	struct amdgpu_device *adev = amdgpu_ttm_adev(vmbo->bo.tbo.bdev);
 
 	mutex_lock(&adev->shadow_list_lock);
-	list_add_tail(&bo->shadow_list, &adev->shadow_list);
+	list_add_tail(&vmbo->shadow_list, &adev->shadow_list);
 	mutex_unlock(&adev->shadow_list_lock);
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index a8c702634e1b..18cb2f28e4de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -109,9 +109,6 @@ struct amdgpu_bo {
 #ifdef CONFIG_MMU_NOTIFIER
 	struct mmu_interval_notifier	notifier;
 #endif
-
-	struct list_head		shadow_list;
-
 	struct kgd_mem                  *kfd_bo;
 };
 
@@ -127,6 +124,7 @@ struct amdgpu_bo_user {
 struct amdgpu_bo_vm {
 	struct amdgpu_bo		bo;
 	struct amdgpu_bo		*shadow;
+	struct list_head		shadow_list;
 	struct amdgpu_vm_bo_base        entries[];
 };
 
@@ -333,7 +331,7 @@ u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);
 int amdgpu_bo_validate(struct amdgpu_bo *bo);
 void amdgpu_bo_get_memory(struct amdgpu_bo *bo, uint64_t *vram_mem,
 				uint64_t *gtt_mem, uint64_t *cpu_mem);
-void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo);
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo);
 int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
 			     struct dma_fence **fence);
 uint32_t amdgpu_bo_get_preferred_pin_domain(struct amdgpu_device *adev,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 4c4c56581780..812c225538a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -936,7 +936,7 @@ static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
 	}
 
 	(*vmbo)->shadow->parent = amdgpu_bo_ref(bo);
-	amdgpu_bo_add_to_shadow_list((*vmbo)->shadow);
+	amdgpu_bo_add_to_shadow_list((*vmbo));
 
 	return 0;
 }
-- 
2.31.1

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback
  2021-06-14 19:26 [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback Nirmoy Das
  2021-06-14 19:26 ` [PATCH 2/2] drm/amdgpu: move shadow_list to amdgpu_bo_vm Nirmoy Das
@ 2021-06-14 20:00 ` Alex Deucher
  2021-06-15  8:42   ` Das, Nirmoy
  2021-06-15  6:53 ` Christian König
  2 siblings, 1 reply; 10+ messages in thread
From: Alex Deucher @ 2021-06-14 20:00 UTC (permalink / raw)
  To: Nirmoy Das; +Cc: Christian Koenig, amd-gfx list

On Mon, Jun 14, 2021 at 3:27 PM Nirmoy Das <nirmoy.das@amd.com> wrote:
>
> Make provision to pass different ttm BO destroy callback
> while creating a amdgpu_bo.
>
> v2: pass destroy callback using amdgpu_bo_param.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 52 +++++++++++++++++-----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
>  2 files changed, 42 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 9092ac12a270..f4f57a73d095 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -12,7 +12,7 @@
>   *
>   * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>   * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * FITNEsS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
>   * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
>   * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>   * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE

Unrelated whitespace change.  Please drop.

Alex

> @@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct amdgpu_bo *bo)
>         }
>  }
>
> -static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
> +static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)
>  {
> -       struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
>         struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> -       struct amdgpu_bo_user *ubo;
>
>         if (bo->tbo.pin_count > 0)
>                 amdgpu_bo_subtract_pin_size(bo);
> @@ -87,18 +85,40 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>         if (bo->tbo.base.import_attach)
>                 drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
>         drm_gem_object_release(&bo->tbo.base);
> +       amdgpu_bo_unref(&bo->parent);
> +}
> +
> +static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
> +{
> +       struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> +
> +       amdgpu_bo_destroy_base(tbo);
> +       kvfree(bo);
> +}
> +
> +static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
> +{
> +       struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> +       struct amdgpu_bo_user *ubo;
> +
> +       amdgpu_bo_destroy_base(tbo);
> +       ubo = to_amdgpu_bo_user(bo);
> +       kfree(ubo->metadata);
> +       kvfree(bo);
> +}
> +
> +static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
> +{
> +       struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
> +       struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> +
> +       amdgpu_bo_destroy_base(tbo);
>         /* in case amdgpu_device_recover_vram got NULL of bo->parent */
>         if (!list_empty(&bo->shadow_list)) {
>                 mutex_lock(&adev->shadow_list_lock);
>                 list_del_init(&bo->shadow_list);
>                 mutex_unlock(&adev->shadow_list_lock);
>         }
> -       amdgpu_bo_unref(&bo->parent);
> -
> -       if (bo->tbo.type != ttm_bo_type_kernel) {
> -               ubo = to_amdgpu_bo_user(bo);
> -               kfree(ubo->metadata);
> -       }
>
>         kvfree(bo);
>  }
> @@ -115,8 +135,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>   */
>  bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
>  {
> -       if (bo->destroy == &amdgpu_bo_destroy)
> +       if (bo->destroy == &amdgpu_bo_destroy ||
> +           bo->destroy == &amdgpu_bo_user_destroy ||
> +           bo->destroy == &amdgpu_bo_vm_destroy)
>                 return true;
> +
>         return false;
>  }
>
> @@ -592,9 +615,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>         if (bp->type == ttm_bo_type_kernel)
>                 bo->tbo.priority = 1;
>
> +       if (!bp->destroy)
> +               bp->destroy = &amdgpu_bo_destroy;
> +
>         r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type,
>                                  &bo->placement, page_align, &ctx,  NULL,
> -                                bp->resv, &amdgpu_bo_destroy);
> +                                bp->resv, bp->destroy);
>         if (unlikely(r != 0))
>                 return r;
>
> @@ -658,6 +684,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
>         int r;
>
>         bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
> +       bp->destroy = &amdgpu_bo_user_destroy;
>         r = amdgpu_bo_create(adev, bp, &bo_ptr);
>         if (r)
>                 return r;
> @@ -689,6 +716,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
>          * num of amdgpu_vm_pt entries.
>          */
>         BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
> +       bp->destroy = &amdgpu_bo_vm_destroy;
>         r = amdgpu_bo_create(adev, bp, &bo_ptr);
>         if (r)
>                 return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index e36b84516b4e..a8c702634e1b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -55,7 +55,8 @@ struct amdgpu_bo_param {
>         u64                             flags;
>         enum ttm_bo_type                type;
>         bool                            no_wait_gpu;
> -       struct dma_resv *resv;
> +       struct dma_resv                 *resv;
> +       void                            (*destroy)(struct ttm_buffer_object *bo);
>  };
>
>  /* bo virtual addresses in a vm */
> --
> 2.31.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback
  2021-06-14 19:26 [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback Nirmoy Das
  2021-06-14 19:26 ` [PATCH 2/2] drm/amdgpu: move shadow_list to amdgpu_bo_vm Nirmoy Das
  2021-06-14 20:00 ` [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback Alex Deucher
@ 2021-06-15  6:53 ` Christian König
  2021-06-15  8:41   ` Das, Nirmoy
  2 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2021-06-15  6:53 UTC (permalink / raw)
  To: Nirmoy Das, amd-gfx

Am 14.06.21 um 21:26 schrieb Nirmoy Das:
> Make provision to pass different ttm BO destroy callback
> while creating a amdgpu_bo.
>
> v2: pass destroy callback using amdgpu_bo_param.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 52 +++++++++++++++++-----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
>   2 files changed, 42 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 9092ac12a270..f4f57a73d095 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -12,7 +12,7 @@
>    *
>    * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>    * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * FITNEsS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
>    * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
>    * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>    * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> @@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct amdgpu_bo *bo)
>   	}
>   }
>
> -static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
> +static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)
>   {
> -	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
>   	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> -	struct amdgpu_bo_user *ubo;
>
>   	if (bo->tbo.pin_count > 0)
>   		amdgpu_bo_subtract_pin_size(bo);
> @@ -87,18 +85,40 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>   	if (bo->tbo.base.import_attach)
>   		drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
>   	drm_gem_object_release(&bo->tbo.base);
> +	amdgpu_bo_unref(&bo->parent);
> +}
> +
> +static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
> +{
> +	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> +
> +	amdgpu_bo_destroy_base(tbo);
> +	kvfree(bo);
> +}
> +
> +static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
> +{
> +	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> +	struct amdgpu_bo_user *ubo;
> +
> +	amdgpu_bo_destroy_base(tbo);
> +	ubo = to_amdgpu_bo_user(bo);
> +	kfree(ubo->metadata);
> +	kvfree(bo);

Why not freeing the metadata first and having the kvfree() in 
amdgpu_bo_destroy()?

Apart from that looks good to me.

Christian.

> +}
> +
> +static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
> +{
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
> +	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> +
> +	amdgpu_bo_destroy_base(tbo);
>   	/* in case amdgpu_device_recover_vram got NULL of bo->parent */
>   	if (!list_empty(&bo->shadow_list)) {
>   		mutex_lock(&adev->shadow_list_lock);
>   		list_del_init(&bo->shadow_list);
>   		mutex_unlock(&adev->shadow_list_lock);
>   	}
> -	amdgpu_bo_unref(&bo->parent);
> -
> -	if (bo->tbo.type != ttm_bo_type_kernel) {
> -		ubo = to_amdgpu_bo_user(bo);
> -		kfree(ubo->metadata);
> -	}
>
>   	kvfree(bo);
>   }
> @@ -115,8 +135,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>    */
>   bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
>   {
> -	if (bo->destroy == &amdgpu_bo_destroy)
> +	if (bo->destroy == &amdgpu_bo_destroy ||
> +	    bo->destroy == &amdgpu_bo_user_destroy ||
> +	    bo->destroy == &amdgpu_bo_vm_destroy)
>   		return true;
> +
>   	return false;
>   }
>
> @@ -592,9 +615,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   	if (bp->type == ttm_bo_type_kernel)
>   		bo->tbo.priority = 1;
>
> +	if (!bp->destroy)
> +		bp->destroy = &amdgpu_bo_destroy;
> +
>   	r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type,
>   				 &bo->placement, page_align, &ctx,  NULL,
> -				 bp->resv, &amdgpu_bo_destroy);
> +				 bp->resv, bp->destroy);
>   	if (unlikely(r != 0))
>   		return r;
>
> @@ -658,6 +684,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
>   	int r;
>
>   	bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
> +	bp->destroy = &amdgpu_bo_user_destroy;
>   	r = amdgpu_bo_create(adev, bp, &bo_ptr);
>   	if (r)
>   		return r;
> @@ -689,6 +716,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
>   	 * num of amdgpu_vm_pt entries.
>   	 */
>   	BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
> +	bp->destroy = &amdgpu_bo_vm_destroy;
>   	r = amdgpu_bo_create(adev, bp, &bo_ptr);
>   	if (r)
>   		return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index e36b84516b4e..a8c702634e1b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -55,7 +55,8 @@ struct amdgpu_bo_param {
>   	u64				flags;
>   	enum ttm_bo_type		type;
>   	bool				no_wait_gpu;
> -	struct dma_resv	*resv;
> +	struct dma_resv			*resv;
> +	void				(*destroy)(struct ttm_buffer_object *bo);
>   };
>
>   /* bo virtual addresses in a vm */
> --
> 2.31.1
>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: move shadow_list to amdgpu_bo_vm
  2021-06-14 19:26 ` [PATCH 2/2] drm/amdgpu: move shadow_list to amdgpu_bo_vm Nirmoy Das
@ 2021-06-15  6:55   ` Christian König
  0 siblings, 0 replies; 10+ messages in thread
From: Christian König @ 2021-06-15  6:55 UTC (permalink / raw)
  To: Nirmoy Das, amd-gfx

Am 14.06.21 um 21:26 schrieb Nirmoy Das:
> Move shadow_list to struct amdgpu_bo_vm as shadow BOs
> are part of PT/PD BOs.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  5 +++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 13 +++++++------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  6 ++----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     |  2 +-
>   4 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index f2636f411a25..3f51b142fc83 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -4063,6 +4063,7 @@ static int amdgpu_device_recover_vram(struct amdgpu_device *adev)
>   {
>   	struct dma_fence *fence = NULL, *next = NULL;
>   	struct amdgpu_bo *shadow;
> +	struct amdgpu_bo_vm *vmbo;
>   	long r = 1, tmo;
>   
>   	if (amdgpu_sriov_runtime(adev))
> @@ -4072,8 +4073,8 @@ static int amdgpu_device_recover_vram(struct amdgpu_device *adev)
>   
>   	dev_info(adev->dev, "recover vram bo from shadow start\n");
>   	mutex_lock(&adev->shadow_list_lock);
> -	list_for_each_entry(shadow, &adev->shadow_list, shadow_list) {
> -
> +	list_for_each_entry(vmbo, &adev->shadow_list, shadow_list) {
> +		shadow = &vmbo->bo;
>   		/* No need to recover an evicted BO */
>   		if (shadow->tbo.mem.mem_type != TTM_PL_TT ||
>   		    shadow->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET ||
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index f4f57a73d095..9f1e6bd8601b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -111,12 +111,13 @@ static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
>   {
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
>   	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> +	struct amdgpu_bo_vm *vmbo = to_amdgpu_bo_vm(bo);
>   
>   	amdgpu_bo_destroy_base(tbo);
>   	/* in case amdgpu_device_recover_vram got NULL of bo->parent */
> -	if (!list_empty(&bo->shadow_list)) {
> +	if (!list_empty(&vmbo->shadow_list)) {
>   		mutex_lock(&adev->shadow_list_lock);
> -		list_del_init(&bo->shadow_list);
> +		list_del_init(&vmbo->shadow_list);
>   		mutex_unlock(&adev->shadow_list_lock);
>   	}
>   
> @@ -592,7 +593,6 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   	if (bo == NULL)
>   		return -ENOMEM;
>   	drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
> -	INIT_LIST_HEAD(&bo->shadow_list);
>   	bo->vm_bo = NULL;
>   	bo->preferred_domains = bp->preferred_domain ? bp->preferred_domain :
>   		bp->domain;
> @@ -722,6 +722,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
>   		return r;
>   
>   	*vmbo_ptr = to_amdgpu_bo_vm(bo_ptr);
> +	INIT_LIST_HEAD(&(*vmbo_ptr)->shadow_list);
>   	return r;
>   }
>   
> @@ -766,12 +767,12 @@ int amdgpu_bo_validate(struct amdgpu_bo *bo)
>    *
>    * Insert a BO to the shadow list.
>    */
> -void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo)
> +void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo)
>   {
> -	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(vmbo->bo.tbo.bdev);
>   
>   	mutex_lock(&adev->shadow_list_lock);
> -	list_add_tail(&bo->shadow_list, &adev->shadow_list);
> +	list_add_tail(&vmbo->shadow_list, &adev->shadow_list);
>   	mutex_unlock(&adev->shadow_list_lock);
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index a8c702634e1b..18cb2f28e4de 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -109,9 +109,6 @@ struct amdgpu_bo {
>   #ifdef CONFIG_MMU_NOTIFIER
>   	struct mmu_interval_notifier	notifier;
>   #endif
> -
> -	struct list_head		shadow_list;
> -
>   	struct kgd_mem                  *kfd_bo;
>   };
>   
> @@ -127,6 +124,7 @@ struct amdgpu_bo_user {
>   struct amdgpu_bo_vm {
>   	struct amdgpu_bo		bo;
>   	struct amdgpu_bo		*shadow;
> +	struct list_head		shadow_list;

We would actually need an amdgpu_bo_shadow for this, but not in this 
patch probably.

Christian.

>   	struct amdgpu_vm_bo_base        entries[];
>   };
>   
> @@ -333,7 +331,7 @@ u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);
>   int amdgpu_bo_validate(struct amdgpu_bo *bo);
>   void amdgpu_bo_get_memory(struct amdgpu_bo *bo, uint64_t *vram_mem,
>   				uint64_t *gtt_mem, uint64_t *cpu_mem);
> -void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo);
> +void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo);
>   int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
>   			     struct dma_fence **fence);
>   uint32_t amdgpu_bo_get_preferred_pin_domain(struct amdgpu_device *adev,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 4c4c56581780..812c225538a7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -936,7 +936,7 @@ static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
>   	}
>   
>   	(*vmbo)->shadow->parent = amdgpu_bo_ref(bo);
> -	amdgpu_bo_add_to_shadow_list((*vmbo)->shadow);
> +	amdgpu_bo_add_to_shadow_list((*vmbo));
>   
>   	return 0;
>   }

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback
  2021-06-15  6:53 ` Christian König
@ 2021-06-15  8:41   ` Das, Nirmoy
  0 siblings, 0 replies; 10+ messages in thread
From: Das, Nirmoy @ 2021-06-15  8:41 UTC (permalink / raw)
  To: Christian König, amd-gfx


On 6/15/2021 8:53 AM, Christian König wrote:
> Am 14.06.21 um 21:26 schrieb Nirmoy Das:
>> Make provision to pass different ttm BO destroy callback
>> while creating a amdgpu_bo.
>>
>> v2: pass destroy callback using amdgpu_bo_param.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 52 +++++++++++++++++-----
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
>>   2 files changed, 42 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 9092ac12a270..f4f57a73d095 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -12,7 +12,7 @@
>>    *
>>    * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>>    * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> - * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO 
>> EVENT SHALL
>> + * FITNEsS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO 
>> EVENT SHALL
>>    * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE 
>> FOR ANY CLAIM,
>>    * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, 
>> TORT OR
>>    * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE 
>> SOFTWARE OR THE
>> @@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct 
>> amdgpu_bo *bo)
>>       }
>>   }
>>
>> -static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>> +static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)
>>   {
>> -    struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
>>       struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
>> -    struct amdgpu_bo_user *ubo;
>>
>>       if (bo->tbo.pin_count > 0)
>>           amdgpu_bo_subtract_pin_size(bo);
>> @@ -87,18 +85,40 @@ static void amdgpu_bo_destroy(struct 
>> ttm_buffer_object *tbo)
>>       if (bo->tbo.base.import_attach)
>>           drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
>>       drm_gem_object_release(&bo->tbo.base);
>> +    amdgpu_bo_unref(&bo->parent);
>> +}
>> +
>> +static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>> +{
>> +    struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
>> +
>> +    amdgpu_bo_destroy_base(tbo);
>> +    kvfree(bo);
>> +}
>> +
>> +static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
>> +{
>> +    struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
>> +    struct amdgpu_bo_user *ubo;
>> +
>> +    amdgpu_bo_destroy_base(tbo);
>> +    ubo = to_amdgpu_bo_user(bo);
>> +    kfree(ubo->metadata);
>> +    kvfree(bo);
>
> Why not freeing the metadata first and having the kvfree() in 
> amdgpu_bo_destroy()?


Didn't think about that, it would be me much cleaner.


Thanks,

Nirmoy

>
> Apart from that looks good to me.
>
> Christian.
>
>> +}
>> +
>> +static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
>> +{
>> +    struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
>> +    struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
>> +
>> +    amdgpu_bo_destroy_base(tbo);
>>       /* in case amdgpu_device_recover_vram got NULL of bo->parent */
>>       if (!list_empty(&bo->shadow_list)) {
>>           mutex_lock(&adev->shadow_list_lock);
>>           list_del_init(&bo->shadow_list);
>>           mutex_unlock(&adev->shadow_list_lock);
>>       }
>> -    amdgpu_bo_unref(&bo->parent);
>> -
>> -    if (bo->tbo.type != ttm_bo_type_kernel) {
>> -        ubo = to_amdgpu_bo_user(bo);
>> -        kfree(ubo->metadata);
>> -    }
>>
>>       kvfree(bo);
>>   }
>> @@ -115,8 +135,11 @@ static void amdgpu_bo_destroy(struct 
>> ttm_buffer_object *tbo)
>>    */
>>   bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
>>   {
>> -    if (bo->destroy == &amdgpu_bo_destroy)
>> +    if (bo->destroy == &amdgpu_bo_destroy ||
>> +        bo->destroy == &amdgpu_bo_user_destroy ||
>> +        bo->destroy == &amdgpu_bo_vm_destroy)
>>           return true;
>> +
>>       return false;
>>   }
>>
>> @@ -592,9 +615,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>>       if (bp->type == ttm_bo_type_kernel)
>>           bo->tbo.priority = 1;
>>
>> +    if (!bp->destroy)
>> +        bp->destroy = &amdgpu_bo_destroy;
>> +
>>       r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, 
>> bp->type,
>>                    &bo->placement, page_align, &ctx, NULL,
>> -                 bp->resv, &amdgpu_bo_destroy);
>> +                 bp->resv, bp->destroy);
>>       if (unlikely(r != 0))
>>           return r;
>>
>> @@ -658,6 +684,7 @@ int amdgpu_bo_create_user(struct amdgpu_device 
>> *adev,
>>       int r;
>>
>>       bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
>> +    bp->destroy = &amdgpu_bo_user_destroy;
>>       r = amdgpu_bo_create(adev, bp, &bo_ptr);
>>       if (r)
>>           return r;
>> @@ -689,6 +716,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
>>        * num of amdgpu_vm_pt entries.
>>        */
>>       BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
>> +    bp->destroy = &amdgpu_bo_vm_destroy;
>>       r = amdgpu_bo_create(adev, bp, &bo_ptr);
>>       if (r)
>>           return r;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> index e36b84516b4e..a8c702634e1b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> @@ -55,7 +55,8 @@ struct amdgpu_bo_param {
>>       u64                flags;
>>       enum ttm_bo_type        type;
>>       bool                no_wait_gpu;
>> -    struct dma_resv    *resv;
>> +    struct dma_resv            *resv;
>> +    void                (*destroy)(struct ttm_buffer_object *bo);
>>   };
>>
>>   /* bo virtual addresses in a vm */
>> -- 
>> 2.31.1
>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback
  2021-06-14 20:00 ` [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback Alex Deucher
@ 2021-06-15  8:42   ` Das, Nirmoy
  0 siblings, 0 replies; 10+ messages in thread
From: Das, Nirmoy @ 2021-06-15  8:42 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Christian Koenig, amd-gfx list


On 6/14/2021 10:00 PM, Alex Deucher wrote:
> On Mon, Jun 14, 2021 at 3:27 PM Nirmoy Das <nirmoy.das@amd.com> wrote:
>> Make provision to pass different ttm BO destroy callback
>> while creating a amdgpu_bo.
>>
>> v2: pass destroy callback using amdgpu_bo_param.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 52 +++++++++++++++++-----
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
>>   2 files changed, 42 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 9092ac12a270..f4f57a73d095 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -12,7 +12,7 @@
>>    *
>>    * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>>    * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> - * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
>> + * FITNEsS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
>>    * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
>>    * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>>    * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> Unrelated whitespace change.  Please drop.


Thanks Alex. I will remove it in time.


>
> Alex
>
>> @@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct amdgpu_bo *bo)
>>          }
>>   }
>>
>> -static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>> +static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)
>>   {
>> -       struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
>>          struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
>> -       struct amdgpu_bo_user *ubo;
>>
>>          if (bo->tbo.pin_count > 0)
>>                  amdgpu_bo_subtract_pin_size(bo);
>> @@ -87,18 +85,40 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>>          if (bo->tbo.base.import_attach)
>>                  drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
>>          drm_gem_object_release(&bo->tbo.base);
>> +       amdgpu_bo_unref(&bo->parent);
>> +}
>> +
>> +static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>> +{
>> +       struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
>> +
>> +       amdgpu_bo_destroy_base(tbo);
>> +       kvfree(bo);
>> +}
>> +
>> +static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
>> +{
>> +       struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
>> +       struct amdgpu_bo_user *ubo;
>> +
>> +       amdgpu_bo_destroy_base(tbo);
>> +       ubo = to_amdgpu_bo_user(bo);
>> +       kfree(ubo->metadata);
>> +       kvfree(bo);
>> +}
>> +
>> +static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
>> +{
>> +       struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
>> +       struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
>> +
>> +       amdgpu_bo_destroy_base(tbo);
>>          /* in case amdgpu_device_recover_vram got NULL of bo->parent */
>>          if (!list_empty(&bo->shadow_list)) {
>>                  mutex_lock(&adev->shadow_list_lock);
>>                  list_del_init(&bo->shadow_list);
>>                  mutex_unlock(&adev->shadow_list_lock);
>>          }
>> -       amdgpu_bo_unref(&bo->parent);
>> -
>> -       if (bo->tbo.type != ttm_bo_type_kernel) {
>> -               ubo = to_amdgpu_bo_user(bo);
>> -               kfree(ubo->metadata);
>> -       }
>>
>>          kvfree(bo);
>>   }
>> @@ -115,8 +135,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>>    */
>>   bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
>>   {
>> -       if (bo->destroy == &amdgpu_bo_destroy)
>> +       if (bo->destroy == &amdgpu_bo_destroy ||
>> +           bo->destroy == &amdgpu_bo_user_destroy ||
>> +           bo->destroy == &amdgpu_bo_vm_destroy)
>>                  return true;
>> +
>>          return false;
>>   }
>>
>> @@ -592,9 +615,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>>          if (bp->type == ttm_bo_type_kernel)
>>                  bo->tbo.priority = 1;
>>
>> +       if (!bp->destroy)
>> +               bp->destroy = &amdgpu_bo_destroy;
>> +
>>          r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type,
>>                                   &bo->placement, page_align, &ctx,  NULL,
>> -                                bp->resv, &amdgpu_bo_destroy);
>> +                                bp->resv, bp->destroy);
>>          if (unlikely(r != 0))
>>                  return r;
>>
>> @@ -658,6 +684,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
>>          int r;
>>
>>          bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
>> +       bp->destroy = &amdgpu_bo_user_destroy;
>>          r = amdgpu_bo_create(adev, bp, &bo_ptr);
>>          if (r)
>>                  return r;
>> @@ -689,6 +716,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
>>           * num of amdgpu_vm_pt entries.
>>           */
>>          BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
>> +       bp->destroy = &amdgpu_bo_vm_destroy;
>>          r = amdgpu_bo_create(adev, bp, &bo_ptr);
>>          if (r)
>>                  return r;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> index e36b84516b4e..a8c702634e1b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> @@ -55,7 +55,8 @@ struct amdgpu_bo_param {
>>          u64                             flags;
>>          enum ttm_bo_type                type;
>>          bool                            no_wait_gpu;
>> -       struct dma_resv *resv;
>> +       struct dma_resv                 *resv;
>> +       void                            (*destroy)(struct ttm_buffer_object *bo);
>>   };
>>
>>   /* bo virtual addresses in a vm */
>> --
>> 2.31.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cnirmoy.das%40amd.com%7C759d1f627e9f4253baa508d92f6f24bb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637592976669056718%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=74RcdxNGmdFbtidZgHVcYPKR3Nycx4TKNQqfS9%2B6IgI%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback
  2021-06-15 10:48 ` Christian König
@ 2021-06-15 11:45   ` Das, Nirmoy
  0 siblings, 0 replies; 10+ messages in thread
From: Das, Nirmoy @ 2021-06-15 11:45 UTC (permalink / raw)
  To: Christian König, amd-gfx; +Cc: Christian.Koenig


On 6/15/2021 12:48 PM, Christian König wrote:
>
>
> Am 15.06.21 um 11:23 schrieb Nirmoy Das:
>> Make provision to pass different ttm BO destroy callback
>> while creating a amdgpu_bo.
>>
>> v2: remove whitespace.
>>      call amdgpu_bo_destroy_base() at the end for cleaner code.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 48 ++++++++++++++++------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
>>   2 files changed, 38 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 9092ac12a270..8473d153650f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct 
>> amdgpu_bo *bo)
>>       }
>>   }
>>
>> -static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>> +static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)
>
> I think you don't even need to rename the function.
>
>>   {
>> -    struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
>>       struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
>> -    struct amdgpu_bo_user *ubo;
>>
>>       if (bo->tbo.pin_count > 0)
>>           amdgpu_bo_subtract_pin_size(bo);
>> @@ -87,20 +85,38 @@ static void amdgpu_bo_destroy(struct 
>> ttm_buffer_object *tbo)
>>       if (bo->tbo.base.import_attach)
>>           drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
>>       drm_gem_object_release(&bo->tbo.base);
>> +    amdgpu_bo_unref(&bo->parent);
>> +    kvfree(bo);
>> +}
>> +
>> +static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>> +{
>> +    amdgpu_bo_destroy_base(tbo);
>> +}
>
> Nor has that wrapper here.
>
> Apart from that looks good to me.


Thanks, let me resend with that.


Nirmoy

>
> Christian.
>
>> +
>> +static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
>> +{
>> +    struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
>> +    struct amdgpu_bo_user *ubo;
>> +
>> +    ubo = to_amdgpu_bo_user(bo);
>> +    kfree(ubo->metadata);
>> +    amdgpu_bo_destroy_base(tbo);
>> +}
>> +
>> +static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
>> +{
>> +    struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
>> +    struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
>> +
>>       /* in case amdgpu_device_recover_vram got NULL of bo->parent */
>>       if (!list_empty(&bo->shadow_list)) {
>>           mutex_lock(&adev->shadow_list_lock);
>>           list_del_init(&bo->shadow_list);
>>           mutex_unlock(&adev->shadow_list_lock);
>>       }
>> -    amdgpu_bo_unref(&bo->parent);
>> -
>> -    if (bo->tbo.type != ttm_bo_type_kernel) {
>> -        ubo = to_amdgpu_bo_user(bo);
>> -        kfree(ubo->metadata);
>> -    }
>>
>> -    kvfree(bo);
>> +    amdgpu_bo_destroy_base(tbo);
>>   }
>>
>>   /**
>> @@ -115,8 +131,11 @@ static void amdgpu_bo_destroy(struct 
>> ttm_buffer_object *tbo)
>>    */
>>   bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
>>   {
>> -    if (bo->destroy == &amdgpu_bo_destroy)
>> +    if (bo->destroy == &amdgpu_bo_destroy ||
>> +        bo->destroy == &amdgpu_bo_user_destroy ||
>> +        bo->destroy == &amdgpu_bo_vm_destroy)
>>           return true;
>> +
>>       return false;
>>   }
>>
>> @@ -592,9 +611,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>>       if (bp->type == ttm_bo_type_kernel)
>>           bo->tbo.priority = 1;
>>
>> +    if (!bp->destroy)
>> +        bp->destroy = &amdgpu_bo_destroy;
>> +
>>       r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, 
>> bp->type,
>>                    &bo->placement, page_align, &ctx, NULL,
>> -                 bp->resv, &amdgpu_bo_destroy);
>> +                 bp->resv, bp->destroy);
>>       if (unlikely(r != 0))
>>           return r;
>>
>> @@ -658,6 +680,7 @@ int amdgpu_bo_create_user(struct amdgpu_device 
>> *adev,
>>       int r;
>>
>>       bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
>> +    bp->destroy = &amdgpu_bo_user_destroy;
>>       r = amdgpu_bo_create(adev, bp, &bo_ptr);
>>       if (r)
>>           return r;
>> @@ -689,6 +712,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
>>        * num of amdgpu_vm_pt entries.
>>        */
>>       BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
>> +    bp->destroy = &amdgpu_bo_vm_destroy;
>>       r = amdgpu_bo_create(adev, bp, &bo_ptr);
>>       if (r)
>>           return r;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> index e36b84516b4e..a8c702634e1b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> @@ -55,7 +55,8 @@ struct amdgpu_bo_param {
>>       u64                flags;
>>       enum ttm_bo_type        type;
>>       bool                no_wait_gpu;
>> -    struct dma_resv    *resv;
>> +    struct dma_resv            *resv;
>> +    void                (*destroy)(struct ttm_buffer_object *bo);
>>   };
>>
>>   /* bo virtual addresses in a vm */
>> -- 
>> 2.31.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cnirmoy.das%40amd.com%7Cb321ddc590404256e9c808d92feb2a0a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637593509331489349%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Y3zQrnAfnzSalGRkNRp9Ocgr4lOZZ0MtCPLJoghvD0c%3D&amp;reserved=0 
>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback
  2021-06-15  9:23 Nirmoy Das
@ 2021-06-15 10:48 ` Christian König
  2021-06-15 11:45   ` Das, Nirmoy
  0 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2021-06-15 10:48 UTC (permalink / raw)
  To: Nirmoy Das, amd-gfx; +Cc: Christian.Koenig



Am 15.06.21 um 11:23 schrieb Nirmoy Das:
> Make provision to pass different ttm BO destroy callback
> while creating a amdgpu_bo.
>
> v2: remove whitespace.
>      call amdgpu_bo_destroy_base() at the end for cleaner code.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 48 ++++++++++++++++------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
>   2 files changed, 38 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 9092ac12a270..8473d153650f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct amdgpu_bo *bo)
>   	}
>   }
>
> -static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
> +static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)

I think you don't even need to rename the function.

>   {
> -	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
>   	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> -	struct amdgpu_bo_user *ubo;
>
>   	if (bo->tbo.pin_count > 0)
>   		amdgpu_bo_subtract_pin_size(bo);
> @@ -87,20 +85,38 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>   	if (bo->tbo.base.import_attach)
>   		drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
>   	drm_gem_object_release(&bo->tbo.base);
> +	amdgpu_bo_unref(&bo->parent);
> +	kvfree(bo);
> +}
> +
> +static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
> +{
> +	amdgpu_bo_destroy_base(tbo);
> +}

Nor has that wrapper here.

Apart from that looks good to me.

Christian.

> +
> +static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
> +{
> +	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> +	struct amdgpu_bo_user *ubo;
> +
> +	ubo = to_amdgpu_bo_user(bo);
> +	kfree(ubo->metadata);
> +	amdgpu_bo_destroy_base(tbo);
> +}
> +
> +static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
> +{
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
> +	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> +
>   	/* in case amdgpu_device_recover_vram got NULL of bo->parent */
>   	if (!list_empty(&bo->shadow_list)) {
>   		mutex_lock(&adev->shadow_list_lock);
>   		list_del_init(&bo->shadow_list);
>   		mutex_unlock(&adev->shadow_list_lock);
>   	}
> -	amdgpu_bo_unref(&bo->parent);
> -
> -	if (bo->tbo.type != ttm_bo_type_kernel) {
> -		ubo = to_amdgpu_bo_user(bo);
> -		kfree(ubo->metadata);
> -	}
>
> -	kvfree(bo);
> +	amdgpu_bo_destroy_base(tbo);
>   }
>
>   /**
> @@ -115,8 +131,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>    */
>   bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
>   {
> -	if (bo->destroy == &amdgpu_bo_destroy)
> +	if (bo->destroy == &amdgpu_bo_destroy ||
> +	    bo->destroy == &amdgpu_bo_user_destroy ||
> +	    bo->destroy == &amdgpu_bo_vm_destroy)
>   		return true;
> +
>   	return false;
>   }
>
> @@ -592,9 +611,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   	if (bp->type == ttm_bo_type_kernel)
>   		bo->tbo.priority = 1;
>
> +	if (!bp->destroy)
> +		bp->destroy = &amdgpu_bo_destroy;
> +
>   	r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type,
>   				 &bo->placement, page_align, &ctx,  NULL,
> -				 bp->resv, &amdgpu_bo_destroy);
> +				 bp->resv, bp->destroy);
>   	if (unlikely(r != 0))
>   		return r;
>
> @@ -658,6 +680,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
>   	int r;
>
>   	bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
> +	bp->destroy = &amdgpu_bo_user_destroy;
>   	r = amdgpu_bo_create(adev, bp, &bo_ptr);
>   	if (r)
>   		return r;
> @@ -689,6 +712,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
>   	 * num of amdgpu_vm_pt entries.
>   	 */
>   	BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
> +	bp->destroy = &amdgpu_bo_vm_destroy;
>   	r = amdgpu_bo_create(adev, bp, &bo_ptr);
>   	if (r)
>   		return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index e36b84516b4e..a8c702634e1b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -55,7 +55,8 @@ struct amdgpu_bo_param {
>   	u64				flags;
>   	enum ttm_bo_type		type;
>   	bool				no_wait_gpu;
> -	struct dma_resv	*resv;
> +	struct dma_resv			*resv;
> +	void				(*destroy)(struct ttm_buffer_object *bo);
>   };
>
>   /* bo virtual addresses in a vm */
> --
> 2.31.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback
@ 2021-06-15  9:23 Nirmoy Das
  2021-06-15 10:48 ` Christian König
  0 siblings, 1 reply; 10+ messages in thread
From: Nirmoy Das @ 2021-06-15  9:23 UTC (permalink / raw)
  To: amd-gfx; +Cc: Nirmoy Das, Christian.Koenig

Make provision to pass different ttm BO destroy callback
while creating a amdgpu_bo.

v2: remove whitespace.
    call amdgpu_bo_destroy_base() at the end for cleaner code.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 48 ++++++++++++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
 2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 9092ac12a270..8473d153650f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct amdgpu_bo *bo)
 	}
 }

-static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)
 {
-	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
 	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
-	struct amdgpu_bo_user *ubo;

 	if (bo->tbo.pin_count > 0)
 		amdgpu_bo_subtract_pin_size(bo);
@@ -87,20 +85,38 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
 	if (bo->tbo.base.import_attach)
 		drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
 	drm_gem_object_release(&bo->tbo.base);
+	amdgpu_bo_unref(&bo->parent);
+	kvfree(bo);
+}
+
+static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+{
+	amdgpu_bo_destroy_base(tbo);
+}
+
+static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
+{
+	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+	struct amdgpu_bo_user *ubo;
+
+	ubo = to_amdgpu_bo_user(bo);
+	kfree(ubo->metadata);
+	amdgpu_bo_destroy_base(tbo);
+}
+
+static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
+{
+	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
+	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+
 	/* in case amdgpu_device_recover_vram got NULL of bo->parent */
 	if (!list_empty(&bo->shadow_list)) {
 		mutex_lock(&adev->shadow_list_lock);
 		list_del_init(&bo->shadow_list);
 		mutex_unlock(&adev->shadow_list_lock);
 	}
-	amdgpu_bo_unref(&bo->parent);
-
-	if (bo->tbo.type != ttm_bo_type_kernel) {
-		ubo = to_amdgpu_bo_user(bo);
-		kfree(ubo->metadata);
-	}

-	kvfree(bo);
+	amdgpu_bo_destroy_base(tbo);
 }

 /**
@@ -115,8 +131,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
  */
 bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
 {
-	if (bo->destroy == &amdgpu_bo_destroy)
+	if (bo->destroy == &amdgpu_bo_destroy ||
+	    bo->destroy == &amdgpu_bo_user_destroy ||
+	    bo->destroy == &amdgpu_bo_vm_destroy)
 		return true;
+
 	return false;
 }

@@ -592,9 +611,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 	if (bp->type == ttm_bo_type_kernel)
 		bo->tbo.priority = 1;

+	if (!bp->destroy)
+		bp->destroy = &amdgpu_bo_destroy;
+
 	r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type,
 				 &bo->placement, page_align, &ctx,  NULL,
-				 bp->resv, &amdgpu_bo_destroy);
+				 bp->resv, bp->destroy);
 	if (unlikely(r != 0))
 		return r;

@@ -658,6 +680,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
 	int r;

 	bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
+	bp->destroy = &amdgpu_bo_user_destroy;
 	r = amdgpu_bo_create(adev, bp, &bo_ptr);
 	if (r)
 		return r;
@@ -689,6 +712,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
 	 * num of amdgpu_vm_pt entries.
 	 */
 	BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
+	bp->destroy = &amdgpu_bo_vm_destroy;
 	r = amdgpu_bo_create(adev, bp, &bo_ptr);
 	if (r)
 		return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index e36b84516b4e..a8c702634e1b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -55,7 +55,8 @@ struct amdgpu_bo_param {
 	u64				flags;
 	enum ttm_bo_type		type;
 	bool				no_wait_gpu;
-	struct dma_resv	*resv;
+	struct dma_resv			*resv;
+	void				(*destroy)(struct ttm_buffer_object *bo);
 };

 /* bo virtual addresses in a vm */
--
2.31.1

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-06-15 11:45 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-14 19:26 [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback Nirmoy Das
2021-06-14 19:26 ` [PATCH 2/2] drm/amdgpu: move shadow_list to amdgpu_bo_vm Nirmoy Das
2021-06-15  6:55   ` Christian König
2021-06-14 20:00 ` [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback Alex Deucher
2021-06-15  8:42   ` Das, Nirmoy
2021-06-15  6:53 ` Christian König
2021-06-15  8:41   ` Das, Nirmoy
2021-06-15  9:23 Nirmoy Das
2021-06-15 10:48 ` Christian König
2021-06-15 11:45   ` Das, Nirmoy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.