Re: [PATCH 6/7] platform/x86/amd/pmf: Add support to get sps default APTS index values

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

On Tue, 27 Feb 2024, Shyam Sundar S K wrote:

> During the driver probe, the default cache values for the static slider
> would be obtained by evaluating the APTS method. Add support to use
> these values as the thermal settings to be updated on the system based
> on the changing platform-profiles.
> 
> 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>

> @@ -79,11 +80,56 @@ static void amd_pmf_dump_sps_defaults_v2(struct amd_pmf_static_slider_granular_v
>  	pr_debug("dc_battery_saver: %u\n", data->sps_idx.dc_battery_saver);
>  	pr_debug("Static Slider APTS state index data - END\n");
>  }
> +
> +static void amd_pmf_dump_apts_sps_defaults(struct amd_pmf_apts_granular *info)
> +{
> +	int i;
> +
> +	pr_debug("Static Slider APTS index default values data - BEGIN");
> +
> +	for (i = 0; i < APTS_MAX_STATES; i++) {
> +		pr_debug("index[%d]: table_version = %u\n", i, info->val[i].table_version);
> +		pr_debug("index[%d]: fan_table_idx = %u\n", i, info->val[i].fan_table_idx);
> +		pr_debug("index[%d]: pmf_ppt = %u\n", i, info->val[i].pmf_ppt);
> +		pr_debug("index[%d]: ppt_pmf_apu_only = %u\n", i, info->val[i].ppt_pmf_apu_only);
> +		pr_debug("index[%d]: stt_min_limit = %u\n", i, info->val[i].stt_min_limit);
> +		pr_debug("index[%d]: stt_skin_temp_limit_apu = %u\n",
> +			 i, info->val[i].stt_skin_temp_limit_apu);
> +		pr_debug("index[%d]: stt_skin_temp_limit_hs2 = %u\n",
> +			 i, info->val[i].stt_skin_temp_limit_hs2);

Again, underscores seem mostly unnecessary. There's also inconsistency in 
capitalization and space/underscore usage compared with the existing 
printouts now that I looked also the existing pr_debug()s. Please try to 
make things mostly consistent.

> +	}
> +
> +	pr_debug("Static Slider APTS index default values data - END");
> +}
>  #else
>  static void amd_pmf_dump_sps_defaults(struct amd_pmf_static_slider_granular *data) {}
>  static void amd_pmf_dump_sps_defaults_v2(struct amd_pmf_static_slider_granular_v2 *data) {}
> +static void amd_pmf_dump_apts_sps_defaults(struct amd_pmf_apts_granular *info) {}
>  #endif
>  
> +static void amd_pmf_load_apts_defaults_sps_v2(struct amd_pmf_dev *pdev)
> +{
> +	struct amd_pmf_apts_granular_output output;
> +	int i;
> +
> +	memset(&apts_config_store, 0, sizeof(apts_config_store));
> +
> +	for (i = 0; i < APTS_MAX_STATES; i++) {
> +		apts_get_static_slider_granular_v2(pdev, &output, i);
> +		apts_config_store.val[i].table_version = output.val.table_version;
> +		apts_config_store.val[i].fan_table_idx = output.val.fan_table_idx;
> +		apts_config_store.val[i].pmf_ppt = output.val.pmf_ppt;
> +		apts_config_store.val[i].ppt_pmf_apu_only = output.val.ppt_pmf_apu_only;
> +		apts_config_store.val[i].stt_min_limit = output.val.stt_min_limit;
> +		apts_config_store.val[i].stt_skin_temp_limit_apu =
> +							output.val.stt_skin_temp_limit_apu;
> +		apts_config_store.val[i].stt_skin_temp_limit_hs2 =
> +							output.val.stt_skin_temp_limit_hs2;

Add a temporary variabled for apts_config_store.val[i] to make these 
shorter?


-- 
 i.