Hi All,
This patch series adds a platform_profile_cycle function to the platform
profile module, which allows modules to easily switch between the
enabled profiles.
Use it in ideapad-laptop and thinkpad-acpi modules.
Best regards,
Gergo
Changes in v4:
- move the cycle to the platform profile module where it can switch
between the enabled profiles
- add a patch to use it in the thinkpad-acpi module
Changes in v3:
- add dytc_profile_cycle function
Changes in v2:
- only switch platform profiles if supported, otherwise keep the
behavior.
[3]: https://lore.kernel.org/all/7c358ad8dd6de7889fa887954145a181501ac362.1712236099.git.soyer@irl.hu/
[2]: https://lore.kernel.org/all/797884d8cab030d3a2b656dba67f3c423cc58be7.1712174794.git.soyer@irl.hu/
[1]: https://lore.kernel.org/all/85254ce8e87570c05e7f04d6507701bef954ed75.1712149429.git.soyer@irl.hu/
---
Gergo Koteles (3):
ACPI: platform-profile: add platform_profile_cycle()
platform/x86: ideapad-laptop: switch platform profiles using thermal
management key
platform/x86: thinkpad_acpi: use platform_profile_cycle()
drivers/acpi/platform_profile.c | 42 +++++++++++++++++++++++++++
drivers/platform/x86/ideapad-laptop.c | 7 +++--
drivers/platform/x86/thinkpad_acpi.c | 19 ++----------
include/linux/platform_profile.h | 1 +
4 files changed, 50 insertions(+), 19 deletions(-)
base-commit: 39cd87c4eb2b893354f3b850f916353f2658ae6f
--
2.44.0
Some laptops have a key to switch platform profiles.
Add a platform_profile_cycle() function to cycle between the enabled
profiles.
Signed-off-by: Gergo Koteles <[email protected]>
---
drivers/acpi/platform_profile.c | 42 ++++++++++++++++++++++++++++++++
include/linux/platform_profile.h | 1 +
2 files changed, 43 insertions(+)
diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
index d418462ab791..1579f380d469 100644
--- a/drivers/acpi/platform_profile.c
+++ b/drivers/acpi/platform_profile.c
@@ -136,6 +136,48 @@ void platform_profile_notify(void)
}
EXPORT_SYMBOL_GPL(platform_profile_notify);
+int platform_profile_cycle(void)
+{
+ enum platform_profile_option profile;
+ enum platform_profile_option next;
+ int err;
+
+ err = mutex_lock_interruptible(&profile_lock);
+ if (err)
+ return err;
+
+ if (!cur_profile) {
+ mutex_unlock(&profile_lock);
+ return -ENODEV;
+ }
+
+ err = cur_profile->profile_get(cur_profile, &profile);
+ if (err) {
+ mutex_unlock(&profile_lock);
+ return err;
+ }
+
+ next = ffs(cur_profile->choices[0] >> (profile + 1)) + profile;
+
+ /* current profile is the highest, select the lowest */
+ if (next == profile)
+ next = ffs(cur_profile->choices[0]) - 1;
+
+ if (WARN_ON((next < 0) || (next >= ARRAY_SIZE(profile_names)))) {
+ mutex_unlock(&profile_lock);
+ return -EINVAL;
+ }
+
+ err = cur_profile->profile_set(cur_profile, next);
+ mutex_unlock(&profile_lock);
+
+ if (!err)
+ sysfs_notify(acpi_kobj, NULL, "platform_profile");
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(platform_profile_cycle);
+
int platform_profile_register(struct platform_profile_handler *pprof)
{
int err;
diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
index e5cbb6841f3a..f5492ed413f3 100644
--- a/include/linux/platform_profile.h
+++ b/include/linux/platform_profile.h
@@ -36,6 +36,7 @@ struct platform_profile_handler {
int platform_profile_register(struct platform_profile_handler *pprof);
int platform_profile_remove(void);
+int platform_profile_cycle(void);
void platform_profile_notify(void);
#endif /*_PLATFORM_PROFILE_H_*/
--
2.44.0
Ideapad laptops have thermal management or performance mode switch key
(Fn + Q). They report KEY_PROG4.
If supported, cycle between platform profiles instead.
Tested on Yoga7 14ARB7.
Signed-off-by: Gergo Koteles <[email protected]>
---
drivers/platform/x86/ideapad-laptop.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 901849810ce2..4893b1aef696 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1181,8 +1181,11 @@ static void ideapad_check_special_buttons(struct ideapad_private *priv)
switch (bit) {
case 6: /* Z570 */
case 0: /* Z580 */
- /* Thermal Management button */
- ideapad_input_report(priv, 65);
+ /* Thermal Management / Performance Mode button */
+ if (priv->dytc)
+ platform_profile_cycle();
+ else
+ ideapad_input_report(priv, 65);
break;
case 1:
/* OneKey Theater button */
--
2.44.0
Some Thinkpads have a 'mode' button that switches between platform
profiles.
Use the new platform_module_cycle function instead of the existing
switch-based one.
Signed-off-by: Gergo Koteles <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 19 ++-----------------
1 file changed, 2 insertions(+), 17 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 82429e59999d..771aaa7ae4cf 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -11190,23 +11190,8 @@ static void tpacpi_driver_event(const unsigned int hkey_event)
else
dytc_control_amt(!dytc_amt_active);
}
- if (hkey_event == TP_HKEY_EV_PROFILE_TOGGLE) {
- switch (dytc_current_profile) {
- case PLATFORM_PROFILE_LOW_POWER:
- dytc_profile_set(NULL, PLATFORM_PROFILE_BALANCED);
- break;
- case PLATFORM_PROFILE_BALANCED:
- dytc_profile_set(NULL, PLATFORM_PROFILE_PERFORMANCE);
- break;
- case PLATFORM_PROFILE_PERFORMANCE:
- dytc_profile_set(NULL, PLATFORM_PROFILE_LOW_POWER);
- break;
- default:
- pr_warn("Profile HKEY unexpected profile %d", dytc_current_profile);
- }
- /* Notify user space the profile changed */
- platform_profile_notify();
- }
+ if (hkey_event == TP_HKEY_EV_PROFILE_TOGGLE)
+ platform_profile_cycle();
}
static void hotkey_driver_event(const unsigned int scancode)
--
2.44.0
Hi Gergo,
please Cc people who commented your changes.
see below:
On 05/04/2024 05:05, Gergo Koteles wrote:
> Some laptops have a key to switch platform profiles.
>
> Add a platform_profile_cycle() function to cycle between the enabled
> profiles.
>
> Signed-off-by: Gergo Koteles <[email protected]>
> ---
> drivers/acpi/platform_profile.c | 42 ++++++++++++++++++++++++++++++++
> include/linux/platform_profile.h | 1 +
> 2 files changed, 43 insertions(+)
>
> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> index d418462ab791..1579f380d469 100644
> --- a/drivers/acpi/platform_profile.c
> +++ b/drivers/acpi/platform_profile.c
> @@ -136,6 +136,48 @@ void platform_profile_notify(void)
> }
> EXPORT_SYMBOL_GPL(platform_profile_notify);
>
> +int platform_profile_cycle(void)
> +{
> + enum platform_profile_option profile;
> + enum platform_profile_option next;
> + int err;
> +
> + err = mutex_lock_interruptible(&profile_lock);
> + if (err)
> + return err;
> +
> + if (!cur_profile) {
> + mutex_unlock(&profile_lock);
> + return -ENODEV;
> + }
> +
> + err = cur_profile->profile_get(cur_profile, &profile);
> + if (err) {
> + mutex_unlock(&profile_lock);
> + return err;
> + }
> +
> + next = ffs(cur_profile->choices[0] >> (profile + 1)) + profile;
> +
> + /* current profile is the highest, select the lowest */
> + if (next == profile)
> + next = ffs(cur_profile->choices[0]) - 1;
> +
> + if (WARN_ON((next < 0) || (next >= ARRAY_SIZE(profile_names)))) {
> + mutex_unlock(&profile_lock);
> + return -EINVAL;
> + }
Why do you need to do this?
That can be simplified by:
[ ... ]
err = cur_profile->profile_get(cur_profile, &profile);
if (err)
goto out;
profile = (profile + 1) % ARRAY_SIZE(profile_names);
err = cur_profile->profile_set(cur_profile, next);
out:
mutex_unlock(&profile_lock);
[ ... ]
> + err = cur_profile->profile_set(cur_profile, next);
> + mutex_unlock(&profile_lock);
> +
> + if (!err)
> + sysfs_notify(acpi_kobj, NULL, "platform_profile");
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(platform_profile_cycle);
> +
> int platform_profile_register(struct platform_profile_handler *pprof)
> {
> int err;
> diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
> index e5cbb6841f3a..f5492ed413f3 100644
> --- a/include/linux/platform_profile.h
> +++ b/include/linux/platform_profile.h
> @@ -36,6 +36,7 @@ struct platform_profile_handler {
>
> int platform_profile_register(struct platform_profile_handler *pprof);
> int platform_profile_remove(void);
> +int platform_profile_cycle(void);
> void platform_profile_notify(void);
>
> #endif /*_PLATFORM_PROFILE_H_*/
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Hi Daniel,
On Fri, 2024-04-05 at 08:48 +0200, Daniel Lezcano wrote:
> Hi Gergo,
>
> please Cc people who commented your changes.
Thanks for this info, next time :)
> > +int platform_profile_cycle(void)
> > +{
> > + enum platform_profile_option profile;
> > + enum platform_profile_option next;
> > + int err;
> > +
> > + err = mutex_lock_interruptible(&profile_lock);
> > + if (err)
> > + return err;
> > +
> > + if (!cur_profile) {
> > + mutex_unlock(&profile_lock);
> > + return -ENODEV;
> > + }
> > +
> > + err = cur_profile->profile_get(cur_profile, &profile);
> > + if (err) {
> > + mutex_unlock(&profile_lock);
> > + return err;
> > + }
> > +
> > + next = ffs(cur_profile->choices[0] >> (profile + 1)) + profile;
> > +
> > + /* current profile is the highest, select the lowest */
> > + if (next == profile)
> > + next = ffs(cur_profile->choices[0]) - 1;
> > +
> > + if (WARN_ON((next < 0) || (next >= ARRAY_SIZE(profile_names)))) {
> > + mutex_unlock(&profile_lock);
> > + return -EINVAL;
> > + }
>
> Why do you need to do this?
>
Many platform drivers use the platform profile module. They support
different sets of profiles, not all. They sets the corresponding bits
in the choices variable.
like this:
set_bit(PLATFORM_PROFILE_BALANCED, platform_profile_handler.choices);
The platform_profile_cycle() cycles through the enabled profiles.
Best regards,
Gergo
> That can be simplified by:
>
> [ ... ]
>
> err = cur_profile->profile_get(cur_profile, &profile);
> if (err)
> goto out;
>
> profile = (profile + 1) % ARRAY_SIZE(profile_names);
>
> err = cur_profile->profile_set(cur_profile, next);
> out:
> mutex_unlock(&profile_lock);
>
> [ ... ]
>
> > + err = cur_profile->profile_set(cur_profile, next);
> > + mutex_unlock(&profile_lock);
> > +
> > + if (!err)
> > + sysfs_notify(acpi_kobj, NULL, "platform_profile");
> > +
> > + return err;
> > +}
> > +EXPORT_SYMBOL_GPL(platform_profile_cycle);
> > +
> > int platform_profile_register(struct platform_profile_handler *pprof)
> > {
> > int err;
> > diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
> > index e5cbb6841f3a..f5492ed413f3 100644
> > --- a/include/linux/platform_profile.h
> > +++ b/include/linux/platform_profile.h
> > @@ -36,6 +36,7 @@ struct platform_profile_handler {
> >
> > int platform_profile_register(struct platform_profile_handler *pprof);
> > int platform_profile_remove(void);
> > +int platform_profile_cycle(void);
> > void platform_profile_notify(void);
> >
> > #endif /*_PLATFORM_PROFILE_H_*/
>
Hi
2024. április 5., péntek 5:05 keltezéssel, Gergo Koteles <[email protected]> írta:
> Some laptops have a key to switch platform profiles.
>
> Add a platform_profile_cycle() function to cycle between the enabled
> profiles.
>
> Signed-off-by: Gergo Koteles <[email protected]>
> ---
> drivers/acpi/platform_profile.c | 42 ++++++++++++++++++++++++++++++++
> include/linux/platform_profile.h | 1 +
> 2 files changed, 43 insertions(+)
>
> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> index d418462ab791..1579f380d469 100644
> --- a/drivers/acpi/platform_profile.c
> +++ b/drivers/acpi/platform_profile.c
> @@ -136,6 +136,48 @@ void platform_profile_notify(void)
> }
> EXPORT_SYMBOL_GPL(platform_profile_notify);
>
> +int platform_profile_cycle(void)
> +{
> + enum platform_profile_option profile;
> + enum platform_profile_option next;
> + int err;
> +
> + err = mutex_lock_interruptible(&profile_lock);
> + if (err)
> + return err;
> +
> + if (!cur_profile) {
> + mutex_unlock(&profile_lock);
> + return -ENODEV;
> + }
> +
> + err = cur_profile->profile_get(cur_profile, &profile);
> + if (err) {
> + mutex_unlock(&profile_lock);
> + return err;
> + }
> +
> + next = ffs(cur_profile->choices[0] >> (profile + 1)) + profile;
> +
> + /* current profile is the highest, select the lowest */
> + if (next == profile)
> + next = ffs(cur_profile->choices[0]) - 1;
I think you can use `find_next_bit()` or similar instead.
> +
> + if (WARN_ON((next < 0) || (next >= ARRAY_SIZE(profile_names)))) {
> + mutex_unlock(&profile_lock);
> + return -EINVAL;
> + }
> +
> + err = cur_profile->profile_set(cur_profile, next);
> + mutex_unlock(&profile_lock);
> +
> + if (!err)
> + sysfs_notify(acpi_kobj, NULL, "platform_profile");
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(platform_profile_cycle);
> +
> int platform_profile_register(struct platform_profile_handler *pprof)
> {
> int err;
> diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
> index e5cbb6841f3a..f5492ed413f3 100644
> --- a/include/linux/platform_profile.h
> +++ b/include/linux/platform_profile.h
> @@ -36,6 +36,7 @@ struct platform_profile_handler {
>
> int platform_profile_register(struct platform_profile_handler *pprof);
> int platform_profile_remove(void);
> +int platform_profile_cycle(void);
> void platform_profile_notify(void);
>
> #endif /*_PLATFORM_PROFILE_H_*/
> --
> 2.44.0
>
Regards,
Barnabás Pőcze
Hi Barnabás,
On Fri, 2024-04-05 at 16:34 +0000, Barnabás Pőcze wrote:
> > + next = ffs(cur_profile->choices[0] >> (profile + 1)) + profile;
> > +
> > + /* current profile is the highest, select the lowest */
> > + if (next == profile)
> > + next = ffs(cur_profile->choices[0]) - 1;
>
> I think you can use `find_next_bit()` or similar instead.
>
Thanks, it looks much better with find_next_bit_wrap.
>
Best regards,
Gergo