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(-)

-- 
2.34.1

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,