Re: [patch V2] drm/amdkfd: Access gpuvm_export_dmabuf() api

[David Francis <David.Francis@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 3984 bytes --]


>
> Does this read well.
>
> drm/amdkfd: Access gpuvm_export_dmabuf() API to get Dmabuf
>
> Directly invoking the function amdgpu_gem_prime_export() from within
>
> KFD is not correct. By utilizing the KFD API to obtain Dmabuf, the
>
> implementation can prevent the creation of multiple instances of
>
> struct dma_buf.
>
> Regards,
>
> Ramesh
>
Looks good.

With the new commit message, patch is

Reviewed-by: David Francis<David.Francis@amd.com>

> On 2023-06-22 17:10, Ramesh Errabolu wrote:
>
>     Call KFD api to get Dmabuf instead of calling GEM Prime API
>
> Would appreciate a more detailed commit message to explain why the
> KFD API is preferred over the GEM API.
> With or without that change, this is
> Reviewed-by: David Francis<David.Francis@amd.com>  <mailto:David.Francis@amd.com>
>
>     Signed-off-by: Ramesh Errabolu<Ramesh.Errabolu@amd.com>  <mailto:Ramesh.Errabolu@amd.com>
>
>     ---
>
>       drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 11 +++++------
>
>       1 file changed, 5 insertions(+), 6 deletions(-)
>
>     diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>
>     index cf1db0ab3471..40ac093b5035 100644
>
>     --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>
>     +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>
>     @@ -1852,15 +1852,14 @@ static uint32_t get_process_num_bos(struct kfd_process *p)
>
>              return num_of_bos;
>
>       }
>
>     -static int criu_get_prime_handle(struct drm_gem_object *gobj, int flags,
>
>     +static int criu_get_prime_handle(struct kgd_mem *mem, int flags,
>
>                                            u32 *shared_fd)
>
>       {
>
>              struct dma_buf *dmabuf;
>
>              int ret;
>
>     -       dmabuf = amdgpu_gem_prime_export(gobj, flags);
>
>     -       if (IS_ERR(dmabuf)) {
>
>     -               ret = PTR_ERR(dmabuf);
>
>     +       ret = amdgpu_amdkfd_gpuvm_export_dmabuf(mem, &dmabuf);
>
>     +       if (ret) {
>
>                      pr_err("dmabuf export failed for the BO\n");
>
>                      return ret;
>
>              }
>
>     @@ -1940,7 +1939,7 @@ static int criu_checkpoint_bos(struct kfd_process *p,
>
>                              }
>
>                              if (bo_bucket->alloc_flags
>
>                                  & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM | KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
>
>     -                               ret = criu_get_prime_handle(&dumper_bo->tbo.base,
>
>     +                               ret = criu_get_prime_handle(kgd_mem,
>
>                                                      bo_bucket->alloc_flags &
>
>                                                      KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE ? DRM_RDWR : 0,
>
>                                                      &bo_bucket->dmabuf_fd);
>
>     @@ -2402,7 +2401,7 @@ static int criu_restore_bo(struct kfd_process *p,
>
>              /* create the dmabuf object and export the bo */
>
>              if (bo_bucket->alloc_flags
>
>                  & (KFD_IOC_ALLOC_MEM_FLAGS_VRAM | KFD_IOC_ALLOC_MEM_FLAGS_GTT)) {
>
>     -               ret = criu_get_prime_handle(&kgd_mem->bo->tbo.base, DRM_RDWR,
>
>     +               ret = criu_get_prime_handle(kgd_mem, DRM_RDWR,
>
>                                                  &bo_bucket->dmabuf_fd);
>
>                      if (ret)
>
>                              return ret;
>
>     --
>
>     2.25.1
>

[-- Attachment #2: Type: text/html, Size: 10391 bytes --]