From: Alex Deucher <alexander.deuc...@amd.com>

[ Upstream commit b95b5391684b39695887afb4a13cccee7820f5d6 ]

Memory allocations should be done in sw_init.  hw_init should
just be hardware programming needed to initialize the IP block.
This is how most other IP blocks work.  Move the GPU memory
allocations from psp hw_init to psp sw_init and move the memory
free to sw_fini.  This also fixes a potential GPU memory leak
if psp hw_init fails.

Reviewed-by: Hawking Zhang <hawking.zh...@amd.com>
Signed-off-by: Alex Deucher <alexander.deuc...@amd.com>
Signed-off-by: Sasha Levin <sas...@kernel.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 95 ++++++++++++-------------
 1 file changed, 47 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 86e2090bbd6e..57e9932d8a04 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -314,7 +314,39 @@ static int psp_sw_init(void *handle)
                }
        }
 
+       ret = amdgpu_bo_create_kernel(adev, PSP_1_MEG, PSP_1_MEG,
+                                     amdgpu_sriov_vf(adev) ?
+                                     AMDGPU_GEM_DOMAIN_VRAM : 
AMDGPU_GEM_DOMAIN_GTT,
+                                     &psp->fw_pri_bo,
+                                     &psp->fw_pri_mc_addr,
+                                     &psp->fw_pri_buf);
+       if (ret)
+               return ret;
+
+       ret = amdgpu_bo_create_kernel(adev, PSP_FENCE_BUFFER_SIZE, PAGE_SIZE,
+                                     AMDGPU_GEM_DOMAIN_VRAM,
+                                     &psp->fence_buf_bo,
+                                     &psp->fence_buf_mc_addr,
+                                     &psp->fence_buf);
+       if (ret)
+               goto failed1;
+
+       ret = amdgpu_bo_create_kernel(adev, PSP_CMD_BUFFER_SIZE, PAGE_SIZE,
+                                     AMDGPU_GEM_DOMAIN_VRAM,
+                                     &psp->cmd_buf_bo, &psp->cmd_buf_mc_addr,
+                                     (void **)&psp->cmd_buf_mem);
+       if (ret)
+               goto failed2;
+
        return 0;
+
+failed2:
+       amdgpu_bo_free_kernel(&psp->fw_pri_bo,
+                             &psp->fw_pri_mc_addr, &psp->fw_pri_buf);
+failed1:
+       amdgpu_bo_free_kernel(&psp->fence_buf_bo,
+                             &psp->fence_buf_mc_addr, &psp->fence_buf);
+       return ret;
 }
 
 static int psp_sw_fini(void *handle)
@@ -344,6 +376,13 @@ static int psp_sw_fini(void *handle)
        kfree(cmd);
        cmd = NULL;
 
+       amdgpu_bo_free_kernel(&psp->fw_pri_bo,
+                             &psp->fw_pri_mc_addr, &psp->fw_pri_buf);
+       amdgpu_bo_free_kernel(&psp->fence_buf_bo,
+                             &psp->fence_buf_mc_addr, &psp->fence_buf);
+       amdgpu_bo_free_kernel(&psp->cmd_buf_bo, &psp->cmd_buf_mc_addr,
+                             (void **)&psp->cmd_buf_mem);
+
        return 0;
 }
 
@@ -2580,51 +2619,18 @@ static int psp_load_fw(struct amdgpu_device *adev)
        struct psp_context *psp = &adev->psp;
 
        if (amdgpu_sriov_vf(adev) && amdgpu_in_reset(adev)) {
-               psp_ring_stop(psp, PSP_RING_TYPE__KM); /* should not destroy 
ring, only stop */
-               goto skip_memalloc;
-       }
-
-       if (amdgpu_sriov_vf(adev)) {
-               ret = amdgpu_bo_create_kernel(adev, PSP_1_MEG, PSP_1_MEG,
-                                               AMDGPU_GEM_DOMAIN_VRAM,
-                                               &psp->fw_pri_bo,
-                                               &psp->fw_pri_mc_addr,
-                                               &psp->fw_pri_buf);
+               /* should not destroy ring, only stop */
+               psp_ring_stop(psp, PSP_RING_TYPE__KM);
        } else {
-               ret = amdgpu_bo_create_kernel(adev, PSP_1_MEG, PSP_1_MEG,
-                                               AMDGPU_GEM_DOMAIN_GTT,
-                                               &psp->fw_pri_bo,
-                                               &psp->fw_pri_mc_addr,
-                                               &psp->fw_pri_buf);
-       }
-
-       if (ret)
-               goto failed;
-
-       ret = amdgpu_bo_create_kernel(adev, PSP_FENCE_BUFFER_SIZE, PAGE_SIZE,
-                                       AMDGPU_GEM_DOMAIN_VRAM,
-                                       &psp->fence_buf_bo,
-                                       &psp->fence_buf_mc_addr,
-                                       &psp->fence_buf);
-       if (ret)
-               goto failed;
-
-       ret = amdgpu_bo_create_kernel(adev, PSP_CMD_BUFFER_SIZE, PAGE_SIZE,
-                                     AMDGPU_GEM_DOMAIN_VRAM,
-                                     &psp->cmd_buf_bo, &psp->cmd_buf_mc_addr,
-                                     (void **)&psp->cmd_buf_mem);
-       if (ret)
-               goto failed;
+               memset(psp->fence_buf, 0, PSP_FENCE_BUFFER_SIZE);
 
-       memset(psp->fence_buf, 0, PSP_FENCE_BUFFER_SIZE);
-
-       ret = psp_ring_init(psp, PSP_RING_TYPE__KM);
-       if (ret) {
-               DRM_ERROR("PSP ring init failed!\n");
-               goto failed;
+               ret = psp_ring_init(psp, PSP_RING_TYPE__KM);
+               if (ret) {
+                       DRM_ERROR("PSP ring init failed!\n");
+                       goto failed;
+               }
        }
 
-skip_memalloc:
        ret = psp_hw_start(psp);
        if (ret)
                goto failed;
@@ -2730,13 +2736,6 @@ static int psp_hw_fini(void *handle)
        psp_tmr_terminate(psp);
        psp_ring_destroy(psp, PSP_RING_TYPE__KM);
 
-       amdgpu_bo_free_kernel(&psp->fw_pri_bo,
-                             &psp->fw_pri_mc_addr, &psp->fw_pri_buf);
-       amdgpu_bo_free_kernel(&psp->fence_buf_bo,
-                             &psp->fence_buf_mc_addr, &psp->fence_buf);
-       amdgpu_bo_free_kernel(&psp->cmd_buf_bo, &psp->cmd_buf_mc_addr,
-                             (void **)&psp->cmd_buf_mem);
-
        return 0;
 }
 
-- 
2.35.1

Reply via email to