drm/amdgpu/psp: move PSP memory alloc from hw_init to sw_init

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.Zhang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 0bd22ebcc3d1119308508a4421bd2e1b7d612575..0787f2e36f2ab69f91919e81973ce79fe860f0ee 100644 (file)
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -356,7 +356,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)
@@ -390,6 +422,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;
 }
 
@@ -2469,51 +2508,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;
@@ -2631,13 +2637,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;
 }