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 <ray.huang at amd.com>
> ---
> 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 {
>
