Re: [PATCH v4 1/2] platforms/x86: Add AMD system management interface

[Nathan Fontenot <nafonten@amd.com>]

On 2/9/22 19:10, Song Liu wrote:
> On Wed, Feb 9, 2022 at 1:08 PM Nathan Fontenot <nafonten@amd.com> wrote:
>>
> [...]
>>> +
>>> +static const struct hsmp_msg_desc msg_desc_table[] = {
>>> +     /* num_args, response_size, type */
>>> +     {0, 0, RSVD},   /* RESERVED */
>>> +     {1, 1, GET},    /* HSMP_TEST */
>>> +     {0, 1, GET},    /* HSMP_GET_SMU_VER */
>>> +     {0, 1, GET},    /* HSMP_GET_PROTO_VER */
>>> +     {0, 1, GET},    /* HSMP_GET_SOCKET_POWER */
>>> +     {1, 0, SET},    /* HSMP_SET_SOCKET_POWER_LIMIT */
>>> +     {0, 1, GET},    /* HSMP_GET_SOCKET_POWER_LIMIT */
>>> +     {0, 1, GET},    /* HSMP_GET_SOCKET_POWER_LIMIT_MAX */
>>> +     {1, 0, SET},    /* HSMP_SET_BOOST_LIMIT */
>>> +     {1, 0, SET},    /* HSMP_SET_BOOST_LIMIT_SOCKET */
>>> +     {1, 1, GET},    /* HSMP_GET_BOOST_LIMIT */
>>> +     {0, 1, GET},    /* HSMP_GET_PROC_HOT */
>>> +     {1, 0, SET},    /* HSMP_SET_XGMI_LINK_WIDTH */
>>> +     {1, 0, SET},    /* HSMP_SET_DF_PSTATE */
>>> +     {0, 0, SET},    /* HSMP_SET_AUTO_DF_PSTATE */
>>> +     {0, 2, GET},    /* HSMP_GET_FCLK_MCLK */
>>> +     {0, 1, GET},    /* HSMP_GET_CCLK_THROTTLE_LIMIT */
>>> +     {0, 1, GET},    /* HSMP_GET_C0_PERCENT */
>>> +     {1, 0, SET},    /* HSMP_SET_NBIO_DPM_LEVEL */
>>> +     {0, 0, RSVD},   /* RESERVED */
>>> +     {0, 1, GET},    /* HSMP_GET_DDR_BANDWIDTH */
>>> +     {0, 1, GET},    /* HSMP_GET_TEMP_MONITOR */
>>> +};
>>
>> The hsmp_msg_desc, hsmp_msg_type, and msg_desc_table are used by the driver for
>> validating user data. These aren't part of the user API.
>>
>> Perhaps these should be defined in the driver itself instead of being a part of
>> the uapi header.
> 
> This was my idea. While I agree it is a little weird to have these
> tables in a uapi
> header, I think it is helpful to give the user some reference about
> proper num_args
> and response_size for each message. I don't have a better idea to achieve this.
> 

I like the idea to give users a reference on args and responses for each HSMP function.
If this table is kept in the uapi header perhaps we should add a short description of
what the expected args and responses are for each HSMP function with a pointer to the
full documentation of the HSMP functions in the PPR.

-Nathan

> Thanks,
> Song