Re: [PATCH 0/3] cpufreq/amd-pstate: Set initial min_freq to lowest_nonlinear_freq

[Mario Limonciello <mario.limonciello@amd.com>]

On 10/3/2024 03:39, Dhananjay Ugwekar wrote:
> According to the AMD architectural programmer's manual volume 2 [1],
> in section "17.6.4.1 CPPC_CAPABILITY_1" lowest_nonlinear_perf is described
> as "Reports the most energy efficient performance level (in terms of
> performance per watt). Above this threshold, lower performance levels
> generally result in increased energy efficiency. Reducing performance
> below this threshold does not result in total energy savings for a given
> computation, although it reduces instantaneous power consumption". So
> lowest_nonlinear_perf is the most power efficient performance level, and
> going below that would lead to a worse performance/watt.
> 
> Also setting the minimum frequency to lowest_nonlinear_freq (instead of
> lowest_freq) allows the CPU to idle at a higher frequency which leads
> to more time being spent in a deeper idle state (as trivial idle tasks
> are completed sooner). This has shown a power benefit in some systems.
> In other systems, power consumption has increased but so has the
> throughput/watt.
> 
> Our objective here is to update the initial lower frequency limit to
> lowest_nonlinear_freq, while allowing the user to later update the lower
> limit to anywhere between lowest_freq to highest_freq for the platform.
> 
> Currently, amd-pstate driver sets the cpudata->req[0] qos_request (lets
> call it amd_pstate_req) to the lowest_freq value at init time, and
> cpufreq.c sets the min_freq_req to 0 (which also gets resolved to the
> lowest_freq eventually). Writing to scaling_min_freq, only updates
> min_freq_req qos_request, while the amd_pstate_req always stays same as the
>   initial value. This leads to the amd_pstate_req becoming the hard lower
> limit (due to the nature of priority lists used to manage the min_freq
> requests). Hence, if we update the amd_pstate_req to lowest_nonlinear_freq
> from amd-pstate driver code, user will never be able to set
> scaling_min_freq to a value lower than that.
> 
> This problem is occurring due to the existence of two different sources
> of lower frequency limits, i.e. cpufreq.c and amd-pstate.c. Removing the
> cpudata->req[0], and updating the min_freq_req itself from amd-pstate
> driver at init time fixes this issue and gives flexibility to the driver
> code as well as allows the user to independently update the lower limit
> later on.
> 
> So, add a callback in cpufreq_driver to update the min_freq_req from
> cpufreq drivers and use it to set the initial min_freq to
> lowest_nonlinear_freq for amd-pstate driver (active, passive and guided
> modes) and cleanup the old min_freq qos request code.
> 
> Link: https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdf [1]
> 
> Dhananjay Ugwekar (3):
>    cpufreq: Add a callback to update the min_freq_req from drivers
>    cpufreq/amd-pstate: Set the initial min_freq to lowest_nonlinear_freq
>    cpufreq/amd-pstate: Cleanup the old min_freq qos request remnants
> 
>   drivers/cpufreq/amd-pstate.c | 35 +++++++++++++++++------------------
>   drivers/cpufreq/amd-pstate.h |  4 ++--
>   drivers/cpufreq/cpufreq.c    |  6 +++++-
>   include/linux/cpufreq.h      |  6 ++++++
>   4 files changed, 30 insertions(+), 21 deletions(-)
> 

Thanks for the series, it looks good to me and from my testing works as 
intended.

For the series:
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

Rafael, Viresh,

Can I get your A-b on patch 1?  I'll take it with other new amd-pstate 
content coming this cycle then.

Thansk,