Re: [PATCH v3 04/14 RESEND] cpufreq: Add Hardware P-State (HWP) driver

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.05.2023 03:02, Marek Marczykowski-Górecki wrote:
> On Wed, May 10, 2023 at 04:19:57PM +0200, Jan Beulich wrote:
>> On 10.05.2023 15:54, Jason Andryuk wrote:
>>> On Mon, May 8, 2023 at 2:33 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>> On 05.05.2023 17:35, Jason Andryuk wrote:
>>>>> On Fri, May 5, 2023 at 3:01 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>> The other issue is that if you select "hwp" as the governor, but HWP
>>>>> hardware support is not available, then hwp_available() needs to reset
>>>>> the governor back to the default.  This feels like a layering
>>>>> violation.
>>>>
>>>> Layering violation - yes. But why would the governor need resetting in
>>>> this case? If HWP was asked for but isn't available, I don't think any
>>>> other cpufreq handling (and hence governor) should be put in place.
>>>> And turning off cpufreq altogether (if necessary in the first place)
>>>> wouldn't, to me, feel as much like a layering violation.
>>>
>>> My goal was for Xen to use HWP if available and fallback to the acpi
>>> cpufreq driver if not.  That to me seems more user-friendly than
>>> disabling cpufreq.
>>>
>>>             if ( hwp_available() )
>>>                 ret = hwp_register_driver();
>>>             else
>>>                 ret = cpufreq_register_driver(&acpi_cpufreq_driver);
>>
>> That's fine as a (future) default, but for now using hwp requires a
>> command line option, and if that option says "hwp" then it ought to
>> be hwp imo.
> 
> As a downstrem distribution, I'd strongly prefer to have an option that
> would enable HWP when present and fallback to other driver otherwise,
> even if that isn't the default upstream. I can't possibly require large
> group of users (either HWP-having or HWP-not-having) to edit the Xen
> cmdline to get power management working well.
> 
> If the meaning for cpufreq=hwp absolutely must include "nothing if HWP
> is not available", then maybe it should be named cpufreq=try-hwp
> instead, or cpufreq=prefer-hwp or something else like this?

Any new sub-option needs to fit the existing ones in its meaning. I
could see e.g. "cpufreq=xen" alone to effect what you want (once hwp
becomes available for use by default). But (for now at least) I
continue to think that a request for "hwp" ought to mean HWP.

Jan