[PATCH 1/2] drm/amdkfd: cleanup device pointer dereference chains

[Joshi, Mukul]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Yunxiang
> Li
> Sent: Thursday, October 10, 2024 11:19 AM
> To: amd-gfx at lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Koenig, Christian
> <Christian.Koenig at amd.com>; Gui, Jack <Jack.Gui at amd.com>; Li, Yunxiang
> (Teddy) <Yunxiang.Li at amd.com>
> Subject: [PATCH 1/2] drm/amdkfd: cleanup device pointer dereference chains
>
> Pull out some duplicated dereference chains into variables, and in some cases grab
> struct device pointer directly from amdgpu_device instead of via drm_device.
>
> Signed-off-by: Yunxiang Li <Yunxiang.Li at amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 32 +++++++++++++-----------
>  1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index d665ecdcd12fc..c334432e55b14 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -1051,6 +1051,7 @@ static void kfd_process_destroy_pdds(struct
> kfd_process *p)
>
>       for (i = 0; i < p->n_pdds; i++) {
>               struct kfd_process_device *pdd = p->pdds[i];
> +             struct amdgpu_device *adev = pdd->dev->adev;
>
>               pr_debug("Releasing pdd (topology id %d) for process (pasid
> 0x%x)\n",
>                               pdd->dev->id, p->pasid);
> @@ -1059,8 +1060,8 @@ static void kfd_process_destroy_pdds(struct
> kfd_process *p)
>               kfd_process_device_destroy_ib_mem(pdd);
>
>               if (pdd->drm_file) {
> -                     amdgpu_amdkfd_gpuvm_release_process_vm(
> -                                     pdd->dev->adev, pdd->drm_priv);
> +                     amdgpu_amdkfd_gpuvm_release_process_vm(adev,
> +                                                            pdd->drm_priv);
>                       fput(pdd->drm_file);
>               }
>
> @@ -1073,15 +1074,14 @@ static void kfd_process_destroy_pdds(struct
> kfd_process *p)
>               kfd_free_process_doorbells(pdd->dev->kfd, pdd);
>
>               if (pdd->dev->kfd->shared_resources.enable_mes)
> -                     amdgpu_amdkfd_free_gtt_mem(pdd->dev->adev,
> -                                                &pdd->proc_ctx_bo);
> +                     amdgpu_amdkfd_free_gtt_mem(adev, &pdd->proc_ctx_bo);
>               /*
>                * before destroying pdd, make sure to report availability
>                * for auto suspend
>                */
>               if (pdd->runtime_inuse) {
> -                     pm_runtime_mark_last_busy(adev_to_drm(pdd->dev-
> >adev)->dev);
> -                     pm_runtime_put_autosuspend(adev_to_drm(pdd->dev-
> >adev)->dev);
> +                     pm_runtime_mark_last_busy(adev->dev);
> +                     pm_runtime_put_autosuspend(adev->dev);
>                       pdd->runtime_inuse = false;
>               }
>
> @@ -1606,6 +1606,8 @@ struct kfd_process_device
> *kfd_create_process_device_data(struct kfd_node *dev,
>                                                       struct kfd_process *p)
>  {
>       struct kfd_process_device *pdd = NULL;
> +     struct amdgpu_device *adev = dev->adev;
> +     struct device *bdev = adev->dev;
>       int retval = 0;
>
>       if (WARN_ON_ONCE(p->n_pdds >= MAX_GPU_INSTANCE)) @@ -
> 1631,14 +1633,17 @@ struct kfd_process_device
> *kfd_create_process_device_data(struct kfd_node *dev,
>       atomic64_set(&pdd->evict_duration_counter, 0);
>
>       if (dev->kfd->shared_resources.enable_mes) {
> -             retval = amdgpu_amdkfd_alloc_gtt_mem(dev->adev,
> +             retval = amdgpu_amdkfd_alloc_gtt_mem(adev,
>                                               AMDGPU_MES_PROC_CTX_SIZE,
>                                               &pdd->proc_ctx_bo,
>                                               &pdd->proc_ctx_gpu_addr,
>                                               &pdd->proc_ctx_cpu_ptr,
>                                               false);
> +             retval = amdgpu_amdkfd_alloc_gtt_mem(
> +                     adev, AMDGPU_MES_PROC_CTX_SIZE, &pdd-
> >proc_ctx_bo,
> +                     &pdd->proc_ctx_gpu_addr, &pdd->proc_ctx_cpu_ptr, false);

Looks like you are duplicating the amdgpu_amdkfd_alloc_gtt_mem call here.

Regards,
Mukul

>               if (retval) {
> -                     dev_err(dev->adev->dev,
> +                     dev_err(bdev,
>                               "failed to allocate process context bo\n");
>                       goto err_free_pdd;
>               }
> @@ -1647,10 +1652,8 @@ struct kfd_process_device
> *kfd_create_process_device_data(struct kfd_node *dev,
>
>       p->pdds[p->n_pdds++] = pdd;
>       if (kfd_dbg_is_per_vmid_supported(pdd->dev))
> -             pdd->spi_dbg_override = pdd->dev->kfd2kgd->disable_debug_trap(
> -                                                     pdd->dev->adev,
> -                                                     false,
> -                                                     0);
> +             pdd->spi_dbg_override =
> +                     pdd->dev->kfd2kgd->disable_debug_trap(adev, false, 0);
>
>       /* Init idr used for memory handle translation */
>       idr_init(&pdd->alloc_idr);
> @@ -1750,11 +1753,12 @@ struct kfd_process_device
> *kfd_bind_process_to_device(struct kfd_node *dev,
>                                                       struct kfd_process *p)
>  {
>       struct kfd_process_device *pdd;
> +     struct device *bdev = dev->adev->dev;
>       int err;
>
>       pdd = kfd_get_process_device_data(dev, p);
>       if (!pdd) {
> -             dev_err(dev->adev->dev, "Process device data doesn't exist\n");
> +             dev_err(bdev, "Process device data doesn't exist\n");
>               return ERR_PTR(-ENOMEM);
>       }
>
> @@ -1767,7 +1771,7 @@ struct kfd_process_device
> *kfd_bind_process_to_device(struct kfd_node *dev,
>        * pdd is destroyed.
>        */
>       if (!pdd->runtime_inuse) {
> -             err = pm_runtime_get_sync(adev_to_drm(dev->adev)->dev);
> +             err = pm_runtime_get_sync(bdev);
>               if (err < 0) {
>                       pm_runtime_put_autosuspend(adev_to_drm(dev->adev)-
> >dev);
>                       return ERR_PTR(err);
> --
> 2.34.1