Re: [PATCH] drm/amdgpu: clean up psp ip if hw_init failed v2

[Paul Menzel]

Dear Alice,

Thank you for your patch.

Am 20.04.22 um 21:37 schrieb Alice Wong:
Call psp_hw_fini when psp_hw_init failed.

Please amend the commit message, and add the motivation/reasoning too [1].

I think it’s common, if a patch (series) is rerolled to list the changes 
between the versions.


Kind regards,

Paul


[1]: https://cbea.ms/git-commit/

Signed-off-by: Alice Wong <shiwei.w...@amd.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 57 +++++++++++++------------
  1 file changed, 29 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 5b9e48d44f5b..52b14efa848a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -2537,6 +2537,34 @@ static int psp_load_fw(struct amdgpu_device *adev)
        return ret;
  }
  
+static int psp_hw_fini(void *handle)
+{
+       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+       struct psp_context *psp = &adev->psp;
+
+       if (psp->ta_fw) {
+               psp_ras_terminate(psp);
+               psp_securedisplay_terminate(psp);
+               psp_rap_terminate(psp);
+               psp_dtm_terminate(psp);
+               psp_hdcp_terminate(psp);
+       }
+
+       psp_asd_terminate(psp);
+
+       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;
+}
+
  static int psp_hw_init(void *handle)
  {
        int ret;
@@ -2563,37 +2591,10 @@ static int psp_hw_init(void *handle)
  failed:
        adev->firmware.load_type = AMDGPU_FW_LOAD_DIRECT;
        mutex_unlock(&adev->firmware.mutex);
+       psp_hw_fini(handle);
        return -EINVAL;
  }
  
-static int psp_hw_fini(void *handle)
-{
-       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-       struct psp_context *psp = &adev->psp;
-
-       if (psp->ta_fw) {
-               psp_ras_terminate(psp);
-               psp_securedisplay_terminate(psp);
-               psp_rap_terminate(psp);
-               psp_dtm_terminate(psp);
-               psp_hdcp_terminate(psp);
-       }
-
-       psp_asd_terminate(psp);
-
-       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;
-}
-
  static int psp_suspend(void *handle)
  {
        int ret;