| Message ID | 20231018070241.2041529-11-Shyam-sundar.S-k@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce PMF Smart PC Solution Builder Feature | expand |
On Wed, 18 Oct 2023, Shyam Sundar S K wrote: > PMF driver sends constant inputs to TA which its gets via the other > subsystems in the kernel. To debug certain TA issues knowing what inputs > being sent to TA becomes critical. Add debug facility to the driver which > can isolate Smart PC and TA related issues. > > Also, make source_as_str() as non-static function as this helper is > required outside of sps.c file. > > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> > --- > drivers/platform/x86/amd/pmf/pmf.h | 3 +++ > drivers/platform/x86/amd/pmf/spc.c | 37 +++++++++++++++++++++++++++ > drivers/platform/x86/amd/pmf/tee-if.c | 1 + > 3 files changed, 41 insertions(+) > > diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h > index 216a9f795436..593930519039 100644 > --- a/drivers/platform/x86/amd/pmf/pmf.h > +++ b/drivers/platform/x86/amd/pmf/pmf.h > @@ -602,6 +602,7 @@ bool is_pprof_balanced(struct amd_pmf_dev *pmf); > int amd_pmf_power_slider_update_event(struct amd_pmf_dev *dev); > const char *amd_pmf_source_as_str(unsigned int state); > > +const char *amd_pmf_source_as_str(unsigned int state); > > int apmf_update_fan_idx(struct amd_pmf_dev *pdev, bool manual, u32 idx); > int amd_pmf_set_sps_power_limits(struct amd_pmf_dev *pmf); > @@ -632,4 +633,6 @@ int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev); > > /* Smart PC - TA interfaces */ > void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in); > +void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in); > + > #endif /* PMF_H */ > diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c > index bd5061fdfdbe..512e0c66efdc 100644 > --- a/drivers/platform/x86/amd/pmf/spc.c > +++ b/drivers/platform/x86/amd/pmf/spc.c > @@ -14,6 +14,43 @@ > #include <linux/units.h> > #include "pmf.h" > > +#ifdef CONFIG_AMD_PMF_DEBUG > +static const char *ta_slider_as_str(unsigned int state) > +{ > + switch (state) { > + case TA_BEST_PERFORMANCE: > + return "PERFORMANCE"; > + case TA_BETTER_PERFORMANCE: > + return "BALANCED"; > + case TA_BEST_BATTERY: > + return "POWER_SAVER"; > + default: > + return "Unknown TA Slider State"; > + } > +} > + > +void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in) > +{ > + dev_dbg(dev->dev, "==== TA inputs START ====\n"); > + dev_dbg(dev->dev, "Slider State : %s\n", ta_slider_as_str(in->ev_info.power_slider)); > + dev_dbg(dev->dev, "Power Source : %s\n", amd_pmf_source_as_str(in->ev_info.power_source)); > + dev_dbg(dev->dev, "Battery Percentage : %u\n", in->ev_info.bat_percentage); > + dev_dbg(dev->dev, "Designed Battery Capacity : %u\n", in->ev_info.bat_design); > + dev_dbg(dev->dev, "Fully Charged Capacity : %u\n", in->ev_info.full_charge_capacity); > + dev_dbg(dev->dev, "Drain Rate : %d\n", in->ev_info.drain_rate); > + dev_dbg(dev->dev, "Socket Power : %u\n", in->ev_info.socket_power); > + dev_dbg(dev->dev, "Skin Temperature : %u\n", in->ev_info.skin_temperature); > + dev_dbg(dev->dev, "Avg C0 Residency : %u\n", in->ev_info.avg_c0residency); > + dev_dbg(dev->dev, "Max C0 Residency : %u\n", in->ev_info.max_c0residency); > + dev_dbg(dev->dev, "GFX Busy : %u\n", in->ev_info.gfx_busy); > + dev_dbg(dev->dev, "Connected Display Count : %u\n", in->ev_info.monitor_count); > + dev_dbg(dev->dev, "LID State : %s\n", in->ev_info.lid_state ? "Close" : "Open"); "open" / "closed" is generic enough that I think it would warrant adding include/linux/string_choices.h helper for it (it should be lowercase there but that difference probably is not an issue for these debug prints). I'd also remove that extra space before :.
Hi Ilpo, Apologies for the long delay. On 10/18/2023 2:30 PM, Ilpo Järvinen wrote: > On Wed, 18 Oct 2023, Shyam Sundar S K wrote: > >> PMF driver sends constant inputs to TA which its gets via the other >> subsystems in the kernel. To debug certain TA issues knowing what inputs >> being sent to TA becomes critical. Add debug facility to the driver which >> can isolate Smart PC and TA related issues. >> >> Also, make source_as_str() as non-static function as this helper is >> required outside of sps.c file. >> >> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> >> --- >> drivers/platform/x86/amd/pmf/pmf.h | 3 +++ >> drivers/platform/x86/amd/pmf/spc.c | 37 +++++++++++++++++++++++++++ >> drivers/platform/x86/amd/pmf/tee-if.c | 1 + >> 3 files changed, 41 insertions(+) >> >> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h >> index 216a9f795436..593930519039 100644 >> --- a/drivers/platform/x86/amd/pmf/pmf.h >> +++ b/drivers/platform/x86/amd/pmf/pmf.h >> @@ -602,6 +602,7 @@ bool is_pprof_balanced(struct amd_pmf_dev *pmf); >> int amd_pmf_power_slider_update_event(struct amd_pmf_dev *dev); >> const char *amd_pmf_source_as_str(unsigned int state); >> >> +const char *amd_pmf_source_as_str(unsigned int state); >> >> int apmf_update_fan_idx(struct amd_pmf_dev *pdev, bool manual, u32 idx); >> int amd_pmf_set_sps_power_limits(struct amd_pmf_dev *pmf); >> @@ -632,4 +633,6 @@ int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev); >> >> /* Smart PC - TA interfaces */ >> void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in); >> +void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in); >> + >> #endif /* PMF_H */ >> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c >> index bd5061fdfdbe..512e0c66efdc 100644 >> --- a/drivers/platform/x86/amd/pmf/spc.c >> +++ b/drivers/platform/x86/amd/pmf/spc.c >> @@ -14,6 +14,43 @@ >> #include <linux/units.h> >> #include "pmf.h" >> >> +#ifdef CONFIG_AMD_PMF_DEBUG >> +static const char *ta_slider_as_str(unsigned int state) >> +{ >> + switch (state) { >> + case TA_BEST_PERFORMANCE: >> + return "PERFORMANCE"; >> + case TA_BETTER_PERFORMANCE: >> + return "BALANCED"; >> + case TA_BEST_BATTERY: >> + return "POWER_SAVER"; >> + default: >> + return "Unknown TA Slider State"; >> + } >> +} >> + >> +void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in) >> +{ >> + dev_dbg(dev->dev, "==== TA inputs START ====\n"); >> + dev_dbg(dev->dev, "Slider State : %s\n", ta_slider_as_str(in->ev_info.power_slider)); >> + dev_dbg(dev->dev, "Power Source : %s\n", amd_pmf_source_as_str(in->ev_info.power_source)); >> + dev_dbg(dev->dev, "Battery Percentage : %u\n", in->ev_info.bat_percentage); >> + dev_dbg(dev->dev, "Designed Battery Capacity : %u\n", in->ev_info.bat_design); >> + dev_dbg(dev->dev, "Fully Charged Capacity : %u\n", in->ev_info.full_charge_capacity); >> + dev_dbg(dev->dev, "Drain Rate : %d\n", in->ev_info.drain_rate); >> + dev_dbg(dev->dev, "Socket Power : %u\n", in->ev_info.socket_power); >> + dev_dbg(dev->dev, "Skin Temperature : %u\n", in->ev_info.skin_temperature); >> + dev_dbg(dev->dev, "Avg C0 Residency : %u\n", in->ev_info.avg_c0residency); >> + dev_dbg(dev->dev, "Max C0 Residency : %u\n", in->ev_info.max_c0residency); >> + dev_dbg(dev->dev, "GFX Busy : %u\n", in->ev_info.gfx_busy); >> + dev_dbg(dev->dev, "Connected Display Count : %u\n", in->ev_info.monitor_count); >> + dev_dbg(dev->dev, "LID State : %s\n", in->ev_info.lid_state ? "Close" : "Open"); > > "open" / "closed" is generic enough that I think it would warrant adding > include/linux/string_choices.h helper for it (it should be lowercase > there but that difference probably is not an issue for these debug > prints). > > I'd also remove that extra space before :. > Seems like string_choices.h does not have helper for open/close. So did not include this change in the next revision. But I have addressed all your other remarks in v4. Thanks, Shyam
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h index 216a9f795436..593930519039 100644 --- a/drivers/platform/x86/amd/pmf/pmf.h +++ b/drivers/platform/x86/amd/pmf/pmf.h @@ -602,6 +602,7 @@ bool is_pprof_balanced(struct amd_pmf_dev *pmf); int amd_pmf_power_slider_update_event(struct amd_pmf_dev *dev); const char *amd_pmf_source_as_str(unsigned int state); +const char *amd_pmf_source_as_str(unsigned int state); int apmf_update_fan_idx(struct amd_pmf_dev *pdev, bool manual, u32 idx); int amd_pmf_set_sps_power_limits(struct amd_pmf_dev *pmf); @@ -632,4 +633,6 @@ int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev); /* Smart PC - TA interfaces */ void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in); +void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in); + #endif /* PMF_H */ diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c index bd5061fdfdbe..512e0c66efdc 100644 --- a/drivers/platform/x86/amd/pmf/spc.c +++ b/drivers/platform/x86/amd/pmf/spc.c @@ -14,6 +14,43 @@ #include <linux/units.h> #include "pmf.h" +#ifdef CONFIG_AMD_PMF_DEBUG +static const char *ta_slider_as_str(unsigned int state) +{ + switch (state) { + case TA_BEST_PERFORMANCE: + return "PERFORMANCE"; + case TA_BETTER_PERFORMANCE: + return "BALANCED"; + case TA_BEST_BATTERY: + return "POWER_SAVER"; + default: + return "Unknown TA Slider State"; + } +} + +void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in) +{ + dev_dbg(dev->dev, "==== TA inputs START ====\n"); + dev_dbg(dev->dev, "Slider State : %s\n", ta_slider_as_str(in->ev_info.power_slider)); + dev_dbg(dev->dev, "Power Source : %s\n", amd_pmf_source_as_str(in->ev_info.power_source)); + dev_dbg(dev->dev, "Battery Percentage : %u\n", in->ev_info.bat_percentage); + dev_dbg(dev->dev, "Designed Battery Capacity : %u\n", in->ev_info.bat_design); + dev_dbg(dev->dev, "Fully Charged Capacity : %u\n", in->ev_info.full_charge_capacity); + dev_dbg(dev->dev, "Drain Rate : %d\n", in->ev_info.drain_rate); + dev_dbg(dev->dev, "Socket Power : %u\n", in->ev_info.socket_power); + dev_dbg(dev->dev, "Skin Temperature : %u\n", in->ev_info.skin_temperature); + dev_dbg(dev->dev, "Avg C0 Residency : %u\n", in->ev_info.avg_c0residency); + dev_dbg(dev->dev, "Max C0 Residency : %u\n", in->ev_info.max_c0residency); + dev_dbg(dev->dev, "GFX Busy : %u\n", in->ev_info.gfx_busy); + dev_dbg(dev->dev, "Connected Display Count : %u\n", in->ev_info.monitor_count); + dev_dbg(dev->dev, "LID State : %s\n", in->ev_info.lid_state ? "Close" : "Open"); + dev_dbg(dev->dev, "==== TA inputs END ====\n"); +} +#else +void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in) {} +#endif + static void amd_pmf_get_smu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in) { u16 max, avg = 0; diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c index d48f980fb1db..0eba258f4040 100644 --- a/drivers/platform/x86/amd/pmf/tee-if.c +++ b/drivers/platform/x86/amd/pmf/tee-if.c @@ -182,6 +182,7 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev) } if (ta_sm->pmf_result == TA_PMF_TYPE_SUCCESS && out->actions_count) { + amd_pmf_dump_ta_inputs(dev, in); dev_dbg(dev->dev, "action count:%u result:%x\n", out->actions_count, ta_sm->pmf_result); amd_pmf_apply_policies(dev, out);