Re: [External] Re: [PATCH v2] platform/x86: thinkpad_acpi: performance mode interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks Benjamin!

On 9/16/2020 5:31 AM, Benjamin Berg wrote:
Hi Mark,

On Fri, 2020-08-21 at 13:53 -0400, Mark Pearson wrote:
+		H  - High performance mode. Maximum power and temperature available.
+		M* - High performance mode but performance is limited to medium as system is
+		     in lapmode. Power and temperature maximums reduced to a safe threshold.
+		M  - Medium performance mode (aka 'balance'). Lower maximum power and temperatures
+		     but better battery life.
+		L  - Low performance mode (aka 'quiet'). Reduced power setting gives lower
+		     temperatures and extended battery life. Fans run quieter.

I tested the patch yesterday and I see some minor issues right now.

First, right now change notifications do not happen for a lapmode
change. The sysfs file will switch between "H" and "M*" without any
notification. This will be an easy fix.
OK - I'll look into this and get that implemented. Good catch.

Second, I personally see the current "M*" more as a degraded
performance mode rather than an effective balanced mode. For example, H
and M* match in the sense that the machine will be more noisy as fans
are turned on more aggressively.

Third, we still have a mismatch when writing the file. i.e. you write
"H" but you will read "M*".

So, I am thinking of using H* instead. That means we directly represent
the firmwares performance mode rather than making an interpretation
about an "effective" state. And with the * we continue to convey that
there is a major impact on performance due to other factors.
I agree - that makes sense to me. I'll make that change.

Thanks for the review
Mark


Benjamin

+What:		/sys/devices/platform/thinkpad_acpi/dytc_lapmod
e
+Date:		August 2020
+Contact:	Mark Pearson <markpearson@xxxxxxxxxx>
+Description:
+		Reads returns the current value of the lapmode sensor.
+
+		0 - desk mode is detected
+		1 - lap mode is detected
+
+What:		/sys/devices/platform/thinkpad_acpi/psensor_sta
te
+Date:		August 2020
+Contact:	Nitin Joshi <njoshi1@xxxxxxxxxx>
+Description:
+		Reads returns the current value of the palm sensor.
+
+		0 - palm not detected
+		1 - palm detected
diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
index 6b57c52d8f13..b98f0de9e063 100644
--- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
+++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
@@ -52,6 +52,7 @@ detailed description):
  	- LCD Shadow (PrivacyGuard) enable and disable
  	- Lap mode sensor
          - Palm sensor (aka psensor)
+	- Thermal mode status and control
A compatibility table by model and feature is maintained on the web
  site, http://ibm-acpi.sf.net/. I appreciate any success or failure
@@ -1465,6 +1466,40 @@ Note - some platforms have a limitation
whereby the EC firmware cannot
  determine if the sensor is installed or not. On these platforms the
psensor
  state will always be reported as true to avoid high power being used
incorrectly.
+DYTC Thermal mode status and control
+------------------------------------
+
+sysfs: dytc_perfmode
+
+Lenovo Thinkpad platforms with DYTC version 5 and newer have
enhanced firmware to
+provide improved performance control.
+
+The firmware can be controlled by hotkeys (FN+H, FN+M, FN+L) to
switch the
+operating mode between three different modes. This sysfs node
provides a better
+interface for user space to use.
+
+The modes available are:
+
+H - High performance. Maximum power is available and the temperature
is
+allowed to increase to the maximum for the platform.
+
+M - Medium performance (aka balance). In this mode power will be
limited and
+the laptop will remain cooler.
+
+L - Low performance (aka quiet). In this mode power consumption is
reduced and
+the device will be cooler and quieter.
+
+Note: High performance mode is only available when the device is in
'deskmode'. If
+the device detects that it is on a lap then it will automatically
drop into medium
+mode to maintain a safer operating temperature.
+
+The sysfs entry provides the ability to return the current status
and to set the
+desired mode. For example::
+
+        echo H > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
+        echo M > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
+        echo L > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
+
  EXPERIMENTAL: UWB
  -----------------
diff --git a/drivers/platform/x86/thinkpad_acpi.c
b/drivers/platform/x86/thinkpad_acpi.c
index 41b75dd4755c..8fcb660aa5a2 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9817,18 +9817,43 @@ static struct ibm_struct
lcdshadow_driver_data = {
  };
/*******************************************************************
******
- * DYTC subdriver, for the Lenovo lapmode feature
+ * DYTC subdriver, for the Lenovo lap and performance mode feature
   */
+#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_GET          2 /* To get current IC function and
mode */
-#define DYTC_GET_LAPMODE_BIT 17 /* Set when in lapmode */
+#define DYTC_CMD_RESET    0x1ff /* To reset back to default */
-static bool dytc_lapmode;
+#define DYTC_QUERY_ENABLE_BIT 8  /* Bit 8 - 0 = disabled, 1 =
enabled */
+#define DYTC_QUERY_SUBREV_BIT 16 /* Bits 16 - 27 - sub revisision */
+#define DYTC_QUERY_REV_BIT    28 /* Bits 28 - 31 - revision */
-static void dytc_lapmode_notify_change(void)
-{
-	sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "dytc_lapmode");
-}
+#define DYTC_GET_FUNCTION_BIT 8  /* Bits 8-11 - function setting */
+#define DYTC_GET_MODE_BIT     12 /* Bits 12-15 - mode setting */
+#define DYTC_GET_LAPMODE_BIT  17 /* Bit 17 - lapmode. Set when on
lap */
+
+#define DYTC_SET_FUNCTION_BIT 12 /* Bits 12-15 - funct 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 balance */
+
+#define DYTC_DISABLE_CQL ((DYTC_MODE_BALANCE << DYTC_SET_MODE_BIT) |
\
+		(DYTC_FUNCTION_CQL << DYTC_SET_FUNCTION_BIT) | \
+		DYTC_CMD_SET)
+#define DYTC_ENABLE_CQL (DYTC_DISABLE_CQL | (1 <<
DYTC_SET_VALID_BIT))
+
+static bool dytc_lapmode;
+static int  dytc_perfmode;
+static bool dytc_available;
+static bool dytc_ignore_next_event;
static int dytc_command(int command, int *output)
  {
@@ -9843,6 +9868,87 @@ static int dytc_command(int command, int
*output)
  	return 0;
  }
+static int dytc_perfmode_get(int *perfmode, int *funcmode)
+{
+	int output, err;
+
+	if (!dytc_available)
+		return -ENODEV;
+
+	err = dytc_command(DYTC_CMD_GET, &output);
+	if (err)
+		return err;
+	*funcmode = (output >> DYTC_GET_FUNCTION_BIT) & 0xF;
+
+	if (*funcmode == DYTC_FUNCTION_CQL) {
+		int dummy;
+		/*
+		 * We can't get the mode when in CQL mode - so we
disable CQL
+		 * mode retrieve the mode and then enable it again.
+		 * As disabling/enabling CQL triggers an event we set a
flag to
+		 * ignore these events. This will be cleared by the
event handler
+		 */
+		dytc_ignore_next_event = true;
+		err = dytc_command(DYTC_DISABLE_CQL, &dummy);
+		if (err)
+			return err;
+		err = dytc_command(DYTC_CMD_GET, &output);
+		if (err)
+			return err;
+		/* Again ignore this event */
+		dytc_ignore_next_event = true;
+		err = dytc_command(DYTC_ENABLE_CQL, &dummy);
+		if (err)
+			return err;
+	}
+	*perfmode = (output >> DYTC_GET_MODE_BIT) & 0xF;
+	return 0;
+}
+
+static int dytc_perfmode_set(int perfmode)
+{
+	int err, dytc_set;
+	int output;
+	int cur_perfmode, cur_funcmode;
+
+	if (!dytc_available)
+		return -ENODEV;
+
+	if (perfmode == DYTC_MODE_BALANCE) {
+		/* To get back to balance mode we just issue a reset
command */
+		err = dytc_command(DYTC_CMD_RESET, &output);
+		if (err)
+			return err;
+	} else {
+		/* Determine if we are in CQL mode. This alters the
commands we do */
+		err = dytc_perfmode_get(&cur_perfmode, &cur_funcmode);
+		if (err)
+			return err;
+
+		if (cur_funcmode == DYTC_FUNCTION_CQL) {
+			/* To set the mode we need to disable CQL
first*/
+			dytc_ignore_next_event = true; /*ignore event*/
+			err = dytc_command(DYTC_DISABLE_CQL, &output);
+			if (err)
+				return err;
+		}
+		dytc_set = (1 << DYTC_SET_VALID_BIT) |
+			(DYTC_FUNCTION_MMC << DYTC_SET_FUNCTION_BIT) |
+			(perfmode << DYTC_SET_MODE_BIT) |
+			DYTC_CMD_SET;
+		err = dytc_command(dytc_set, &output);
+		if (err)
+			return err;
+		if (cur_funcmode == DYTC_FUNCTION_CQL) {
+			dytc_ignore_next_event = true; /*ignore event*/
+			err = dytc_command(DYTC_ENABLE_CQL, &output);
+			if (err)
+				return err;
+		}
+	}
+	return 0;
+}
+
  static int dytc_lapmode_get(bool *state)
  {
  	int output, err;
@@ -9854,45 +9960,130 @@ static int dytc_lapmode_get(bool *state)
  	return 0;
  }
-static void dytc_lapmode_refresh(void)
+static void dytc_refresh(void)
  {
-	bool new_state;
+	bool lapmode;
+	int perfmode, funcmode;
  	int err;
- err = dytc_lapmode_get(&new_state);
-	if (err || (new_state == dytc_lapmode))
+	err = dytc_lapmode_get(&lapmode);
+	if (err)
+		return;
+	if (dytc_ignore_next_event) {
+		dytc_ignore_next_event = false; /*clear setting*/
  		return;
+	}
+	if (lapmode != dytc_lapmode) {
+		dytc_lapmode = lapmode;
+		sysfs_notify(&tpacpi_pdev->dev.kobj, NULL,
"dytc_lapmode");
+	}
+	err = dytc_perfmode_get(&perfmode, &funcmode);
+	if (err)
+		return;
+	if (perfmode != dytc_perfmode) {
+		dytc_perfmode = perfmode;
+		sysfs_notify(&tpacpi_pdev->dev.kobj, NULL,
"dytc_perfmode");
+	}
+}
+
+/* sysfs perfmode entry */
+static ssize_t dytc_perfmode_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	int err;
+	int perfmode, funcmode;
+
+	err = dytc_perfmode_get(&perfmode, &funcmode);
+	if (err)
+		return err;
- dytc_lapmode = new_state;
-	dytc_lapmode_notify_change();
+	switch (perfmode) {
+	case DYTC_MODE_PERFORM:
+		/* High performance is only available in deskmode */
+		if (funcmode == DYTC_FUNCTION_CQL)
+			return sprintf(buf, "M*\n");
+		else
+			return sprintf(buf, "H\n");
+	case DYTC_MODE_QUIET:
+		return sprintf(buf, "L\n");
+	case DYTC_MODE_BALANCE:
+		return sprintf(buf, "M\n");
+	default:
+		return sprintf(buf, "Unknown (%d)\n", perfmode);
+	}
  }
+static ssize_t dytc_perfmode_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	int err;
+
+	switch (buf[0]) {
+	case 'l':
+	case 'L':
+		err = dytc_perfmode_set(DYTC_MODE_QUIET);
+		break;
+	case 'm':
+	case 'M':
+		err = dytc_perfmode_set(DYTC_MODE_BALANCE);
+		break;
+	case 'h':
+	case 'H':
+		err = dytc_perfmode_set(DYTC_MODE_PERFORM);
+		break;
+	default:
+		err = -EINVAL;
+		pr_err("Unknown operating mode. Ignoring\n");
+		break;
+	}
+	if (err)
+		return err;
+
+	tpacpi_disclose_usertask(attr->attr.name,
+				"Performance mode set to %c\n",
buf[0]);
+	return count;
+}
+
+static DEVICE_ATTR_RW(dytc_perfmode);
+
+static struct attribute *dytc_perfmode_attributes[] = {
+	&dev_attr_dytc_perfmode.attr,
+	NULL
+};
+
+static const struct attribute_group dytc_perf_attr_group = {
+	.attrs = dytc_perfmode_attributes
+};
+
  /* sysfs lapmode entry */
  static ssize_t dytc_lapmode_show(struct device *dev,
  					struct device_attribute *attr,
  					char *buf)
  {
-	return snprintf(buf, PAGE_SIZE, "%d\n", dytc_lapmode);
+	return sprintf(buf, "%d\n", dytc_lapmode);
  }
static DEVICE_ATTR_RO(dytc_lapmode); -static struct attribute *dytc_attributes[] = {
+static struct attribute *dytc_lap_attributes[] = {
  	&dev_attr_dytc_lapmode.attr,
-	NULL,
+	NULL
  };
-static const struct attribute_group dytc_attr_group = {
-	.attrs = dytc_attributes,
+static const struct attribute_group dytc_lap_attr_group = {
+	.attrs = dytc_lap_attributes
  };
static int tpacpi_dytc_init(struct ibm_init_struct *iibm)
  {
-	int err;
+	int err, output;
- err = dytc_lapmode_get(&dytc_lapmode);
-	/* If support isn't available (ENODEV) then don't return an
error
-	 * but just don't create the sysfs group
+	err = dytc_command(DYTC_CMD_QUERY, &output);
+	/*
+	 * If support isn't available (ENODEV) then don't return an
error
+	 * just don't create the sysfs group
  	 */
  	if (err == -ENODEV)
  		return 0;
@@ -9900,14 +10091,38 @@ static int tpacpi_dytc_init(struct
ibm_init_struct *iibm)
  	if (err)
  		return err;
- /* Platform supports this feature - create the group */
-	err = sysfs_create_group(&tpacpi_pdev->dev.kobj,
&dytc_attr_group);
+	/* Check DYTC is enabled and supports mode setting */
+	dytc_available = false;
+	dytc_ignore_next_event = false;
+	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);
+			dytc_available = true;
+			/* Platform supports this feature - create the
group */
+			err = sysfs_create_group(&tpacpi_pdev-
dev.kobj, &dytc_perf_attr_group);
+			if (err)
+				return err;
+
+			err = dytc_lapmode_get(&dytc_lapmode);
+			if (err)
+				return err;
+			err = sysfs_create_group(&tpacpi_pdev-
dev.kobj, &dytc_lap_attr_group);
+		}
+	}
  	return err;
  }
static void dytc_exit(void)
  {
-	sysfs_remove_group(&tpacpi_pdev->dev.kobj, &dytc_attr_group);
+	if (dytc_available) {
+		sysfs_remove_group(&tpacpi_pdev->dev.kobj,
&dytc_lap_attr_group);
+		sysfs_remove_group(&tpacpi_pdev->dev.kobj,
&dytc_perf_attr_group);
+	}
  }
static struct ibm_struct dytc_driver_data = {
@@ -10057,7 +10272,7 @@ static void tpacpi_driver_event(const
unsigned int hkey_event)
  	}
if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED)
-		dytc_lapmode_refresh();
+		dytc_refresh();
if ((hkey_event == TP_HKEY_EV_PALM_DETECTED) ||
  		(hkey_event == TP_HKEY_EV_PALM_UNDETECTED))


_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel



[Index of Archives]     [Linux ACPI]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Photo]     [Yosemite Photos]     [Yosemite Advice]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux