Re: [PATCH v3 2/2] platform/x86/amd/pmf: Add PMF debug facilities

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Tue, 9 May 2023, Shyam Sundar S K wrote:

> At times, when the mode transitions fail to happen, the current
> driver does not give enough debug information on why the transition
> failed or the default preset values did not load. Having an on-demand
> logs guarded by CONFIG would be helpful in such cases.
> 
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> v3:
>  - Based on review-hans branch
>  - Address Ilpo's review remarks
>  - use the right format specifiers while printing the data
> 
> v2:
>  - Based on review-hans branch
>  - Drop extra CONFIG item based on feedback from Hans
> 
>  drivers/platform/x86/amd/pmf/auto-mode.c | 23 +++++++++++++++++++++++
>  drivers/platform/x86/amd/pmf/cnqf.c      | 19 +++++++++++++++++++
>  2 files changed, 42 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd/pmf/auto-mode.c b/drivers/platform/x86/amd/pmf/auto-mode.c
> index 64425201d353..cb2fb4df39e9 100644
> --- a/drivers/platform/x86/amd/pmf/auto-mode.c
> +++ b/drivers/platform/x86/amd/pmf/auto-mode.c
> @@ -179,11 +179,34 @@ void amd_pmf_trans_automode(struct amd_pmf_dev *dev, int socket_power, ktime_t t
>  			config_store.transition[i].applied = false;
>  			update = true;
>  		}
> +
> +#ifdef CONFIG_AMD_PMF_DEBUG
> +		dev_dbg(dev->dev, "[AUTO MODE] average_power : %d mW mode: %s\n", avg_power,
> +			state_as_str(config_store.current_mode));
> +
> +		dev_dbg(dev->dev, "[AUTO MODE] time: %lld ms timer: %u ms tc: %u ms\n",
> +			time_elapsed_ms, config_store.transition[i].timer,
> +			config_store.transition[i].time_constant);
> +
> +		dev_dbg(dev->dev, "[AUTO MODE] shiftup: %u pt: %u mW pf: %u mW pd: %u mW\n",
> +			config_store.transition[i].shifting_up,
> +			config_store.transition[i].power_threshold,
> +			config_store.mode_set[i].power_floor,
> +			config_store.transition[i].power_delta);
> +#endif
>  	}
>  
>  	dev_dbg(dev->dev, "[AUTO_MODE] avg power: %u mW mode: %s\n", avg_power,
>  		state_as_str(config_store.current_mode));
>  
> +#ifdef CONFIG_AMD_PMF_DEBUG
> +	dev_dbg(dev->dev, "[AUTO MODE] priority1: %u priority2: %u priority3: %u priority4: %u\n",
> +		config_store.transition[0].applied,
> +		config_store.transition[1].applied,
> +		config_store.transition[2].applied,
> +		config_store. transition[3].applied);

Extra space.

-- 
 i.

> +#endif
> +
>  	if (update) {
>  		for (j = 0; j < AUTO_TRANSITION_MAX; j++) {
>  			/* Apply the mode with highest priority indentified */
> diff --git a/drivers/platform/x86/amd/pmf/cnqf.c b/drivers/platform/x86/amd/pmf/cnqf.c
> index 35af7c18f600..539b186e9027 100644
> --- a/drivers/platform/x86/amd/pmf/cnqf.c
> +++ b/drivers/platform/x86/amd/pmf/cnqf.c
> @@ -175,6 +175,13 @@ int amd_pmf_trans_cnqf(struct amd_pmf_dev *dev, int socket_power, ktime_t time_l
>  		config_store.trans_param[src][i].count++;
>  
>  		tp = &config_store.trans_param[src][i];
> +
> +#ifdef CONFIG_AMD_PMF_DEBUG
> +		dev_dbg(dev->dev, "avg_power: %u mW total_power: %u mW count: %u timer: %u ms\n",
> +			avg_power, config_store.trans_param[src][i].total_power,
> +			config_store.trans_param[src][i].count,
> +			config_store.trans_param[src][i].timer);
> +#endif
>  		if (tp->timer >= tp->time_constant && tp->count) {
>  			avg_power = tp->total_power / tp->count;
>  
> @@ -195,6 +202,18 @@ int amd_pmf_trans_cnqf(struct amd_pmf_dev *dev, int socket_power, ktime_t time_l
>  	dev_dbg(dev->dev, "[CNQF] Avg power: %u mW socket power: %u mW mode:%s\n",
>  		avg_power, socket_power, state_as_str(config_store.current_mode));
>  
> +#ifdef CONFIG_AMD_PMF_DEBUG
> +	dev_dbg(dev->dev, "[CNQF] priority1: %u priority2: %u priority3: %u\n",
> +		config_store.trans_param[src][0].priority,
> +		config_store.trans_param[src][1].priority,
> +		config_store.trans_param[src][2].priority);
> +
> +	dev_dbg(dev->dev, "[CNQF] priority4: %u priority5: %u priority6: %u\n",
> +		config_store.trans_param[src][3].priority,
> +		config_store.trans_param[src][4].priority,
> +		config_store.trans_param[src][5].priority);
> +#endif
> +
>  	for (j = 0; j < CNQF_TRANSITION_MAX; j++) {
>  		/* apply the highest priority */
>  		if (config_store.trans_param[src][j].priority) {
>