* [PATCH v1 0/3] send message to pmfw when SMT changes @ 2023-03-22 5:48 Wenyou Yang 2023-03-22 5:48 ` [PATCH v1 1/3] cpu/smt: add a notifier to notify the " Wenyou Yang ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Wenyou Yang @ 2023-03-22 5:48 UTC (permalink / raw) To: alexander.deucher, christian.koenig, Xinhui.Pan Cc: Wenyou Yang, ying.li, kunliu13, amd-gfx, richardqi.liang When the CPU SMT changes on the fly, send the message to pmfw to notify the SMT status changed. Wenyou Yang (3): cpu/smt: add a notifier to notify the SMT changes drm/amd/pm: send the SMT-enable message to pmfw drm/amd/pm: vangogh: support to send SMT enable message drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 41 +++++++++++++++++++ drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 5 +++ .../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 | 19 +++++++++ include/linux/cpu.h | 5 +++ kernel/cpu.c | 11 ++++- 7 files changed, 84 insertions(+), 3 deletions(-) -- 2.39.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v1 1/3] cpu/smt: add a notifier to notify the SMT changes 2023-03-22 5:48 [PATCH v1 0/3] send message to pmfw when SMT changes Wenyou Yang @ 2023-03-22 5:48 ` Wenyou Yang 2023-03-22 5:48 ` [PATCH v1 2/3] drm/amd/pm: send the SMT-enable message to pmfw Wenyou Yang 2023-03-22 5:48 ` [PATCH v1 3/3] drm/amd/pm: vangogh: support to send SMT enable message Wenyou Yang 2 siblings, 0 replies; 12+ messages in thread From: Wenyou Yang @ 2023-03-22 5:48 UTC (permalink / raw) To: alexander.deucher, christian.koenig, Xinhui.Pan Cc: Wenyou Yang, ying.li, kunliu13, amd-gfx, richardqi.liang Add the notifier chain to notify the cpu SMT status changes Signed-off-by: Wenyou Yang <WenYou.Yang@amd.com> --- include/linux/cpu.h | 5 +++++ kernel/cpu.c | 11 ++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/include/linux/cpu.h b/include/linux/cpu.h index 314802f98b9d..9a842317fe2d 100644 --- a/include/linux/cpu.h +++ b/include/linux/cpu.h @@ -213,6 +213,11 @@ enum cpuhp_smt_control { CPU_SMT_NOT_IMPLEMENTED, }; +enum cpuhp_smt_status { + SMT_ENABLED, + SMT_DISABLED, +}; + #if defined(CONFIG_SMP) && defined(CONFIG_HOTPLUG_SMT) extern enum cpuhp_smt_control cpu_smt_control; extern void cpu_smt_disable(bool force); diff --git a/kernel/cpu.c b/kernel/cpu.c index 6c0a92ca6bb5..accae0fa9868 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -89,6 +89,9 @@ static DEFINE_PER_CPU(struct cpuhp_cpu_state, cpuhp_state) = { cpumask_t cpus_booted_once_mask; #endif +RAW_NOTIFIER_HEAD(smt_notifier_head); +EXPORT_SYMBOL(smt_notifier_head); + #if defined(CONFIG_LOCKDEP) && defined(CONFIG_SMP) static struct lockdep_map cpuhp_state_up_map = STATIC_LOCKDEP_MAP_INIT("cpuhp_state-up", &cpuhp_state_up_map); @@ -2281,8 +2284,10 @@ int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval) */ cpuhp_offline_cpu_device(cpu); } - if (!ret) + if (!ret) { cpu_smt_control = ctrlval; + raw_notifier_call_chain(&smt_notifier_head, SMT_DISABLED, NULL); + } cpu_maps_update_done(); return ret; } @@ -2303,7 +2308,11 @@ int cpuhp_smt_enable(void) /* See comment in cpuhp_smt_disable() */ cpuhp_online_cpu_device(cpu); } + if (!ret) + raw_notifier_call_chain(&smt_notifier_head, SMT_ENABLED, NULL); + cpu_maps_update_done(); + return ret; } #endif -- 2.39.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 2/3] drm/amd/pm: send the SMT-enable message to pmfw 2023-03-22 5:48 [PATCH v1 0/3] send message to pmfw when SMT changes Wenyou Yang 2023-03-22 5:48 ` [PATCH v1 1/3] cpu/smt: add a notifier to notify the " Wenyou Yang @ 2023-03-22 5:48 ` Wenyou Yang 2023-03-23 17:41 ` [v1,2/3] " Limonciello, Mario 2023-03-22 5:48 ` [PATCH v1 3/3] drm/amd/pm: vangogh: support to send SMT enable message Wenyou Yang 2 siblings, 1 reply; 12+ messages in thread From: Wenyou Yang @ 2023-03-22 5:48 UTC (permalink / raw) To: alexander.deucher, christian.koenig, Xinhui.Pan Cc: Wenyou Yang, ying.li, kunliu13, amd-gfx, richardqi.liang When the CPU SMT status change in the fly, sent the SMT-enable message to pmfw to notify it that the SMT status changed. Signed-off-by: Wenyou Yang <WenYou.Yang@amd.com> --- drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 41 +++++++++++++++++++ drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 5 +++ 2 files changed, 46 insertions(+) diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c index b5d64749990e..5cd85a9d149d 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c @@ -22,6 +22,7 @@ #define SWSMU_CODE_LAYER_L1 +#include <linux/cpu.h> #include <linux/firmware.h> #include <linux/pci.h> @@ -69,6 +70,14 @@ 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); +static int smt_notifier_callback(struct notifier_block *nb, unsigned long action, void *data); + +extern struct raw_notifier_head smt_notifier_head; + +static struct notifier_block smt_notifier = { + .notifier_call = smt_notifier_callback, +}; + static int smu_sys_get_pp_feature_mask(void *handle, char *buf) { @@ -625,6 +634,8 @@ static int smu_set_funcs(struct amdgpu_device *adev) return 0; } +static struct smu_context *current_smu; + static int smu_early_init(void *handle) { struct amdgpu_device *adev = (struct amdgpu_device *)handle; @@ -645,6 +656,7 @@ static int smu_early_init(void *handle) mutex_init(&smu->message_lock); adev->powerplay.pp_handle = smu; + current_smu = smu; adev->powerplay.pp_funcs = &swsmu_pm_funcs; r = smu_set_funcs(adev); @@ -1105,6 +1117,8 @@ static int smu_sw_init(void *handle) if (!smu->ppt_funcs->get_fan_control_mode) smu->adev->pm.no_fan = true; + raw_notifier_chain_register(&smt_notifier_head, &smt_notifier); + return 0; } @@ -1122,6 +1136,8 @@ static int smu_sw_fini(void *handle) smu_fini_microcode(smu); + raw_notifier_chain_unregister(&smt_notifier_head, &smt_notifier); + return 0; } @@ -3241,3 +3257,28 @@ int smu_send_hbm_bad_channel_flag(struct smu_context *smu, uint32_t size) return ret; } + +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 = current_smu; + int ret = NOTIFY_OK; + + ret = (action == SMT_ENABLED) ? + smu_set_cpu_smt_enable(smu, true) : + smu_set_cpu_smt_enable(smu, false); + if (ret) + ret = NOTIFY_BAD; + + return ret; +} 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..7c6594bba796 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h @@ -1354,6 +1354,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 enable); }; typedef enum { -- 2.39.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [v1,2/3] drm/amd/pm: send the SMT-enable message to pmfw 2023-03-22 5:48 ` [PATCH v1 2/3] drm/amd/pm: send the SMT-enable message to pmfw Wenyou Yang @ 2023-03-23 17:41 ` Limonciello, Mario 2023-03-23 18:06 ` Limonciello, Mario 2023-03-24 2:14 ` Yang, WenYou 0 siblings, 2 replies; 12+ messages in thread From: Limonciello, Mario @ 2023-03-23 17:41 UTC (permalink / raw) To: Wenyou Yang, alexander.deucher, christian.koenig, Xinhui.Pan Cc: ying.li, kunliu13, richardqi.liang, amd-gfx On 3/22/2023 00:48, Wenyou Yang wrote: > When the CPU SMT status change in the fly, sent the SMT-enable > message to pmfw to notify it that the SMT status changed. > > Signed-off-by: Wenyou Yang <WenYou.Yang@amd.com> > --- > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 41 +++++++++++++++++++ > drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 5 +++ > 2 files changed, 46 insertions(+) > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > index b5d64749990e..5cd85a9d149d 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > @@ -22,6 +22,7 @@ > > #define SWSMU_CODE_LAYER_L1 > > +#include <linux/cpu.h> > #include <linux/firmware.h> > #include <linux/pci.h> > > @@ -69,6 +70,14 @@ 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); > > +static int smt_notifier_callback(struct notifier_block *nb, unsigned long action, void *data); > + > +extern struct raw_notifier_head smt_notifier_head; > + > +static struct notifier_block smt_notifier = { > + .notifier_call = smt_notifier_callback, > +}; > + > static int smu_sys_get_pp_feature_mask(void *handle, > char *buf) > { > @@ -625,6 +634,8 @@ static int smu_set_funcs(struct amdgpu_device *adev) > return 0; > } > > +static struct smu_context *current_smu; > + > static int smu_early_init(void *handle) > { > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > @@ -645,6 +656,7 @@ static int smu_early_init(void *handle) > mutex_init(&smu->message_lock); > > adev->powerplay.pp_handle = smu; > + current_smu = smu; > adev->powerplay.pp_funcs = &swsmu_pm_funcs; > > r = smu_set_funcs(adev); > @@ -1105,6 +1117,8 @@ static int smu_sw_init(void *handle) > if (!smu->ppt_funcs->get_fan_control_mode) > smu->adev->pm.no_fan = true; > > + raw_notifier_chain_register(&smt_notifier_head, &smt_notifier); > + > return 0; > } > > @@ -1122,6 +1136,8 @@ static int smu_sw_fini(void *handle) > > smu_fini_microcode(smu); > > + raw_notifier_chain_unregister(&smt_notifier_head, &smt_notifier); > + > return 0; > } > > @@ -3241,3 +3257,28 @@ int smu_send_hbm_bad_channel_flag(struct smu_context *smu, uint32_t size) > > return ret; > } > + > +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 = current_smu; > + int ret = NOTIFY_OK; This initialization is pointless, it's clobbered in the next line. > + > + ret = (action == SMT_ENABLED) ? > + smu_set_cpu_smt_enable(smu, true) : > + smu_set_cpu_smt_enable(smu, false); How about this instead, it should be more readable: ret = smu_set_cpu_smt_enable(smu, action == SMT_ENABLED); > + if (ret) > + ret = NOTIFY_BAD; > + > + return ret; How about instead: dev_dbg(adev->dev, "failed to %sable SMT: %d\n", action == SMT_ENABLED ? "en" : "dis", ret); return ret ? NOTIFY_BAD : NOTIFY_OK; > +} > 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..7c6594bba796 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h > +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h > @@ -1354,6 +1354,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 enable); > }; > > typedef enum { ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v1,2/3] drm/amd/pm: send the SMT-enable message to pmfw 2023-03-23 17:41 ` [v1,2/3] " Limonciello, Mario @ 2023-03-23 18:06 ` Limonciello, Mario 2023-03-24 2:29 ` Lazar, Lijo 2023-03-24 2:14 ` Yang, WenYou 1 sibling, 1 reply; 12+ messages in thread From: Limonciello, Mario @ 2023-03-23 18:06 UTC (permalink / raw) To: Wenyou Yang, alexander.deucher, christian.koenig, Xinhui.Pan Cc: ying.li, kunliu13, richardqi.liang, amd-gfx On 3/23/2023 12:41, Limonciello, Mario wrote: > On 3/22/2023 00:48, Wenyou Yang wrote: >> When the CPU SMT status change in the fly, sent the SMT-enable >> message to pmfw to notify it that the SMT status changed. >> >> Signed-off-by: Wenyou Yang <WenYou.Yang@amd.com> >> --- >> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 41 +++++++++++++++++++ >> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 5 +++ >> 2 files changed, 46 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c >> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c >> index b5d64749990e..5cd85a9d149d 100644 >> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c >> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c >> @@ -22,6 +22,7 @@ >> #define SWSMU_CODE_LAYER_L1 >> +#include <linux/cpu.h> >> #include <linux/firmware.h> >> #include <linux/pci.h> >> @@ -69,6 +70,14 @@ 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); >> +static int smt_notifier_callback(struct notifier_block *nb, unsigned >> long action, void *data); >> + >> +extern struct raw_notifier_head smt_notifier_head; >> + >> +static struct notifier_block smt_notifier = { >> + .notifier_call = smt_notifier_callback, >> +}; >> + >> static int smu_sys_get_pp_feature_mask(void *handle, >> char *buf) >> { >> @@ -625,6 +634,8 @@ static int smu_set_funcs(struct amdgpu_device *adev) >> return 0; >> } >> +static struct smu_context *current_smu; >> + >> static int smu_early_init(void *handle) >> { >> struct amdgpu_device *adev = (struct amdgpu_device *)handle; >> @@ -645,6 +656,7 @@ static int smu_early_init(void *handle) >> mutex_init(&smu->message_lock); >> adev->powerplay.pp_handle = smu; >> + current_smu = smu; Although this series is intended for the Van Gogh case right now, I dont't think this would scale well for multiple GPUs in a system. I think that instead you may want to move the notifier callback to be a level "higher" in amdgpu. Perhaps amdgpu_device.c? Then when that notifier call is received you'll want to walk through the PCI device space to find any GPUs that are bound with AMDGPU a series of wrappers/calls that end up calling smu_set_cpu_smt_enable with the approriate arguments. >> adev->powerplay.pp_funcs = &swsmu_pm_funcs; >> r = smu_set_funcs(adev); >> @@ -1105,6 +1117,8 @@ static int smu_sw_init(void *handle) >> if (!smu->ppt_funcs->get_fan_control_mode) >> smu->adev->pm.no_fan = true; >> + raw_notifier_chain_register(&smt_notifier_head, &smt_notifier); >> + >> return 0; >> } >> @@ -1122,6 +1136,8 @@ static int smu_sw_fini(void *handle) >> smu_fini_microcode(smu); >> + raw_notifier_chain_unregister(&smt_notifier_head, &smt_notifier); >> + >> return 0; >> } >> @@ -3241,3 +3257,28 @@ int smu_send_hbm_bad_channel_flag(struct >> smu_context *smu, uint32_t size) >> return ret; >> } >> + >> +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 = current_smu; >> + int ret = NOTIFY_OK; > > This initialization is pointless, it's clobbered in the next line. > >> + >> + ret = (action == SMT_ENABLED) ? >> + smu_set_cpu_smt_enable(smu, true) : >> + smu_set_cpu_smt_enable(smu, false); > > How about this instead, it should be more readable: > > ret = smu_set_cpu_smt_enable(smu, action == SMT_ENABLED); > >> + if (ret) >> + ret = NOTIFY_BAD; >> + >> + return ret; > > How about instead: > > dev_dbg(adev->dev, "failed to %sable SMT: %d\n", action == > SMT_ENABLED ? "en" : "dis", ret); > > return ret ? NOTIFY_BAD : NOTIFY_OK; > >> +} >> 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..7c6594bba796 100644 >> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h >> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h >> @@ -1354,6 +1354,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 enable); >> }; >> typedef enum { > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v1,2/3] drm/amd/pm: send the SMT-enable message to pmfw 2023-03-23 18:06 ` Limonciello, Mario @ 2023-03-24 2:29 ` Lazar, Lijo 2023-03-24 17:49 ` Limonciello, Mario 0 siblings, 1 reply; 12+ messages in thread From: Lazar, Lijo @ 2023-03-24 2:29 UTC (permalink / raw) To: Limonciello, Mario, Wenyou Yang, alexander.deucher, christian.koenig, Xinhui.Pan Cc: ying.li, kunliu13, amd-gfx, richardqi.liang On 3/23/2023 11:36 PM, Limonciello, Mario wrote: > On 3/23/2023 12:41, Limonciello, Mario wrote: >> On 3/22/2023 00:48, Wenyou Yang wrote: >>> When the CPU SMT status change in the fly, sent the SMT-enable >>> message to pmfw to notify it that the SMT status changed. >>> >>> Signed-off-by: Wenyou Yang <WenYou.Yang@amd.com> >>> --- >>> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 41 +++++++++++++++++++ >>> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 5 +++ >>> 2 files changed, 46 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c >>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c >>> index b5d64749990e..5cd85a9d149d 100644 >>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c >>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c >>> @@ -22,6 +22,7 @@ >>> #define SWSMU_CODE_LAYER_L1 >>> +#include <linux/cpu.h> >>> #include <linux/firmware.h> >>> #include <linux/pci.h> >>> @@ -69,6 +70,14 @@ 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); >>> +static int smt_notifier_callback(struct notifier_block *nb, unsigned >>> long action, void *data); >>> + >>> +extern struct raw_notifier_head smt_notifier_head; >>> + >>> +static struct notifier_block smt_notifier = { >>> + .notifier_call = smt_notifier_callback, >>> +}; >>> + >>> static int smu_sys_get_pp_feature_mask(void *handle, >>> char *buf) >>> { >>> @@ -625,6 +634,8 @@ static int smu_set_funcs(struct amdgpu_device *adev) >>> return 0; >>> } >>> +static struct smu_context *current_smu; >>> + >>> static int smu_early_init(void *handle) >>> { >>> struct amdgpu_device *adev = (struct amdgpu_device *)handle; >>> @@ -645,6 +656,7 @@ static int smu_early_init(void *handle) >>> mutex_init(&smu->message_lock); >>> adev->powerplay.pp_handle = smu; >>> + current_smu = smu; > > Although this series is intended for the Van Gogh case right now, I > dont't think this would scale well for multiple GPUs in a system. > > I think that instead you may want to move the notifier callback to be a > level "higher" in amdgpu. Perhaps amdgpu_device.c? Then when that > notifier call is received you'll want to walk through the PCI device > space to find any GPUs that are bound with AMDGPU a series of > wrappers/calls that end up calling smu_set_cpu_smt_enable with the > approriate arguments. > This is not required when the notifier is registered only within Vangogh ppt function. Then follow Evan's suggestion of keeping the notifier block inside smu. From the notifier block, it can find the smu block and then call cpu_smt_enable/disable. That way notifier callback comes only once even with multiple dGPUs + Vangogh and processed for the corresponding smu. This notifier doesn't need to be registered for platforms only with dGPUs or APUs which don't need this. Thanks, Lijo > >>> adev->powerplay.pp_funcs = &swsmu_pm_funcs; >>> r = smu_set_funcs(adev); >>> @@ -1105,6 +1117,8 @@ static int smu_sw_init(void *handle) >>> if (!smu->ppt_funcs->get_fan_control_mode) >>> smu->adev->pm.no_fan = true; >>> + raw_notifier_chain_register(&smt_notifier_head, &smt_notifier); >>> + >>> return 0; >>> } >>> @@ -1122,6 +1136,8 @@ static int smu_sw_fini(void *handle) >>> smu_fini_microcode(smu); >>> + raw_notifier_chain_unregister(&smt_notifier_head, &smt_notifier); >>> + >>> return 0; >>> } >>> @@ -3241,3 +3257,28 @@ int smu_send_hbm_bad_channel_flag(struct >>> smu_context *smu, uint32_t size) >>> return ret; >>> } >>> + >>> +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 = current_smu; >>> + int ret = NOTIFY_OK; >> >> This initialization is pointless, it's clobbered in the next line. >> >>> + >>> + ret = (action == SMT_ENABLED) ? >>> + smu_set_cpu_smt_enable(smu, true) : >>> + smu_set_cpu_smt_enable(smu, false); >> >> How about this instead, it should be more readable: >> >> ret = smu_set_cpu_smt_enable(smu, action == SMT_ENABLED); >> >>> + if (ret) >>> + ret = NOTIFY_BAD; >>> + >>> + return ret; >> >> How about instead: >> >> dev_dbg(adev->dev, "failed to %sable SMT: %d\n", action == >> SMT_ENABLED ? "en" : "dis", ret); >> >> return ret ? NOTIFY_BAD : NOTIFY_OK; >> >>> +} >>> 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..7c6594bba796 100644 >>> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h >>> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h >>> @@ -1354,6 +1354,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 enable); >>> }; >>> typedef enum { >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [v1,2/3] drm/amd/pm: send the SMT-enable message to pmfw 2023-03-24 2:29 ` Lazar, Lijo @ 2023-03-24 17:49 ` Limonciello, Mario 2023-03-25 1:49 ` Lazar, Lijo 0 siblings, 1 reply; 12+ messages in thread From: Limonciello, Mario @ 2023-03-24 17:49 UTC (permalink / raw) To: Lazar, Lijo, Yang, WenYou, Deucher, Alexander, Koenig, Christian, Pan, Xinhui Cc: Li, Ying, amd-gfx, Liang, Richard qi, Liu, Kun [AMD Official Use Only - General] > -----Original Message----- > From: Lazar, Lijo <Lijo.Lazar@amd.com> > Sent: Thursday, March 23, 2023 21:29 > To: Limonciello, Mario <Mario.Limonciello@amd.com>; Yang, WenYou > <WenYou.Yang@amd.com>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Koenig, Christian > <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com> > Cc: Li, Ying <YING.LI@amd.com>; Liu, Kun <Kun.Liu2@amd.com>; Liang, > Richard qi <Richardqi.Liang@amd.com>; amd-gfx@lists.freedesktop.org > Subject: Re: [v1,2/3] drm/amd/pm: send the SMT-enable message to pmfw > > > > On 3/23/2023 11:36 PM, Limonciello, Mario wrote: > > On 3/23/2023 12:41, Limonciello, Mario wrote: > >> On 3/22/2023 00:48, Wenyou Yang wrote: > >>> When the CPU SMT status change in the fly, sent the SMT-enable > >>> message to pmfw to notify it that the SMT status changed. > >>> > >>> Signed-off-by: Wenyou Yang <WenYou.Yang@amd.com> > >>> --- > >>> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 41 > +++++++++++++++++++ > >>> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 5 +++ > >>> 2 files changed, 46 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > >>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > >>> index b5d64749990e..5cd85a9d149d 100644 > >>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > >>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > >>> @@ -22,6 +22,7 @@ > >>> #define SWSMU_CODE_LAYER_L1 > >>> +#include <linux/cpu.h> > >>> #include <linux/firmware.h> > >>> #include <linux/pci.h> > >>> @@ -69,6 +70,14 @@ 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); > >>> +static int smt_notifier_callback(struct notifier_block *nb, unsigned > >>> long action, void *data); > >>> + > >>> +extern struct raw_notifier_head smt_notifier_head; > >>> + > >>> +static struct notifier_block smt_notifier = { > >>> + .notifier_call = smt_notifier_callback, > >>> +}; > >>> + > >>> static int smu_sys_get_pp_feature_mask(void *handle, > >>> char *buf) > >>> { > >>> @@ -625,6 +634,8 @@ static int smu_set_funcs(struct amdgpu_device > *adev) > >>> return 0; > >>> } > >>> +static struct smu_context *current_smu; > >>> + > >>> static int smu_early_init(void *handle) > >>> { > >>> struct amdgpu_device *adev = (struct amdgpu_device *)handle; > >>> @@ -645,6 +656,7 @@ static int smu_early_init(void *handle) > >>> mutex_init(&smu->message_lock); > >>> adev->powerplay.pp_handle = smu; > >>> + current_smu = smu; > > > > Although this series is intended for the Van Gogh case right now, I > > dont't think this would scale well for multiple GPUs in a system. > > > > I think that instead you may want to move the notifier callback to be a > > level "higher" in amdgpu. Perhaps amdgpu_device.c? Then when that > > notifier call is received you'll want to walk through the PCI device > > space to find any GPUs that are bound with AMDGPU a series of > > wrappers/calls that end up calling smu_set_cpu_smt_enable with the > > approriate arguments. > > > > This is not required when the notifier is registered only within Vangogh > ppt function. Then follow Evan's suggestion of keeping the notifier > block inside smu. From the notifier block, it can find the smu block and > then call cpu_smt_enable/disable. That way notifier callback comes only > once even with multiple dGPUs + Vangogh and processed for the > corresponding smu. > > This notifier doesn't need to be registered for platforms only with > dGPUs or APUs which don't need this. They don't right now, but I was thinking how this could scale to other APUs or dGPUs if they are interested in adding support for this message too. > > Thanks, > Lijo > > > > >>> adev->powerplay.pp_funcs = &swsmu_pm_funcs; > >>> r = smu_set_funcs(adev); > >>> @@ -1105,6 +1117,8 @@ static int smu_sw_init(void *handle) > >>> if (!smu->ppt_funcs->get_fan_control_mode) > >>> smu->adev->pm.no_fan = true; > >>> + raw_notifier_chain_register(&smt_notifier_head, &smt_notifier); > >>> + > >>> return 0; > >>> } > >>> @@ -1122,6 +1136,8 @@ static int smu_sw_fini(void *handle) > >>> smu_fini_microcode(smu); > >>> + raw_notifier_chain_unregister(&smt_notifier_head, &smt_notifier); > >>> + > >>> return 0; > >>> } > >>> @@ -3241,3 +3257,28 @@ int smu_send_hbm_bad_channel_flag(struct > >>> smu_context *smu, uint32_t size) > >>> return ret; > >>> } > >>> + > >>> +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 = current_smu; > >>> + int ret = NOTIFY_OK; > >> > >> This initialization is pointless, it's clobbered in the next line. > >> > >>> + > >>> + ret = (action == SMT_ENABLED) ? > >>> + smu_set_cpu_smt_enable(smu, true) : > >>> + smu_set_cpu_smt_enable(smu, false); > >> > >> How about this instead, it should be more readable: > >> > >> ret = smu_set_cpu_smt_enable(smu, action == SMT_ENABLED); > >> > >>> + if (ret) > >>> + ret = NOTIFY_BAD; > >>> + > >>> + return ret; > >> > >> How about instead: > >> > >> dev_dbg(adev->dev, "failed to %sable SMT: %d\n", action == > >> SMT_ENABLED ? "en" : "dis", ret); > >> > >> return ret ? NOTIFY_BAD : NOTIFY_OK; > >> > >>> +} > >>> 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..7c6594bba796 100644 > >>> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h > >>> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h > >>> @@ -1354,6 +1354,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 enable); > >>> }; > >>> typedef enum { > >> > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v1,2/3] drm/amd/pm: send the SMT-enable message to pmfw 2023-03-24 17:49 ` Limonciello, Mario @ 2023-03-25 1:49 ` Lazar, Lijo 0 siblings, 0 replies; 12+ messages in thread From: Lazar, Lijo @ 2023-03-25 1:49 UTC (permalink / raw) To: Limonciello, Mario, Yang, WenYou, Deucher, Alexander, Koenig, Christian, Pan, Xinhui Cc: Li, Ying, amd-gfx, Liang, Richard qi, Liu, Kun [-- Attachment #1: Type: text/plain, Size: 7934 bytes --] [AMD Official Use Only - General] The notifier block is embedded in smu context of a device. If there are 4 devices and 3 of them are interested, they register using the notifier block within their smu context. Notifier is called in a chain and when received they use the container_of to get the smu context and communicate with the respective device's FW on the actions to do. BTW, I don't know why dGPU PMFW would be interested in SMT change. On AMD APU + dGPU we already have things like smartshift and it will take care if communicated to APU alone. Thanks, Lijo ________________________________ From: Limonciello, Mario <Mario.Limonciello@amd.com> Sent: Friday, March 24, 2023 11:19:55 PM To: Lazar, Lijo <Lijo.Lazar@amd.com>; Yang, WenYou <WenYou.Yang@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com> Cc: Li, Ying <YING.LI@amd.com>; Liu, Kun <Kun.Liu2@amd.com>; Liang, Richard qi <Richardqi.Liang@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org> Subject: RE: [v1,2/3] drm/amd/pm: send the SMT-enable message to pmfw [AMD Official Use Only - General] > -----Original Message----- > From: Lazar, Lijo <Lijo.Lazar@amd.com> > Sent: Thursday, March 23, 2023 21:29 > To: Limonciello, Mario <Mario.Limonciello@amd.com>; Yang, WenYou > <WenYou.Yang@amd.com>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Koenig, Christian > <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com> > Cc: Li, Ying <YING.LI@amd.com>; Liu, Kun <Kun.Liu2@amd.com>; Liang, > Richard qi <Richardqi.Liang@amd.com>; amd-gfx@lists.freedesktop.org > Subject: Re: [v1,2/3] drm/amd/pm: send the SMT-enable message to pmfw > > > > On 3/23/2023 11:36 PM, Limonciello, Mario wrote: > > On 3/23/2023 12:41, Limonciello, Mario wrote: > >> On 3/22/2023 00:48, Wenyou Yang wrote: > >>> When the CPU SMT status change in the fly, sent the SMT-enable > >>> message to pmfw to notify it that the SMT status changed. > >>> > >>> Signed-off-by: Wenyou Yang <WenYou.Yang@amd.com> > >>> --- > >>> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 41 > +++++++++++++++++++ > >>> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 5 +++ > >>> 2 files changed, 46 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > >>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > >>> index b5d64749990e..5cd85a9d149d 100644 > >>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > >>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > >>> @@ -22,6 +22,7 @@ > >>> #define SWSMU_CODE_LAYER_L1 > >>> +#include <linux/cpu.h> > >>> #include <linux/firmware.h> > >>> #include <linux/pci.h> > >>> @@ -69,6 +70,14 @@ 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); > >>> +static int smt_notifier_callback(struct notifier_block *nb, unsigned > >>> long action, void *data); > >>> + > >>> +extern struct raw_notifier_head smt_notifier_head; > >>> + > >>> +static struct notifier_block smt_notifier = { > >>> + .notifier_call = smt_notifier_callback, > >>> +}; > >>> + > >>> static int smu_sys_get_pp_feature_mask(void *handle, > >>> char *buf) > >>> { > >>> @@ -625,6 +634,8 @@ static int smu_set_funcs(struct amdgpu_device > *adev) > >>> return 0; > >>> } > >>> +static struct smu_context *current_smu; > >>> + > >>> static int smu_early_init(void *handle) > >>> { > >>> struct amdgpu_device *adev = (struct amdgpu_device *)handle; > >>> @@ -645,6 +656,7 @@ static int smu_early_init(void *handle) > >>> mutex_init(&smu->message_lock); > >>> adev->powerplay.pp_handle = smu; > >>> + current_smu = smu; > > > > Although this series is intended for the Van Gogh case right now, I > > dont't think this would scale well for multiple GPUs in a system. > > > > I think that instead you may want to move the notifier callback to be a > > level "higher" in amdgpu. Perhaps amdgpu_device.c? Then when that > > notifier call is received you'll want to walk through the PCI device > > space to find any GPUs that are bound with AMDGPU a series of > > wrappers/calls that end up calling smu_set_cpu_smt_enable with the > > approriate arguments. > > > > This is not required when the notifier is registered only within Vangogh > ppt function. Then follow Evan's suggestion of keeping the notifier > block inside smu. From the notifier block, it can find the smu block and > then call cpu_smt_enable/disable. That way notifier callback comes only > once even with multiple dGPUs + Vangogh and processed for the > corresponding smu. > > This notifier doesn't need to be registered for platforms only with > dGPUs or APUs which don't need this. They don't right now, but I was thinking how this could scale to other APUs or dGPUs if they are interested in adding support for this message too. > > Thanks, > Lijo > > > > >>> adev->powerplay.pp_funcs = &swsmu_pm_funcs; > >>> r = smu_set_funcs(adev); > >>> @@ -1105,6 +1117,8 @@ static int smu_sw_init(void *handle) > >>> if (!smu->ppt_funcs->get_fan_control_mode) > >>> smu->adev->pm.no_fan = true; > >>> + raw_notifier_chain_register(&smt_notifier_head, &smt_notifier); > >>> + > >>> return 0; > >>> } > >>> @@ -1122,6 +1136,8 @@ static int smu_sw_fini(void *handle) > >>> smu_fini_microcode(smu); > >>> + raw_notifier_chain_unregister(&smt_notifier_head, &smt_notifier); > >>> + > >>> return 0; > >>> } > >>> @@ -3241,3 +3257,28 @@ int smu_send_hbm_bad_channel_flag(struct > >>> smu_context *smu, uint32_t size) > >>> return ret; > >>> } > >>> + > >>> +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 = current_smu; > >>> + int ret = NOTIFY_OK; > >> > >> This initialization is pointless, it's clobbered in the next line. > >> > >>> + > >>> + ret = (action == SMT_ENABLED) ? > >>> + smu_set_cpu_smt_enable(smu, true) : > >>> + smu_set_cpu_smt_enable(smu, false); > >> > >> How about this instead, it should be more readable: > >> > >> ret = smu_set_cpu_smt_enable(smu, action == SMT_ENABLED); > >> > >>> + if (ret) > >>> + ret = NOTIFY_BAD; > >>> + > >>> + return ret; > >> > >> How about instead: > >> > >> dev_dbg(adev->dev, "failed to %sable SMT: %d\n", action == > >> SMT_ENABLED ? "en" : "dis", ret); > >> > >> return ret ? NOTIFY_BAD : NOTIFY_OK; > >> > >>> +} > >>> 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..7c6594bba796 100644 > >>> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h > >>> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h > >>> @@ -1354,6 +1354,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 enable); > >>> }; > >>> typedef enum { > >> > > [-- Attachment #2: Type: text/html, Size: 12828 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [v1,2/3] drm/amd/pm: send the SMT-enable message to pmfw 2023-03-23 17:41 ` [v1,2/3] " Limonciello, Mario 2023-03-23 18:06 ` Limonciello, Mario @ 2023-03-24 2:14 ` Yang, WenYou 1 sibling, 0 replies; 12+ messages in thread From: Yang, WenYou @ 2023-03-24 2:14 UTC (permalink / raw) To: Limonciello, Mario, Deucher, Alexander, Koenig, Christian, Pan, Xinhui Cc: Li, Ying, Liang, Richard qi, amd-gfx, Liu, Kun [AMD Official Use Only - General] > -----Original Message----- > From: Limonciello, Mario <Mario.Limonciello@amd.com> > Sent: Friday, March 24, 2023 1:42 AM > To: Yang, WenYou <WenYou.Yang@amd.com>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Koenig, Christian > <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com> > Cc: Li, Ying <YING.LI@amd.com>; Liu, Kun <Kun.Liu2@amd.com>; amd- > gfx@lists.freedesktop.org; Liang, Richard qi <Richardqi.Liang@amd.com> > Subject: Re: [v1,2/3] drm/amd/pm: send the SMT-enable message to pmfw > > On 3/22/2023 00:48, Wenyou Yang wrote: > > When the CPU SMT status change in the fly, sent the SMT-enable message > > to pmfw to notify it that the SMT status changed. > > > > Signed-off-by: Wenyou Yang <WenYou.Yang@amd.com> > > --- > > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 41 > +++++++++++++++++++ > > drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 5 +++ > > 2 files changed, 46 insertions(+) > > > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > > index b5d64749990e..5cd85a9d149d 100644 > > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > > @@ -22,6 +22,7 @@ > > > > #define SWSMU_CODE_LAYER_L1 > > > > +#include <linux/cpu.h> > > #include <linux/firmware.h> > > #include <linux/pci.h> > > > > @@ -69,6 +70,14 @@ 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); > > > > +static int smt_notifier_callback(struct notifier_block *nb, unsigned > > +long action, void *data); > > + > > +extern struct raw_notifier_head smt_notifier_head; > > + > > +static struct notifier_block smt_notifier = { > > + .notifier_call = smt_notifier_callback, }; > > + > > static int smu_sys_get_pp_feature_mask(void *handle, > > char *buf) > > { > > @@ -625,6 +634,8 @@ static int smu_set_funcs(struct amdgpu_device > *adev) > > return 0; > > } > > > > +static struct smu_context *current_smu; > > + > > static int smu_early_init(void *handle) > > { > > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > @@ > > -645,6 +656,7 @@ static int smu_early_init(void *handle) > > mutex_init(&smu->message_lock); > > > > adev->powerplay.pp_handle = smu; > > + current_smu = smu; > > adev->powerplay.pp_funcs = &swsmu_pm_funcs; > > > > r = smu_set_funcs(adev); > > @@ -1105,6 +1117,8 @@ static int smu_sw_init(void *handle) > > if (!smu->ppt_funcs->get_fan_control_mode) > > smu->adev->pm.no_fan = true; > > > > + raw_notifier_chain_register(&smt_notifier_head, &smt_notifier); > > + > > return 0; > > } > > > > @@ -1122,6 +1136,8 @@ static int smu_sw_fini(void *handle) > > > > smu_fini_microcode(smu); > > > > + raw_notifier_chain_unregister(&smt_notifier_head, &smt_notifier); > > + > > return 0; > > } > > > > @@ -3241,3 +3257,28 @@ int smu_send_hbm_bad_channel_flag(struct > > smu_context *smu, uint32_t size) > > > > return ret; > > } > > + > > +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 = current_smu; > > + int ret = NOTIFY_OK; > > This initialization is pointless, it's clobbered in the next line. Yes, accept. > > > + > > + ret = (action == SMT_ENABLED) ? > > + smu_set_cpu_smt_enable(smu, true) : > > + smu_set_cpu_smt_enable(smu, false); > > How about this instead, it should be more readable: > > ret = smu_set_cpu_smt_enable(smu, action == SMT_ENABLED); Accept. > > > + if (ret) > > + ret = NOTIFY_BAD; > > + > > + return ret; > > How about instead: > > dev_dbg(adev->dev, "failed to %sable SMT: %d\n", action == > SMT_ENABLED ? "en" : "dis", ret); > > return ret ? NOTIFY_BAD : NOTIFY_OK; > Accept. Thanks Wenyou > > +} > > 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..7c6594bba796 100644 > > --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h > > +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h > > @@ -1354,6 +1354,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 enable); > > }; > > > > typedef enum { ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v1 3/3] drm/amd/pm: vangogh: support to send SMT enable message 2023-03-22 5:48 [PATCH v1 0/3] send message to pmfw when SMT changes Wenyou Yang 2023-03-22 5:48 ` [PATCH v1 1/3] cpu/smt: add a notifier to notify the " Wenyou Yang 2023-03-22 5:48 ` [PATCH v1 2/3] drm/amd/pm: send the SMT-enable message to pmfw Wenyou Yang @ 2023-03-22 5:48 ` Wenyou Yang 2023-03-23 17:14 ` [v1,3/3] " Limonciello, Mario 2 siblings, 1 reply; 12+ messages in thread From: Wenyou Yang @ 2023-03-22 5:48 UTC (permalink / raw) To: alexander.deucher, christian.koenig, Xinhui.Pan Cc: Wenyou Yang, ying.li, kunliu13, amd-gfx, richardqi.liang Add the support to PPSMC_MSG_SetCClkSMTEnable(0x58) message to pmfw for vangogh. Signed-off-by: Wenyou Yang <WenYou.Yang@amd.com> --- .../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 | 19 +++++++++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) 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..2b182dbc6f9c 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 0x54 //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..f0eeb42df96b 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c @@ -141,6 +141,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] = { @@ -2428,6 +2429,23 @@ 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) +{ + int ret = 0; + + if (enable) { + ret = smu_cmn_send_smc_msg_with_param(smu, + SMU_MSG_SetCClkSMTEnable, + 1, NULL); + } else { + ret = smu_cmn_send_smc_msg_with_param(smu, + SMU_MSG_SetCClkSMTEnable, + 0, NULL); + } + + return ret; +} + static const struct pptable_funcs vangogh_ppt_funcs = { .check_fw_status = smu_v11_0_check_fw_status, @@ -2474,6 +2492,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) -- 2.39.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [v1,3/3] drm/amd/pm: vangogh: support to send SMT enable message 2023-03-22 5:48 ` [PATCH v1 3/3] drm/amd/pm: vangogh: support to send SMT enable message Wenyou Yang @ 2023-03-23 17:14 ` Limonciello, Mario 2023-03-24 2:07 ` Yang, WenYou 0 siblings, 1 reply; 12+ messages in thread From: Limonciello, Mario @ 2023-03-23 17:14 UTC (permalink / raw) To: Wenyou Yang, alexander.deucher, christian.koenig, Xinhui.Pan Cc: ying.li, kunliu13, richardqi.liang, amd-gfx On 3/22/2023 00:48, Wenyou Yang wrote: > Add the support to PPSMC_MSG_SetCClkSMTEnable(0x58) message to pmfw > for vangogh. > > Signed-off-by: Wenyou Yang <WenYou.Yang@amd.com> > --- > .../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 | 19 +++++++++++++++++++ > 3 files changed, 23 insertions(+), 2 deletions(-) > > 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..2b182dbc6f9c 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 0x54 This doesn't make sense that the PPSMC_Message_Count would be smaller than the biggest message. This should be: #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..f0eeb42df96b 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c > @@ -141,6 +141,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] = { > @@ -2428,6 +2429,23 @@ 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) > +{ > + int ret = 0; > + > + if (enable) { > + ret = smu_cmn_send_smc_msg_with_param(smu, > + SMU_MSG_SetCClkSMTEnable, > + 1, NULL); > + } else { > + ret = smu_cmn_send_smc_msg_with_param(smu, > + SMU_MSG_SetCClkSMTEnable, > + 0, NULL); > + } > + > + return ret; > +} > + > static const struct pptable_funcs vangogh_ppt_funcs = { > > .check_fw_status = smu_v11_0_check_fw_status, > @@ -2474,6 +2492,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) ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [v1,3/3] drm/amd/pm: vangogh: support to send SMT enable message 2023-03-23 17:14 ` [v1,3/3] " Limonciello, Mario @ 2023-03-24 2:07 ` Yang, WenYou 0 siblings, 0 replies; 12+ messages in thread From: Yang, WenYou @ 2023-03-24 2:07 UTC (permalink / raw) To: Limonciello, Mario, Deucher, Alexander, Koenig, Christian, Pan, Xinhui Cc: Li, Ying, Liang, Richard qi, amd-gfx, Liu, Kun [AMD Official Use Only - General] > -----Original Message----- > From: Limonciello, Mario <Mario.Limonciello@amd.com> > Sent: Friday, March 24, 2023 1:15 AM > To: Yang, WenYou <WenYou.Yang@amd.com>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Koenig, Christian > <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com> > Cc: Li, Ying <YING.LI@amd.com>; Liu, Kun <Kun.Liu2@amd.com>; amd- > gfx@lists.freedesktop.org; Liang, Richard qi <Richardqi.Liang@amd.com> > Subject: Re: [v1,3/3] drm/amd/pm: vangogh: support to send SMT enable > message > > On 3/22/2023 00:48, Wenyou Yang wrote: > > Add the support to PPSMC_MSG_SetCClkSMTEnable(0x58) message to > pmfw > > for vangogh. > > > > Signed-off-by: Wenyou Yang <WenYou.Yang@amd.com> > > --- > > .../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 | 19 > +++++++++++++++++++ > > 3 files changed, 23 insertions(+), 2 deletions(-) > > > > 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..2b182dbc6f9c 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 0x54 > > This doesn't make sense that the PPSMC_Message_Count would be smaller > than the biggest message. This should be: > > #define PPSMC_Message_Count 0x59 > Accepted. Thanks Wenyou > > > > //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..f0eeb42df96b 100644 > > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c > > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c > > @@ -141,6 +141,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] = { @@ -2428,6 > +2429,23 @@ 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) { > > + int ret = 0; > > + > > + if (enable) { > > + ret = smu_cmn_send_smc_msg_with_param(smu, > > + > SMU_MSG_SetCClkSMTEnable, > > + 1, NULL); > > + } else { > > + ret = smu_cmn_send_smc_msg_with_param(smu, > > + > SMU_MSG_SetCClkSMTEnable, > > + 0, NULL); > > + } > > + > > + return ret; > > +} > > + > > static const struct pptable_funcs vangogh_ppt_funcs = { > > > > .check_fw_status = smu_v11_0_check_fw_status, @@ -2474,6 > +2492,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) ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-03-25 1:49 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-22 5:48 [PATCH v1 0/3] send message to pmfw when SMT changes Wenyou Yang 2023-03-22 5:48 ` [PATCH v1 1/3] cpu/smt: add a notifier to notify the " Wenyou Yang 2023-03-22 5:48 ` [PATCH v1 2/3] drm/amd/pm: send the SMT-enable message to pmfw Wenyou Yang 2023-03-23 17:41 ` [v1,2/3] " Limonciello, Mario 2023-03-23 18:06 ` Limonciello, Mario 2023-03-24 2:29 ` Lazar, Lijo 2023-03-24 17:49 ` Limonciello, Mario 2023-03-25 1:49 ` Lazar, Lijo 2023-03-24 2:14 ` Yang, WenYou 2023-03-22 5:48 ` [PATCH v1 3/3] drm/amd/pm: vangogh: support to send SMT enable message Wenyou Yang 2023-03-23 17:14 ` [v1,3/3] " Limonciello, Mario 2023-03-24 2:07 ` Yang, WenYou
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.