Hi,
On 9/17/20 1:50 PM, Bastien Nocera wrote:
Hey,
On Thu, 2020-09-17 at 13:22 +0200, Hans de Goede wrote:
Hi Elia, Mark, et al.
Elia, Mark I'm mailing you both because both of you have pdx86
patches pending to add a vendor
specific sysfs-attribute for selecting performance-profiles for resp.
HP and Lenovo Thinkpad laptops.
I think that this shows that we might need to start thinking
about a generic kernel API for this, otherwise we will
end up with slight different options per vendor ...
Some comments below based on possible use in power-profiles-daemon:
https://www.hadess.net/2020/09/power-profiles-daemon-new-project.html
So it seems we may need something like:
/sys/class/system_performance_profile
Where we would then get e.g.:
/sys/class/system_performance_profile/thinkpad_acpi/performance_profi
le
And then we need to standardize on the names/values which
performance_profile can show / accept when written too.
The big question is what do we do if there are more then 3 profiles?
The Intel P-State driver in the kernel supports 4 separate ones (plus
default):
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpufreq/intel_pstate.c#n591
which we crammed into 3 profiles:
https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/blob/master/src/ppd-driver-intel-pstate.c#L209-226
One option would be something like the following:
cat
/sys/class/system_performance_profile/thinkpad_acpi/performance_profi
le
low-power [balanced] performance
Are the square brackets to show the currently selected profile? I'd
rather it was a separate sysfs attribute. I would expect to only ever
read the list of supported profiles once, and then monitor an "active
profile" attribute.
See my new proposal in reaction to Benjamin's email.
(a bit like the intel_pstate kernel driver does, but then all the
devices that support Intel P-State support all the profiles, so it's
not a good example ;)
cat
/sys/class/system_performance_profile/thinkpad_acpi/extra_performance
_profiles
extra-low-power balanced-performance-mix
So we add an optional extra_performance_profiles sysfs attribute and
we ask all
drivers implemeting this class to implement at least the 3 standard
profiles
(by mapping 3 of their options to these) and optional they can offer
extra
profiles (with free form names) in the extra_performance_profiles
sysfs attribute under the class-device.
The idea behind putting the extra profiles in a separate sysfs-
attribute
is that reading the main performance_profile attribute will always
show
one selected, even if one of the extra profiles is actually in use,
then the driver should also show the closest standardized profile as
being active.
I think it's fine having more than 3 profiles. Something like power-
profiles-daemon would likely trying to match them all to one of the 3
profiles it uses as an interface, or forcing the use of those 3
profiles, depending on what that profile behaves.
This will allow userspace code to always rely on the standard
interface
both for getting a representation of the currently active profile as
well
as for setting the active profile.
Elia, Mark, I assume that both of you want to get your patches for
this
upstream sooner, rather then later. But I think we should put them on
hold until we have an agreement on a shared userspace API for this.
Compared to the WIP lenovo-dytc "perfmode" driver, we're missing
something to advertise the unavailability of a profile, and the reason
for that unavailability.
UGh, do we really need to export this though. We have the lap_mode thing
already; and that is something which we will need for other reasons in
the future too. Any UI for selecting performance modes can display a
warning when lap_mode is true saying that: "The laptop has detected that it
is sitting on someone's lap and that performance may be limited
because of this." (feel free to improve the text).
I guess we could split the "value" attribute from my reply to Benjamin's
email into "configured_value" (rw) and "actual_value" (rw) attributes.
If we have the info we might as well export it I guess,.
Regards,
Hans
