linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Pearson <markpearson@lenovo.com>
To: <markpearson@lenovo.com>
Cc: <rafael@kernel.org>, <hdegoede@redhat.com>,
	<linux-acpi@vger.kernel.org>,
	<platform-driver-x86@vger.kernel.org>
Subject: [RFC] ACPI: platform-profile: support for AC vs DC modes
Date: Tue, 1 Mar 2022 15:15:54 -0500	[thread overview]
Message-ID: <20220301201554.4417-1-markpearson@lenovo.com> (raw)
In-Reply-To: <markpearson@lenovo.com>

Looking for feedback on this feature. Whether it is worth
pursuing and any concerns with the general implementation.

I've recently been working on PSC platform profile mode support
for Lenovo AMD platforms (patch proposed upstream last week). 
One of the interesting pieces with the Lenovo PSC implementation
is it supports different profiles for AC (plugged in) vs DC
(running from battery).

I was thinking of adding this support in the thinkpad_acpi driver,
but it seems it would be nicer to make this generally available for
all platforms that offer profile support.

This implementation allows the user to set one profile for when a
system is plugged in, and a different profile for when they are
unplugged. I imagine this would be used so that performance mode
is used when plugged in and low-power used when unplugged (as an
example). The user could configure it to match their preference.

If the user doesn't configure a DC profile it behaves the same as
previously and any ACPI power events will be ignored. If the user
configures a DC profile then when a system is unplugged it will
automatically configure this setting.

I've added platform_profile_ac and platform_profile_dc sysfs nodes.
The platform_profile and platform_profile_ac nodes will behave the
same when setting a profile to maintain backwards compatibility.

If you read the platform_profile it will return the currently
active profile.
If you read the platform_profile_ac or platform_profile_dc node it
will return the configured profile. This is something missing from
the current implementation that I think is a nice bonus.

User space implementation could potentially be used to do the same
idea, but having this available allows users to configure from
cmdline or use scripts seemed valuable.

Note - I'm aware that I still need to:
 1) Update the API documentation file
 2) Implement a disable/unconfigure on the profile_dc setting
But I figured this was far enough along that it would be good to get
comments.

Thanks in advance for any feedback.

Signed-off-by: Mark Pearson <markpearson@lenovo.com>
---
 drivers/acpi/platform_profile.c  | 130 +++++++++++++++++++++++++++++--
 include/linux/platform_profile.h |   1 +
 2 files changed, 125 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
index d418462ab791..e4246e6632cf 100644
--- a/drivers/acpi/platform_profile.c
+++ b/drivers/acpi/platform_profile.c
@@ -7,6 +7,7 @@
 #include <linux/init.h>
 #include <linux/mutex.h>
 #include <linux/platform_profile.h>
+#include <linux/power_supply.h>
 #include <linux/sysfs.h>
 
 static struct platform_profile_handler *cur_profile;
@@ -22,6 +23,51 @@ static const char * const profile_names[] = {
 };
 static_assert(ARRAY_SIZE(profile_names) == PLATFORM_PROFILE_LAST);
 
+static struct notifier_block ac_nb;
+static int cur_profile_ac;
+static int cur_profile_dc;
+
+static int platform_profile_set(void)
+{
+	int profile, err;
+
+	if (cur_profile_dc == PLATFORM_PROFILE_UNCONFIGURED)
+		profile = cur_profile_ac;
+	else {
+		if (power_supply_is_system_supplied() > 0)
+			profile = cur_profile_ac;
+		else
+			profile = cur_profile_dc;
+	}
+
+	err = mutex_lock_interruptible(&profile_lock);
+	if (err)
+		return err;
+
+	err = cur_profile->profile_set(cur_profile, profile);
+	if (err)
+		return err;
+
+	sysfs_notify(acpi_kobj, NULL, "platform_profile");
+	mutex_unlock(&profile_lock);
+	return 0;
+}
+
+static int platform_profile_acpi_event(struct notifier_block *nb,
+					unsigned long val,
+					void *data)
+{
+	struct acpi_bus_event *entry = (struct acpi_bus_event *)data;
+
+	WARN_ON(cur_profile_dc == PLATFORM_PROFILE_UNCONFIGURED);
+
+	/* if power supply changed, then update profile */
+	if (strcmp(entry->device_class, "ac_adapter") == 0)
+		return platform_profile_set();
+
+	return 0;
+}
+
 static ssize_t platform_profile_choices_show(struct device *dev,
 					struct device_attribute *attr,
 					char *buf)
@@ -77,9 +123,34 @@ static ssize_t platform_profile_show(struct device *dev,
 	return sysfs_emit(buf, "%s\n", profile_names[profile]);
 }
 
-static ssize_t platform_profile_store(struct device *dev,
+static ssize_t configured_profile_show(struct device *dev,
 			    struct device_attribute *attr,
-			    const char *buf, size_t count)
+			    char *buf, int profile)
+{
+	if (profile == PLATFORM_PROFILE_UNCONFIGURED)
+		return sysfs_emit(buf, "Not-configured\n");
+
+	/* Check that profile is valid index */
+	if (WARN_ON((profile < 0) || (profile >= ARRAY_SIZE(profile_names))))
+		return -EIO;
+	return sysfs_emit(buf, "%s\n", profile_names[profile]);
+}
+
+static ssize_t platform_profile_ac_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	return configured_profile_show(dev, attr, buf, cur_profile_ac);
+}
+
+static ssize_t platform_profile_dc_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	return configured_profile_show(dev, attr, buf, cur_profile_dc);
+}
+
+static int profile_select(const char *buf)
 {
 	int err, i;
 
@@ -105,11 +176,50 @@ static ssize_t platform_profile_store(struct device *dev,
 		return -EOPNOTSUPP;
 	}
 
-	err = cur_profile->profile_set(cur_profile, i);
-	if (!err)
-		sysfs_notify(acpi_kobj, NULL, "platform_profile");
-
 	mutex_unlock(&profile_lock);
+	return i;
+}
+
+static ssize_t platform_profile_store(struct device *dev,
+			    struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	int profile, err;
+
+	profile	= profile_select(buf);
+	if (profile < 0)
+		return profile;
+
+	cur_profile_ac = profile;
+	err = platform_profile_set();
+	if (err)
+		return err;
+	return count;
+}
+
+static ssize_t platform_profile_ac_store(struct device *dev,
+			    struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	return platform_profile_store(dev, attr, buf, count);
+}
+
+static ssize_t platform_profile_dc_store(struct device *dev,
+			    struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	int profile, err;
+
+	profile = profile_select(buf);
+	if (profile < 0)
+		return profile;
+
+	/* We need to register for ACPI events now */
+	if (cur_profile_dc == PLATFORM_PROFILE_UNCONFIGURED)
+		register_acpi_notifier(&ac_nb);
+
+	cur_profile_dc = profile;
+	err = platform_profile_set();
 	if (err)
 		return err;
 	return count;
@@ -117,10 +227,14 @@ static ssize_t platform_profile_store(struct device *dev,
 
 static DEVICE_ATTR_RO(platform_profile_choices);
 static DEVICE_ATTR_RW(platform_profile);
+static DEVICE_ATTR_RW(platform_profile_ac);
+static DEVICE_ATTR_RW(platform_profile_dc);
 
 static struct attribute *platform_profile_attrs[] = {
 	&dev_attr_platform_profile_choices.attr,
 	&dev_attr_platform_profile.attr,
+	&dev_attr_platform_profile_ac.attr,
+	&dev_attr_platform_profile_dc.attr,
 	NULL
 };
 
@@ -161,7 +275,9 @@ int platform_profile_register(struct platform_profile_handler *pprof)
 	}
 
 	cur_profile = pprof;
+	cur_profile_ac = cur_profile_dc = PLATFORM_PROFILE_UNCONFIGURED;
 	mutex_unlock(&profile_lock);
+	ac_nb.notifier_call = platform_profile_acpi_event;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(platform_profile_register);
@@ -169,6 +285,8 @@ EXPORT_SYMBOL_GPL(platform_profile_register);
 int platform_profile_remove(void)
 {
 	sysfs_remove_group(acpi_kobj, &platform_profile_group);
+	if (cur_profile_dc != PLATFORM_PROFILE_UNCONFIGURED)
+		unregister_acpi_notifier(&ac_nb);
 
 	mutex_lock(&profile_lock);
 	cur_profile = NULL;
diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
index e5cbb6841f3a..34566256bb60 100644
--- a/include/linux/platform_profile.h
+++ b/include/linux/platform_profile.h
@@ -15,6 +15,7 @@
  * If more options are added please update profile_names array in
  * platform_profile.c and sysfs-platform_profile documentation.
  */
+#define PLATFORM_PROFILE_UNCONFIGURED -1
 
 enum platform_profile_option {
 	PLATFORM_PROFILE_LOW_POWER,
-- 
2.34.1


             reply	other threads:[~2022-03-01 20:18 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-01 20:15 Mark Pearson [this message]
2022-03-03  2:53 ` [RFC] ACPI: platform-profile: support for AC vs DC modes Mario Limonciello
2022-03-03 17:08   ` [External] " Mark Pearson
2022-03-03 17:40     ` Limonciello, Mario
2022-03-08 14:39 ` Hans de Goede
2022-03-08 14:50   ` Limonciello, Mario
2022-03-08 15:16     ` Hans de Goede
2022-03-08 15:55       ` Limonciello, Mario
2022-03-08 16:10         ` Hans de Goede
2022-03-08 17:44           ` [External] " Mark Pearson
2022-03-14 12:45             ` Hans de Goede
2022-03-14 13:39               ` Limonciello, Mario
2022-03-14 14:43                 ` Hans de Goede
2022-03-14 14:59                   ` Mark Pearson
2022-03-14 15:05                     ` Hans de Goede
2022-03-14 15:31                   ` Hans de Goede
2022-03-14 15:32                     ` Mark Pearson
2022-03-14 16:56                       ` Hans de Goede
2022-03-14 17:10                         ` Limonciello, Mario
2022-03-14 17:13                         ` Mark Pearson

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=20220301201554.4417-1-markpearson@lenovo.com \
    --to=markpearson@lenovo.com \
    --cc=hdegoede@redhat.com \
    --cc=linux-acpi@vger.kernel.org \
    --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;
as well as URLs for NNTP newsgroup(s).