From: Antheas Kapenekakis <lkml@antheas.dev>
To: mario.limonciello@amd.com, mpearson-lenovo@squebb.ca
Cc: ilpo.jarvinen@linux.intel.com, lenb@kernel.org,
linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
platform-driver-x86@vger.kernel.org, rafael@kernel.org,
hdegoede@redhat.com, me@kylegospodneti.ch, luke@ljones.dev,
Antheas Kapenekakis <lkml@antheas.dev>
Subject: [PATCH v2 1/2] ACPI: platform_profile: Add handlers that do not occlude power options
Date: Thu, 27 Feb 2025 16:36:02 +0100 [thread overview]
Message-ID: <20250227153603.131046-2-lkml@antheas.dev> (raw)
In-Reply-To: <20250227153603.131046-1-lkml@antheas.dev>
Currently, only the subset of supported profiles are exposed when
using platform profile. This is a big problem when e.g., asus-wmi
and amd-pmf are loaded together, as they have conflicting low
power options. This causes ppd and tuned to miss the low power
option increasing power consumption. In 2025, this is only expected
to increase, as more laptops ship with amd-pmf notifiers and this
starts to become an issue in more manufacturers.
Therefore, this commit introduces the concept of secondary handlers.
These are handlers that are able to accept all options and do not
partake in the process of selecting which profiles are visible.
These handlers still have a probe function which is used for their
endpoint and their endpoint works normally and will block other
options. However, when called from the legacy endpoint, all options
will be sent to them. It is the expectation that secondary handlers
will pick the closest profile they have to what was sent.
In the absence of a primary handler, the options shown in the legacy
endpoint will be the union of all options of all secondary handlers.
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/acpi/platform_profile.c | 57 +++++++++++++++++++++++++++-----
include/linux/platform_profile.h | 5 +++
2 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
index 2ad53cc6aae5..55e8bb6adf8e 100644
--- a/drivers/acpi/platform_profile.c
+++ b/drivers/acpi/platform_profile.c
@@ -63,17 +63,18 @@ static ssize_t _commmon_choices_show(unsigned long *choices, char *buf)
* _store_class_profile - Set the profile for a class device
* @dev: The class device
* @data: The profile to set
+ * @enforce_valid: For secondary handlers, enforce that the profile is valid
*
* Return: 0 on success, -errno on failure
*/
-static int _store_class_profile(struct device *dev, void *data)
+static int _store_class_profile(struct device *dev, void *data, bool enforce_valid)
{
struct platform_profile_handler *handler;
int *bit = (int *)data;
lockdep_assert_held(&profile_lock);
handler = to_pprof_handler(dev);
- if (!test_bit(*bit, handler->choices))
+ if ((enforce_valid || !handler->ops->secondary) && !test_bit(*bit, handler->choices))
return -EOPNOTSUPP;
return handler->ops->profile_set(dev, *bit);
@@ -204,7 +205,7 @@ static ssize_t profile_store(struct device *dev,
return -EINVAL;
scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) {
- ret = _store_class_profile(dev, &index);
+ ret = _store_class_profile(dev, &index, true);
if (ret)
return ret;
}
@@ -243,21 +244,37 @@ static const struct class platform_profile_class = {
*
* Return: 0 on success, -errno on failure
*/
-static int _aggregate_choices(struct device *dev, void *data)
+static int _aggregate_choices(struct device *dev, void *data, bool secondary)
{
struct platform_profile_handler *handler;
unsigned long *aggregate = data;
lockdep_assert_held(&profile_lock);
handler = to_pprof_handler(dev);
+
+ if (handler->ops->secondary != secondary)
+ return 0;
+
if (test_bit(PLATFORM_PROFILE_LAST, aggregate))
bitmap_copy(aggregate, handler->choices, PLATFORM_PROFILE_LAST);
+ else if (handler->ops->secondary)
+ bitmap_or(aggregate, handler->choices, aggregate, PLATFORM_PROFILE_LAST);
else
bitmap_and(aggregate, handler->choices, aggregate, PLATFORM_PROFILE_LAST);
return 0;
}
+static int _aggregate_choices_primary(struct device *dev, void *data)
+{
+ return _aggregate_choices(dev, data, false);
+}
+
+static int _aggregate_choices_secondary(struct device *dev, void *data)
+{
+ return _aggregate_choices(dev, data, true);
+}
+
/**
* platform_profile_choices_show - Show the available profile choices for legacy sysfs interface
* @dev: The device
@@ -276,9 +293,16 @@ static ssize_t platform_profile_choices_show(struct device *dev,
set_bit(PLATFORM_PROFILE_LAST, aggregate);
scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) {
err = class_for_each_device(&platform_profile_class, NULL,
- aggregate, _aggregate_choices);
+ aggregate, _aggregate_choices_primary);
if (err)
return err;
+
+ if (test_bit(PLATFORM_PROFILE_LAST, aggregate)) {
+ err = class_for_each_device(&platform_profile_class, NULL,
+ aggregate, _aggregate_choices_secondary);
+ if (err)
+ return err;
+ }
}
/* no profile handler registered any more */
@@ -325,7 +349,7 @@ static int _store_and_notify(struct device *dev, void *data)
enum platform_profile_option *profile = data;
int err;
- err = _store_class_profile(dev, profile);
+ err = _store_class_profile(dev, profile, false);
if (err)
return err;
return _notify_class_profile(dev, NULL);
@@ -384,9 +408,18 @@ static ssize_t platform_profile_store(struct device *dev,
set_bit(PLATFORM_PROFILE_LAST, choices);
scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) {
ret = class_for_each_device(&platform_profile_class, NULL,
- choices, _aggregate_choices);
+ choices, _aggregate_choices_primary);
if (ret)
return ret;
+
+ if (test_bit(PLATFORM_PROFILE_LAST, choices)) {
+ ret = class_for_each_device(
+ &platform_profile_class, NULL, choices,
+ _aggregate_choices_secondary);
+ if (ret)
+ return ret;
+ }
+
if (!test_bit(i, choices))
return -EOPNOTSUPP;
@@ -470,10 +503,18 @@ int platform_profile_cycle(void)
return -EINVAL;
err = class_for_each_device(&platform_profile_class, NULL,
- choices, _aggregate_choices);
+ choices, _aggregate_choices_primary);
if (err)
return err;
+ if (test_bit(PLATFORM_PROFILE_LAST, choices)) {
+ err = class_for_each_device(
+ &platform_profile_class, NULL, choices,
+ _aggregate_choices_secondary);
+ if (err)
+ return err;
+ }
+
/* never iterate into a custom if all drivers supported it */
clear_bit(PLATFORM_PROFILE_CUSTOM, choices);
diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
index 8ab5b0e8eb2c..3593d3311a89 100644
--- a/include/linux/platform_profile.h
+++ b/include/linux/platform_profile.h
@@ -37,11 +37,16 @@ enum platform_profile_option {
* profile in sysfs.
* @profile_set: Callback that will be called when storing a new platform
* profile in sysfs.
+ * @secondary: Set the platform handler as a secondary. Secondary handlers
+ * accept all profile options and are not considered in choosing
+ * which options to show under the legacy sysfs. This way, they do
+ * not occlude the primary handler's choices.
*/
struct platform_profile_ops {
int (*probe)(void *drvdata, unsigned long *choices);
int (*profile_get)(struct device *dev, enum platform_profile_option *profile);
int (*profile_set)(struct device *dev, enum platform_profile_option profile);
+ bool secondary;
};
struct device *platform_profile_register(struct device *dev, const char *name,
--
2.48.1
next prev parent reply other threads:[~2025-02-27 15:36 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-27 15:36 [PATCH v2 0/2] ACPI: platform_profile: fix legacy sysfs with multiple handlers Antheas Kapenekakis
2025-02-27 15:36 ` Antheas Kapenekakis [this message]
2025-02-27 15:36 ` [PATCH v2 2/2] ACPI: platform_profile: make amd-pmf a secondary handler Antheas Kapenekakis
2025-02-27 16:45 ` Mario Limonciello
2025-02-27 17:04 ` Antheas Kapenekakis
2025-02-27 17:10 ` Mario Limonciello
2025-02-27 17:18 ` Antheas Kapenekakis
2025-02-27 17:23 ` Mario Limonciello
2025-02-27 17:28 ` Antheas Kapenekakis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250227153603.131046-2-lkml@antheas.dev \
--to=lkml@antheas.dev \
--cc=hdegoede@redhat.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luke@ljones.dev \
--cc=mario.limonciello@amd.com \
--cc=me@kylegospodneti.ch \
--cc=mpearson-lenovo@squebb.ca \
--cc=platform-driver-x86@vger.kernel.org \
--cc=rafael@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox