[AMD Public Use]
Reviewed-by: John Clements <john.clements@xxxxxxx>
-----Original Message-----
From: Zhou1, Tao <Tao.Zhou1@xxxxxxx>
Sent: Wednesday, May 6, 2020 4:39 PM
To: Zhang, Hawking <Hawking.Zhang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Clements, John <John.Clements@xxxxxxx>; Gao, Likun <Likun.Gao@xxxxxxx>; Chen, Guchun <Guchun.Chen@xxxxxxx>; Li, Dennis <Dennis.Li@xxxxxxx>
Cc: Zhang, Hawking <Hawking.Zhang@xxxxxxx>
Subject: RE: [PATCH 1/4] drm/amdgpu: switch to common xgmi ta helpers
[AMD Public Use]
The series is:
Reviewed-by: Tao Zhou <tao.zhou1@xxxxxxx>
> -----Original Message-----
> From: Hawking Zhang <Hawking.Zhang@xxxxxxx>
> Sent: 2020年5月6日 14:39
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; Clements, John <John.Clements@xxxxxxx>;
> Gao, Likun <Likun.Gao@xxxxxxx>; Chen, Guchun <Guchun.Chen@xxxxxxx>;
> Li, Dennis <Dennis.Li@xxxxxxx>; Zhou1, Tao <Tao.Zhou1@xxxxxxx>
> Cc: Zhang, Hawking <Hawking.Zhang@xxxxxxx>
> Subject: [PATCH 1/4] drm/amdgpu: switch to common xgmi ta helpers
>
> get_hive_id/get_node_id/get_topology_info/set_topology_info
> are common xgmi command supported by TA for all the ASICs that support
> xgmi link. They should be implemented as common helper functions to
> avoid duplicated code per IP generation
>
> Signed-off-by: Hawking Zhang <Hawking.Zhang@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 115
> ++++++++++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h | 24 +++----
> drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 121
> --------------------------------
> 3 files changed, 123 insertions(+), 137 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index f061ad6..bb5b510 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -664,6 +664,121 @@ int psp_xgmi_initialize(struct psp_context *psp)
> return ret;
> }
>
> +int psp_xgmi_get_hive_id(struct psp_context *psp, uint64_t *hive_id) {
> + struct ta_xgmi_shared_memory *xgmi_cmd;
> + int ret;
> +
> + xgmi_cmd = (struct ta_xgmi_shared_memory*)psp-
> >xgmi_context.xgmi_shared_buf;
> + memset(xgmi_cmd, 0, sizeof(struct ta_xgmi_shared_memory));
> +
> + xgmi_cmd->cmd_id = TA_COMMAND_XGMI__GET_HIVE_ID;
> +
> + /* Invoke xgmi ta to get hive id */
> + ret = psp_xgmi_invoke(psp, xgmi_cmd->cmd_id);
> + if (ret)
> + return ret;
> +
> + *hive_id = xgmi_cmd->xgmi_out_message.get_hive_id.hive_id;
> +
> + return 0;
> +}
> +
> +int psp_xgmi_get_node_id(struct psp_context *psp, uint64_t *node_id) {
> + struct ta_xgmi_shared_memory *xgmi_cmd;
> + int ret;
> +
> + xgmi_cmd = (struct ta_xgmi_shared_memory*)psp-
> >xgmi_context.xgmi_shared_buf;
> + memset(xgmi_cmd, 0, sizeof(struct ta_xgmi_shared_memory));
> +
> + xgmi_cmd->cmd_id = TA_COMMAND_XGMI__GET_NODE_ID;
> +
> + /* Invoke xgmi ta to get the node id */
> + ret = psp_xgmi_invoke(psp, xgmi_cmd->cmd_id);
> + if (ret)
> + return ret;
> +
> + *node_id = xgmi_cmd->xgmi_out_message.get_node_id.node_id;
> +
> + return 0;
> +}
> +
> +int psp_xgmi_get_topology_info(struct psp_context *psp,
> + int number_devices,
> + struct psp_xgmi_topology_info *topology) {
> + struct ta_xgmi_shared_memory *xgmi_cmd;
> + struct ta_xgmi_cmd_get_topology_info_input *topology_info_input;
> + struct ta_xgmi_cmd_get_topology_info_output
> *topology_info_output;
> + int i;
> + int ret;
> +
> + if (!topology || topology->num_nodes >
> TA_XGMI__MAX_CONNECTED_NODES)
> + return -EINVAL;
> +
> + xgmi_cmd = (struct ta_xgmi_shared_memory*)psp-
> >xgmi_context.xgmi_shared_buf;
> + memset(xgmi_cmd, 0, sizeof(struct ta_xgmi_shared_memory));
> +
> + /* Fill in the shared memory with topology information as input */
> + topology_info_input = &xgmi_cmd-
> >xgmi_in_message.get_topology_info;
> + xgmi_cmd->cmd_id =
> TA_COMMAND_XGMI__GET_GET_TOPOLOGY_INFO;
> + topology_info_input->num_nodes = number_devices;
> +
> + for (i = 0; i < topology_info_input->num_nodes; i++) {
> + topology_info_input->nodes[i].node_id = topology-
> >nodes[i].node_id;
> + topology_info_input->nodes[i].num_hops = topology-
> >nodes[i].num_hops;
> + topology_info_input->nodes[i].is_sharing_enabled =
> topology->nodes[i].is_sharing_enabled;
> + topology_info_input->nodes[i].sdma_engine = topology-
> >nodes[i].sdma_engine;
> + }
> +
> + /* Invoke xgmi ta to get the topology information */
> + ret = psp_xgmi_invoke(psp,
> TA_COMMAND_XGMI__GET_GET_TOPOLOGY_INFO);
> + if (ret)
> + return ret;
> +
> + /* Read the output topology information from the shared memory */
> + topology_info_output = &xgmi_cmd-
> >xgmi_out_message.get_topology_info;
> + topology->num_nodes = xgmi_cmd-
> >xgmi_out_message.get_topology_info.num_nodes;
> + for (i = 0; i < topology->num_nodes; i++) {
> + topology->nodes[i].node_id = topology_info_output-
> >nodes[i].node_id;
> + topology->nodes[i].num_hops = topology_info_output-
> >nodes[i].num_hops;
> + topology->nodes[i].is_sharing_enabled =
> topology_info_output->nodes[i].is_sharing_enabled;
> + topology->nodes[i].sdma_engine = topology_info_output-
> >nodes[i].sdma_engine;
> + }
> +
> + return 0;
> +}
> +
> +int psp_xgmi_set_topology_info(struct psp_context *psp,
> + int number_devices,
> + struct psp_xgmi_topology_info *topology) {
> + struct ta_xgmi_shared_memory *xgmi_cmd;
> + struct ta_xgmi_cmd_get_topology_info_input *topology_info_input;
> + int i;
> +
> + if (!topology || topology->num_nodes >
> TA_XGMI__MAX_CONNECTED_NODES)
> + return -EINVAL;
> +
> + xgmi_cmd = (struct ta_xgmi_shared_memory*)psp-
> >xgmi_context.xgmi_shared_buf;
> + memset(xgmi_cmd, 0, sizeof(struct ta_xgmi_shared_memory));
> +
> + topology_info_input = &xgmi_cmd-
> >xgmi_in_message.get_topology_info;
> + xgmi_cmd->cmd_id = TA_COMMAND_XGMI__SET_TOPOLOGY_INFO;
> + topology_info_input->num_nodes = number_devices;
> +
> + for (i = 0; i < topology_info_input->num_nodes; i++) {
> + topology_info_input->nodes[i].node_id = topology-
> >nodes[i].node_id;
> + topology_info_input->nodes[i].num_hops = topology-
> >nodes[i].num_hops;
> + topology_info_input->nodes[i].is_sharing_enabled = 1;
> + topology_info_input->nodes[i].sdma_engine = topology-
> >nodes[i].sdma_engine;
> + }
> +
> + /* Invoke xgmi ta to set topology information */
> + return psp_xgmi_invoke(psp,
> TA_COMMAND_XGMI__SET_TOPOLOGY_INFO);
> +}
> +
> // ras begin
> static int psp_ras_init_shared_buf(struct psp_context *psp) { diff
> --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> index 7fcd63d..263bd8e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> @@ -95,12 +95,6 @@ struct psp_funcs
> enum psp_ring_type ring_type);
> bool (*smu_reload_quirk)(struct psp_context *psp);
> int (*mode1_reset)(struct psp_context *psp);
> - int (*xgmi_get_node_id)(struct psp_context *psp, uint64_t *node_id);
> - int (*xgmi_get_hive_id)(struct psp_context *psp, uint64_t *hive_id);
> - int (*xgmi_get_topology_info)(struct psp_context *psp, int
> number_devices,
> - struct psp_xgmi_topology_info *topology);
> - int (*xgmi_set_topology_info)(struct psp_context *psp, int
> number_devices,
> - struct psp_xgmi_topology_info *topology);
> int (*ras_trigger_error)(struct psp_context *psp,
> struct ta_ras_trigger_error_input *info);
> int (*ras_cure_posion)(struct psp_context *psp, uint64_t *mode_ptr);
> @@ -316,16 +310,6 @@ struct amdgpu_psp_funcs {
> ((psp)->funcs->smu_reload_quirk ? (psp)->funcs-
> >smu_reload_quirk((psp)) : false) #define psp_mode1_reset(psp) \
> ((psp)->funcs->mode1_reset ? (psp)->funcs-
> >mode1_reset((psp)) : false) -#define psp_xgmi_get_node_id(psp,
> >node_id) \
> - ((psp)->funcs->xgmi_get_node_id ? (psp)->funcs-
> >xgmi_get_node_id((psp), (node_id)) : -EINVAL)
> -#define psp_xgmi_get_hive_id(psp, hive_id) \
> - ((psp)->funcs->xgmi_get_hive_id ? (psp)->funcs-
> >xgmi_get_hive_id((psp), (hive_id)) : -EINVAL)
> -#define psp_xgmi_get_topology_info(psp, num_device, topology) \
> - ((psp)->funcs->xgmi_get_topology_info ? \
> - (psp)->funcs->xgmi_get_topology_info((psp), (num_device),
> (topology)) : -EINVAL)
> -#define psp_xgmi_set_topology_info(psp, num_device, topology) \
> - ((psp)->funcs->xgmi_set_topology_info ? \
> - (psp)->funcs->xgmi_set_topology_info((psp), (num_device),
> (topology)) : -EINVAL)
> #define psp_rlc_autoload(psp) \
> ((psp)->funcs->rlc_autoload_start ? (psp)->funcs-
> >rlc_autoload_start((psp)) : 0) #define psp_mem_training_init(psp) \
> >@@ -
> 369,6 +353,14 @@ int psp_update_vcn_sram(struct amdgpu_device *adev,
> int inst_idx, int psp_xgmi_initialize(struct psp_context *psp); int
> psp_xgmi_terminate(struct psp_context *psp); int
> psp_xgmi_invoke(struct psp_context *psp, uint32_t ta_cmd_id);
> +int psp_xgmi_get_hive_id(struct psp_context *psp, uint64_t *hive_id);
> +int psp_xgmi_get_node_id(struct psp_context *psp, uint64_t *node_id);
> +int psp_xgmi_get_topology_info(struct psp_context *psp,
> + int number_devices,
> + struct psp_xgmi_topology_info *topology); int
> +psp_xgmi_set_topology_info(struct psp_context *psp,
> + int number_devices,
> + struct psp_xgmi_topology_info *topology);
>
> int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id); int
> psp_ras_enable_features(struct psp_context *psp, diff --git
> a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> index 97c80f1..4f6c0df 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> @@ -524,123 +524,6 @@ static int psp_v11_0_mode1_reset(struct
> psp_context *psp)
> return 0;
> }
>
> -/* TODO: Fill in follow functions once PSP firmware interface for
> XGMI is ready.
> - * For now, return success and hack the hive_id so high level code
> can
> - * start testing
> - */
> -static int psp_v11_0_xgmi_get_topology_info(struct psp_context *psp,
> - int number_devices, struct psp_xgmi_topology_info *topology)
> -{
> - struct ta_xgmi_shared_memory *xgmi_cmd;
> - struct ta_xgmi_cmd_get_topology_info_input *topology_info_input;
> - struct ta_xgmi_cmd_get_topology_info_output
> *topology_info_output;
> - int i;
> - int ret;
> -
> - if (!topology || topology->num_nodes >
> TA_XGMI__MAX_CONNECTED_NODES)
> - return -EINVAL;
> -
> - xgmi_cmd = (struct ta_xgmi_shared_memory*)psp-
> >xgmi_context.xgmi_shared_buf;
> - memset(xgmi_cmd, 0, sizeof(struct ta_xgmi_shared_memory));
> -
> - /* Fill in the shared memory with topology information as input */
> - topology_info_input = &xgmi_cmd-
> >xgmi_in_message.get_topology_info;
> - xgmi_cmd->cmd_id =
> TA_COMMAND_XGMI__GET_GET_TOPOLOGY_INFO;
> - topology_info_input->num_nodes = number_devices;
> -
> - for (i = 0; i < topology_info_input->num_nodes; i++) {
> - topology_info_input->nodes[i].node_id = topology-
> >nodes[i].node_id;
> - topology_info_input->nodes[i].num_hops = topology-
> >nodes[i].num_hops;
> - topology_info_input->nodes[i].is_sharing_enabled =
> topology->nodes[i].is_sharing_enabled;
> - topology_info_input->nodes[i].sdma_engine = topology-
> >nodes[i].sdma_engine;
> - }
> -
> - /* Invoke xgmi ta to get the topology information */
> - ret = psp_xgmi_invoke(psp,
> TA_COMMAND_XGMI__GET_GET_TOPOLOGY_INFO);
> - if (ret)
> - return ret;
> -
> - /* Read the output topology information from the shared memory */
> - topology_info_output = &xgmi_cmd-
> >xgmi_out_message.get_topology_info;
> - topology->num_nodes = xgmi_cmd-
> >xgmi_out_message.get_topology_info.num_nodes;
> - for (i = 0; i < topology->num_nodes; i++) {
> - topology->nodes[i].node_id = topology_info_output-
> >nodes[i].node_id;
> - topology->nodes[i].num_hops = topology_info_output-
> >nodes[i].num_hops;
> - topology->nodes[i].is_sharing_enabled =
> topology_info_output->nodes[i].is_sharing_enabled;
> - topology->nodes[i].sdma_engine = topology_info_output-
> >nodes[i].sdma_engine;
> - }
> -
> - return 0;
> -}
> -
> -static int psp_v11_0_xgmi_set_topology_info(struct psp_context *psp,
> - int number_devices, struct psp_xgmi_topology_info *topology)
> -{
> - struct ta_xgmi_shared_memory *xgmi_cmd;
> - struct ta_xgmi_cmd_get_topology_info_input *topology_info_input;
> - int i;
> -
> - if (!topology || topology->num_nodes >
> TA_XGMI__MAX_CONNECTED_NODES)
> - return -EINVAL;
> -
> - xgmi_cmd = (struct ta_xgmi_shared_memory*)psp-
> >xgmi_context.xgmi_shared_buf;
> - memset(xgmi_cmd, 0, sizeof(struct ta_xgmi_shared_memory));
> -
> - topology_info_input = &xgmi_cmd-
> >xgmi_in_message.get_topology_info;
> - xgmi_cmd->cmd_id = TA_COMMAND_XGMI__SET_TOPOLOGY_INFO;
> - topology_info_input->num_nodes = number_devices;
> -
> - for (i = 0; i < topology_info_input->num_nodes; i++) {
> - topology_info_input->nodes[i].node_id = topology-
> >nodes[i].node_id;
> - topology_info_input->nodes[i].num_hops = topology-
> >nodes[i].num_hops;
> - topology_info_input->nodes[i].is_sharing_enabled = 1;
> - topology_info_input->nodes[i].sdma_engine = topology-
> >nodes[i].sdma_engine;
> - }
> -
> - /* Invoke xgmi ta to set topology information */
> - return psp_xgmi_invoke(psp,
> TA_COMMAND_XGMI__SET_TOPOLOGY_INFO);
> -}
> -
> -static int psp_v11_0_xgmi_get_hive_id(struct psp_context *psp,
> uint64_t
> *hive_id) -{
> - struct ta_xgmi_shared_memory *xgmi_cmd;
> - int ret;
> -
> - xgmi_cmd = (struct ta_xgmi_shared_memory*)psp-
> >xgmi_context.xgmi_shared_buf;
> - memset(xgmi_cmd, 0, sizeof(struct ta_xgmi_shared_memory));
> -
> - xgmi_cmd->cmd_id = TA_COMMAND_XGMI__GET_HIVE_ID;
> -
> - /* Invoke xgmi ta to get hive id */
> - ret = psp_xgmi_invoke(psp, xgmi_cmd->cmd_id);
> - if (ret)
> - return ret;
> -
> - *hive_id = xgmi_cmd->xgmi_out_message.get_hive_id.hive_id;
> -
> - return 0;
> -}
> -
> -static int psp_v11_0_xgmi_get_node_id(struct psp_context *psp,
> uint64_t
> *node_id) -{
> - struct ta_xgmi_shared_memory *xgmi_cmd;
> - int ret;
> -
> - xgmi_cmd = (struct ta_xgmi_shared_memory*)psp-
> >xgmi_context.xgmi_shared_buf;
> - memset(xgmi_cmd, 0, sizeof(struct ta_xgmi_shared_memory));
> -
> - xgmi_cmd->cmd_id = TA_COMMAND_XGMI__GET_NODE_ID;
> -
> - /* Invoke xgmi ta to get the node id */
> - ret = psp_xgmi_invoke(psp, xgmi_cmd->cmd_id);
> - if (ret)
> - return ret;
> -
> - *node_id = xgmi_cmd->xgmi_out_message.get_node_id.node_id;
> -
> - return 0;
> -}
> -
> static int psp_v11_0_ras_trigger_error(struct psp_context *psp,
> struct ta_ras_trigger_error_input *info) { @@ -995,10 +878,6 @@
> static const struct psp_funcs psp_v11_0_funcs = {
> .ring_stop = psp_v11_0_ring_stop,
> .ring_destroy = psp_v11_0_ring_destroy,
> .mode1_reset = psp_v11_0_mode1_reset,
> - .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
> - .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
> - .xgmi_get_hive_id = psp_v11_0_xgmi_get_hive_id,
> - .xgmi_get_node_id = psp_v11_0_xgmi_get_node_id,
> .ras_trigger_error = psp_v11_0_ras_trigger_error,
> .ras_cure_posion = psp_v11_0_ras_cure_posion,
> .rlc_autoload_start = psp_v11_0_rlc_autoload_start,
> --
> 2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
