Yeah, when I had a glance at this func, flashed a similar idea.
A little comment inline, please confirm it.

Regards,
Jerry

On 07/04/2017 02:10 PM, Huang Rui wrote:
We would like to use a reserve vram to store all non-psp firmware data when it
is submmited. And needn't alloc/free when each firmware submits again and again,
we can reuse that just one page size buffer.

Signed-off-by: Huang Rui <[email protected]>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 39 +++++++++++++++++----------------
  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  5 +++++
  2 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 7b43c60..59700a2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -118,33 +118,18 @@ psp_cmd_submit_buf(struct psp_context *psp,
                   int index)
  {
        int ret;
-       struct amdgpu_bo *cmd_buf_bo;
-       uint64_t cmd_buf_mc_addr;
-       struct psp_gfx_cmd_resp *cmd_buf_mem;
-       struct amdgpu_device *adev = psp->adev;

-       ret = amdgpu_bo_create_kernel(adev, PSP_CMD_BUFFER_SIZE, PAGE_SIZE,
-                                     AMDGPU_GEM_DOMAIN_VRAM,
-                                     &cmd_buf_bo, &cmd_buf_mc_addr,
-                                     (void **)&cmd_buf_mem);
-       if (ret)
-               return ret;
-
-       memset(cmd_buf_mem, 0, PSP_CMD_BUFFER_SIZE);
+       memset(psp->cmd_buf_mem, 0, PSP_CMD_BUFFER_SIZE);

-       memcpy(cmd_buf_mem, cmd, sizeof(struct psp_gfx_cmd_resp));
+       memcpy(psp->cmd_buf_mem, cmd, sizeof(struct psp_gfx_cmd_resp));

-       ret = psp_cmd_submit(psp, ucode, cmd_buf_mc_addr,
+       ret = psp_cmd_submit(psp, ucode, psp->cmd_buf_mc_addr,
                             fence_mc_addr, index);

        while (*((unsigned int *)psp->fence_buf) != index) {
                msleep(1);
        }

-       amdgpu_bo_free_kernel(&cmd_buf_bo,
-                             &cmd_buf_mc_addr,
-                             (void **)&cmd_buf_mem);
-
        return ret;
  }

@@ -352,6 +337,13 @@ static int psp_load_fw(struct amdgpu_device *adev)
                                      &psp->fence_buf_mc_addr,
                                      &psp->fence_buf);
        if (ret)
+               goto failed_mem2;
+
+       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_mem1;

        memset(psp->fence_buf, 0, PSP_FENCE_BUFFER_SIZE);
@@ -379,9 +371,13 @@ static int psp_load_fw(struct amdgpu_device *adev)
        return 0;

We may also need to update the error handling for psp_ring_init()
as "goto failed_mem;"


  failed_mem:
+       amdgpu_bo_free_kernel(&psp->cmd_buf_bo,
+                             &psp->cmd_buf_mc_addr,
+                             (void **)&psp->cmd_buf_mem);
+failed_mem1:
        amdgpu_bo_free_kernel(&psp->fence_buf_bo,
                              &psp->fence_buf_mc_addr, &psp->fence_buf);
-failed_mem1:
+failed_mem2:
        amdgpu_bo_free_kernel(&psp->fw_pri_bo,
                              &psp->fw_pri_mc_addr, &psp->fw_pri_buf);
  failed:
@@ -451,6 +447,11 @@ static int psp_hw_fini(void *handle)
                                      &psp->asd_shared_mc_addr,
                                      &psp->asd_shared_buf);

+       if (psp->cmd_buf_bo)
+               amdgpu_bo_free_kernel(&psp->cmd_buf_bo,
+                                     &psp->cmd_buf_mc_addr,
+                                     (void **)&psp->cmd_buf_mem);
+
        kfree(psp->cmd);
        psp->cmd = NULL;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 1a1c8b4..538fa9d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -108,6 +108,11 @@ struct psp_context
        struct amdgpu_bo                *fence_buf_bo;
        uint64_t                        fence_buf_mc_addr;
        void                            *fence_buf;
+
+       /* cmd buffer */
+       struct amdgpu_bo                *cmd_buf_bo;
+       uint64_t                        cmd_buf_mc_addr;
+       struct psp_gfx_cmd_resp         *cmd_buf_mem;
  };

  struct amdgpu_psp_funcs {

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to