Re: [PATCH v9 06/13] cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors

From: Wyes Karny
Date: Fri Jan 06 2023 - 00:23:10 EST


Hi Perry,

On 12/25/2022 10:04 PM, Perry Yuan wrote:
----------------------------------->8-------------------------------
> +static struct cpufreq_driver amd_pstate_epp_driver = {
> + .flags = CPUFREQ_CONST_LOOPS,
> + .verify = amd_pstate_epp_verify_policy,
> + .setpolicy = amd_pstate_epp_set_policy,
> + .init = amd_pstate_epp_cpu_init,
> + .exit = amd_pstate_epp_cpu_exit,
> + .name = "amd_pstate_epp",

Small nit, amd_pstate_driver name is "amd-pstate", whereas here for
amd_pstate_epp_driver name "amd_pstate_epp" is used.
Should amd_pstate_driver renamed "amd_pstate"?

> + .attr = amd_pstate_epp_attr,
> +};
> +
> static int __init amd_pstate_init(void)
> {
> int ret;
> @@ -656,7 +1012,8 @@ static int __init amd_pstate_init(void)
> /* capability check */
> if (boot_cpu_has(X86_FEATURE_CPPC)) {
> pr_debug("AMD CPPC MSR based functionality is supported\n");
> - amd_pstate_driver.adjust_perf = amd_pstate_adjust_perf;
> + if (cppc_state == AMD_PSTATE_PASSIVE)
> + default_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
> } else {
> pr_debug("AMD CPPC shared memory based functionality is supported\n");
> static_call_update(amd_pstate_enable, cppc_enable);
> @@ -667,14 +1024,13 @@ static int __init amd_pstate_init(void)
> /* enable amd pstate feature */
> ret = amd_pstate_enable(true);
> if (ret) {
> - pr_err("failed to enable amd-pstate with return %d\n", ret);
> + pr_err("failed to enable with return %d\n", ret);
> return ret;
> }
>
> - ret = cpufreq_register_driver(&amd_pstate_driver);
> + ret = cpufreq_register_driver(default_pstate_driver);
> if (ret)
> - pr_err("failed to register amd_pstate_driver with return %d\n",
> - ret);
> + pr_err("failed to register with return %d\n", ret);
>
> return ret;
> }
> @@ -696,6 +1052,12 @@ static int __init amd_pstate_param(char *str)
> if (cppc_state == AMD_PSTATE_DISABLE)
> pr_info("driver is explicitly disabled\n");
>
> + if (cppc_state == AMD_PSTATE_ACTIVE)
> + default_pstate_driver = &amd_pstate_epp_driver;
> +
> + if (cppc_state == AMD_PSTATE_PASSIVE)
> + default_pstate_driver = &amd_pstate_driver;
> +
> return 0;
> }
>
> diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> index dae2ce0f6735..8341a2a2948a 100644
> --- a/include/linux/amd-pstate.h
> +++ b/include/linux/amd-pstate.h
> @@ -47,6 +47,10 @@ struct amd_aperf_mperf {
> * @prev: Last Aperf/Mperf/tsc count value read from register
> * @freq: current cpu frequency value
> * @boost_supported: check whether the Processor or SBIOS supports boost mode
> + * @epp_policy: Last saved policy used to set energy-performance preference
> + * @epp_cached: Cached CPPC energy-performance preference value
> + * @policy: Cpufreq policy value
> + * @cppc_cap1_cached Cached MSR_AMD_CPPC_CAP1 register value
> *
> * The amd_cpudata is key private data for each CPU thread in AMD P-State, and
> * represents all the attributes and goals that AMD P-State requests at runtime.
> @@ -72,6 +76,12 @@ struct amd_cpudata {
>
> u64 freq;
> bool boost_supported;
> +
> + /* EPP feature related attributes*/
> + s16 epp_policy;
> + s16 epp_cached;
> + u32 policy;
> + u64 cppc_cap1_cached;
> };
>
> /*

--
Thanks & Regards,
Wyes