RE: [PATCH 1/3] drm/amdgpu: drop asd shared memory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



RE - Do we need to keep that memory around for the TAs or do they use some other memory?

Hi Alex,

Each TA owns its shared memory for the cmd that gfx send to TEE. The command could be different per TA and might be using simultaneously so the shared memory can't be shared among Tas.

Originally, we also created shared buffer for ASD driver. but since gfx driver is not privileged to talk to ASD directly. The shared buffer is actually never used.

Regards,
Hawking
-----Original Message-----
From: Deucher, Alexander <Alexander.Deucher@xxxxxxx> 
Sent: 2019年12月3日 0:44
To: Zhang, Hawking <Hawking.Zhang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Min, Frank <Frank.Min@xxxxxxx>; Clements, John <John.Clements@xxxxxxx>
Cc: Zhang, Hawking <Hawking.Zhang@xxxxxxx>
Subject: RE: [PATCH 1/3] drm/amdgpu: drop asd shared memory

> -----Original Message-----
> From: Hawking Zhang <Hawking.Zhang@xxxxxxx>
> Sent: Monday, December 2, 2019 1:04 AM
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Min, Frank <Frank.Min@xxxxxxx>; 
> Clements, John <John.Clements@xxxxxxx>; Deucher, Alexander 
> <Alexander.Deucher@xxxxxxx>
> Cc: Zhang, Hawking <Hawking.Zhang@xxxxxxx>
> Subject: [PATCH 1/3] drm/amdgpu: drop asd shared memory
> 
> asd shared memory is not needed since drivers doesn't invoke any 
> further cmd to asd directly after the asd loading. trust application 
> is the one who needs to talk to asd after the initialization
> 

Do we need to keep that memory around for the TAs or do they use some other memory?

Alex

> Change-Id: I728afa4c7e8b67bc06678b10e92ac064ba10173e
> Signed-off-by: Hawking Zhang <Hawking.Zhang@xxxxxxx>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 44 
> +++++++------------------ drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h | 12 
> ++++---
>  2 files changed, 18 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index d8ef7098ffdf..bdc9e7ae4892 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -309,35 +309,17 @@ static int psp_tmr_load(struct psp_context *psp)
>  	return ret;
>  }
> 
> -static void psp_prep_asd_cmd_buf(struct psp_gfx_cmd_resp *cmd,
> -				 uint64_t asd_mc, uint64_t asd_mc_shared,
> -				 uint32_t size, uint32_t shared_size)
> +static void psp_prep_asd_load_cmd_buf(struct psp_gfx_cmd_resp *cmd,
> +				uint64_t asd_mc, uint32_t size)
>  {
>  	cmd->cmd_id = GFX_CMD_ID_LOAD_ASD;
>  	cmd->cmd.cmd_load_ta.app_phy_addr_lo = lower_32_bits(asd_mc);
>  	cmd->cmd.cmd_load_ta.app_phy_addr_hi = upper_32_bits(asd_mc);
>  	cmd->cmd.cmd_load_ta.app_len = size;
> 
> -	cmd->cmd.cmd_load_ta.cmd_buf_phy_addr_lo =
> lower_32_bits(asd_mc_shared);
> -	cmd->cmd.cmd_load_ta.cmd_buf_phy_addr_hi =
> upper_32_bits(asd_mc_shared);
> -	cmd->cmd.cmd_load_ta.cmd_buf_len = shared_size;
> -}
> -
> -static int psp_asd_init(struct psp_context *psp) -{
> -	int ret;
> -
> -	/*
> -	 * Allocate 16k memory aligned to 4k from Frame Buffer (local
> -	 * physical) for shared ASD <-> Driver
> -	 */
> -	ret = amdgpu_bo_create_kernel(psp->adev,
> PSP_ASD_SHARED_MEM_SIZE,
> -				      PAGE_SIZE,
> AMDGPU_GEM_DOMAIN_VRAM,
> -				      &psp->asd_shared_bo,
> -				      &psp->asd_shared_mc_addr,
> -				      &psp->asd_shared_buf);
> -
> -	return ret;
> +	cmd->cmd.cmd_load_ta.cmd_buf_phy_addr_lo = 0;
> +	cmd->cmd.cmd_load_ta.cmd_buf_phy_addr_hi = 0;
> +	cmd->cmd.cmd_load_ta.cmd_buf_len = 0;
>  }
> 
>  static int psp_asd_load(struct psp_context *psp) @@ -359,11 +341,15 
> @@ static int psp_asd_load(struct psp_context *psp)
>  	memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>  	memcpy(psp->fw_pri_buf, psp->asd_start_addr, psp-
> >asd_ucode_size);
> 
> -	psp_prep_asd_cmd_buf(cmd, psp->fw_pri_mc_addr, psp-
> >asd_shared_mc_addr,
> -			     psp->asd_ucode_size,
> PSP_ASD_SHARED_MEM_SIZE);
> +	psp_prep_asd_load_cmd_buf(cmd, psp->fw_pri_mc_addr,
> +				  psp->asd_ucode_size);
> 
>  	ret = psp_cmd_submit_buf(psp, NULL, cmd,
>  				 psp->fence_buf_mc_addr);
> +	if (!ret) {
> +		psp->asd_context.asd_initialized = true;
> +		psp->asd_context.session_id = cmd->resp.session_id;
> +	}
> 
>  	kfree(cmd);
> 
> @@ -1198,12 +1184,6 @@ static int psp_hw_start(struct psp_context *psp)
>  		return ret;
>  	}
> 
> -	ret = psp_asd_init(psp);
> -	if (ret) {
> -		DRM_ERROR("PSP asd init failed!\n");
> -		return ret;
> -	}
> -
>  	ret = psp_asd_load(psp);
>  	if (ret) {
>  		DRM_ERROR("PSP load asd failed!\n"); @@ -1611,8 +1591,6 @@ static 
> int psp_hw_fini(void *handle)
>  			      &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->asd_shared_bo, &psp-
> >asd_shared_mc_addr,
> -			      &psp->asd_shared_buf);
>  	amdgpu_bo_free_kernel(&psp->cmd_buf_bo, &psp-
> >cmd_buf_mc_addr,
>  			      (void **)&psp->cmd_buf_mem);
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> index 6de0ee97861f..a4d7690ea577 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> @@ -32,7 +32,6 @@
> 
>  #define PSP_FENCE_BUFFER_SIZE	0x1000
>  #define PSP_CMD_BUFFER_SIZE	0x1000
> -#define PSP_ASD_SHARED_MEM_SIZE 0x4000  #define 
> PSP_XGMI_SHARED_MEM_SIZE 0x4000  #define PSP_RAS_SHARED_MEM_SIZE 
> 0x8000
>  #define PSP_1_MEG		0x100000
> @@ -130,6 +129,11 @@ struct psp_xgmi_topology_info {
>  	struct psp_xgmi_node_info
> 	nodes[AMDGPU_XGMI_MAX_CONNECTED_NODES];
>  };
> 
> +struct psp_asd_context {
> +	bool			asd_initialized;
> +	uint32_t		session_id;
> +};
> +
>  struct psp_xgmi_context {
>  	uint8_t				initialized;
>  	uint32_t			session_id;
> @@ -238,15 +242,12 @@ struct psp_context
>  	struct amdgpu_bo		*tmr_bo;
>  	uint64_t			tmr_mc_addr;
> 
> -	/* asd firmware and buffer */
> +	/* asd firmware */
>  	const struct firmware		*asd_fw;
>  	uint32_t			asd_fw_version;
>  	uint32_t			asd_feature_version;
>  	uint32_t			asd_ucode_size;
>  	uint8_t				*asd_start_addr;
> -	struct amdgpu_bo		*asd_shared_bo;
> -	uint64_t			asd_shared_mc_addr;
> -	void				*asd_shared_buf;
> 
>  	/* fence buffer */
>  	struct amdgpu_bo		*fence_buf_bo;
> @@ -281,6 +282,7 @@ struct psp_context
>  	uint32_t			ta_dtm_ucode_size;
>  	uint8_t				*ta_dtm_start_addr;
> 
> +	struct psp_asd_context		asd_context;
>  	struct psp_xgmi_context		xgmi_context;
>  	struct psp_ras_context		ras;
>  	struct psp_hdcp_context 	hdcp_context;
> --
> 2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux