On 2025/2/11 22:08, Sumit Gupta wrote: > > >> >> On 2025/2/11 18:44, Viresh Kumar wrote: >>> On 11-02-25, 16:07, Sumit Gupta wrote: >>>> This patchset supports the Autonomous Performance Level Selection mode >>>> in the cppc_cpufreq driver. The feature is part of the existing CPPC >>>> specification and already present in Intel and AMD specific pstate >>>> cpufreq drivers. The patchset adds the support in generic acpi cppc >>>> cpufreq driver. >>> >>> Is there an overlap with: >>> >>> https://lore.kernel.org/all/20250206131428.3261578-1-zhenglifeng1@xxxxxxxxxx/ >>> >>> ? >> >> Ha, it looks like we're doing something very similar. >> > > Hi Viresh, > > Thank you for pointing to [1]. > > There seems to be some common points about updating the 'energy_perf' > and 'auto_sel' registers for autonomous mode but the current patchset > has more comprehensive changes to support Autonomous mode with the > cppc_cpufreq driver. > > The patches in [1]: > 1) Make the cpc register read/write API’s generic and improves error > handling for 'CPC_IN_PCC'. > 2) Expose sysfs under 'cppc_cpufreq_attr' to update 'auto_select', > 'auto_act_window' and 'epp' registers. > > The current patch series: > 1) Exposes sysfs under 'cppc_attrs' to keep CPC registers together. > 2) Updates existing API’s to use new registers and creates new API > with similar semantics to get all perf_ctrls. > 3) Renames some existing API’s for clarity. > 4) Use these existing API’s from acpi_cppc sysfs to update the CPC > registers used in Autonomous mode: > 'auto_select', 'epp', 'min_perf', 'max_perf' registers. > 5) Add separate 'cppc_cpufreq_epp' instance of the 'cppc_cpufreq' > driver to apply different limit and policy for Autonomous mode. > Having it separate will avoid confusion between SW and HW mode. > Also, it will be easy to scale and add new features in future > without interference. Similar approach is used in Intel and AMD > pstate drivers. > > Please share inputs about the preferred approach. > > Best Regards, > Sumit Gupta > > [1] https://lore.kernel.org/all/20250206131428.3261578-1-zhenglifeng1@xxxxxxxxxx/ > > Hi Sumit, Thanks for upstreaming this. I think the changes to cppc_acpi in this patchset is inappropriate. 1) cppc_attrs are common sysfs for any system that supports CPPC. That means, these interfaces would appear even if the cpufreq driver has already managing it, e.g. amd-pstate and cppc_cpufreq. This would create multiple interfaces to modify the same CPPC regs, which may probably introduce concurrency and data consistency issues. Instead, exposing the interfaces under cppc_cpufreq_attr decouples the write access to CPPC regs. 2) It's inappropriate to call cpufreq_cpu_get() in cppc_acpi. This file currently provides interfaces for cpufreq drivers to use. It has no ABI dependency on cpufreq at the moment. Apart from the changes to cppc_acpi, I think the whole patchset in [1] can be independent to this patchset. In other words, adding the cppc_cpufreq_epp_driver could be standalone to discuss. I think combining the use of ->setpolicy() and setting EPP could be a use case? Could you explain more on the motivation of adding a new cppc_cpufreq_epp_driver? [1] https://lore.kernel.org/all/20250206131428.3261578-1-zhenglifeng1@xxxxxxxxxx/ Regards, Lifeng >>> >>>> It adds a new 'cppc_cpufreq_epp' instance of the 'cppc_cpufreq' driver >>>> for supporting the Autonomous Performance Level Selection and Energy >>>> Performance Preference (EPP). >>>> Autonomous selection will get enabled during boot if 'cppc_auto_sel' >>>> boot argument is passed or the 'Autonomous Selection Enable' register >>>> is already set before kernel boot. When enabled, the hardware is >>>> allowed to autonomously select the CPU frequency within the min and >>>> max perf boundaries using the Engergy Performance Preference hints. >>>> The EPP values range from '0x0'(performance preference) to '0xFF' >>>> (energy efficiency preference). >>>> >>>> It also exposes the acpi_cppc sysfs nodes to update the epp, auto_sel >>>> and {min|max_perf} registers for changing the hints to hardware for >>>> Autonomous selection. >>>> >>>> In a followup patch, plan to add support to dynamically switch the >>>> cpufreq driver instance from 'cppc_cpufreq_epp' to 'cppc_cpufreq' and >>>> vice-versa without reboot. >>>> >>>> The patches are divided into below groups: >>>> - Patch [1-2]: Improvements. Can be applied independently. >>>> - Patch [3-4]: sysfs store nodes for Auto mode. Depend on Patch [1-2]. >>>> - Patch [5]: Support for 'cppc_cpufreq_epp'. Uses a macro from [3]. >>>> >>>> Sumit Gupta (5): >>>> ACPI: CPPC: add read perf ctrls api and rename few existing >>>> ACPI: CPPC: expand macro to create store acpi_cppc sysfs node >>>> ACPI: CPPC: support updating epp, auto_sel and {min|max_perf} from >>>> sysfs >>>> Documentation: ACPI: add autonomous mode ctrls info in cppc_sysfs.txt >>>> cpufreq: CPPC: Add cppc_cpufreq_epp instance for Autonomous mode >>>> >>>> Documentation/admin-guide/acpi/cppc_sysfs.rst | 28 ++ >>>> .../admin-guide/kernel-parameters.txt | 11 + >>>> drivers/acpi/cppc_acpi.c | 311 ++++++++++++++++-- >>>> drivers/cpufreq/cppc_cpufreq.c | 260 ++++++++++++++- >>>> include/acpi/cppc_acpi.h | 19 +- >>>> 5 files changed, 572 insertions(+), 57 deletions(-) >>> >>