RE: [PATCH v5 18/18] xen/cpufreq: Adapt SET/GET_CPUFREQ_CPPC xen_sysctl_pm_op for amd-cppc driver

["Penny, Zheng" <penny.zheng@xxxxxxx>]

[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: Tuesday, June 17, 2025 6:38 PM
> To: Penny, Zheng <penny.zheng@xxxxxxx>
> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Anthony PERARD
> <anthony.perard@xxxxxxxxxx>; Andrew Cooper <andrew.cooper3@xxxxxxxxxx>;
> Orzel, Michal <Michal.Orzel@xxxxxxx>; Julien Grall <julien@xxxxxxx>; Roger Pau
> Monné <roger.pau@xxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; 
> xen-
> devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v5 18/18] xen/cpufreq: Adapt SET/GET_CPUFREQ_CPPC
> xen_sysctl_pm_op for amd-cppc driver
>
> On 27.05.2025 10:48, Penny Zheng wrote:
> > --- a/tools/misc/xenpm.c
> > +++ b/tools/misc/xenpm.c
> > +
> > +    case XEN_SYSCTL_CPPC_SET_PRESET_BALANCE:
> > +        if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_DESIRED )
> > +            return -EINVAL;
> > +        policy->policy = CPUFREQ_POLICY_BALANCE;
> > +        min_perf = data->caps.lowest_perf;
> > +        max_perf = data->caps.highest_perf;
> > +        epp = CPPC_ENERGY_PERF_BALANCE;
> > +        break;
>
> Isn't this more line "ondemand"? To me, "balance" would mean tying perf to at 
> least
> close around the middle of lowest and highest.
>

The "balance" word comes from the epp value, it is 127, which is the middle 
value
In actual hardware algorithm, the value of Energy Performance Preference 
register(EPP) will be translated to frequency setting,
And it sets the minimum active frequency.
 An EPP of zero sets the min active frequency to Fmax, while an EPP of 255 sets 
the min active frequency to Fmin (~IdleFreq).  It is linear scaling, so epp of 
127 is calculated to the middle of Fmax and Fmin.
And Fmax corresponds to the highest perf, and Fmin corresponds to the 
non-linear lowest perf

> > +    case XEN_SYSCTL_CPPC_SET_PRESET_NONE:
> > +        /*
> > +         * In paasive mode, Xen governor is responsible for perfomance 
> > tuning.
>
> Nit: passive
>
> > +         * we shall set lowest_perf with "lowest_nonlinear_perf" to ensure
> > +         * governoring performance in P-states range.
> > +         */
> > +        min_perf = data->caps.lowest_nonlinear_perf;
> > +        max_perf = data->caps.highest_perf;
>
> As in the earlier patch - I fear I don't understand the comment, and hence 
> why to
> use lowest-nonlinear here remains unclear to me.
>

After previous discussion, I'll use non-linear lowest in all places

> Jan