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. (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. Cheers > > I would like to think that the above proposal is a good start, > if we can quickly (*) decide on an userspace API here > > Regards, > > Hans > > p.s. > > I guess we should also add an optional lap_mode sysfs attribute > to the class-device, to have all the info for the Thinkpads in > one place. > > > *) but not too quickly, it is important we get this right >
