[Public] The variadic wrapper assigns different argument meanings based on number of parameters used and the position. It will be hard to read code based on it. There is an optional timeout parameter which will need a 5-argument function. Just don't want to miss out on that if everyone starts focusing only on cmn helper functions.
Thanks, Lijo ________________________________ From: Wang, Yang(Kevin) <[email protected]> Sent: Thursday, March 19, 2026 11:23:06 AM To: Lazar, Lijo <[email protected]>; [email protected] <[email protected]> Cc: Deucher, Alexander <[email protected]>; Zhang, Hawking <[email protected]>; Feng, Kenneth <[email protected]> Subject: RE: [PATCH 1/3] drm/amd/pm: add variant func smu_cmn_send_msg() to unify msg sending logic [AMD Official Use Only - AMD Internal Distribution Only] This patch simplifies the development flow with a unified interface and reduces overhead for developers. Most developers can implement features without focusing on low-level details, while complex message sending scenarios can still use the low-level msg sending APIs directly. Btw, The scattered APIs hurt maintainability, and most developers should focus on business logic rather than low-level details. Thus, the "cmn" helper is necessary, it only simplifies the common path while keeping full flexibility for advanced use cases. We do not have to choose one or the other exclusively. Best Regards, Kevin -----Original Message----- From: Lazar, Lijo <[email protected]> Sent: Thursday, March 19, 2026 11:46 AM To: Wang, Yang(Kevin) <[email protected]>; [email protected] Cc: Deucher, Alexander <[email protected]>; Zhang, Hawking <[email protected]>; Feng, Kenneth <[email protected]> Subject: Re: [PATCH 1/3] drm/amd/pm: add variant func smu_cmn_send_msg() to unify msg sending logic On 19-Mar-26 7:39 AM, Yang Wang wrote: > add variant func smu_cmn_send_msg() to unify smu message sending > logic, and enabling support for newer ASIC interfaces such as SMU v15 and > upcoming devices. > (support multi-param/multi-response, standardize code across all smu > code layers) > > The smu_cmn_send_msg() API will expand to the following prototypes based on > the number of input parameters. > e.g: > 1. r = smu_cmn_send_msg(smu, msg_id); > 2. r = smu_cmn_send_msg(smu, msg_id, &read_arg); 3. r = > smu_cmn_send_msg(smu, msg_id, param, &read_arg); 4. r = > smu_cmn_send_msg(smu, msg_id, > num_param, [param0, param1, ...], > num_response, [arg0, arg1, ...] > The intent behind message control is to make the message protocol transparent to IP versions and specific IP versions to have more control over them - if they need to override a message mechanisms, add specific timeouts to particular messages etc. Overall, they are expected to move away from using 'cmn' and directly use message control operations. That also avoids redundant memory copies of in/out arguments. Thanks, Lijo > Signed-off-by: Yang Wang <[email protected]> > --- > drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c | 79 ++++++++++++++++++++++++++ > drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h | 22 +++++++ > 2 files changed, 101 insertions(+) > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c > b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c > index a644579903f4..bc2ac5ae6a48 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c > @@ -193,6 +193,85 @@ int smu_cmn_send_smc_msg(struct smu_context *smu, > read_arg); > } > > +static inline int smu_cmn_send_msg_internal(struct smu_context *smu, enum > smu_message_type msg, > + int num_in_args, u32 *in_args, > + int num_out_args, u32 *out_args) { > + struct smu_msg_ctl *ctl = &smu->msg_ctl; > + struct smu_msg_args args = { 0 }; > + int ret; > + > + if (msg >= SMU_MSG_MAX_COUNT) > + return -EINVAL; > + > + if ((num_in_args >= ARRAY_SIZE(args.args) || num_in_args < 0) || > + (num_out_args >= ARRAY_SIZE(args.out_args) || num_out_args < 0)) > + return -EINVAL; > + > + if ((num_in_args > 0 && !in_args) || (num_out_args > 0 && !out_args)) > + return -EINVAL; > + > + if (!ctl->ops || !ctl->ops->send_msg) > + return -EOPNOTSUPP; > + > + args.msg = msg; > + args.num_args = num_in_args; > + args.num_out_args = num_out_args; > + args.flags = 0; > + args.timeout = 0; > + > + if (num_in_args) > + memcpy(&args.args[0], in_args, num_in_args * sizeof(u32)); > + > + ret = ctl->ops->send_msg(ctl, &args); > + if (ret) > + return ret; > + > + if (num_out_args) > + memcpy(out_args, &args.out_args[0], num_out_args * sizeof(u32)); > + > + return ret; > +} > + > +/* > + * NOTE: To ensure compatibility with the behavioral logic of the > +legacy API, > + * it is required to explicitly set the parameter "param" to 0 when > +invoking > + * the msg_0 and msg_1 functions. > + * */ > + > +int __smu_cmn_send_msg_0(struct smu_context *smu, enum > +smu_message_type msg) { > + return __smu_cmn_send_msg_2(smu, msg, 0, NULL); } > + > +int __smu_cmn_send_msg_1(struct smu_context *smu, enum smu_message_type msg, > + u32 *read_arg) > +{ > + return __smu_cmn_send_msg_2(smu, msg, 0, read_arg); } > + > +int __smu_cmn_send_msg_2(struct smu_context *smu, enum smu_message_type msg, > + u32 param, u32 *read_arg) > +{ > + int ret; > + > + if (read_arg) > + ret = smu_cmn_send_msg_internal(smu, msg, 1, ¶m, 1, > read_arg); > + else > + ret = smu_cmn_send_msg_internal(smu, msg, 1, ¶m, 0, NULL); > + > + return ret; > +} > + > +int __smu_cmn_send_msg_4(struct smu_context *smu, enum smu_message_type msg, > + int num_in_args, u32 *in_args, > + int num_out_args, u32 *out_args) > +{ > + return smu_cmn_send_msg_internal(smu, msg, > + num_in_args, in_args, > + num_out_args, out_args); > +} > + > int smu_cmn_send_debug_smc_msg(struct smu_context *smu, > uint32_t msg) > { > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h > b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h > index e4d282d8bcae..f48356c22dbb 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h > @@ -209,6 +209,28 @@ int smu_cmn_dpm_pcie_gen_idx(int gen); > int smu_cmn_dpm_pcie_width_idx(int width); > int smu_cmn_check_fw_version(struct smu_context *smu); > > +int __smu_cmn_send_msg_0(struct smu_context *smu, enum > +smu_message_type msg); int __smu_cmn_send_msg_1(struct smu_context *smu, > enum smu_message_type msg, > + u32 *read_arg); > +int __smu_cmn_send_msg_2(struct smu_context *smu, enum smu_message_type msg, > + u32 param, u32 *read_arg); > +int __smu_cmn_send_msg_4(struct smu_context *smu, enum smu_message_type msg, > + int num_in_args, u32 *in_args, > + int num_out_args, u32 *out_args); > + > +/* > +* The smu_cmn_send_msg() API will expand to the following prototypes based > on the number of input parameters. > +* e.g: > +* 1. r = smu_cmn_send_msg(smu, msg_id); > +* 2. r = smu_cmn_send_msg(smu, msg_id, &read_arg); > +* 3. r = smu_cmn_send_msg(smu, msg_id, param, &read_arg); > +* 4. r = smu_cmn_send_msg(smu, msg_id, > +* num_param, [param0, param1, ...], > +* num_response, [arg0, arg1, ...] > +*/ > +#define smu_cmn_send_msg(smu, msg, ...) \ > + CONCATENATE(__smu_cmn_send_msg_, COUNT_ARGS(__VA_ARGS__))(smu, msg, > +##__VA_ARGS__) > + > /*SMU gpu metrics */ > > /* Attribute ID mapping */
