Re: [PATCH v2 1/2] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello Mario,

On 6/4/2026 12:23 AM, Mario Limonciello wrote:
>> --- a/drivers/cpufreq/amd-pstate.h
>> +++ b/drivers/cpufreq/amd-pstate.h
>> @@ -128,6 +128,8 @@ struct amd_cpudata {
>>       u8    epp_default_dc;
>>       bool    dynamic_epp;
>>       bool    raw_epp;
>> +    /* Indicates that EPP has been successfully programmed at least once since boot. */
>> +    bool    epp_hw_programmed;
>>       struct notifier_block power_nb;
>>         /* platform profile */
> 
> I'm having a hard time following why this is needed.  Let me explain my chain of thought.
> 
> We start at amd_pstate_epp_cpu_init().
> * cpudata (and thus cppc_req_cached) is initalized to 0 via kzalloc()
>    - On a server we initialize "epp_default" via a lookup to the HW (amd_pstate_get_epp).
>    - On a non-server we initialize "epp_default" to 0x80.
> * we call amd_pstate_set_epp() with epp_deafult as the argument.
> 
> Now in the shared mem backend (shmem_set_epp) we do a lookup of epp_cached and it should be 0.  We do a comparison of the argument sent to the function, and this should be 0x80.
> 
> So how do we get into a case that amd_pstate_set_epp() is actually called with 0?

This was the path I traced yesterday considering amd_dynamic_epp enabled for
a shared memory system that is plugged into the wall:

  amd_pstate_epp_cpu_init()
    cpudata->epp_default_ac = AMD_CPPC_EPP_PERFORMANCE;
    cpudata->epp_default_dc = AMD_CPPC_EPP_BALANCE_PERFORMANCE;
    cpudata->current_profile = PLATFORM_PROFILE_BALANCED;

    /* cpudata->cppc_req_cached has epp_cached as 0 */

    amd_pstate_set_dynamic_epp(policy)
      epp = amd_pstate_get_balanced_epp(policy); /* returns cpudata->epp_default_ac which is EPP_PERFORMANCE */

      amd_pstate_set_epp(policy, epp /* AMD_CPPC_EPP_PERFORMANCE */)
        if (epp == epp_cached)
          return;

        /*
         * Skips cppc_set_epp_perf(cpudata->cpu, &perf_ctrls, 1)
         * where the last argument "enable" enables EPP via the
         * AUTO_SEL_ENABLE path.
         */

> 
> There are some trace points specifically for this purpose - you could confirm there really is such a call by using them at bootup (or by starting in passive and with a mode change to active at runtime).

It should show up in the traces yes!

> 
> Then we call amd_pstate_set_epp() with that epp_default value.

I think this patch has some merit, although the second patch, I'm not
too sure of. After some digging, it seems we always had AUTO_SEL_ENABLE
and ENERGY_PERF right from the time CPPC was introduced (including Zen1
systems that had CPPC support).

Marco, do you have actually have a system that neither has
AUTO_SEL_ENABLE, nor ENERGY_PERF? Seems very unlikely a configuration
like that made it out.

-- 
Thanks and Regards,
Prateek