linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method
@ 2025-04-10 16:54 Armin Wolf
  2025-04-10 16:54 ` [PATCH 1/3] ACPI: OSI: Stop advertising support for "3.0 _SCP Extensions" Armin Wolf
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Armin Wolf @ 2025-04-10 16:54 UTC (permalink / raw)
  To: rafael, rui.zhang; +Cc: lenb, linux-acpi, linux-kernel

The ACPI specification defines an interface for the operating system
to change the preferred cooling mode of a given ACPI thermal zone.
This interface takes the form of a special ACPI control method called
_SCP (see section 11.4.13 for details) and is already supported by the
ACPI thermal driver.

However this support as many issues:

 - the kernel advertises support for the "3.0 _SCP Extensions" yet the
   ACPI thermal driver does not support those extensions. This may
   confuse the ACPI firmware.

 - the execution of the _SCP control method happens after the driver
   retrieved the trip point values. This conflicts with the ACPI
   specification:

	"OSPM will automatically evaluate _ACx and _PSV objects after
	 executing _SCP."

 - the cooling mode is hardcoded to active cooling and cannot be
   changed by the user.

Those issues are fixed in this patch series. In the end the user
will be able to tell the ACPI firmware wether he prefers active or
passive cooling. This setting will also be interesting for
applications like TLP (https://linrunner.de/tlp/index.html).

The whole series was tested on various devices supporting the _SCP
control method and on a device without the _SCP control method and
appears to work flawlessly.

Armin Wolf (3):
  ACPI: OSI: Stop advertising support for "3.0 _SCP Extensions"
  ACPI: thermal: Execute _SCP before reading trip points
  ACPI: thermal: Allow userspace applications to change the cooling mode

 .../ABI/testing/sysfs-driver-thermal          |  14 ++
 MAINTAINERS                                   |   1 +
 drivers/acpi/osi.c                            |   1 -
 drivers/acpi/thermal.c                        | 129 ++++++++++++++++--
 4 files changed, 133 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-thermal

--
2.39.5


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/3] ACPI: OSI: Stop advertising support for "3.0 _SCP Extensions"
  2025-04-10 16:54 [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method Armin Wolf
@ 2025-04-10 16:54 ` Armin Wolf
  2025-04-10 16:54 ` [PATCH 2/3] ACPI: thermal: Execute _SCP before reading trip points Armin Wolf
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Armin Wolf @ 2025-04-10 16:54 UTC (permalink / raw)
  To: rafael, rui.zhang; +Cc: lenb, linux-acpi, linux-kernel

As specified in section 5.7.2 of the ACPI specification the feature
group string "3.0 _SCP Extensions" implies that the operating system
evaluates the _SCP control method with additional parameters.

However the ACPI thermal driver evaluates the _SCP control method
without those additional parameters, conflicting with the above
feature group string advertised to the firmware thru _OSI.

Stop advertising support for this feature string to avoid confusing
the ACPI firmware.

Fixes: e5f660ebef68 ("ACPI / osi: Collect _OSI handling into one single file")
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/acpi/osi.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
index df9328c850bd..f2c943b934be 100644
--- a/drivers/acpi/osi.c
+++ b/drivers/acpi/osi.c
@@ -42,7 +42,6 @@ static struct acpi_osi_entry
 osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = {
 	{"Module Device", true},
 	{"Processor Device", true},
-	{"3.0 _SCP Extensions", true},
 	{"Processor Aggregator Device", true},
 };

--
2.39.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/3] ACPI: thermal: Execute _SCP before reading trip points
  2025-04-10 16:54 [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method Armin Wolf
  2025-04-10 16:54 ` [PATCH 1/3] ACPI: OSI: Stop advertising support for "3.0 _SCP Extensions" Armin Wolf
@ 2025-04-10 16:54 ` Armin Wolf
  2025-04-10 16:54 ` [PATCH 3/3] ACPI: thermal: Allow userspace applications to change the cooling mode Armin Wolf
  2025-04-25 23:20 ` [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method Armin Wolf
  3 siblings, 0 replies; 14+ messages in thread
From: Armin Wolf @ 2025-04-10 16:54 UTC (permalink / raw)
  To: rafael, rui.zhang; +Cc: lenb, linux-acpi, linux-kernel

As specified in section 11.4.13 of the ACPI specification the
operating system is required to evaluate the _ACx and _PSV objects
after executing the _SCP control method.

Move the execution of the _SCP control method before the invocation
of acpi_thermal_get_trip_points() to avoid missing updates to the
_ACx and _PSV objects.

Fixes: b09872a652d3 ("ACPI: thermal: Fold acpi_thermal_get_info() into its caller")
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/acpi/thermal.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 0c874186f8ae..5c2defe55898 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -803,6 +803,12 @@ static int acpi_thermal_add(struct acpi_device *device)

 	acpi_thermal_aml_dependency_fix(tz);

+	/*
+	 * Set the cooling mode [_SCP] to active cooling. This needs to happen before
+	 * we retrieve the trip point values.
+	 */
+	acpi_execute_simple_method(tz->device->handle, "_SCP", ACPI_THERMAL_MODE_ACTIVE);
+
 	/* Get trip points [_ACi, _PSV, etc.] (required). */
 	acpi_thermal_get_trip_points(tz);

@@ -814,10 +820,6 @@ static int acpi_thermal_add(struct acpi_device *device)
 	if (result)
 		goto free_memory;

-	/* Set the cooling mode [_SCP] to active cooling. */
-	acpi_execute_simple_method(tz->device->handle, "_SCP",
-				   ACPI_THERMAL_MODE_ACTIVE);
-
 	/* Determine the default polling frequency [_TZP]. */
 	if (tzp)
 		tz->polling_frequency = tzp;
--
2.39.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/3] ACPI: thermal: Allow userspace applications to change the cooling mode
  2025-04-10 16:54 [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method Armin Wolf
  2025-04-10 16:54 ` [PATCH 1/3] ACPI: OSI: Stop advertising support for "3.0 _SCP Extensions" Armin Wolf
  2025-04-10 16:54 ` [PATCH 2/3] ACPI: thermal: Execute _SCP before reading trip points Armin Wolf
@ 2025-04-10 16:54 ` Armin Wolf
  2025-04-25 23:20 ` [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method Armin Wolf
  3 siblings, 0 replies; 14+ messages in thread
From: Armin Wolf @ 2025-04-10 16:54 UTC (permalink / raw)
  To: rafael, rui.zhang; +Cc: lenb, linux-acpi, linux-kernel

Users might want to signal the ACPI firmware whether active or passive
cooling is preferred. This is already possible under Windows using the
Windows power settings.

Add a new "cooling_mode" sysfs attribute which can be used by users to
change the cooling mode of a given thermal zone. Only thermal zones
supporting the _SCP control method will expose this new attribute.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 .../ABI/testing/sysfs-driver-thermal          |  14 ++
 MAINTAINERS                                   |   1 +
 drivers/acpi/thermal.c                        | 129 ++++++++++++++++--
 3 files changed, 132 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-thermal

diff --git a/Documentation/ABI/testing/sysfs-driver-thermal b/Documentation/ABI/testing/sysfs-driver-thermal
new file mode 100644
index 000000000000..bf2349f31863
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-thermal
@@ -0,0 +1,14 @@
+What:		/sys/bus/acpi/devices/LNXTHERM:*/cooling_mode
+Date:		April 2025
+KernelVersion:	6.16
+Contact:	Armin Wolf <W_Armin@gmx.de>
+Description:
+		A string representing the preferred cooling mode of the
+		associated ACPI thermal zone:
+
+		- "active" for preferring active cooling
+
+		- "passive" for preferring passive cooling
+
+		The exact characteristics of both cooling modes depend
+		on the underlying ACPI firmware implementation.
diff --git a/MAINTAINERS b/MAINTAINERS
index 96b827049501..fd3102723518 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -382,6 +382,7 @@ R:	Zhang Rui <rui.zhang@intel.com>
 L:	linux-acpi@vger.kernel.org
 S:	Supported
 B:	https://bugzilla.kernel.org
+F:	Documentation/ABI/testing/sysfs-driver-thermal
 F:	drivers/acpi/*thermal*

 ACPI VIOT DRIVER
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 5c2defe55898..52d0c777a93a 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -15,11 +15,14 @@

 #define pr_fmt(fmt) "ACPI: thermal: " fmt

+#include <linux/cleanup.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/dmi.h>
 #include <linux/init.h>
 #include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
 #include <linux/types.h>
 #include <linux/jiffies.h>
 #include <linux/kmod.h>
@@ -40,7 +43,6 @@
 #define ACPI_THERMAL_NOTIFY_DEVICES	0x82
 #define ACPI_THERMAL_NOTIFY_CRITICAL	0xF0
 #define ACPI_THERMAL_NOTIFY_HOT		0xF1
-#define ACPI_THERMAL_MODE_ACTIVE	0x00

 #define ACPI_THERMAL_MAX_ACTIVE		10
 #define ACPI_THERMAL_MAX_LIMIT_STR_LEN	65
@@ -85,6 +87,16 @@ MODULE_PARM_DESC(psv, "Disable or override all passive trip points.");

 static struct workqueue_struct *acpi_thermal_pm_queue;

+enum acpi_thermal_cooling_mode {
+	ACPI_THERMAL_MODE_ACTIVE	= 0x00,
+	ACPI_THERMAL_MODE_PASSIVE	= 0x01,
+};
+
+static const char * const acpi_thermal_cooling_mode_strings[] = {
+	[ACPI_THERMAL_MODE_ACTIVE]	= "active",
+	[ACPI_THERMAL_MODE_PASSIVE]	= "passive",
+};
+
 struct acpi_thermal_trip {
 	unsigned long temp_dk;
 	struct acpi_handle_list devices;
@@ -119,6 +131,9 @@ struct acpi_thermal {
 	struct work_struct thermal_check_work;
 	struct mutex thermal_check_lock;
 	refcount_t thermal_check_count;
+	bool supports_cooling_mode;
+	struct mutex cooling_mode_lock;       /* Protects cooling mode updates */
+	enum acpi_thermal_cooling_mode cooling_mode;
 };

 /* --------------------------------------------------------------------------
@@ -328,7 +343,6 @@ static void acpi_queue_thermal_check(struct acpi_thermal *tz)
 static void acpi_thermal_trips_update(struct acpi_thermal *tz, u32 event)
 {
 	struct adjust_trip_data atd = { .tz = tz, .event = event };
-	struct acpi_device *adev = tz->device;

 	/*
 	 * Use thermal_zone_for_each_trip() to carry out the trip points
@@ -340,8 +354,6 @@ static void acpi_thermal_trips_update(struct acpi_thermal *tz, u32 event)
 	thermal_zone_for_each_trip(tz->thermal_zone,
 				   acpi_thermal_adjust_trip, &atd);
 	acpi_queue_thermal_check(tz);
-	acpi_bus_generate_netlink_event(adev->pnp.device_class,
-					dev_name(&adev->dev), event, 0);
 }

 static int acpi_thermal_get_critical_trip(struct acpi_thermal *tz)
@@ -473,6 +485,18 @@ static void acpi_thermal_get_trip_points(struct acpi_thermal *tz)
 		tz->trips.active[i].trip.temp_dk = THERMAL_TEMP_INVALID;
 }

+static int acpi_thermal_set_cooling_mode(struct acpi_thermal *tz,
+					 enum acpi_thermal_cooling_mode mode)
+{
+	acpi_status status;
+
+	status = acpi_execute_simple_method(tz->device->handle, "_SCP", mode);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	return 0;
+}
+
 /* sys I/F for generic thermal sysfs support */

 static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
@@ -683,6 +707,8 @@ static void acpi_thermal_notify(acpi_handle handle, u32 event, void *data)
 	case ACPI_THERMAL_NOTIFY_THRESHOLDS:
 	case ACPI_THERMAL_NOTIFY_DEVICES:
 		acpi_thermal_trips_update(tz, event);
+		acpi_bus_generate_netlink_event(device->pnp.device_class, dev_name(&device->dev),
+						event, 0);
 		break;
 	default:
 		acpi_handle_debug(device->handle, "Unsupported event [0x%x]\n",
@@ -777,6 +803,65 @@ static void acpi_thermal_free_thermal_zone(struct acpi_thermal *tz)
 	kfree(tz);
 }

+static ssize_t cooling_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct acpi_thermal *tz = acpi_driver_data(to_acpi_device(dev));
+
+	guard(mutex)(&tz->cooling_mode_lock);
+
+	return sysfs_emit(buf, "%s\n", acpi_thermal_cooling_mode_strings[tz->cooling_mode]);
+}
+
+static ssize_t cooling_mode_store(struct device *dev, struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct acpi_thermal *tz = acpi_driver_data(to_acpi_device(dev));
+	int ret, mode;
+
+	mode = sysfs_match_string(acpi_thermal_cooling_mode_strings, buf);
+	if (mode < 0)
+		return mode;
+
+	guard(mutex)(&tz->cooling_mode_lock);
+
+	ret = acpi_thermal_set_cooling_mode(tz, mode);
+	if (ret < 0)
+		return ret;
+
+	tz->cooling_mode = mode;
+	acpi_thermal_trips_update(tz, ACPI_THERMAL_NOTIFY_THRESHOLDS);
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(cooling_mode);
+
+static struct attribute *acpi_thermal_attrs[] = {
+	&dev_attr_cooling_mode.attr,
+	NULL
+};
+
+static umode_t acpi_thermal_group_is_visible(struct kobject *kobj, struct attribute *attr, int idx)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct acpi_thermal *tz = acpi_driver_data(to_acpi_device(dev));
+
+	if (tz->supports_cooling_mode)
+		return attr->mode;
+
+	return 0;
+}
+
+static const struct attribute_group acpi_thermal_group = {
+	.is_visible = acpi_thermal_group_is_visible,
+	.attrs = acpi_thermal_attrs,
+};
+
+static const struct attribute_group *acpi_thermal_groups[] = {
+	&acpi_thermal_group,
+	NULL
+};
+
 static int acpi_thermal_add(struct acpi_device *device)
 {
 	struct thermal_trip trip_table[ACPI_THERMAL_MAX_NR_TRIPS] = { 0 };
@@ -786,7 +871,7 @@ static int acpi_thermal_add(struct acpi_device *device)
 	int crit_temp, hot_temp;
 	int passive_delay = 0;
 	int result;
-	int i;
+	int ret, i;

 	if (!device)
 		return -EINVAL;
@@ -795,6 +880,10 @@ static int acpi_thermal_add(struct acpi_device *device)
 	if (!tz)
 		return -ENOMEM;

+	ret = devm_mutex_init(&device->dev, &tz->cooling_mode_lock);
+	if (ret < 0)
+		return ret;
+
 	tz->device = device;
 	strscpy(tz->name, device->pnp.bus_id);
 	strscpy(acpi_device_name(device), ACPI_THERMAL_DEVICE_NAME);
@@ -803,11 +892,18 @@ static int acpi_thermal_add(struct acpi_device *device)

 	acpi_thermal_aml_dependency_fix(tz);

-	/*
-	 * Set the cooling mode [_SCP] to active cooling. This needs to happen before
-	 * we retrieve the trip point values.
-	 */
-	acpi_execute_simple_method(tz->device->handle, "_SCP", ACPI_THERMAL_MODE_ACTIVE);
+	tz->supports_cooling_mode = acpi_has_method(tz->device->handle, "_SCP");
+	if (tz->supports_cooling_mode) {
+		/*
+		 * Set the initial cooling mode to active cooling. This needs to happen
+		 * before we retrieve the trip point values.
+		 */
+		ret = acpi_thermal_set_cooling_mode(tz, ACPI_THERMAL_MODE_ACTIVE);
+		if (ret < 0)
+			dev_err(&tz->device->dev, "Failed to set initial cooling mode\n");
+
+		tz->cooling_mode = ACPI_THERMAL_MODE_ACTIVE;
+	}

 	/* Get trip points [_ACi, _PSV, etc.] (required). */
 	acpi_thermal_get_trip_points(tz);
@@ -924,7 +1020,7 @@ static int acpi_thermal_suspend(struct device *dev)
 static int acpi_thermal_resume(struct device *dev)
 {
 	struct acpi_thermal *tz;
-	int i, j, power_state;
+	int ret, i, j, power_state;

 	if (!dev)
 		return -EINVAL;
@@ -933,6 +1029,12 @@ static int acpi_thermal_resume(struct device *dev)
 	if (!tz)
 		return -EINVAL;

+	if (tz->supports_cooling_mode) {
+		ret = acpi_thermal_set_cooling_mode(tz, tz->cooling_mode);
+		if (ret < 0)
+			dev_err(&tz->device->dev, "Failed to restore cooling mode\n");
+	}
+
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
 		struct acpi_thermal_trip *acpi_trip = &tz->trips.active[i].trip;

@@ -969,7 +1071,10 @@ static struct acpi_driver acpi_thermal_driver = {
 		.add = acpi_thermal_add,
 		.remove = acpi_thermal_remove,
 		},
-	.drv.pm = &acpi_thermal_pm,
+	.drv = {
+		.dev_groups = acpi_thermal_groups,
+		.pm = &acpi_thermal_pm,
+	},
 };

 static int thermal_act(const struct dmi_system_id *d)
--
2.39.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method
  2025-04-10 16:54 [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method Armin Wolf
                   ` (2 preceding siblings ...)
  2025-04-10 16:54 ` [PATCH 3/3] ACPI: thermal: Allow userspace applications to change the cooling mode Armin Wolf
@ 2025-04-25 23:20 ` Armin Wolf
  2025-04-26 13:12   ` Rafael J. Wysocki
  3 siblings, 1 reply; 14+ messages in thread
From: Armin Wolf @ 2025-04-25 23:20 UTC (permalink / raw)
  To: rafael, rui.zhang; +Cc: lenb, linux-acpi, linux-kernel

Am 10.04.25 um 18:54 schrieb Armin Wolf:

> The ACPI specification defines an interface for the operating system
> to change the preferred cooling mode of a given ACPI thermal zone.
> This interface takes the form of a special ACPI control method called
> _SCP (see section 11.4.13 for details) and is already supported by the
> ACPI thermal driver.
>
> However this support as many issues:
>
>   - the kernel advertises support for the "3.0 _SCP Extensions" yet the
>     ACPI thermal driver does not support those extensions. This may
>     confuse the ACPI firmware.
>
>   - the execution of the _SCP control method happens after the driver
>     retrieved the trip point values. This conflicts with the ACPI
>     specification:
>
> 	"OSPM will automatically evaluate _ACx and _PSV objects after
> 	 executing _SCP."
>
>   - the cooling mode is hardcoded to active cooling and cannot be
>     changed by the user.
>
> Those issues are fixed in this patch series. In the end the user
> will be able to tell the ACPI firmware wether he prefers active or
> passive cooling. This setting will also be interesting for
> applications like TLP (https://linrunner.de/tlp/index.html).
>
> The whole series was tested on various devices supporting the _SCP
> control method and on a device without the _SCP control method and
> appears to work flawlessly.

Any updates on this? I can proof that the new interface for setting the cooling mode
works. Additionally the first two patches fix two issues inside the underlying code
itself, so having them inside the mainline tree would be beneficial to users.

Thanks,
Armin Wolf

>
> Armin Wolf (3):
>    ACPI: OSI: Stop advertising support for "3.0 _SCP Extensions"
>    ACPI: thermal: Execute _SCP before reading trip points
>    ACPI: thermal: Allow userspace applications to change the cooling mode
>
>   .../ABI/testing/sysfs-driver-thermal          |  14 ++
>   MAINTAINERS                                   |   1 +
>   drivers/acpi/osi.c                            |   1 -
>   drivers/acpi/thermal.c                        | 129 ++++++++++++++++--
>   4 files changed, 133 insertions(+), 12 deletions(-)
>   create mode 100644 Documentation/ABI/testing/sysfs-driver-thermal
>
> --
> 2.39.5
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method
  2025-04-25 23:20 ` [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method Armin Wolf
@ 2025-04-26 13:12   ` Rafael J. Wysocki
  2025-04-26 22:52     ` Armin Wolf
  0 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2025-04-26 13:12 UTC (permalink / raw)
  To: Armin Wolf; +Cc: rafael, rui.zhang, lenb, linux-acpi, linux-kernel

On Sat, Apr 26, 2025 at 1:20 AM Armin Wolf <W_Armin@gmx.de> wrote:
>
> Am 10.04.25 um 18:54 schrieb Armin Wolf:
>
> > The ACPI specification defines an interface for the operating system
> > to change the preferred cooling mode of a given ACPI thermal zone.
> > This interface takes the form of a special ACPI control method called
> > _SCP (see section 11.4.13 for details) and is already supported by the
> > ACPI thermal driver.
> >
> > However this support as many issues:
> >
> >   - the kernel advertises support for the "3.0 _SCP Extensions" yet the
> >     ACPI thermal driver does not support those extensions. This may
> >     confuse the ACPI firmware.
> >
> >   - the execution of the _SCP control method happens after the driver
> >     retrieved the trip point values. This conflicts with the ACPI
> >     specification:
> >
> >       "OSPM will automatically evaluate _ACx and _PSV objects after
> >        executing _SCP."
> >
> >   - the cooling mode is hardcoded to active cooling and cannot be
> >     changed by the user.
> >
> > Those issues are fixed in this patch series. In the end the user
> > will be able to tell the ACPI firmware wether he prefers active or
> > passive cooling. This setting will also be interesting for
> > applications like TLP (https://linrunner.de/tlp/index.html).
> >
> > The whole series was tested on various devices supporting the _SCP
> > control method and on a device without the _SCP control method and
> > appears to work flawlessly.
>
> Any updates on this? I can proof that the new interface for setting the cooling mode
> works. Additionally the first two patches fix two issues inside the underlying code
> itself, so having them inside the mainline tree would be beneficial to users.

Sure.

I'm going to get to them next week, probably on Monday.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method
  2025-04-26 13:12   ` Rafael J. Wysocki
@ 2025-04-26 22:52     ` Armin Wolf
  2025-04-28 12:31       ` Armin Wolf
  0 siblings, 1 reply; 14+ messages in thread
From: Armin Wolf @ 2025-04-26 22:52 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: rui.zhang, lenb, linux-acpi, linux-kernel

Am 26.04.25 um 15:12 schrieb Rafael J. Wysocki:

> On Sat, Apr 26, 2025 at 1:20 AM Armin Wolf <W_Armin@gmx.de> wrote:
>> Am 10.04.25 um 18:54 schrieb Armin Wolf:
>>
>>> The ACPI specification defines an interface for the operating system
>>> to change the preferred cooling mode of a given ACPI thermal zone.
>>> This interface takes the form of a special ACPI control method called
>>> _SCP (see section 11.4.13 for details) and is already supported by the
>>> ACPI thermal driver.
>>>
>>> However this support as many issues:
>>>
>>>    - the kernel advertises support for the "3.0 _SCP Extensions" yet the
>>>      ACPI thermal driver does not support those extensions. This may
>>>      confuse the ACPI firmware.
>>>
>>>    - the execution of the _SCP control method happens after the driver
>>>      retrieved the trip point values. This conflicts with the ACPI
>>>      specification:
>>>
>>>        "OSPM will automatically evaluate _ACx and _PSV objects after
>>>         executing _SCP."
>>>
>>>    - the cooling mode is hardcoded to active cooling and cannot be
>>>      changed by the user.
>>>
>>> Those issues are fixed in this patch series. In the end the user
>>> will be able to tell the ACPI firmware wether he prefers active or
>>> passive cooling. This setting will also be interesting for
>>> applications like TLP (https://linrunner.de/tlp/index.html).
>>>
>>> The whole series was tested on various devices supporting the _SCP
>>> control method and on a device without the _SCP control method and
>>> appears to work flawlessly.
>> Any updates on this? I can proof that the new interface for setting the cooling mode
>> works. Additionally the first two patches fix two issues inside the underlying code
>> itself, so having them inside the mainline tree would be beneficial to users.
> Sure.
>
> I'm going to get to them next week, probably on Monday.

Ok, thanks.

Armin Wolf


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method
  2025-04-26 22:52     ` Armin Wolf
@ 2025-04-28 12:31       ` Armin Wolf
  2025-04-28 12:34         ` Rafael J. Wysocki
  0 siblings, 1 reply; 14+ messages in thread
From: Armin Wolf @ 2025-04-28 12:31 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: rui.zhang, lenb, linux-acpi, linux-kernel

Am 27.04.25 um 00:52 schrieb Armin Wolf:

> Am 26.04.25 um 15:12 schrieb Rafael J. Wysocki:
>
>> On Sat, Apr 26, 2025 at 1:20 AM Armin Wolf <W_Armin@gmx.de> wrote:
>>> Am 10.04.25 um 18:54 schrieb Armin Wolf:
>>>
>>>> The ACPI specification defines an interface for the operating system
>>>> to change the preferred cooling mode of a given ACPI thermal zone.
>>>> This interface takes the form of a special ACPI control method called
>>>> _SCP (see section 11.4.13 for details) and is already supported by the
>>>> ACPI thermal driver.
>>>>
>>>> However this support as many issues:
>>>>
>>>>    - the kernel advertises support for the "3.0 _SCP Extensions" 
>>>> yet the
>>>>      ACPI thermal driver does not support those extensions. This may
>>>>      confuse the ACPI firmware.
>>>>
>>>>    - the execution of the _SCP control method happens after the driver
>>>>      retrieved the trip point values. This conflicts with the ACPI
>>>>      specification:
>>>>
>>>>        "OSPM will automatically evaluate _ACx and _PSV objects after
>>>>         executing _SCP."
>>>>
>>>>    - the cooling mode is hardcoded to active cooling and cannot be
>>>>      changed by the user.
>>>>
>>>> Those issues are fixed in this patch series. In the end the user
>>>> will be able to tell the ACPI firmware wether he prefers active or
>>>> passive cooling. This setting will also be interesting for
>>>> applications like TLP (https://linrunner.de/tlp/index.html).
>>>>
>>>> The whole series was tested on various devices supporting the _SCP
>>>> control method and on a device without the _SCP control method and
>>>> appears to work flawlessly.
>>> Any updates on this? I can proof that the new interface for setting 
>>> the cooling mode
>>> works. Additionally the first two patches fix two issues inside the 
>>> underlying code
>>> itself, so having them inside the mainline tree would be beneficial 
>>> to users.
>> Sure.
>>
>> I'm going to get to them next week, probably on Monday.
>
> Ok, thanks.
>
> Armin Wolf
>
I am a bit ashamed of myself but i think we need to put this patch series on hold after all :(.

The reason of this is that i am confused by the ACPI specification regarding _SCP:

	11.1.2.1. OSPM Change of Cooling Policy

	When OSPM changes the platform’s cooling policy from one cooling mode to the other, the following occurs:

     	1. OSPM notifies the platform of the new cooling mode by running the Set Cooling Policy (_SCP) control method in all thermal zones and invoking the OS-specific Set Cooling Policy interface to all participating devices in each thermal zone.

     	2. Thresholds are updated in the hardware and OSPM is notified of the change.

     	3. OSPM re-evaluates the active and passive cooling temperature trip points for the zone and all devices in the zone to obtain the new temperature thresholds.

This section of the ACPI specification tells me that we need to evaluate the _SCP control method of all ACPI thermal zones
at the same time, yet section 11.4.13. tells me that each _SCP control methods belongs to the individual thermal zone.

The reason why i am concerned by this is because Windows adheres to section 11.1.2.1. and only exposes this setting
as a global tunable. This might cause device manufacturers to depend on this behavior and lead to strange things
should two thermal zones have different _SCP settings.

I will ask the UEFI mailing list which behavior is expected by the ACPI specification. Until then i suggest that
we put this patch series on hold.

Thanks,
Armin Wolf


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method
  2025-04-28 12:31       ` Armin Wolf
@ 2025-04-28 12:34         ` Rafael J. Wysocki
  2025-05-02 22:29           ` Armin Wolf
  0 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2025-04-28 12:34 UTC (permalink / raw)
  To: Armin Wolf; +Cc: Rafael J. Wysocki, rui.zhang, lenb, linux-acpi, linux-kernel

On Mon, Apr 28, 2025 at 2:31 PM Armin Wolf <W_Armin@gmx.de> wrote:
>
> Am 27.04.25 um 00:52 schrieb Armin Wolf:
>
> > Am 26.04.25 um 15:12 schrieb Rafael J. Wysocki:
> >
> >> On Sat, Apr 26, 2025 at 1:20 AM Armin Wolf <W_Armin@gmx.de> wrote:
> >>> Am 10.04.25 um 18:54 schrieb Armin Wolf:
> >>>
> >>>> The ACPI specification defines an interface for the operating system
> >>>> to change the preferred cooling mode of a given ACPI thermal zone.
> >>>> This interface takes the form of a special ACPI control method called
> >>>> _SCP (see section 11.4.13 for details) and is already supported by the
> >>>> ACPI thermal driver.
> >>>>
> >>>> However this support as many issues:
> >>>>
> >>>>    - the kernel advertises support for the "3.0 _SCP Extensions"
> >>>> yet the
> >>>>      ACPI thermal driver does not support those extensions. This may
> >>>>      confuse the ACPI firmware.
> >>>>
> >>>>    - the execution of the _SCP control method happens after the driver
> >>>>      retrieved the trip point values. This conflicts with the ACPI
> >>>>      specification:
> >>>>
> >>>>        "OSPM will automatically evaluate _ACx and _PSV objects after
> >>>>         executing _SCP."
> >>>>
> >>>>    - the cooling mode is hardcoded to active cooling and cannot be
> >>>>      changed by the user.
> >>>>
> >>>> Those issues are fixed in this patch series. In the end the user
> >>>> will be able to tell the ACPI firmware wether he prefers active or
> >>>> passive cooling. This setting will also be interesting for
> >>>> applications like TLP (https://linrunner.de/tlp/index.html).
> >>>>
> >>>> The whole series was tested on various devices supporting the _SCP
> >>>> control method and on a device without the _SCP control method and
> >>>> appears to work flawlessly.
> >>> Any updates on this? I can proof that the new interface for setting
> >>> the cooling mode
> >>> works. Additionally the first two patches fix two issues inside the
> >>> underlying code
> >>> itself, so having them inside the mainline tree would be beneficial
> >>> to users.
> >> Sure.
> >>
> >> I'm going to get to them next week, probably on Monday.
> >
> > Ok, thanks.
> >
> > Armin Wolf
> >
> I am a bit ashamed of myself but i think we need to put this patch series on hold after all :(.
>
> The reason of this is that i am confused by the ACPI specification regarding _SCP:
>
>         11.1.2.1. OSPM Change of Cooling Policy
>
>         When OSPM changes the platform’s cooling policy from one cooling mode to the other, the following occurs:
>
>         1. OSPM notifies the platform of the new cooling mode by running the Set Cooling Policy (_SCP) control method in all thermal zones and invoking the OS-specific Set Cooling Policy interface to all participating devices in each thermal zone.
>
>         2. Thresholds are updated in the hardware and OSPM is notified of the change.
>
>         3. OSPM re-evaluates the active and passive cooling temperature trip points for the zone and all devices in the zone to obtain the new temperature thresholds.
>
> This section of the ACPI specification tells me that we need to evaluate the _SCP control method of all ACPI thermal zones
> at the same time, yet section 11.4.13. tells me that each _SCP control methods belongs to the individual thermal zone.
>
> The reason why i am concerned by this is because Windows adheres to section 11.1.2.1. and only exposes this setting
> as a global tunable. This might cause device manufacturers to depend on this behavior and lead to strange things
> should two thermal zones have different _SCP settings.
>
> I will ask the UEFI mailing list which behavior is expected by the ACPI specification. Until then i suggest that
> we put this patch series on hold.

Sure, no problem.

Please resend it when you think it is good to go.

Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method
  2025-04-28 12:34         ` Rafael J. Wysocki
@ 2025-05-02 22:29           ` Armin Wolf
  2025-05-16 13:59             ` Rafael J. Wysocki
  0 siblings, 1 reply; 14+ messages in thread
From: Armin Wolf @ 2025-05-02 22:29 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: rui.zhang, lenb, linux-acpi, linux-kernel

Am 28.04.25 um 14:34 schrieb Rafael J. Wysocki:

> On Mon, Apr 28, 2025 at 2:31 PM Armin Wolf <W_Armin@gmx.de> wrote:
>> Am 27.04.25 um 00:52 schrieb Armin Wolf:
>>
>>> Am 26.04.25 um 15:12 schrieb Rafael J. Wysocki:
>>>
>>>> On Sat, Apr 26, 2025 at 1:20 AM Armin Wolf <W_Armin@gmx.de> wrote:
>>>>> Am 10.04.25 um 18:54 schrieb Armin Wolf:
>>>>>
>>>>>> The ACPI specification defines an interface for the operating system
>>>>>> to change the preferred cooling mode of a given ACPI thermal zone.
>>>>>> This interface takes the form of a special ACPI control method called
>>>>>> _SCP (see section 11.4.13 for details) and is already supported by the
>>>>>> ACPI thermal driver.
>>>>>>
>>>>>> However this support as many issues:
>>>>>>
>>>>>>     - the kernel advertises support for the "3.0 _SCP Extensions"
>>>>>> yet the
>>>>>>       ACPI thermal driver does not support those extensions. This may
>>>>>>       confuse the ACPI firmware.
>>>>>>
>>>>>>     - the execution of the _SCP control method happens after the driver
>>>>>>       retrieved the trip point values. This conflicts with the ACPI
>>>>>>       specification:
>>>>>>
>>>>>>         "OSPM will automatically evaluate _ACx and _PSV objects after
>>>>>>          executing _SCP."
>>>>>>
>>>>>>     - the cooling mode is hardcoded to active cooling and cannot be
>>>>>>       changed by the user.
>>>>>>
>>>>>> Those issues are fixed in this patch series. In the end the user
>>>>>> will be able to tell the ACPI firmware wether he prefers active or
>>>>>> passive cooling. This setting will also be interesting for
>>>>>> applications like TLP (https://linrunner.de/tlp/index.html).
>>>>>>
>>>>>> The whole series was tested on various devices supporting the _SCP
>>>>>> control method and on a device without the _SCP control method and
>>>>>> appears to work flawlessly.
>>>>> Any updates on this? I can proof that the new interface for setting
>>>>> the cooling mode
>>>>> works. Additionally the first two patches fix two issues inside the
>>>>> underlying code
>>>>> itself, so having them inside the mainline tree would be beneficial
>>>>> to users.
>>>> Sure.
>>>>
>>>> I'm going to get to them next week, probably on Monday.
>>> Ok, thanks.
>>>
>>> Armin Wolf
>>>
>> I am a bit ashamed of myself but i think we need to put this patch series on hold after all :(.
>>
>> The reason of this is that i am confused by the ACPI specification regarding _SCP:
>>
>>          11.1.2.1. OSPM Change of Cooling Policy
>>
>>          When OSPM changes the platform’s cooling policy from one cooling mode to the other, the following occurs:
>>
>>          1. OSPM notifies the platform of the new cooling mode by running the Set Cooling Policy (_SCP) control method in all thermal zones and invoking the OS-specific Set Cooling Policy interface to all participating devices in each thermal zone.
>>
>>          2. Thresholds are updated in the hardware and OSPM is notified of the change.
>>
>>          3. OSPM re-evaluates the active and passive cooling temperature trip points for the zone and all devices in the zone to obtain the new temperature thresholds.
>>
>> This section of the ACPI specification tells me that we need to evaluate the _SCP control method of all ACPI thermal zones
>> at the same time, yet section 11.4.13. tells me that each _SCP control methods belongs to the individual thermal zone.
>>
>> The reason why i am concerned by this is because Windows adheres to section 11.1.2.1. and only exposes this setting
>> as a global tunable. This might cause device manufacturers to depend on this behavior and lead to strange things
>> should two thermal zones have different _SCP settings.
>>
>> I will ask the UEFI mailing list which behavior is expected by the ACPI specification. Until then i suggest that
>> we put this patch series on hold.
> Sure, no problem.
>
> Please resend it when you think it is good to go.
>
> Thanks!

Alright, the UEFI mailing list gave no response, so i am kind of stuck.

It seems that many firmware implementation only have a single cooling policy register which is set by all _SCP control methods inside the whole system.
The reason for this seems to be that Windows threats this setting as global, but the ACPI specification seemingly does not directly mandate this.

Do you thing we should take the risk and allow users to control each _SCP instance manually?

Apart from that the first two patches should be safe, so you can still pick them. Only the last patch needs some more work.

Thanks,
Armin Wolf


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method
  2025-05-02 22:29           ` Armin Wolf
@ 2025-05-16 13:59             ` Rafael J. Wysocki
  2025-05-16 16:56               ` Rafael J. Wysocki
  2025-05-16 23:01               ` Armin Wolf
  0 siblings, 2 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2025-05-16 13:59 UTC (permalink / raw)
  To: Armin Wolf; +Cc: Rafael J. Wysocki, rui.zhang, lenb, linux-acpi, linux-kernel

On Sat, May 3, 2025 at 12:29 AM Armin Wolf <W_Armin@gmx.de> wrote:
>
> Am 28.04.25 um 14:34 schrieb Rafael J. Wysocki:
>
> > On Mon, Apr 28, 2025 at 2:31 PM Armin Wolf <W_Armin@gmx.de> wrote:
> >> Am 27.04.25 um 00:52 schrieb Armin Wolf:
> >>
> >>> Am 26.04.25 um 15:12 schrieb Rafael J. Wysocki:
> >>>
> >>>> On Sat, Apr 26, 2025 at 1:20 AM Armin Wolf <W_Armin@gmx.de> wrote:
> >>>>> Am 10.04.25 um 18:54 schrieb Armin Wolf:
> >>>>>
> >>>>>> The ACPI specification defines an interface for the operating system
> >>>>>> to change the preferred cooling mode of a given ACPI thermal zone.
> >>>>>> This interface takes the form of a special ACPI control method called
> >>>>>> _SCP (see section 11.4.13 for details) and is already supported by the
> >>>>>> ACPI thermal driver.
> >>>>>>
> >>>>>> However this support as many issues:
> >>>>>>
> >>>>>>     - the kernel advertises support for the "3.0 _SCP Extensions"
> >>>>>> yet the
> >>>>>>       ACPI thermal driver does not support those extensions. This may
> >>>>>>       confuse the ACPI firmware.
> >>>>>>
> >>>>>>     - the execution of the _SCP control method happens after the driver
> >>>>>>       retrieved the trip point values. This conflicts with the ACPI
> >>>>>>       specification:
> >>>>>>
> >>>>>>         "OSPM will automatically evaluate _ACx and _PSV objects after
> >>>>>>          executing _SCP."
> >>>>>>
> >>>>>>     - the cooling mode is hardcoded to active cooling and cannot be
> >>>>>>       changed by the user.
> >>>>>>
> >>>>>> Those issues are fixed in this patch series. In the end the user
> >>>>>> will be able to tell the ACPI firmware wether he prefers active or
> >>>>>> passive cooling. This setting will also be interesting for
> >>>>>> applications like TLP (https://linrunner.de/tlp/index.html).
> >>>>>>
> >>>>>> The whole series was tested on various devices supporting the _SCP
> >>>>>> control method and on a device without the _SCP control method and
> >>>>>> appears to work flawlessly.
> >>>>> Any updates on this? I can proof that the new interface for setting
> >>>>> the cooling mode
> >>>>> works. Additionally the first two patches fix two issues inside the
> >>>>> underlying code
> >>>>> itself, so having them inside the mainline tree would be beneficial
> >>>>> to users.
> >>>> Sure.
> >>>>
> >>>> I'm going to get to them next week, probably on Monday.
> >>> Ok, thanks.
> >>>
> >>> Armin Wolf
> >>>
> >> I am a bit ashamed of myself but i think we need to put this patch series on hold after all :(.
> >>
> >> The reason of this is that i am confused by the ACPI specification regarding _SCP:
> >>
> >>          11.1.2.1. OSPM Change of Cooling Policy
> >>
> >>          When OSPM changes the platform’s cooling policy from one cooling mode to the other, the following occurs:
> >>
> >>          1. OSPM notifies the platform of the new cooling mode by running the Set Cooling Policy (_SCP) control method in all thermal zones and invoking the OS-specific Set Cooling Policy interface to all participating devices in each thermal zone.
> >>
> >>          2. Thresholds are updated in the hardware and OSPM is notified of the change.
> >>
> >>          3. OSPM re-evaluates the active and passive cooling temperature trip points for the zone and all devices in the zone to obtain the new temperature thresholds.
> >>
> >> This section of the ACPI specification tells me that we need to evaluate the _SCP control method of all ACPI thermal zones
> >> at the same time, yet section 11.4.13. tells me that each _SCP control methods belongs to the individual thermal zone.
> >>
> >> The reason why i am concerned by this is because Windows adheres to section 11.1.2.1. and only exposes this setting
> >> as a global tunable. This might cause device manufacturers to depend on this behavior and lead to strange things
> >> should two thermal zones have different _SCP settings.
> >>
> >> I will ask the UEFI mailing list which behavior is expected by the ACPI specification. Until then i suggest that
> >> we put this patch series on hold.
> > Sure, no problem.
> >
> > Please resend it when you think it is good to go.
> >
> > Thanks!
>
> Alright, the UEFI mailing list gave no response, so i am kind of stuck.
>
> It seems that many firmware implementation only have a single cooling policy register which is set by all _SCP control methods inside the whole system.
> The reason for this seems to be that Windows treats this setting as global, but the ACPI specification seemingly does not directly mandate this.
>
> Do you think we should take the risk and allow users to control each _SCP instance manually?

No, I don't.

Doing things that are not done in Windows with ACPI objects is
generally asking for trouble unless there is a specific use case and
there is high confidence that it is actually going to work.

At least to begin with, I wouldn't do it.

> Apart from that the first two patches should be safe, so you can still pick them.

Done.

> Only the last patch needs some more work.

OK

Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method
  2025-05-16 13:59             ` Rafael J. Wysocki
@ 2025-05-16 16:56               ` Rafael J. Wysocki
  2025-05-16 22:51                 ` Armin Wolf
  2025-05-16 23:01               ` Armin Wolf
  1 sibling, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2025-05-16 16:56 UTC (permalink / raw)
  To: Armin Wolf; +Cc: rui.zhang, lenb, linux-acpi, linux-kernel

On Fri, May 16, 2025 at 3:59 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Sat, May 3, 2025 at 12:29 AM Armin Wolf <W_Armin@gmx.de> wrote:
> >
> > Am 28.04.25 um 14:34 schrieb Rafael J. Wysocki:
> >
> > > On Mon, Apr 28, 2025 at 2:31 PM Armin Wolf <W_Armin@gmx.de> wrote:
> > >> Am 27.04.25 um 00:52 schrieb Armin Wolf:
> > >>
> > >>> Am 26.04.25 um 15:12 schrieb Rafael J. Wysocki:
> > >>>
> > >>>> On Sat, Apr 26, 2025 at 1:20 AM Armin Wolf <W_Armin@gmx.de> wrote:
> > >>>>> Am 10.04.25 um 18:54 schrieb Armin Wolf:
> > >>>>>
> > >>>>>> The ACPI specification defines an interface for the operating system
> > >>>>>> to change the preferred cooling mode of a given ACPI thermal zone.
> > >>>>>> This interface takes the form of a special ACPI control method called
> > >>>>>> _SCP (see section 11.4.13 for details) and is already supported by the
> > >>>>>> ACPI thermal driver.
> > >>>>>>
> > >>>>>> However this support as many issues:
> > >>>>>>
> > >>>>>>     - the kernel advertises support for the "3.0 _SCP Extensions"
> > >>>>>> yet the
> > >>>>>>       ACPI thermal driver does not support those extensions. This may
> > >>>>>>       confuse the ACPI firmware.
> > >>>>>>
> > >>>>>>     - the execution of the _SCP control method happens after the driver
> > >>>>>>       retrieved the trip point values. This conflicts with the ACPI
> > >>>>>>       specification:
> > >>>>>>
> > >>>>>>         "OSPM will automatically evaluate _ACx and _PSV objects after
> > >>>>>>          executing _SCP."
> > >>>>>>
> > >>>>>>     - the cooling mode is hardcoded to active cooling and cannot be
> > >>>>>>       changed by the user.
> > >>>>>>
> > >>>>>> Those issues are fixed in this patch series. In the end the user
> > >>>>>> will be able to tell the ACPI firmware wether he prefers active or
> > >>>>>> passive cooling. This setting will also be interesting for
> > >>>>>> applications like TLP (https://linrunner.de/tlp/index.html).
> > >>>>>>
> > >>>>>> The whole series was tested on various devices supporting the _SCP
> > >>>>>> control method and on a device without the _SCP control method and
> > >>>>>> appears to work flawlessly.
> > >>>>> Any updates on this? I can proof that the new interface for setting
> > >>>>> the cooling mode
> > >>>>> works. Additionally the first two patches fix two issues inside the
> > >>>>> underlying code
> > >>>>> itself, so having them inside the mainline tree would be beneficial
> > >>>>> to users.
> > >>>> Sure.
> > >>>>
> > >>>> I'm going to get to them next week, probably on Monday.
> > >>> Ok, thanks.
> > >>>
> > >>> Armin Wolf
> > >>>
> > >> I am a bit ashamed of myself but i think we need to put this patch series on hold after all :(.
> > >>
> > >> The reason of this is that i am confused by the ACPI specification regarding _SCP:
> > >>
> > >>          11.1.2.1. OSPM Change of Cooling Policy
> > >>
> > >>          When OSPM changes the platform’s cooling policy from one cooling mode to the other, the following occurs:
> > >>
> > >>          1. OSPM notifies the platform of the new cooling mode by running the Set Cooling Policy (_SCP) control method in all thermal zones and invoking the OS-specific Set Cooling Policy interface to all participating devices in each thermal zone.
> > >>
> > >>          2. Thresholds are updated in the hardware and OSPM is notified of the change.
> > >>
> > >>          3. OSPM re-evaluates the active and passive cooling temperature trip points for the zone and all devices in the zone to obtain the new temperature thresholds.
> > >>
> > >> This section of the ACPI specification tells me that we need to evaluate the _SCP control method of all ACPI thermal zones
> > >> at the same time, yet section 11.4.13. tells me that each _SCP control methods belongs to the individual thermal zone.

It just says "This object may exist under a thermal zone or a device"
so I don't see any contradiction.  Section 11.4.13 says where it is
located and Section 11.1.2.1 says when to evaluate it.

However, Section 11.4.13 also says "OSPM will automatically evaluate
_ACx and _PSV objects after executing _SCP" which is not arranged for
in your patch [3/3] IIUC.

Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method
  2025-05-16 16:56               ` Rafael J. Wysocki
@ 2025-05-16 22:51                 ` Armin Wolf
  0 siblings, 0 replies; 14+ messages in thread
From: Armin Wolf @ 2025-05-16 22:51 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: rui.zhang, lenb, linux-acpi, linux-kernel

Am 16.05.25 um 18:56 schrieb Rafael J. Wysocki:

> On Fri, May 16, 2025 at 3:59 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Sat, May 3, 2025 at 12:29 AM Armin Wolf <W_Armin@gmx.de> wrote:
>>> Am 28.04.25 um 14:34 schrieb Rafael J. Wysocki:
>>>
>>>> On Mon, Apr 28, 2025 at 2:31 PM Armin Wolf <W_Armin@gmx.de> wrote:
>>>>> Am 27.04.25 um 00:52 schrieb Armin Wolf:
>>>>>
>>>>>> Am 26.04.25 um 15:12 schrieb Rafael J. Wysocki:
>>>>>>
>>>>>>> On Sat, Apr 26, 2025 at 1:20 AM Armin Wolf <W_Armin@gmx.de> wrote:
>>>>>>>> Am 10.04.25 um 18:54 schrieb Armin Wolf:
>>>>>>>>
>>>>>>>>> The ACPI specification defines an interface for the operating system
>>>>>>>>> to change the preferred cooling mode of a given ACPI thermal zone.
>>>>>>>>> This interface takes the form of a special ACPI control method called
>>>>>>>>> _SCP (see section 11.4.13 for details) and is already supported by the
>>>>>>>>> ACPI thermal driver.
>>>>>>>>>
>>>>>>>>> However this support as many issues:
>>>>>>>>>
>>>>>>>>>      - the kernel advertises support for the "3.0 _SCP Extensions"
>>>>>>>>> yet the
>>>>>>>>>        ACPI thermal driver does not support those extensions. This may
>>>>>>>>>        confuse the ACPI firmware.
>>>>>>>>>
>>>>>>>>>      - the execution of the _SCP control method happens after the driver
>>>>>>>>>        retrieved the trip point values. This conflicts with the ACPI
>>>>>>>>>        specification:
>>>>>>>>>
>>>>>>>>>          "OSPM will automatically evaluate _ACx and _PSV objects after
>>>>>>>>>           executing _SCP."
>>>>>>>>>
>>>>>>>>>      - the cooling mode is hardcoded to active cooling and cannot be
>>>>>>>>>        changed by the user.
>>>>>>>>>
>>>>>>>>> Those issues are fixed in this patch series. In the end the user
>>>>>>>>> will be able to tell the ACPI firmware wether he prefers active or
>>>>>>>>> passive cooling. This setting will also be interesting for
>>>>>>>>> applications like TLP (https://linrunner.de/tlp/index.html).
>>>>>>>>>
>>>>>>>>> The whole series was tested on various devices supporting the _SCP
>>>>>>>>> control method and on a device without the _SCP control method and
>>>>>>>>> appears to work flawlessly.
>>>>>>>> Any updates on this? I can proof that the new interface for setting
>>>>>>>> the cooling mode
>>>>>>>> works. Additionally the first two patches fix two issues inside the
>>>>>>>> underlying code
>>>>>>>> itself, so having them inside the mainline tree would be beneficial
>>>>>>>> to users.
>>>>>>> Sure.
>>>>>>>
>>>>>>> I'm going to get to them next week, probably on Monday.
>>>>>> Ok, thanks.
>>>>>>
>>>>>> Armin Wolf
>>>>>>
>>>>> I am a bit ashamed of myself but i think we need to put this patch series on hold after all :(.
>>>>>
>>>>> The reason of this is that i am confused by the ACPI specification regarding _SCP:
>>>>>
>>>>>           11.1.2.1. OSPM Change of Cooling Policy
>>>>>
>>>>>           When OSPM changes the platform’s cooling policy from one cooling mode to the other, the following occurs:
>>>>>
>>>>>           1. OSPM notifies the platform of the new cooling mode by running the Set Cooling Policy (_SCP) control method in all thermal zones and invoking the OS-specific Set Cooling Policy interface to all participating devices in each thermal zone.
>>>>>
>>>>>           2. Thresholds are updated in the hardware and OSPM is notified of the change.
>>>>>
>>>>>           3. OSPM re-evaluates the active and passive cooling temperature trip points for the zone and all devices in the zone to obtain the new temperature thresholds.
>>>>>
>>>>> This section of the ACPI specification tells me that we need to evaluate the _SCP control method of all ACPI thermal zones
>>>>> at the same time, yet section 11.4.13. tells me that each _SCP control methods belongs to the individual thermal zone.
> It just says "This object may exist under a thermal zone or a device"
> so I don't see any contradiction.  Section 11.4.13 says where it is
> located and Section 11.1.2.1 says when to evaluate it.
>
> However, Section 11.4.13 also says "OSPM will automatically evaluate
> _ACx and _PSV objects after executing _SCP" which is not arranged for
> in your patch [3/3] IIUC.
>
> Thanks!

This is incorrect, i do call acpi_thermal_trips_update() after setting _SCP in cooling_mode_store().

Thanks,
Armin Wolf


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method
  2025-05-16 13:59             ` Rafael J. Wysocki
  2025-05-16 16:56               ` Rafael J. Wysocki
@ 2025-05-16 23:01               ` Armin Wolf
  1 sibling, 0 replies; 14+ messages in thread
From: Armin Wolf @ 2025-05-16 23:01 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: rui.zhang, lenb, linux-acpi, linux-kernel

Am 16.05.25 um 15:59 schrieb Rafael J. Wysocki:

> On Sat, May 3, 2025 at 12:29 AM Armin Wolf <W_Armin@gmx.de> wrote:
>> Am 28.04.25 um 14:34 schrieb Rafael J. Wysocki:
>>
>>> On Mon, Apr 28, 2025 at 2:31 PM Armin Wolf <W_Armin@gmx.de> wrote:
>>>> Am 27.04.25 um 00:52 schrieb Armin Wolf:
>>>>
>>>>> Am 26.04.25 um 15:12 schrieb Rafael J. Wysocki:
>>>>>
>>>>>> On Sat, Apr 26, 2025 at 1:20 AM Armin Wolf <W_Armin@gmx.de> wrote:
>>>>>>> Am 10.04.25 um 18:54 schrieb Armin Wolf:
>>>>>>>
>>>>>>>> The ACPI specification defines an interface for the operating system
>>>>>>>> to change the preferred cooling mode of a given ACPI thermal zone.
>>>>>>>> This interface takes the form of a special ACPI control method called
>>>>>>>> _SCP (see section 11.4.13 for details) and is already supported by the
>>>>>>>> ACPI thermal driver.
>>>>>>>>
>>>>>>>> However this support as many issues:
>>>>>>>>
>>>>>>>>      - the kernel advertises support for the "3.0 _SCP Extensions"
>>>>>>>> yet the
>>>>>>>>        ACPI thermal driver does not support those extensions. This may
>>>>>>>>        confuse the ACPI firmware.
>>>>>>>>
>>>>>>>>      - the execution of the _SCP control method happens after the driver
>>>>>>>>        retrieved the trip point values. This conflicts with the ACPI
>>>>>>>>        specification:
>>>>>>>>
>>>>>>>>          "OSPM will automatically evaluate _ACx and _PSV objects after
>>>>>>>>           executing _SCP."
>>>>>>>>
>>>>>>>>      - the cooling mode is hardcoded to active cooling and cannot be
>>>>>>>>        changed by the user.
>>>>>>>>
>>>>>>>> Those issues are fixed in this patch series. In the end the user
>>>>>>>> will be able to tell the ACPI firmware wether he prefers active or
>>>>>>>> passive cooling. This setting will also be interesting for
>>>>>>>> applications like TLP (https://linrunner.de/tlp/index.html).
>>>>>>>>
>>>>>>>> The whole series was tested on various devices supporting the _SCP
>>>>>>>> control method and on a device without the _SCP control method and
>>>>>>>> appears to work flawlessly.
>>>>>>> Any updates on this? I can proof that the new interface for setting
>>>>>>> the cooling mode
>>>>>>> works. Additionally the first two patches fix two issues inside the
>>>>>>> underlying code
>>>>>>> itself, so having them inside the mainline tree would be beneficial
>>>>>>> to users.
>>>>>> Sure.
>>>>>>
>>>>>> I'm going to get to them next week, probably on Monday.
>>>>> Ok, thanks.
>>>>>
>>>>> Armin Wolf
>>>>>
>>>> I am a bit ashamed of myself but i think we need to put this patch series on hold after all :(.
>>>>
>>>> The reason of this is that i am confused by the ACPI specification regarding _SCP:
>>>>
>>>>           11.1.2.1. OSPM Change of Cooling Policy
>>>>
>>>>           When OSPM changes the platform’s cooling policy from one cooling mode to the other, the following occurs:
>>>>
>>>>           1. OSPM notifies the platform of the new cooling mode by running the Set Cooling Policy (_SCP) control method in all thermal zones and invoking the OS-specific Set Cooling Policy interface to all participating devices in each thermal zone.
>>>>
>>>>           2. Thresholds are updated in the hardware and OSPM is notified of the change.
>>>>
>>>>           3. OSPM re-evaluates the active and passive cooling temperature trip points for the zone and all devices in the zone to obtain the new temperature thresholds.
>>>>
>>>> This section of the ACPI specification tells me that we need to evaluate the _SCP control method of all ACPI thermal zones
>>>> at the same time, yet section 11.4.13. tells me that each _SCP control methods belongs to the individual thermal zone.
>>>>
>>>> The reason why i am concerned by this is because Windows adheres to section 11.1.2.1. and only exposes this setting
>>>> as a global tunable. This might cause device manufacturers to depend on this behavior and lead to strange things
>>>> should two thermal zones have different _SCP settings.
>>>>
>>>> I will ask the UEFI mailing list which behavior is expected by the ACPI specification. Until then i suggest that
>>>> we put this patch series on hold.
>>> Sure, no problem.
>>>
>>> Please resend it when you think it is good to go.
>>>
>>> Thanks!
>> Alright, the UEFI mailing list gave no response, so i am kind of stuck.
>>
>> It seems that many firmware implementation only have a single cooling policy register which is set by all _SCP control methods inside the whole system.
>> The reason for this seems to be that Windows treats this setting as global, but the ACPI specification seemingly does not directly mandate this.
>>
>> Do you think we should take the risk and allow users to control each _SCP instance manually?
> No, I don't.
>
> Doing things that are not done in Windows with ACPI objects is
> generally asking for trouble unless there is a specific use case and
> there is high confidence that it is actually going to work.
>
> At least to begin with, I wouldn't do it.

Alright, i will go with a single global interface then.

I am already thinking of using the platform profile interface (with performance and quiet profiles) instead of a custom
sysfs interface to allow for better integration with tuned/platform-profiles-daemon/etc.

I am currently a bit preoccupied so this will take a while.

>> Apart from that the first two patches should be safe, so you can still pick them.
> Done.

Thanks :)

Armin Wolf

>> Only the last patch needs some more work.
> OK
>
> Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-05-16 23:01 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-10 16:54 [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method Armin Wolf
2025-04-10 16:54 ` [PATCH 1/3] ACPI: OSI: Stop advertising support for "3.0 _SCP Extensions" Armin Wolf
2025-04-10 16:54 ` [PATCH 2/3] ACPI: thermal: Execute _SCP before reading trip points Armin Wolf
2025-04-10 16:54 ` [PATCH 3/3] ACPI: thermal: Allow userspace applications to change the cooling mode Armin Wolf
2025-04-25 23:20 ` [PATCH 0/3] ACPI: thermal: Properly support the _SCP control method Armin Wolf
2025-04-26 13:12   ` Rafael J. Wysocki
2025-04-26 22:52     ` Armin Wolf
2025-04-28 12:31       ` Armin Wolf
2025-04-28 12:34         ` Rafael J. Wysocki
2025-05-02 22:29           ` Armin Wolf
2025-05-16 13:59             ` Rafael J. Wysocki
2025-05-16 16:56               ` Rafael J. Wysocki
2025-05-16 22:51                 ` Armin Wolf
2025-05-16 23:01               ` Armin Wolf

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).