RE: [PATCH v3 2/2] drm/amd/pm: vangogh: send the SMT enable message to pmfw

From: Yang, WenYou
Date: Wed Mar 29 2023 - 03:29:19 EST

[AMD Official Use Only - General]



> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@xxxxxxx>
> Sent: Wednesday, March 29, 2023 12:18 PM
> To: Yang, WenYou <WenYou.Yang@xxxxxxx>; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; Koenig, Christian
> <Christian.Koenig@xxxxxxx>; Pan, Xinhui <Xinhui.Pan@xxxxxxx>; Quan, Evan
> <Evan.Quan@xxxxxxx>; bp@xxxxxxx; peterz@xxxxxxxxxxxxx;
> jpoimboe@xxxxxxxxxx; Phillips, Kim <kim.phillips@xxxxxxx>; tglx@xxxxxxxxxxxxx
> Cc: Yuan, Perry <Perry.Yuan@xxxxxxx>; Liang, Richard qi
> <Richardqi.Liang@xxxxxxx>; Li, Ying <YING.LI@xxxxxxx>; Liu, Kun
> <Kun.Liu2@xxxxxxx>; gpiccoli@xxxxxxxxxx; amd-gfx@xxxxxxxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v3 2/2] drm/amd/pm: vangogh: send the SMT enable
> message to pmfw
>
>
> On 3/28/23 20:51, Wenyou Yang wrote:
> > When the CPU SMT status is changed in the fly, sent the SMT enable
> > message to pmfw to notify it that the SMT status changed.
> >
> > Add the support to send PPSMC_MSG_SetCClkSMTEnable(0x58) message to
> > pmfw for vangogh.
> >
> > Signed-off-by: Wenyou Yang <WenYou.Yang@xxxxxxx>
> > ---
> > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 5 +++
> > drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 7 +++
> > .../pm/swsmu/inc/pmfw_if/smu_v11_5_ppsmc.h | 3 +-
> > drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h | 3 +-
> > .../gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c | 43
> +++++++++++++++++++
> > 5 files changed, 59 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > index b5d64749990e..d53d2acc9b46 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > @@ -69,6 +69,8 @@ static int smu_set_fan_speed_rpm(void *handle,
> uint32_t speed);
> > static int smu_set_gfx_cgpg(struct smu_context *smu, bool enabled);
> > static int smu_set_mp1_state(void *handle, enum pp_mp1_state
> > mp1_state);
> >
> > +extern struct raw_notifier_head smt_notifier_head;
> > +
> > static int smu_sys_get_pp_feature_mask(void *handle,
> > char *buf)
> > {
> > @@ -1122,6 +1124,9 @@ static int smu_sw_fini(void *handle)
> >
> > smu_fini_microcode(smu);
> >
> > + if (smu->nb.notifier_call != NULL)
> > + raw_notifier_chain_unregister(&smt_notifier_head, &smu->nb);
> > +
> > return 0;
> > }
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> > b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> > index 09469c750a96..4d51ac5ec8ba 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> > @@ -566,6 +566,8 @@ struct smu_context
> >
> > struct firmware pptable_firmware;
> >
> > + struct notifier_block nb;
> > +
> > u32 param_reg;
> > u32 msg_reg;
> > u32 resp_reg;
> > @@ -1354,6 +1356,11 @@ struct pptable_funcs {
> > * @init_pptable_microcode: Prepare the pptable microcode to upload
> via PSP
> > */
> > int (*init_pptable_microcode)(struct smu_context *smu);
> > +
> > + /**
> > + * @set_cpu_smt_enable: Set the CPU SMT status.
> > + */
> > + int (*set_cpu_smt_enable)(struct smu_context *smu, bool smt_enable);
> > };
> >
> > typedef enum {
> > diff --git
> > a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v11_5_ppsmc.h
> > b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v11_5_ppsmc.h
> > index 7471e2df2828..a6bfa1912c42 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v11_5_ppsmc.h
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v11_5_ppsmc.h
> > @@ -111,7 +111,8 @@
> > #define PPSMC_MSG_GetGfxOffStatus 0x50
> > #define PPSMC_MSG_GetGfxOffEntryCount 0x51
> > #define PPSMC_MSG_LogGfxOffResidency 0x52
> > -#define PPSMC_Message_Count 0x53
> > +#define PPSMC_MSG_SetCClkSMTEnable 0x58
> > +#define PPSMC_Message_Count 0x59
> >
> > //Argument for PPSMC_MSG_GfxDeviceDriverReset
> > enum {
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> > b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> > index 297b70b9388f..820812d910bf 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> > @@ -245,7 +245,8 @@
> > __SMU_DUMMY_MAP(AllowGpo), \
> > __SMU_DUMMY_MAP(Mode2Reset), \
> > __SMU_DUMMY_MAP(RequestI2cTransaction), \
> > - __SMU_DUMMY_MAP(GetMetricsTable),
> > + __SMU_DUMMY_MAP(GetMetricsTable), \
> > + __SMU_DUMMY_MAP(SetCClkSMTEnable),
> >
> > #undef __SMU_DUMMY_MAP
> > #define __SMU_DUMMY_MAP(type) SMU_MSG_##type
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> > b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> > index 7433dcaa16e0..07f8822f2eb0 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> > @@ -35,6 +35,7 @@
> > #include "asic_reg/gc/gc_10_3_0_offset.h"
> > #include "asic_reg/gc/gc_10_3_0_sh_mask.h"
> > #include <asm/processor.h>
> > +#include <linux/cpu.h>
> >
> > /*
> > * DO NOT use these for err/warn/info/debug messages.
> > @@ -70,6 +71,8 @@
> > FEATURE_MASK(FEATURE_DCFCLK_DPM_BIT)| \
> > FEATURE_MASK(FEATURE_GFX_DPM_BIT))
> >
> > +extern struct raw_notifier_head smt_notifier_head;
> > +
> > static struct cmn2asic_msg_mapping
> vangogh_message_map[SMU_MSG_MAX_COUNT] = {
> > MSG_MAP(TestMessage, PPSMC_MSG_TestMessage,
> 0),
> > MSG_MAP(GetSmuVersion, PPSMC_MSG_GetSmuVersion,
> 0),
> > @@ -141,6 +144,7 @@ static struct cmn2asic_msg_mapping
> vangogh_message_map[SMU_MSG_MAX_COUNT] = {
> > MSG_MAP(GetGfxOffStatus,
> PPSMC_MSG_GetGfxOffStatus, 0),
> > MSG_MAP(GetGfxOffEntryCount,
> PPSMC_MSG_GetGfxOffEntryCount, 0),
> > MSG_MAP(LogGfxOffResidency,
> PPSMC_MSG_LogGfxOffResidency, 0),
> > + MSG_MAP(SetCClkSMTEnable,
> PPSMC_MSG_SetCClkSMTEnable,
> 0),
> > };
> >
> > static struct cmn2asic_mapping
> > vangogh_feature_mask_map[SMU_FEATURE_COUNT] = { @@ -221,6 +225,9
> @@ static const uint8_t vangogh_throttler_map[] = {
> > [THROTTLER_STATUS_BIT_TDC_CVIP] =
> (SMU_THROTTLER_TDC_CVIP_BIT),
> > };
> >
> > +static int smt_notifier_callback(struct notifier_block *nb,
> > + unsigned long action, void *data);
> > +
> > static int vangogh_tables_init(struct smu_context *smu)
> > {
> > struct smu_table_context *smu_table = &smu->smu_table; @@ -477,6
> > +484,9 @@ static int vangogh_init_smc_tables(struct smu_context *smu)
> > smu->cpu_core_num = 4;
> > #endif
> >
> > + smu->nb.notifier_call = smt_notifier_callback;
> > + raw_notifier_chain_register(&smt_notifier_head, &smu->nb);
> > +
> > return smu_v11_0_init_smc_tables(smu);
> > }
> >
> > @@ -2428,6 +2438,12 @@ static u32 vangogh_get_gfxoff_entrycount(struct
> smu_context *smu, uint64_t *entr
> > return ret;
> > }
> >
> > +static int vangogh_set_cpu_smt_enable(struct smu_context *smu, bool
> > +enable) {
> > + return smu_cmn_send_smc_msg_with_param(smu,
> SMU_MSG_SetCClkSMTEnable,
> > + enable ? 1 : 0, NULL);
> > +}
> > +
> > static const struct pptable_funcs vangogh_ppt_funcs = {
> >
> > .check_fw_status = smu_v11_0_check_fw_status, @@ -2474,6 +2490,7
> @@
> > static const struct pptable_funcs vangogh_ppt_funcs = {
> > .get_power_limit = vangogh_get_power_limit,
> > .set_power_limit = vangogh_set_power_limit,
> > .get_vbios_bootup_values = smu_v11_0_get_vbios_bootup_values,
> > + .set_cpu_smt_enable = vangogh_set_cpu_smt_enable,
> > };
> >
> > void vangogh_set_ppt_funcs(struct smu_context *smu) @@ -2486,3
> > +2503,29 @@ void vangogh_set_ppt_funcs(struct smu_context *smu)
> > smu->is_apu = true;
> > smu_v11_0_set_smu_mailbox_registers(smu);
> > }
> > +
> > +static int smu_set_cpu_smt_enable(struct smu_context *smu, bool
> > +enable) {
> > + int ret = -EINVAL;
> > +
> > + if (smu->ppt_funcs && smu->ppt_funcs->set_cpu_smt_enable)
> > + ret = smu->ppt_funcs->set_cpu_smt_enable(smu, enable);
> > +
> > + return ret;
> > +}
> > +
> > +static int smt_notifier_callback(struct notifier_block *nb,
> > + unsigned long action, void *data) {
> > + struct smu_context *smu = container_of(nb, struct smu_context, nb);
> > + int ret;
> > +
> > + smu = container_of(nb, struct smu_context, nb);
> > +
> > + ret = smu_set_cpu_smt_enable(smu, action == SMT_ENABLED);
> > +
> > + dev_dbg(smu->adev->dev, "failed to set cclk_pd_limit for
> SMT %sabled: %d\n",
> > + action == SMT_ENABLED ? "en" : "dis", ret);
> > +
> > + return ret ? NOTIFY_BAD : NOTIFY_OK; }
>
> Is there a particular PMFW version requirement for this message to work?
>
> Perhaps you should only register/unregister for this notifier when that minimum
> PMFW is present.
>
> Otherwise this is going to lead to smu_set_cpu_smt_enable always failing on
> older PMFW and the notifier callback will return NOTIFY_BAD every time.

Thank you for your suggestion, Will add it.

Best Regards,
Wenyou