[PATCH v4 10/17] platform/x86/amd/pmf: Add facility to dump TA inputs

[Shyam Sundar S K]

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 at amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k at 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