Re: [PATCH v2 3/3] drm/amdgpu: drm/amdkfd: add amdgpu_kfd_bo struct

[Nirmoy <nirmodas@xxxxxxx>]

On 3/5/21 1:37 AM, Felix Kuehling wrote:
> Am 2021-03-03 um 7:04 a.m. schrieb Christian König:
> > Am 03.03.21 um 10:25 schrieb Nirmoy Das:
> > > Implement a new struct based on amdgpu_bo base class
> > > for BOs created by kfd device so that kfd related memeber
> > > of amdgpu_bo can be moved there.
> > You should probably restructure which patch has which code in it here.
> >
> > The first one adds the general infrastructure and makes the necessary
> > modification to allow allocating BO structures with different
> > structure size.
> >
> > And the second then adds the amdgpu_kfd_bo structure so that the KFD
> > can use it.
> >
> > You should also double check with Felix if we don't support importing
> > BOs from elsewhere here and if that approach is correct.
> We do support importing graphics BOs into KFD processes.


Ok so this approach is not going to work then.


Thanks,

Nirmoy


> Regards,
>    Felix
>
>
> > Regards,
> > Christian.
> >
> > > v2: rename AMDGPU_GEM_USER_KFD -> AMDGPU_GEM_CREATE_KFD
> > >
> > > Signed-off-by: Nirmoy Das <nirmoy.das@xxxxxxx>
> > > ---
> > >    .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 10 ++++--
> > >    drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c        |  3 +-
> > >    drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    | 32 ++++++++++++++-----
> > >    drivers/gpu/drm/amd/amdgpu/amdgpu_object.h    |  8 ++++-
> > >    4 files changed, 40 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > > index 57798707cd5f..1f52ae4de609 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > > @@ -1152,6 +1152,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> > >        struct sg_table *sg = NULL;
> > >        uint64_t user_addr = 0;
> > >        struct amdgpu_bo *bo;
> > > +    struct amdgpu_kfd_bo *kbo;
> > >        struct amdgpu_bo_param bp;
> > >        u32 domain, alloc_domain;
> > >        u64 alloc_flags;
> > > @@ -1227,17 +1228,20 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> > >        bp.flags = alloc_flags;
> > >        bp.type = bo_type;
> > >        bp.resv = NULL;
> > > -    ret = amdgpu_kfd_bo_create(adev, &bp, &bo);
> > > +    ret = amdgpu_kfd_bo_create(adev, &bp, &kbo);
> > >        if (ret) {
> > >            pr_debug("Failed to create BO on domain %s. ret %d\n",
> > >                    domain_string(alloc_domain), ret);
> > >            goto err_bo_create;
> > >        }
> > > +
> > > +    bo = &kbo->bo;
> > >        if (bo_type == ttm_bo_type_sg) {
> > >            bo->tbo.sg = sg;
> > >            bo->tbo.ttm->sg = sg;
> > >        }
> > > -    bo->kfd_bo = *mem;
> > > +
> > > +    kbo->kfd_bo = *mem;
> > >        (*mem)->bo = bo;
> > >        if (user_addr)
> > >            bo->flags |= AMDGPU_AMDKFD_USERPTR_BO;
> > > @@ -1261,7 +1265,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> > >
> > >    allocate_init_user_pages_failed:
> > >        remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
> > > -    amdgpu_bo_unref(&bo);
> > > +    amdgpu_kfd_bo_unref(&kbo);
> > >        /* Don't unreserve system mem limit twice */
> > >        goto err_reserve_limit;
> > >    err_bo_create:
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> > > index 1da67cf812b1..eaaf4940abcb 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> > > @@ -102,6 +102,7 @@ static bool amdgpu_mn_invalidate_hsa(struct
> > > mmu_interval_notifier *mni,
> > >                         unsigned long cur_seq)
> > >    {
> > >        struct amdgpu_bo *bo = container_of(mni, struct amdgpu_bo,
> > > notifier);
> > > +    struct amdgpu_kfd_bo *kbo = container_of(bo, struct
> > > amdgpu_kfd_bo, bo);
> > >        struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> > >
> > >        if (!mmu_notifier_range_blockable(range))
> > > @@ -111,7 +112,7 @@ static bool amdgpu_mn_invalidate_hsa(struct
> > > mmu_interval_notifier *mni,
> > >
> > >        mmu_interval_set_seq(mni, cur_seq);
> > >
> > > -    amdgpu_amdkfd_evict_userptr(bo->kfd_bo, bo->notifier.mm);
> > > +    amdgpu_amdkfd_evict_userptr(kbo->kfd_bo, bo->notifier.mm);
> > >        mutex_unlock(&adev->notifier_lock);
> > >
> > >        return true;
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > > index 1b41b4870c99..787eb99119a2 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > > @@ -551,8 +551,10 @@ static int amdgpu_bo_do_create(struct
> > > amdgpu_device *adev,
> > >
> > >        acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
> > >                           sizeof(struct amdgpu_bo));
> > > +    if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
> > > +        bp->bo_ptr_size = sizeof(struct amdgpu_bo);
> > >
> > > -    bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
> > > +    bo = kzalloc(bp->bo_ptr_size, GFP_KERNEL);
> > >        if (bo == NULL)
> > >            return -ENOMEM;
> > >        drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base,
> > > size);
> > > @@ -714,35 +716,37 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
> > >
> > >    int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
> > >                 struct amdgpu_bo_param *bp,
> > > -             struct amdgpu_bo **bo_ptr)
> > > +             struct amdgpu_kfd_bo **kfd_bo_ptr)
> > >    {
> > > +    struct amdgpu_bo *bo_ptr;
> > >        u64 flags = bp->flags;
> > >        int r;
> > >
> > >        bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
> > >        bp->flags = bp->flags | AMDGPU_GEM_CREATE_KFD;
> > > -    r = amdgpu_bo_do_create(adev, bp, bo_ptr);
> > > +    bp->bo_ptr_size = sizeof(struct amdgpu_kfd_bo);
> > > +    r = amdgpu_bo_do_create(adev, bp, &bo_ptr);
> > >        if (r)
> > >            return r;
> > >
> > > +    *kfd_bo_ptr = (struct amdgpu_kfd_bo *)bo_ptr;
> > >        if ((flags & AMDGPU_GEM_CREATE_SHADOW) && !(adev->flags &
> > > AMD_IS_APU)) {
> > >            if (!bp->resv)
> > > -            WARN_ON(dma_resv_lock((*bo_ptr)->tbo.base.resv,
> > > +            WARN_ON(dma_resv_lock((*kfd_bo_ptr)->bo.tbo.base.resv,
> > >                                NULL));
> > >
> > > -        r = amdgpu_bo_create_shadow(adev, bp->size, *bo_ptr);
> > > +        r = amdgpu_bo_create_shadow(adev, bp->size,
> > > &(*kfd_bo_ptr)->bo);
> > >
> > >            if (!bp->resv)
> > > -            dma_resv_unlock((*bo_ptr)->tbo.base.resv);
> > > +            dma_resv_unlock((*kfd_bo_ptr)->bo.tbo.base.resv);
> > >
> > >            if (r)
> > > -            amdgpu_bo_unref(bo_ptr);
> > > +            amdgpu_kfd_bo_unref(kfd_bo_ptr);
> > >        }
> > >
> > >        return r;
> > >    }
> > >
> > > -
> > >    /**
> > >     * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
> > >     * @bo: pointer to the buffer object
> > > @@ -910,6 +914,18 @@ void amdgpu_bo_unref(struct amdgpu_bo **bo)
> > >        *bo = NULL;
> > >    }
> > >
> > > +void amdgpu_kfd_bo_unref(struct amdgpu_kfd_bo **kbo)
> > > +{
> > > +    struct ttm_buffer_object *tbo;
> > > +
> > > +    if ((*kbo) == NULL)
> > > +        return;
> > > +
> > > +    tbo = &((*kbo)->bo.tbo);
> > > +    ttm_bo_put(tbo);
> > > +    *kbo = NULL;
> > > +}
> > > +
> > >    /**
> > >     * amdgpu_bo_pin_restricted - pin an &amdgpu_bo buffer object
> > >     * @bo: &amdgpu_bo buffer object to be pinned
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > > index 665ee0015f06..fa98a1fe2574 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > > @@ -43,6 +43,8 @@ struct amdgpu_bo_param {
> > >        u32                domain;
> > >        u32                preferred_domain;
> > >        u64                flags;
> > > +    /* size of a subclass using amdgpu_bo as base class */
> > > +    u32                bo_ptr_size;
> > >        enum ttm_bo_type        type;
> > >        bool                no_wait_gpu;
> > >        struct dma_resv    *resv;
> > > @@ -109,7 +111,10 @@ struct amdgpu_bo {
> > >    #endif
> > >
> > >        struct list_head        shadow_list;
> > > +};
> > >
> > > +struct amdgpu_kfd_bo {
> > > +    struct amdgpu_bo        bo;
> > >        struct kgd_mem                  *kfd_bo;
> > >    };
> > >
> > > @@ -247,7 +252,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
> > >                 struct amdgpu_bo **bo_ptr);
> > >    int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
> > >                 struct amdgpu_bo_param *bp,
> > > -             struct amdgpu_bo **bo_ptr);
> > > +             struct amdgpu_kfd_bo **bo_ptr);
> > >    int amdgpu_bo_create_reserved(struct amdgpu_device *adev,
> > >                      unsigned long size, int align,
> > >                      u32 domain, struct amdgpu_bo **bo_ptr,
> > > @@ -266,6 +271,7 @@ void *amdgpu_bo_kptr(struct amdgpu_bo *bo);
> > >    void amdgpu_bo_kunmap(struct amdgpu_bo *bo);
> > >    struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo);
> > >    void amdgpu_bo_unref(struct amdgpu_bo **bo);
> > > +void amdgpu_kfd_bo_unref(struct amdgpu_kfd_bo **kbo);
> > >    int amdgpu_bo_pin(struct amdgpu_bo *bo, u32 domain);
> > >    int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
> > >                     u64 min_offset, u64 max_offset);
> > > --
> > > 2.30.1
> > >
> > > _______________________________________________
> > > amd-gfx mailing list
> > > amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx