[PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback

[Das, Nirmoy]

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 at 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 at lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cnirmoy.das%40amd.com%7Cb321ddc590404256e9c808d92feb2a0a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637593509331489349%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Y3zQrnAfnzSalGRkNRp9Ocgr4lOZZ0MtCPLJoghvD0c%3D&reserved=0 
>>
>