* [PATCH v8 1/3] Documentation: Add documentation for new platform_profile sysfs attribute
@ 2020-12-30 0:18 Mark Pearson
2020-12-30 0:18 ` [PATCH v8 2/3] ACPI: platform-profile: Add platform profile support Mark Pearson
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Mark Pearson @ 2020-12-30 0:18 UTC (permalink / raw)
To: markpearson; +Cc: hdegoede, mgross, rjw, linux-acpi, platform-driver-x86
On modern systems the platform performance, temperature, fan and other
hardware related characteristics are often dynamically configurable. The
profile is often automatically adjusted to the load by some
automatic-mechanism (which may very well live outside the kernel).
These auto platform-adjustment mechanisms often can be configured with
one of several 'platform-profiles', with either a bias towards low-power
consumption or towards performance (and higher power consumption and
thermals).
Introduce a new platform_profile sysfs API which offers a generic API for
selecting the performance-profile of these automatic-mechanisms.
Co-developed-by: Mark Pearson <markpearson@lenovo.com>
Signed-off-by: Mark Pearson <markpearson@lenovo.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- updated to rst format
Changes in v3, v4, v5
- version bump along with rest of patch series
Changes in v6:
- Split sysfs-platform_profile.rs into ABI text and then admin guide in
userspace-api section. Hope this is correct - I'm guessing a bit.
Changes in v7:
- Correct available_choices to platform_profile_choices
- Improve phrasing as recommended by review
Changes in v8:
- Removed unnecessary empty lines at end of file
.../ABI/testing/sysfs-platform_profile | 24 +++++++++++
Documentation/userspace-api/index.rst | 1 +
.../userspace-api/sysfs-platform_profile.rst | 42 +++++++++++++++++++
3 files changed, 67 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-platform_profile
create mode 100644 Documentation/userspace-api/sysfs-platform_profile.rst
diff --git a/Documentation/ABI/testing/sysfs-platform_profile b/Documentation/ABI/testing/sysfs-platform_profile
new file mode 100644
index 000000000000..9d6b89b66cca
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform_profile
@@ -0,0 +1,24 @@
+What: /sys/firmware/acpi/platform_profile_choices
+Date: October 2020
+Contact: Hans de Goede <hdegoede@redhat.com>
+Description: This file contains a space-separated list of profiles supported for this device.
+
+ Drivers must use the following standard profile-names:
+
+ ============ ============================================
+ low-power Low power consumption
+ cool Cooler operation
+ quiet Quieter operation
+ balanced Balance between low power consumption and performance
+ performance High performance operation
+ ============ ============================================
+
+ Userspace may expect drivers to offer more than one of these
+ standard profile names.
+
+What: /sys/firmware/acpi/platform_profile
+Date: October 2020
+Contact: Hans de Goede <hdegoede@redhat.com>
+Description: Reading this file gives the current selected profile for this
+ device. Writing this file with one of the strings from
+ platform_profile_choices changes the profile to the new value.
diff --git a/Documentation/userspace-api/index.rst b/Documentation/userspace-api/index.rst
index acd2cc2a538d..d29b020e5622 100644
--- a/Documentation/userspace-api/index.rst
+++ b/Documentation/userspace-api/index.rst
@@ -24,6 +24,7 @@ place where this information is gathered.
ioctl/index
iommu
media/index
+ sysfs-platform_profile
.. only:: subproject and html
diff --git a/Documentation/userspace-api/sysfs-platform_profile.rst b/Documentation/userspace-api/sysfs-platform_profile.rst
new file mode 100644
index 000000000000..c33a71263d9e
--- /dev/null
+++ b/Documentation/userspace-api/sysfs-platform_profile.rst
@@ -0,0 +1,42 @@
+=====================================================================
+Platform Profile Selection (e.g. /sys/firmware/acpi/platform_profile)
+=====================================================================
+
+On modern systems the platform performance, temperature, fan and other
+hardware related characteristics are often dynamically configurable. The
+platform configuration is often automatically adjusted to the current
+conditions by some automatic mechanism (which may very well live outside
+the kernel).
+
+These auto platform adjustment mechanisms often can be configured with
+one of several platform profiles, with either a bias towards low power
+operation or towards performance.
+
+The purpose of the platform_profile attribute is to offer a generic sysfs
+API for selecting the platform profile of these automatic mechanisms.
+
+Note that this API is only for selecting the platform profile, it is
+NOT a goal of this API to allow monitoring the resulting performance
+characteristics. Monitoring performance is best done with device/vendor
+specific tools such as e.g. turbostat.
+
+Specifically when selecting a high performance profile the actual achieved
+performance may be limited by various factors such as: the heat generated
+by other components, room temperature, free air flow at the bottom of a
+laptop, etc. It is explicitly NOT a goal of this API to let userspace know
+about any sub-optimal conditions which are impeding reaching the requested
+performance level.
+
+Since numbers on their own cannot represent the multiple variables that a
+profile will adjust (power consumption, heat generation, etc) this API
+uses strings to describe the various profiles. To make sure that userspace
+gets a consistent experience the sysfs-platform_profile ABI document defines
+a fixed set of profile names. Drivers *must* map their internal profile
+representation onto this fixed set.
+
+If there is no good match when mapping then a new profile name may be
+added. Drivers which wish to introduce new profile names must:
+
+ 1. Explain why the existing profile names canot be used.
+ 2. Add the new profile name, along with a clear description of the
+ expected behaviour, to the sysfs-platform_profile ABI documentation.
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v8 2/3] ACPI: platform-profile: Add platform profile support
2020-12-30 0:18 [PATCH v8 1/3] Documentation: Add documentation for new platform_profile sysfs attribute Mark Pearson
@ 2020-12-30 0:18 ` Mark Pearson
2020-12-30 17:40 ` Rafael J. Wysocki
2020-12-30 0:18 ` [PATCH v8 3/3] platform/x86: thinkpad_acpi: " Mark Pearson
2020-12-30 17:35 ` [PATCH v8 1/3] Documentation: Add documentation for new platform_profile sysfs attribute Rafael J. Wysocki
2 siblings, 1 reply; 6+ messages in thread
From: Mark Pearson @ 2020-12-30 0:18 UTC (permalink / raw)
To: markpearson; +Cc: hdegoede, mgross, rjw, linux-acpi, platform-driver-x86
This is the initial implementation of the platform-profile feature.
It provides the details discussed and outlined in the
sysfs-platform_profile document.
Many modern systems have the ability to modify the operating profile to
control aspects like fan speed, temperature and power levels. This
module provides a common sysfs interface that platform modules can register
against to control their individual profile options.
Signed-off-by: Mark Pearson <markpearson@lenovo.com>
---
Changes in v2:
Address (hopefully) all recommendations from review including:
- reorder includes list alphabetically
- make globals statics and use const as required
- change profile name scanning to use full string
- clean up profile name lists to remove unwanted additions
- use sysfs_emit and sysfs_emit_at appropriately (much nicer!)
- improve error handling. Return errors to user in all cases and use
better error codes where appropriate (ENOOPSUPP)
- clean up sysfs output for better readability
- formatting fixes where needed
- improve structure and enum names to be clearer
- remove cur_profile field from structure. It is now local to the
actual platform driver file (patch 3 in series)
- improve checking so if future profile options are added profile_names
will be updated as well.
- move CONFIG option next to ACPI_THERMAL as it seemed slightly related
- removed MAINTAINERS update as not appropriate (note warning message
is seen when running checkpatch)
Changes in v3:
- Add missed platform_profile.h file
Changes in v4:
- Clean up duplicate entry in Kconfig file
- Add linux/bits.h to include list
- Remove unnecessary items from include list
- Make cur_profile const
- Clean up comments
- formatting clean-ups
- add checking of profile return value to show function
- add checking to store to see if it's a supported profile
- revert ENOTSUPP change in store function
- improved error checking in profile registration
- improved profile naming (now platform_profile_*)
Changes in v5:
- correct 'balance' to 'balanced' to be consistent with documentation
- add WARN_ON when checking profile index in show function
- switch mutex_lock_interruptible back to mutex_lock where appropriate
- add 'platform_profile_last' as final entry in profile entry. Update
implementation to use this appropriately
- Use BITS_TO_LONG and appropriate access functions for choices field
- Correct error handling as recommended
- Sanity check profile fields on registration
- Remove unnecessary init and exit functions
Changed in v6:
- Change default build option to 'm' and clean in formating in Kconfig
- Change enums to be capitalised as requested
- Rename unregister function to remove
Changes in v7
- version bump along with rest of patch series
Changes in v8:
- correct checking for empty choices bitmap in register function
drivers/acpi/Kconfig | 17 +++
drivers/acpi/Makefile | 1 +
drivers/acpi/platform_profile.c | 181 +++++++++++++++++++++++++++++++
include/linux/platform_profile.h | 39 +++++++
4 files changed, 238 insertions(+)
create mode 100644 drivers/acpi/platform_profile.c
create mode 100644 include/linux/platform_profile.h
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index edf1558c1105..5ddff93e38c2 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -326,6 +326,23 @@ config ACPI_THERMAL
To compile this driver as a module, choose M here:
the module will be called thermal.
+config ACPI_PLATFORM_PROFILE
+ tristate "ACPI Platform Profile Driver"
+ default m
+ help
+ This driver adds support for platform-profiles on platforms that
+ support it.
+
+ Platform-profiles can be used to control the platform behaviour. For
+ example whether to operate in a lower power mode, in a higher
+ power performance mode or between the two.
+
+ This driver provides the sysfs interface and is used as the registration
+ point for platform specific drivers.
+
+ Which profiles are supported is determined on a per-platform basis and
+ should be obtained from the platform specific driver.
+
config ACPI_CUSTOM_DSDT_FILE
string "Custom DSDT Table file to include"
default ""
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 44e412506317..c64a8af106c0 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -78,6 +78,7 @@ obj-$(CONFIG_ACPI_PCI_SLOT) += pci_slot.o
obj-$(CONFIG_ACPI_PROCESSOR) += processor.o
obj-$(CONFIG_ACPI) += container.o
obj-$(CONFIG_ACPI_THERMAL) += thermal.o
+obj-$(CONFIG_ACPI_PLATFORM_PROFILE) += platform_profile.o
obj-$(CONFIG_ACPI_NFIT) += nfit/
obj-$(CONFIG_ACPI_NUMA) += numa/
obj-$(CONFIG_ACPI) += acpi_memhotplug.o
diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
new file mode 100644
index 000000000000..f65aa8cd2185
--- /dev/null
+++ b/drivers/acpi/platform_profile.c
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/* Platform profile sysfs interface */
+
+#include <linux/acpi.h>
+#include <linux/bits.h>
+#include <linux/init.h>
+#include <linux/mutex.h>
+#include <linux/platform_profile.h>
+#include <linux/sysfs.h>
+
+static const struct platform_profile_handler *cur_profile;
+static DEFINE_MUTEX(profile_lock);
+
+static const char * const profile_names[] = {
+ [PLATFORM_PROFILE_LOW] = "low-power",
+ [PLATFORM_PROFILE_COOL] = "cool",
+ [PLATFORM_PROFILE_QUIET] = "quiet",
+ [PLATFORM_PROFILE_BALANCED] = "balanced",
+ [PLATFORM_PROFILE_PERFORM] = "performance",
+};
+static_assert(ARRAY_SIZE(profile_names) == PLATFORM_PROFILE_LAST);
+
+static ssize_t platform_profile_choices_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ int len = 0;
+ int err, i;
+
+ err = mutex_lock_interruptible(&profile_lock);
+ if (err)
+ return err;
+
+ if (!cur_profile) {
+ mutex_unlock(&profile_lock);
+ return -ENODEV;
+ }
+
+ for_each_set_bit(i, cur_profile->choices, PLATFORM_PROFILE_LAST) {
+ if (len == 0)
+ len += sysfs_emit_at(buf, len, "%s", profile_names[i]);
+ else
+ len += sysfs_emit_at(buf, len, " %s", profile_names[i]);
+ }
+ len += sysfs_emit_at(buf, len, "\n");
+ mutex_unlock(&profile_lock);
+ return len;
+}
+
+static ssize_t platform_profile_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ enum platform_profile_option profile = PLATFORM_PROFILE_BALANCED;
+ 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(&profile);
+ mutex_unlock(&profile_lock);
+ if (err)
+ return err;
+
+ /* 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_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int err, i;
+
+ err = mutex_lock_interruptible(&profile_lock);
+ if (err)
+ return err;
+
+ if (!cur_profile) {
+ mutex_unlock(&profile_lock);
+ return -ENODEV;
+ }
+
+ /* Scan for a matching profile */
+ i = sysfs_match_string(profile_names, buf);
+ if (i < 0) {
+ mutex_unlock(&profile_lock);
+ return -EINVAL;
+ }
+
+ /* Check that platform supports this profile choice */
+ if (!test_bit(i, cur_profile->choices)) {
+ mutex_unlock(&profile_lock);
+ return -EOPNOTSUPP;
+ }
+
+ err = cur_profile->profile_set(i);
+ mutex_unlock(&profile_lock);
+ if (err)
+ return err;
+ return count;
+}
+
+static DEVICE_ATTR_RO(platform_profile_choices);
+static DEVICE_ATTR_RW(platform_profile);
+
+static struct attribute *platform_profile_attrs[] = {
+ &dev_attr_platform_profile_choices.attr,
+ &dev_attr_platform_profile.attr,
+ NULL
+};
+
+static const struct attribute_group platform_profile_group = {
+ .attrs = platform_profile_attrs
+};
+
+void platform_profile_notify(void)
+{
+ if (!cur_profile)
+ return;
+ sysfs_notify(acpi_kobj, NULL, "platform_profile");
+}
+EXPORT_SYMBOL_GPL(platform_profile_notify);
+
+int platform_profile_register(const struct platform_profile_handler *pprof)
+{
+ int err;
+
+ mutex_lock(&profile_lock);
+ /* We can only have one active profile */
+ if (cur_profile) {
+ mutex_unlock(&profile_lock);
+ return -EEXIST;
+ }
+
+ /* Sanity check the profile handler field are set */
+ if (!pprof || bitmap_empty(pprof->choices, PLATFORM_PROFILE_LAST) ||
+ !pprof->profile_set || !pprof->profile_get) {
+ mutex_unlock(&profile_lock);
+ return -EINVAL;
+ }
+
+ err = sysfs_create_group(acpi_kobj, &platform_profile_group);
+ if (err) {
+ mutex_unlock(&profile_lock);
+ return err;
+ }
+
+ cur_profile = pprof;
+ mutex_unlock(&profile_lock);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(platform_profile_register);
+
+int platform_profile_remove(void)
+{
+ mutex_lock(&profile_lock);
+ if (!cur_profile) {
+ mutex_unlock(&profile_lock);
+ return -ENODEV;
+ }
+
+ sysfs_remove_group(acpi_kobj, &platform_profile_group);
+ cur_profile = NULL;
+ mutex_unlock(&profile_lock);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(platform_profile_remove);
+
+MODULE_AUTHOR("Mark Pearson <markpearson@lenovo.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
new file mode 100644
index 000000000000..9a1e2abd7602
--- /dev/null
+++ b/include/linux/platform_profile.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Platform profile sysfs interface
+ *
+ * See Documentation/ABI/testing/sysfs-platform_profile.rst for more
+ * information.
+ */
+
+#ifndef _PLATFORM_PROFILE_H_
+#define _PLATFORM_PROFILE_H_
+
+#include <linux/bitops.h>
+
+/*
+ * If more options are added please update profile_names
+ * array in platform-profile.c and sysfs-platform-profile.rst
+ * documentation.
+ */
+
+enum platform_profile_option {
+ PLATFORM_PROFILE_LOW,
+ PLATFORM_PROFILE_COOL,
+ PLATFORM_PROFILE_QUIET,
+ PLATFORM_PROFILE_BALANCED,
+ PLATFORM_PROFILE_PERFORM,
+ PLATFORM_PROFILE_LAST, /*must always be last */
+};
+
+struct platform_profile_handler {
+ unsigned long choices[BITS_TO_LONGS(PLATFORM_PROFILE_LAST)];
+ int (*profile_get)(enum platform_profile_option *profile);
+ int (*profile_set)(enum platform_profile_option profile);
+};
+
+int platform_profile_register(const struct platform_profile_handler *pprof);
+int platform_profile_remove(void);
+void platform_profile_notify(void);
+
+#endif /*_PLATFORM_PROFILE_H_*/
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v8 3/3] platform/x86: thinkpad_acpi: Add platform profile support
2020-12-30 0:18 [PATCH v8 1/3] Documentation: Add documentation for new platform_profile sysfs attribute Mark Pearson
2020-12-30 0:18 ` [PATCH v8 2/3] ACPI: platform-profile: Add platform profile support Mark Pearson
@ 2020-12-30 0:18 ` Mark Pearson
2021-01-05 11:02 ` Hans de Goede
2020-12-30 17:35 ` [PATCH v8 1/3] Documentation: Add documentation for new platform_profile sysfs attribute Rafael J. Wysocki
2 siblings, 1 reply; 6+ messages in thread
From: Mark Pearson @ 2020-12-30 0:18 UTC (permalink / raw)
To: markpearson; +Cc: hdegoede, mgross, rjw, linux-acpi, platform-driver-x86
Add support to thinkpad_acpi for Lenovo platforms that have DYTC
version 5 support or newer to use the platform profile feature.
This will allow users to determine and control the platform modes
between low-power, balanced operation and performance modes.
Signed-off-by: Mark Pearson <markpearson@lenovo.com>
---
Changes in v2:
Address (hopefully) all recommendations from review including:
- use IS_ENABLED instead of IS_DEFINED
- update driver to work with all the fixes in platform_profile update
- improve error handling for invalid inputs
- move tracking of current profile mode into this driver
Changes in v3:
- version update for patch series
Changes in v4:
- Rebase on top of palm sensor patch which led to a little bit of file
restructuring/clean up
- Use BIT macro where applicable
- Formatting fixes
- Check sysfs node created on exit function
- implement and use DYTC_SET_COMMAND macro
- in case of failure setting performance mode make sure CQL mode is
enabled again before returning.
- Clean up initialisation and error handling code
Changes in v5:
- Use atomic_int with ignoring events
- Add mutex when accessing ACPI information
- Clean up registration code
- Use cached value in profile_get function
- Add dytc_cql_command function to clean up and provide common handler
- Update to work with changes in platform_profile implementation
Changes in v6:
- Update file to build against update in v6 platform_profile
Changes in v7 & v8:
- version bump along with rest of patch series
drivers/platform/x86/thinkpad_acpi.c | 292 ++++++++++++++++++++++++++-
1 file changed, 286 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 6a4c54db38fb..e08b3548afd1 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -66,6 +66,7 @@
#include <linux/acpi.h>
#include <linux/pci.h>
#include <linux/power_supply.h>
+#include <linux/platform_profile.h>
#include <sound/core.h>
#include <sound/control.h>
#include <sound/initval.h>
@@ -9843,16 +9844,27 @@ static bool has_lapsensor;
static bool palm_state;
static bool lap_state;
-static int lapsensor_get(bool *present, bool *state)
+static int dytc_command(int command, int *output)
{
acpi_handle dytc_handle;
- int output;
- *present = false;
- if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle)))
+ if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle))) {
+ /* Platform doesn't support DYTC */
return -ENODEV;
- if (!acpi_evalf(dytc_handle, &output, NULL, "dd", DYTC_CMD_GET))
+ }
+ if (!acpi_evalf(dytc_handle, output, NULL, "dd", command))
return -EIO;
+ return 0;
+}
+
+static int lapsensor_get(bool *present, bool *state)
+{
+ int output, err;
+
+ *present = false;
+ err = dytc_command(DYTC_CMD_GET, &output);
+ if (err)
+ return err;
*present = true; /*If we get his far, we have lapmode support*/
*state = output & BIT(DYTC_GET_LAPMODE_BIT) ? true : false;
@@ -9971,6 +9983,262 @@ static struct ibm_struct proxsensor_driver_data = {
.exit = proxsensor_exit,
};
+#if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
+
+/*************************************************************************
+ * DYTC Platform Profile interface
+ */
+
+#define DYTC_CMD_QUERY 0 /* To get DYTC status - enable/revision */
+#define DYTC_CMD_SET 1 /* To enable/disable IC function mode */
+#define DYTC_CMD_RESET 0x1ff /* To reset back to default */
+
+#define DYTC_QUERY_ENABLE_BIT 8 /* Bit 8 - 0 = disabled, 1 = enabled */
+#define DYTC_QUERY_SUBREV_BIT 16 /* Bits 16 - 27 - sub revision */
+#define DYTC_QUERY_REV_BIT 28 /* Bits 28 - 31 - revision */
+
+#define DYTC_GET_FUNCTION_BIT 8 /* Bits 8-11 - function setting */
+#define DYTC_GET_MODE_BIT 12 /* Bits 12-15 - mode setting */
+
+#define DYTC_SET_FUNCTION_BIT 12 /* Bits 12-15 - function setting */
+#define DYTC_SET_MODE_BIT 16 /* Bits 16-19 - mode setting */
+#define DYTC_SET_VALID_BIT 20 /* Bit 20 - 1 = on, 0 = off */
+
+#define DYTC_FUNCTION_STD 0 /* Function = 0, standard mode */
+#define DYTC_FUNCTION_CQL 1 /* Function = 1, lap mode */
+#define DYTC_FUNCTION_MMC 11 /* Function = 11, desk mode */
+
+#define DYTC_MODE_PERFORM 2 /* High power mode aka performance */
+#define DYTC_MODE_QUIET 3 /* Low power mode aka quiet */
+#define DYTC_MODE_BALANCE 0xF /* Default mode aka balanced */
+
+#define DYTC_SET_COMMAND(function, mode, on) \
+ (DYTC_CMD_SET | (function) << DYTC_SET_FUNCTION_BIT | \
+ (mode) << DYTC_SET_MODE_BIT | \
+ (on) << DYTC_SET_VALID_BIT)
+
+#define DYTC_DISABLE_CQL DYTC_SET_COMMAND(DYTC_FUNCTION_CQL, DYTC_MODE_BALANCE, 0)
+
+#define DYTC_ENABLE_CQL DYTC_SET_COMMAND(DYTC_FUNCTION_CQL, DYTC_MODE_BALANCE, 1)
+
+static bool dytc_profile_available;
+static enum platform_profile_option dytc_current_profile;
+static atomic_t dytc_ignore_event = ATOMIC_INIT(0);
+static DEFINE_MUTEX(dytc_mutex);
+
+static int convert_dytc_to_profile(int dytcmode, enum platform_profile_option *profile)
+{
+ switch (dytcmode) {
+ case DYTC_MODE_QUIET:
+ *profile = PLATFORM_PROFILE_LOW;
+ break;
+ case DYTC_MODE_BALANCE:
+ *profile = PLATFORM_PROFILE_BALANCED;
+ break;
+ case DYTC_MODE_PERFORM:
+ *profile = PLATFORM_PROFILE_PERFORM;
+ break;
+ default: /* Unknown mode */
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int convert_profile_to_dytc(enum platform_profile_option profile, int *perfmode)
+{
+ switch (profile) {
+ case PLATFORM_PROFILE_QUIET:
+ *perfmode = DYTC_MODE_QUIET;
+ break;
+ case PLATFORM_PROFILE_BALANCED:
+ *perfmode = DYTC_MODE_BALANCE;
+ break;
+ case PLATFORM_PROFILE_PERFORM:
+ *perfmode = DYTC_MODE_PERFORM;
+ break;
+ default: /* Unknown profile */
+ return -EOPNOTSUPP;
+ }
+ return 0;
+}
+
+/*
+ * dytc_profile_get: Function to register with platform_profile
+ * handler. Returns current platform profile.
+ */
+int dytc_profile_get(enum platform_profile_option *profile)
+{
+ *profile = dytc_current_profile;
+ return 0;
+}
+
+/*
+ * Helper function - check if we are in CQL mode and if we are
+ * - disable CQL,
+ * - run the command
+ * - enable CQL
+ * If not in CQL mode, just run the command
+ */
+int dytc_cql_command(int command, int *output)
+{
+ int err, cmd_err, dummy;
+ int cur_funcmode;
+
+ /* Determine if we are in CQL mode. This alters the commands we do */
+ err = dytc_command(DYTC_CMD_GET, output);
+ if (err)
+ return err;
+
+ cur_funcmode = (*output >> DYTC_GET_FUNCTION_BIT) & 0xF;
+ /* Check if we're OK to return immediately */
+ if ((command == DYTC_CMD_GET) && (cur_funcmode != DYTC_FUNCTION_CQL))
+ return 0;
+
+ if (cur_funcmode == DYTC_FUNCTION_CQL) {
+ atomic_inc(&dytc_ignore_event);
+ err = dytc_command(DYTC_DISABLE_CQL, &dummy);
+ if (err)
+ return err;
+ }
+
+ cmd_err = dytc_command(command, output);
+ /* Check return condition after we've restored CQL state */
+
+ if (cur_funcmode == DYTC_FUNCTION_CQL) {
+ err = dytc_command(DYTC_ENABLE_CQL, &dummy);
+ if (err)
+ return err;
+ }
+
+ return cmd_err;
+}
+
+/*
+ * dytc_profile_set: Function to register with platform_profile
+ * handler. Sets current platform profile.
+ */
+int dytc_profile_set(enum platform_profile_option profile)
+{
+ int output;
+ int err;
+
+ if (!dytc_profile_available)
+ return -ENODEV;
+
+ err = mutex_lock_interruptible(&dytc_mutex);
+ if (err)
+ return err;
+
+ if (profile == PLATFORM_PROFILE_BALANCED) {
+ /* To get back to balanced mode we just issue a reset command */
+ err = dytc_command(DYTC_CMD_RESET, &output);
+ if (err)
+ goto unlock;
+ } else {
+ int perfmode;
+
+ err = convert_profile_to_dytc(profile, &perfmode);
+ if (err)
+ goto unlock;
+
+ /* Determine if we are in CQL mode. This alters the commands we do */
+ err = dytc_cql_command(DYTC_SET_COMMAND(DYTC_FUNCTION_MMC, perfmode, 1), &output);
+ if (err)
+ goto unlock;
+ }
+ /* Success - update current profile */
+ dytc_current_profile = profile;
+unlock:
+ mutex_unlock(&dytc_mutex);
+ return err;
+}
+
+static void dytc_profile_refresh(void)
+{
+ enum platform_profile_option profile;
+ int output, err;
+ int perfmode;
+
+ mutex_lock(&dytc_mutex);
+ err = dytc_cql_command(DYTC_CMD_GET, &output);
+ mutex_unlock(&dytc_mutex);
+ if (err)
+ return;
+
+ perfmode = (output >> DYTC_GET_MODE_BIT) & 0xF;
+ convert_dytc_to_profile(perfmode, &profile);
+ if (profile != dytc_current_profile) {
+ dytc_current_profile = profile;
+ platform_profile_notify();
+ }
+}
+
+static struct platform_profile_handler dytc_profile = {
+ .profile_get = dytc_profile_get,
+ .profile_set = dytc_profile_set,
+};
+
+static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
+{
+ int err, output;
+
+ /* Setup supported modes */
+ set_bit(PLATFORM_PROFILE_LOW, dytc_profile.choices);
+ set_bit(PLATFORM_PROFILE_BALANCED, dytc_profile.choices);
+ set_bit(PLATFORM_PROFILE_PERFORM, dytc_profile.choices);
+
+ dytc_profile_available = false;
+ err = dytc_command(DYTC_CMD_QUERY, &output);
+ /*
+ * If support isn't available (ENODEV) then don't return an error
+ * and don't create the sysfs group
+ */
+ if (err == -ENODEV)
+ return 0;
+ /* For all other errors we can flag the failure */
+ if (err)
+ return err;
+
+ /* Check DYTC is enabled and supports mode setting */
+ if (output & BIT(DYTC_QUERY_ENABLE_BIT)) {
+ /* Only DYTC v5.0 and later has this feature. */
+ int dytc_version;
+
+ dytc_version = (output >> DYTC_QUERY_REV_BIT) & 0xF;
+ if (dytc_version >= 5) {
+ dbg_printk(TPACPI_DBG_INIT,
+ "DYTC version %d: thermal mode available\n", dytc_version);
+ /* Create platform_profile structure and register */
+ err = platform_profile_register(&dytc_profile);
+ /*
+ * If for some reason platform_profiles aren't enabled
+ * don't quit terminally.
+ */
+ if (err)
+ return 0;
+
+ dytc_profile_available = true;
+ /* Ensure initial values are correct */
+ dytc_profile_refresh();
+ }
+ }
+ return 0;
+}
+
+static void dytc_profile_exit(void)
+{
+ if (dytc_profile_available) {
+ dytc_profile_available = false;
+ platform_profile_remove();
+ }
+}
+
+static struct ibm_struct dytc_profile_driver_data = {
+ .name = "dytc-profile",
+ .exit = dytc_profile_exit,
+};
+#endif /* CONFIG_ACPI_PLATFORM_PROFILE */
+
/****************************************************************************
****************************************************************************
*
@@ -10019,8 +10287,14 @@ static void tpacpi_driver_event(const unsigned int hkey_event)
mutex_unlock(&kbdlight_mutex);
}
- if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED)
+ if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED) {
lapsensor_refresh();
+#if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
+ /* If we are already accessing DYTC then skip dytc update */
+ if (!atomic_add_unless(&dytc_ignore_event, -1, 0))
+ dytc_profile_refresh();
+#endif
+ }
}
static void hotkey_driver_event(const unsigned int scancode)
@@ -10463,6 +10737,12 @@ static struct ibm_init_struct ibms_init[] __initdata = {
.init = tpacpi_proxsensor_init,
.data = &proxsensor_driver_data,
},
+#if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
+ {
+ .init = tpacpi_dytc_profile_init,
+ .data = &dytc_profile_driver_data,
+ },
+#endif
};
static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v8 1/3] Documentation: Add documentation for new platform_profile sysfs attribute
2020-12-30 0:18 [PATCH v8 1/3] Documentation: Add documentation for new platform_profile sysfs attribute Mark Pearson
2020-12-30 0:18 ` [PATCH v8 2/3] ACPI: platform-profile: Add platform profile support Mark Pearson
2020-12-30 0:18 ` [PATCH v8 3/3] platform/x86: thinkpad_acpi: " Mark Pearson
@ 2020-12-30 17:35 ` Rafael J. Wysocki
2 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2020-12-30 17:35 UTC (permalink / raw)
To: Mark Pearson
Cc: Hans de Goede, Mark Gross, Rafael J. Wysocki,
ACPI Devel Maling List, Platform Driver
On Wed, Dec 30, 2020 at 1:24 AM Mark Pearson <markpearson@lenovo.com> wrote:
>
> On modern systems the platform performance, temperature, fan and other
> hardware related characteristics are often dynamically configurable. The
> profile is often automatically adjusted to the load by some
> automatic-mechanism (which may very well live outside the kernel).
>
> These auto platform-adjustment mechanisms often can be configured with
> one of several 'platform-profiles', with either a bias towards low-power
> consumption or towards performance (and higher power consumption and
> thermals).
>
> Introduce a new platform_profile sysfs API which offers a generic API for
> selecting the performance-profile of these automatic-mechanisms.
>
> Co-developed-by: Mark Pearson <markpearson@lenovo.com>
> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Applied, thanks!
> ---
> Changes in v2:
> - updated to rst format
> Changes in v3, v4, v5
> - version bump along with rest of patch series
> Changes in v6:
> - Split sysfs-platform_profile.rs into ABI text and then admin guide in
> userspace-api section. Hope this is correct - I'm guessing a bit.
> Changes in v7:
> - Correct available_choices to platform_profile_choices
> - Improve phrasing as recommended by review
> Changes in v8:
> - Removed unnecessary empty lines at end of file
>
> .../ABI/testing/sysfs-platform_profile | 24 +++++++++++
> Documentation/userspace-api/index.rst | 1 +
> .../userspace-api/sysfs-platform_profile.rst | 42 +++++++++++++++++++
> 3 files changed, 67 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-platform_profile
> create mode 100644 Documentation/userspace-api/sysfs-platform_profile.rst
>
> diff --git a/Documentation/ABI/testing/sysfs-platform_profile b/Documentation/ABI/testing/sysfs-platform_profile
> new file mode 100644
> index 000000000000..9d6b89b66cca
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform_profile
> @@ -0,0 +1,24 @@
> +What: /sys/firmware/acpi/platform_profile_choices
> +Date: October 2020
> +Contact: Hans de Goede <hdegoede@redhat.com>
> +Description: This file contains a space-separated list of profiles supported for this device.
> +
> + Drivers must use the following standard profile-names:
> +
> + ============ ============================================
> + low-power Low power consumption
> + cool Cooler operation
> + quiet Quieter operation
> + balanced Balance between low power consumption and performance
> + performance High performance operation
> + ============ ============================================
> +
> + Userspace may expect drivers to offer more than one of these
> + standard profile names.
> +
> +What: /sys/firmware/acpi/platform_profile
> +Date: October 2020
> +Contact: Hans de Goede <hdegoede@redhat.com>
> +Description: Reading this file gives the current selected profile for this
> + device. Writing this file with one of the strings from
> + platform_profile_choices changes the profile to the new value.
> diff --git a/Documentation/userspace-api/index.rst b/Documentation/userspace-api/index.rst
> index acd2cc2a538d..d29b020e5622 100644
> --- a/Documentation/userspace-api/index.rst
> +++ b/Documentation/userspace-api/index.rst
> @@ -24,6 +24,7 @@ place where this information is gathered.
> ioctl/index
> iommu
> media/index
> + sysfs-platform_profile
>
> .. only:: subproject and html
>
> diff --git a/Documentation/userspace-api/sysfs-platform_profile.rst b/Documentation/userspace-api/sysfs-platform_profile.rst
> new file mode 100644
> index 000000000000..c33a71263d9e
> --- /dev/null
> +++ b/Documentation/userspace-api/sysfs-platform_profile.rst
> @@ -0,0 +1,42 @@
> +=====================================================================
> +Platform Profile Selection (e.g. /sys/firmware/acpi/platform_profile)
> +=====================================================================
> +
> +On modern systems the platform performance, temperature, fan and other
> +hardware related characteristics are often dynamically configurable. The
> +platform configuration is often automatically adjusted to the current
> +conditions by some automatic mechanism (which may very well live outside
> +the kernel).
> +
> +These auto platform adjustment mechanisms often can be configured with
> +one of several platform profiles, with either a bias towards low power
> +operation or towards performance.
> +
> +The purpose of the platform_profile attribute is to offer a generic sysfs
> +API for selecting the platform profile of these automatic mechanisms.
> +
> +Note that this API is only for selecting the platform profile, it is
> +NOT a goal of this API to allow monitoring the resulting performance
> +characteristics. Monitoring performance is best done with device/vendor
> +specific tools such as e.g. turbostat.
> +
> +Specifically when selecting a high performance profile the actual achieved
> +performance may be limited by various factors such as: the heat generated
> +by other components, room temperature, free air flow at the bottom of a
> +laptop, etc. It is explicitly NOT a goal of this API to let userspace know
> +about any sub-optimal conditions which are impeding reaching the requested
> +performance level.
> +
> +Since numbers on their own cannot represent the multiple variables that a
> +profile will adjust (power consumption, heat generation, etc) this API
> +uses strings to describe the various profiles. To make sure that userspace
> +gets a consistent experience the sysfs-platform_profile ABI document defines
> +a fixed set of profile names. Drivers *must* map their internal profile
> +representation onto this fixed set.
> +
> +If there is no good match when mapping then a new profile name may be
> +added. Drivers which wish to introduce new profile names must:
> +
> + 1. Explain why the existing profile names canot be used.
> + 2. Add the new profile name, along with a clear description of the
> + expected behaviour, to the sysfs-platform_profile ABI documentation.
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v8 2/3] ACPI: platform-profile: Add platform profile support
2020-12-30 0:18 ` [PATCH v8 2/3] ACPI: platform-profile: Add platform profile support Mark Pearson
@ 2020-12-30 17:40 ` Rafael J. Wysocki
0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2020-12-30 17:40 UTC (permalink / raw)
To: Mark Pearson
Cc: Hans de Goede, Mark Gross, Rafael J. Wysocki,
ACPI Devel Maling List, Platform Driver
On Wed, Dec 30, 2020 at 1:24 AM Mark Pearson <markpearson@lenovo.com> wrote:
>
> This is the initial implementation of the platform-profile feature.
> It provides the details discussed and outlined in the
> sysfs-platform_profile document.
>
> Many modern systems have the ability to modify the operating profile to
> control aspects like fan speed, temperature and power levels. This
> module provides a common sysfs interface that platform modules can register
> against to control their individual profile options.
>
> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
> ---
> Changes in v2:
> Address (hopefully) all recommendations from review including:
> - reorder includes list alphabetically
> - make globals statics and use const as required
> - change profile name scanning to use full string
> - clean up profile name lists to remove unwanted additions
> - use sysfs_emit and sysfs_emit_at appropriately (much nicer!)
> - improve error handling. Return errors to user in all cases and use
> better error codes where appropriate (ENOOPSUPP)
> - clean up sysfs output for better readability
> - formatting fixes where needed
> - improve structure and enum names to be clearer
> - remove cur_profile field from structure. It is now local to the
> actual platform driver file (patch 3 in series)
> - improve checking so if future profile options are added profile_names
> will be updated as well.
> - move CONFIG option next to ACPI_THERMAL as it seemed slightly related
> - removed MAINTAINERS update as not appropriate (note warning message
> is seen when running checkpatch)
>
> Changes in v3:
> - Add missed platform_profile.h file
>
> Changes in v4:
> - Clean up duplicate entry in Kconfig file
> - Add linux/bits.h to include list
> - Remove unnecessary items from include list
> - Make cur_profile const
> - Clean up comments
> - formatting clean-ups
> - add checking of profile return value to show function
> - add checking to store to see if it's a supported profile
> - revert ENOTSUPP change in store function
> - improved error checking in profile registration
> - improved profile naming (now platform_profile_*)
>
> Changes in v5:
> - correct 'balance' to 'balanced' to be consistent with documentation
> - add WARN_ON when checking profile index in show function
> - switch mutex_lock_interruptible back to mutex_lock where appropriate
> - add 'platform_profile_last' as final entry in profile entry. Update
> implementation to use this appropriately
> - Use BITS_TO_LONG and appropriate access functions for choices field
> - Correct error handling as recommended
> - Sanity check profile fields on registration
> - Remove unnecessary init and exit functions
>
> Changed in v6:
> - Change default build option to 'm' and clean in formating in Kconfig
> - Change enums to be capitalised as requested
> - Rename unregister function to remove
>
> Changes in v7
> - version bump along with rest of patch series
>
> Changes in v8:
> - correct checking for empty choices bitmap in register function
Applied with the Hans' R-by and I replaced PLATFORM_PROFILE_LOW and
PLATFORM_PROFILE_PERFORM with PLATFORM_PROFILE_LOW_POWER and
PLATFORM_PROFILE_PERFORMANCE, respectively.
Thanks!
> drivers/acpi/Kconfig | 17 +++
> drivers/acpi/Makefile | 1 +
> drivers/acpi/platform_profile.c | 181 +++++++++++++++++++++++++++++++
> include/linux/platform_profile.h | 39 +++++++
> 4 files changed, 238 insertions(+)
> create mode 100644 drivers/acpi/platform_profile.c
> create mode 100644 include/linux/platform_profile.h
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index edf1558c1105..5ddff93e38c2 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -326,6 +326,23 @@ config ACPI_THERMAL
> To compile this driver as a module, choose M here:
> the module will be called thermal.
>
> +config ACPI_PLATFORM_PROFILE
> + tristate "ACPI Platform Profile Driver"
> + default m
> + help
> + This driver adds support for platform-profiles on platforms that
> + support it.
> +
> + Platform-profiles can be used to control the platform behaviour. For
> + example whether to operate in a lower power mode, in a higher
> + power performance mode or between the two.
> +
> + This driver provides the sysfs interface and is used as the registration
> + point for platform specific drivers.
> +
> + Which profiles are supported is determined on a per-platform basis and
> + should be obtained from the platform specific driver.
> +
> config ACPI_CUSTOM_DSDT_FILE
> string "Custom DSDT Table file to include"
> default ""
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 44e412506317..c64a8af106c0 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -78,6 +78,7 @@ obj-$(CONFIG_ACPI_PCI_SLOT) += pci_slot.o
> obj-$(CONFIG_ACPI_PROCESSOR) += processor.o
> obj-$(CONFIG_ACPI) += container.o
> obj-$(CONFIG_ACPI_THERMAL) += thermal.o
> +obj-$(CONFIG_ACPI_PLATFORM_PROFILE) += platform_profile.o
> obj-$(CONFIG_ACPI_NFIT) += nfit/
> obj-$(CONFIG_ACPI_NUMA) += numa/
> obj-$(CONFIG_ACPI) += acpi_memhotplug.o
> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> new file mode 100644
> index 000000000000..f65aa8cd2185
> --- /dev/null
> +++ b/drivers/acpi/platform_profile.c
> @@ -0,0 +1,181 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/* Platform profile sysfs interface */
> +
> +#include <linux/acpi.h>
> +#include <linux/bits.h>
> +#include <linux/init.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_profile.h>
> +#include <linux/sysfs.h>
> +
> +static const struct platform_profile_handler *cur_profile;
> +static DEFINE_MUTEX(profile_lock);
> +
> +static const char * const profile_names[] = {
> + [PLATFORM_PROFILE_LOW] = "low-power",
> + [PLATFORM_PROFILE_COOL] = "cool",
> + [PLATFORM_PROFILE_QUIET] = "quiet",
> + [PLATFORM_PROFILE_BALANCED] = "balanced",
> + [PLATFORM_PROFILE_PERFORM] = "performance",
> +};
> +static_assert(ARRAY_SIZE(profile_names) == PLATFORM_PROFILE_LAST);
> +
> +static ssize_t platform_profile_choices_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + int len = 0;
> + int err, i;
> +
> + err = mutex_lock_interruptible(&profile_lock);
> + if (err)
> + return err;
> +
> + if (!cur_profile) {
> + mutex_unlock(&profile_lock);
> + return -ENODEV;
> + }
> +
> + for_each_set_bit(i, cur_profile->choices, PLATFORM_PROFILE_LAST) {
> + if (len == 0)
> + len += sysfs_emit_at(buf, len, "%s", profile_names[i]);
> + else
> + len += sysfs_emit_at(buf, len, " %s", profile_names[i]);
> + }
> + len += sysfs_emit_at(buf, len, "\n");
> + mutex_unlock(&profile_lock);
> + return len;
> +}
> +
> +static ssize_t platform_profile_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + enum platform_profile_option profile = PLATFORM_PROFILE_BALANCED;
> + 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(&profile);
> + mutex_unlock(&profile_lock);
> + if (err)
> + return err;
> +
> + /* 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_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int err, i;
> +
> + err = mutex_lock_interruptible(&profile_lock);
> + if (err)
> + return err;
> +
> + if (!cur_profile) {
> + mutex_unlock(&profile_lock);
> + return -ENODEV;
> + }
> +
> + /* Scan for a matching profile */
> + i = sysfs_match_string(profile_names, buf);
> + if (i < 0) {
> + mutex_unlock(&profile_lock);
> + return -EINVAL;
> + }
> +
> + /* Check that platform supports this profile choice */
> + if (!test_bit(i, cur_profile->choices)) {
> + mutex_unlock(&profile_lock);
> + return -EOPNOTSUPP;
> + }
> +
> + err = cur_profile->profile_set(i);
> + mutex_unlock(&profile_lock);
> + if (err)
> + return err;
> + return count;
> +}
> +
> +static DEVICE_ATTR_RO(platform_profile_choices);
> +static DEVICE_ATTR_RW(platform_profile);
> +
> +static struct attribute *platform_profile_attrs[] = {
> + &dev_attr_platform_profile_choices.attr,
> + &dev_attr_platform_profile.attr,
> + NULL
> +};
> +
> +static const struct attribute_group platform_profile_group = {
> + .attrs = platform_profile_attrs
> +};
> +
> +void platform_profile_notify(void)
> +{
> + if (!cur_profile)
> + return;
> + sysfs_notify(acpi_kobj, NULL, "platform_profile");
> +}
> +EXPORT_SYMBOL_GPL(platform_profile_notify);
> +
> +int platform_profile_register(const struct platform_profile_handler *pprof)
> +{
> + int err;
> +
> + mutex_lock(&profile_lock);
> + /* We can only have one active profile */
> + if (cur_profile) {
> + mutex_unlock(&profile_lock);
> + return -EEXIST;
> + }
> +
> + /* Sanity check the profile handler field are set */
> + if (!pprof || bitmap_empty(pprof->choices, PLATFORM_PROFILE_LAST) ||
> + !pprof->profile_set || !pprof->profile_get) {
> + mutex_unlock(&profile_lock);
> + return -EINVAL;
> + }
> +
> + err = sysfs_create_group(acpi_kobj, &platform_profile_group);
> + if (err) {
> + mutex_unlock(&profile_lock);
> + return err;
> + }
> +
> + cur_profile = pprof;
> + mutex_unlock(&profile_lock);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(platform_profile_register);
> +
> +int platform_profile_remove(void)
> +{
> + mutex_lock(&profile_lock);
> + if (!cur_profile) {
> + mutex_unlock(&profile_lock);
> + return -ENODEV;
> + }
> +
> + sysfs_remove_group(acpi_kobj, &platform_profile_group);
> + cur_profile = NULL;
> + mutex_unlock(&profile_lock);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(platform_profile_remove);
> +
> +MODULE_AUTHOR("Mark Pearson <markpearson@lenovo.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
> new file mode 100644
> index 000000000000..9a1e2abd7602
> --- /dev/null
> +++ b/include/linux/platform_profile.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Platform profile sysfs interface
> + *
> + * See Documentation/ABI/testing/sysfs-platform_profile.rst for more
> + * information.
> + */
> +
> +#ifndef _PLATFORM_PROFILE_H_
> +#define _PLATFORM_PROFILE_H_
> +
> +#include <linux/bitops.h>
> +
> +/*
> + * If more options are added please update profile_names
> + * array in platform-profile.c and sysfs-platform-profile.rst
> + * documentation.
> + */
> +
> +enum platform_profile_option {
> + PLATFORM_PROFILE_LOW,
> + PLATFORM_PROFILE_COOL,
> + PLATFORM_PROFILE_QUIET,
> + PLATFORM_PROFILE_BALANCED,
> + PLATFORM_PROFILE_PERFORM,
> + PLATFORM_PROFILE_LAST, /*must always be last */
> +};
> +
> +struct platform_profile_handler {
> + unsigned long choices[BITS_TO_LONGS(PLATFORM_PROFILE_LAST)];
> + int (*profile_get)(enum platform_profile_option *profile);
> + int (*profile_set)(enum platform_profile_option profile);
> +};
> +
> +int platform_profile_register(const struct platform_profile_handler *pprof);
> +int platform_profile_remove(void);
> +void platform_profile_notify(void);
> +
> +#endif /*_PLATFORM_PROFILE_H_*/
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v8 3/3] platform/x86: thinkpad_acpi: Add platform profile support
2020-12-30 0:18 ` [PATCH v8 3/3] platform/x86: thinkpad_acpi: " Mark Pearson
@ 2021-01-05 11:02 ` Hans de Goede
0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2021-01-05 11:02 UTC (permalink / raw)
To: Mark Pearson; +Cc: mgross, rjw, linux-acpi, platform-driver-x86
Hi,
Some review remarks inline, mostly things I noticed while running
Rafael's bleeding-edge branch which has 1/3 and 2/3 of this series
combined with this patch.
On 12/30/20 1:18 AM, Mark Pearson wrote:
> Add support to thinkpad_acpi for Lenovo platforms that have DYTC
> version 5 support or newer to use the platform profile feature.
>
> This will allow users to determine and control the platform modes
> between low-power, balanced operation and performance modes.
>
> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
> ---
> Changes in v2:
> Address (hopefully) all recommendations from review including:
> - use IS_ENABLED instead of IS_DEFINED
> - update driver to work with all the fixes in platform_profile update
> - improve error handling for invalid inputs
> - move tracking of current profile mode into this driver
>
> Changes in v3:
> - version update for patch series
>
> Changes in v4:
> - Rebase on top of palm sensor patch which led to a little bit of file
> restructuring/clean up
> - Use BIT macro where applicable
> - Formatting fixes
> - Check sysfs node created on exit function
> - implement and use DYTC_SET_COMMAND macro
> - in case of failure setting performance mode make sure CQL mode is
> enabled again before returning.
> - Clean up initialisation and error handling code
>
> Changes in v5:
> - Use atomic_int with ignoring events
> - Add mutex when accessing ACPI information
> - Clean up registration code
> - Use cached value in profile_get function
> - Add dytc_cql_command function to clean up and provide common handler
> - Update to work with changes in platform_profile implementation
>
> Changes in v6:
> - Update file to build against update in v6 platform_profile
>
> Changes in v7 & v8:
> - version bump along with rest of patch series
>
> drivers/platform/x86/thinkpad_acpi.c | 292 ++++++++++++++++++++++++++-
> 1 file changed, 286 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 6a4c54db38fb..e08b3548afd1 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -66,6 +66,7 @@
> #include <linux/acpi.h>
> #include <linux/pci.h>
> #include <linux/power_supply.h>
> +#include <linux/platform_profile.h>
> #include <sound/core.h>
> #include <sound/control.h>
> #include <sound/initval.h>
> @@ -9843,16 +9844,27 @@ static bool has_lapsensor;
> static bool palm_state;
> static bool lap_state;
>
> -static int lapsensor_get(bool *present, bool *state)
> +static int dytc_command(int command, int *output)
> {
> acpi_handle dytc_handle;
> - int output;
>
> - *present = false;
> - if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle)))
> + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle))) {
> + /* Platform doesn't support DYTC */
> return -ENODEV;
> - if (!acpi_evalf(dytc_handle, &output, NULL, "dd", DYTC_CMD_GET))
> + }
> + if (!acpi_evalf(dytc_handle, output, NULL, "dd", command))
> return -EIO;
> + return 0;
> +}
> +
> +static int lapsensor_get(bool *present, bool *state)
> +{
> + int output, err;
> +
> + *present = false;
> + err = dytc_command(DYTC_CMD_GET, &output);
> + if (err)
> + return err;
>
> *present = true; /*If we get his far, we have lapmode support*/
> *state = output & BIT(DYTC_GET_LAPMODE_BIT) ? true : false;
> @@ -9971,6 +9983,262 @@ static struct ibm_struct proxsensor_driver_data = {
> .exit = proxsensor_exit,
> };
>
> +#if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
> +
> +/*************************************************************************
> + * DYTC Platform Profile interface
> + */
> +
> +#define DYTC_CMD_QUERY 0 /* To get DYTC status - enable/revision */
> +#define DYTC_CMD_SET 1 /* To enable/disable IC function mode */
> +#define DYTC_CMD_RESET 0x1ff /* To reset back to default */
> +
> +#define DYTC_QUERY_ENABLE_BIT 8 /* Bit 8 - 0 = disabled, 1 = enabled */
> +#define DYTC_QUERY_SUBREV_BIT 16 /* Bits 16 - 27 - sub revision */
> +#define DYTC_QUERY_REV_BIT 28 /* Bits 28 - 31 - revision */
> +
> +#define DYTC_GET_FUNCTION_BIT 8 /* Bits 8-11 - function setting */
> +#define DYTC_GET_MODE_BIT 12 /* Bits 12-15 - mode setting */
> +
> +#define DYTC_SET_FUNCTION_BIT 12 /* Bits 12-15 - function setting */
> +#define DYTC_SET_MODE_BIT 16 /* Bits 16-19 - mode setting */
> +#define DYTC_SET_VALID_BIT 20 /* Bit 20 - 1 = on, 0 = off */
> +
> +#define DYTC_FUNCTION_STD 0 /* Function = 0, standard mode */
> +#define DYTC_FUNCTION_CQL 1 /* Function = 1, lap mode */
> +#define DYTC_FUNCTION_MMC 11 /* Function = 11, desk mode */
> +
> +#define DYTC_MODE_PERFORM 2 /* High power mode aka performance */
> +#define DYTC_MODE_QUIET 3 /* Low power mode aka quiet */
> +#define DYTC_MODE_BALANCE 0xF /* Default mode aka balanced */
> +
> +#define DYTC_SET_COMMAND(function, mode, on) \
> + (DYTC_CMD_SET | (function) << DYTC_SET_FUNCTION_BIT | \
> + (mode) << DYTC_SET_MODE_BIT | \
> + (on) << DYTC_SET_VALID_BIT)
> +
> +#define DYTC_DISABLE_CQL DYTC_SET_COMMAND(DYTC_FUNCTION_CQL, DYTC_MODE_BALANCE, 0)
> +
> +#define DYTC_ENABLE_CQL DYTC_SET_COMMAND(DYTC_FUNCTION_CQL, DYTC_MODE_BALANCE, 1)
> +
> +static bool dytc_profile_available;
> +static enum platform_profile_option dytc_current_profile;
> +static atomic_t dytc_ignore_event = ATOMIC_INIT(0);
> +static DEFINE_MUTEX(dytc_mutex);
> +
> +static int convert_dytc_to_profile(int dytcmode, enum platform_profile_option *profile)
> +{
> + switch (dytcmode) {
> + case DYTC_MODE_QUIET:
> + *profile = PLATFORM_PROFILE_LOW;
This needs to be PLATFORM_PROFILE_LOW_POWER now, due to changes Rafael made while
merging 2/3 (more instances of this below).
> + break;
> + case DYTC_MODE_BALANCE:
> + *profile = PLATFORM_PROFILE_BALANCED;
> + break;
> + case DYTC_MODE_PERFORM:
> + *profile = PLATFORM_PROFILE_PERFORM;
This needs to be PLATFORM_PROFILE_PERFORMANCE now, due to changes Rafael made while
merging 2/3 (more instances of this below).
> + break;
> + default: /* Unknown mode */
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int convert_profile_to_dytc(enum platform_profile_option profile, int *perfmode)
> +{
> + switch (profile) {
> + case PLATFORM_PROFILE_QUIET:
QUIET ? Above you translate DYTC_MODE_QUIET to PLATFORM_PROFILE_LOW_POWER and when
setting the choices bits you also use PLATFORM_PROFILE_LOW_POWER, so you should use
PLATFORM_PROFILE_LOW_POWER here too. Or replace all of them with PLATFORM_PROFILE_QUIET,
which might be better if the internal Lenovo docs describe this setting as quiet.
> + *perfmode = DYTC_MODE_QUIET;
> + break;
> + case PLATFORM_PROFILE_BALANCED:
> + *perfmode = DYTC_MODE_BALANCE;
> + break;
> + case PLATFORM_PROFILE_PERFORM:
> + *perfmode = DYTC_MODE_PERFORM;
PERFORMANCE (as above).
> + break;
> + default: /* Unknown profile */
> + return -EOPNOTSUPP;
> + }
> + return 0;
> +}
> +
> +/*
> + * dytc_profile_get: Function to register with platform_profile
> + * handler. Returns current platform profile.
> + */
> +int dytc_profile_get(enum platform_profile_option *profile)
> +{
> + *profile = dytc_current_profile;
> + return 0;
> +}
> +
> +/*
> + * Helper function - check if we are in CQL mode and if we are
> + * - disable CQL,
> + * - run the command
> + * - enable CQL
> + * If not in CQL mode, just run the command
> + */
> +int dytc_cql_command(int command, int *output)
> +{
> + int err, cmd_err, dummy;
> + int cur_funcmode;
> +
> + /* Determine if we are in CQL mode. This alters the commands we do */
> + err = dytc_command(DYTC_CMD_GET, output);
> + if (err)
> + return err;
> +
> + cur_funcmode = (*output >> DYTC_GET_FUNCTION_BIT) & 0xF;
> + /* Check if we're OK to return immediately */
> + if ((command == DYTC_CMD_GET) && (cur_funcmode != DYTC_FUNCTION_CQL))
> + return 0;
> +
> + if (cur_funcmode == DYTC_FUNCTION_CQL) {
> + atomic_inc(&dytc_ignore_event);
> + err = dytc_command(DYTC_DISABLE_CQL, &dummy);
> + if (err)
> + return err;
> + }
> +
> + cmd_err = dytc_command(command, output);
> + /* Check return condition after we've restored CQL state */
> +
> + if (cur_funcmode == DYTC_FUNCTION_CQL) {
> + err = dytc_command(DYTC_ENABLE_CQL, &dummy);
> + if (err)
> + return err;
> + }
> +
> + return cmd_err;
> +}
> +
> +/*
> + * dytc_profile_set: Function to register with platform_profile
> + * handler. Sets current platform profile.
> + */
> +int dytc_profile_set(enum platform_profile_option profile)
> +{
> + int output;
> + int err;
> +
> + if (!dytc_profile_available)
> + return -ENODEV;
> +
> + err = mutex_lock_interruptible(&dytc_mutex);
> + if (err)
> + return err;
> +
> + if (profile == PLATFORM_PROFILE_BALANCED) {
> + /* To get back to balanced mode we just issue a reset command */
> + err = dytc_command(DYTC_CMD_RESET, &output);
> + if (err)
> + goto unlock;
> + } else {
> + int perfmode;
> +
> + err = convert_profile_to_dytc(profile, &perfmode);
> + if (err)
> + goto unlock;
> +
> + /* Determine if we are in CQL mode. This alters the commands we do */
> + err = dytc_cql_command(DYTC_SET_COMMAND(DYTC_FUNCTION_MMC, perfmode, 1), &output);
> + if (err)
> + goto unlock;
> + }
> + /* Success - update current profile */
> + dytc_current_profile = profile;
> +unlock:
> + mutex_unlock(&dytc_mutex);
> + return err;
> +}
> +
> +static void dytc_profile_refresh(void)
> +{
> + enum platform_profile_option profile;
> + int output, err;
> + int perfmode;
> +
> + mutex_lock(&dytc_mutex);
> + err = dytc_cql_command(DYTC_CMD_GET, &output);
> + mutex_unlock(&dytc_mutex);
> + if (err)
> + return;
> +
> + perfmode = (output >> DYTC_GET_MODE_BIT) & 0xF;
> + convert_dytc_to_profile(perfmode, &profile);
> + if (profile != dytc_current_profile) {
> + dytc_current_profile = profile;
> + platform_profile_notify();
> + }
> +}
> +
> +static struct platform_profile_handler dytc_profile = {
> + .profile_get = dytc_profile_get,
> + .profile_set = dytc_profile_set,
> +};
> +
> +static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
> +{
> + int err, output;
> +
> + /* Setup supported modes */
> + set_bit(PLATFORM_PROFILE_LOW, dytc_profile.choices);
> + set_bit(PLATFORM_PROFILE_BALANCED, dytc_profile.choices);
> + set_bit(PLATFORM_PROFILE_PERFORM, dytc_profile.choices);
Same changes necessary as discussed above.
Regards,
Hans
> +
> + dytc_profile_available = false;
> + err = dytc_command(DYTC_CMD_QUERY, &output);
> + /*
> + * If support isn't available (ENODEV) then don't return an error
> + * and don't create the sysfs group
> + */
> + if (err == -ENODEV)
> + return 0;
> + /* For all other errors we can flag the failure */
> + if (err)
> + return err;
> +
> + /* Check DYTC is enabled and supports mode setting */
> + if (output & BIT(DYTC_QUERY_ENABLE_BIT)) {
> + /* Only DYTC v5.0 and later has this feature. */
> + int dytc_version;
> +
> + dytc_version = (output >> DYTC_QUERY_REV_BIT) & 0xF;
> + if (dytc_version >= 5) {
> + dbg_printk(TPACPI_DBG_INIT,
> + "DYTC version %d: thermal mode available\n", dytc_version);
> + /* Create platform_profile structure and register */
> + err = platform_profile_register(&dytc_profile);
> + /*
> + * If for some reason platform_profiles aren't enabled
> + * don't quit terminally.
> + */
> + if (err)
> + return 0;
> +
> + dytc_profile_available = true;
> + /* Ensure initial values are correct */
> + dytc_profile_refresh();
> + }
> + }
> + return 0;
> +}
> +
> +static void dytc_profile_exit(void)
> +{
> + if (dytc_profile_available) {
> + dytc_profile_available = false;
> + platform_profile_remove();
> + }
> +}
> +
> +static struct ibm_struct dytc_profile_driver_data = {
> + .name = "dytc-profile",
> + .exit = dytc_profile_exit,
> +};
> +#endif /* CONFIG_ACPI_PLATFORM_PROFILE */
> +
> /****************************************************************************
> ****************************************************************************
> *
> @@ -10019,8 +10287,14 @@ static void tpacpi_driver_event(const unsigned int hkey_event)
> mutex_unlock(&kbdlight_mutex);
> }
>
> - if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED)
> + if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED) {
> lapsensor_refresh();
> +#if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
> + /* If we are already accessing DYTC then skip dytc update */
> + if (!atomic_add_unless(&dytc_ignore_event, -1, 0))
> + dytc_profile_refresh();
> +#endif
> + }
> }
>
> static void hotkey_driver_event(const unsigned int scancode)
> @@ -10463,6 +10737,12 @@ static struct ibm_init_struct ibms_init[] __initdata = {
> .init = tpacpi_proxsensor_init,
> .data = &proxsensor_driver_data,
> },
> +#if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
> + {
> + .init = tpacpi_dytc_profile_init,
> + .data = &dytc_profile_driver_data,
> + },
> +#endif
> };
>
> static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-05 11:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-30 0:18 [PATCH v8 1/3] Documentation: Add documentation for new platform_profile sysfs attribute Mark Pearson
2020-12-30 0:18 ` [PATCH v8 2/3] ACPI: platform-profile: Add platform profile support Mark Pearson
2020-12-30 17:40 ` Rafael J. Wysocki
2020-12-30 0:18 ` [PATCH v8 3/3] platform/x86: thinkpad_acpi: " Mark Pearson
2021-01-05 11:02 ` Hans de Goede
2020-12-30 17:35 ` [PATCH v8 1/3] Documentation: Add documentation for new platform_profile sysfs attribute Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox