linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices
@ 2021-02-11 20:16 Maximilian Luz
  2021-02-11 20:17 ` [PATCH v2 1/4] ACPI: platform: Hide ACPI_PLATFORM_PROFILE option Maximilian Luz
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Maximilian Luz @ 2021-02-11 20:16 UTC (permalink / raw)
  To: Hans de Goede, Rafael J. Wysocki
  Cc: Maximilian Luz, Mark Gross, Len Brown, Mark Pearson, Jiaxun Yang,
	platform-driver-x86, linux-acpi, linux-kernel

This series adds a driver to provide platform profile support on 5th-
and later generation Microsoft Surface devices with a Surface System
Aggregator Module. On those devices, the platform profile can be used to
influence cooling behavior and power consumption.

To achieve this, a new platform profile is introduced: the
'balanced-performance' profile.

In addition, a couple of fix-ups are performed:
- Hide CONFIG_ACPI_PLATFORM_PROFILE and change drivers so that it is
  selected instead of depended on.
- Fix some references to documentation in a comment.

Note: This series (or more specifically "platform/surface: Add platform
profile driver") depends on the "platform/surface: Add Surface
Aggregator device registry" series.

Changes in v2:
 - Introduce new 'balanced-performance' platform profile and change
   profile mapping in driver.
 - Perform some fix-ups for the ACPI platform profile implementation:
   - Fix some references to documentation in a comment.
   - Hide CONFIG_ACPI_PLATFORM_PROFILE

Maximilian Luz (4):
  ACPI: platform: Hide ACPI_PLATFORM_PROFILE option
  ACPI: platform: Fix file references in comment
  ACPI: platform: Add balanced-performance platform profile
  platform/surface: Add platform profile driver

 .../ABI/testing/sysfs-platform_profile        |  18 +-
 MAINTAINERS                                   |   6 +
 drivers/acpi/Kconfig                          |  16 +-
 drivers/acpi/platform_profile.c               |   1 +
 drivers/platform/surface/Kconfig              |  22 ++
 drivers/platform/surface/Makefile             |   1 +
 .../surface/surface_platform_profile.c        | 190 ++++++++++++++++++
 drivers/platform/x86/Kconfig                  |   4 +-
 include/linux/platform_profile.h              |   6 +-
 9 files changed, 237 insertions(+), 27 deletions(-)
 create mode 100644 drivers/platform/surface/surface_platform_profile.c

-- 
2.30.0


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

* [PATCH v2 1/4] ACPI: platform: Hide ACPI_PLATFORM_PROFILE option
  2021-02-11 20:16 [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices Maximilian Luz
@ 2021-02-11 20:17 ` Maximilian Luz
  2021-02-11 20:17 ` [PATCH v2 2/4] ACPI: platform: Fix file references in comment Maximilian Luz
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Maximilian Luz @ 2021-02-11 20:17 UTC (permalink / raw)
  To: Hans de Goede, Rafael J. Wysocki
  Cc: Maximilian Luz, Mark Gross, Len Brown, Mark Pearson, Jiaxun Yang,
	platform-driver-x86, linux-acpi, linux-kernel

The ACPI_PLATFORM_PROFILE option essentially provides a library and not
really an independent module. Thus it seems to be more user-friendly to
hide this option and simply make drivers depending on it select it.

Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
---
 drivers/acpi/Kconfig         | 16 +---------------
 drivers/platform/x86/Kconfig |  4 ++--
 2 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 5ddff93e38c2..030678965159 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -327,21 +327,7 @@ config ACPI_THERMAL
 	  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.
+	tristate
 
 config ACPI_CUSTOM_DSDT_FILE
 	string "Custom DSDT Table file to include"
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 56353e8c792a..ad4e630e73e2 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -450,7 +450,7 @@ config IDEAPAD_LAPTOP
 	depends on BACKLIGHT_CLASS_DEVICE
 	depends on ACPI_VIDEO || ACPI_VIDEO = n
 	depends on ACPI_WMI || ACPI_WMI = n
-	depends on ACPI_PLATFORM_PROFILE
+	select ACPI_PLATFORM_PROFILE
 	select INPUT_SPARSEKMAP
 	select NEW_LEDS
 	select LEDS_CLASS
@@ -484,7 +484,7 @@ config THINKPAD_ACPI
 	depends on RFKILL || RFKILL = n
 	depends on ACPI_VIDEO || ACPI_VIDEO = n
 	depends on BACKLIGHT_CLASS_DEVICE
-	depends on ACPI_PLATFORM_PROFILE
+	select ACPI_PLATFORM_PROFILE
 	select HWMON
 	select NVRAM
 	select NEW_LEDS
-- 
2.30.0


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

* [PATCH v2 2/4] ACPI: platform: Fix file references in comment
  2021-02-11 20:16 [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices Maximilian Luz
  2021-02-11 20:17 ` [PATCH v2 1/4] ACPI: platform: Hide ACPI_PLATFORM_PROFILE option Maximilian Luz
@ 2021-02-11 20:17 ` Maximilian Luz
  2021-02-11 20:17 ` [PATCH v2 3/4] ACPI: platform: Add balanced-performance platform profile Maximilian Luz
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Maximilian Luz @ 2021-02-11 20:17 UTC (permalink / raw)
  To: Hans de Goede, Rafael J. Wysocki
  Cc: Maximilian Luz, Mark Gross, Len Brown, Mark Pearson, Jiaxun Yang,
	platform-driver-x86, linux-acpi, linux-kernel

The referenced files are named slightly different. Replace '-' with '_'
and drop the .rst ending.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
---
 include/linux/platform_profile.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
index a26542d53058..b4794027e759 100644
--- a/include/linux/platform_profile.h
+++ b/include/linux/platform_profile.h
@@ -12,9 +12,8 @@
 #include <linux/bitops.h>
 
 /*
- * If more options are added please update profile_names
- * array in platform-profile.c and sysfs-platform-profile.rst
- * documentation.
+ * If more options are added please update profile_names array in
+ * platform_profile.c and sysfs-platform_profile documentation.
  */
 
 enum platform_profile_option {
-- 
2.30.0


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

* [PATCH v2 3/4] ACPI: platform: Add balanced-performance platform profile
  2021-02-11 20:16 [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices Maximilian Luz
  2021-02-11 20:17 ` [PATCH v2 1/4] ACPI: platform: Hide ACPI_PLATFORM_PROFILE option Maximilian Luz
  2021-02-11 20:17 ` [PATCH v2 2/4] ACPI: platform: Fix file references in comment Maximilian Luz
@ 2021-02-11 20:17 ` Maximilian Luz
  2021-02-11 20:17 ` [PATCH v2 4/4] platform/surface: Add platform profile driver Maximilian Luz
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Maximilian Luz @ 2021-02-11 20:17 UTC (permalink / raw)
  To: Hans de Goede, Rafael J. Wysocki
  Cc: Maximilian Luz, Mark Gross, Len Brown, Mark Pearson, Jiaxun Yang,
	platform-driver-x86, linux-acpi, linux-kernel

Some devices, including most Microsoft Surface devices, have a platform
profile somewhere inbetween balanced and performance. More specifically,
adding this profile allows the following mapping on Surface devices:

  Vendor Name           Platform Profile
  ------------------------------------------
  Battery Saver         low-power
  Recommended           balanced
  Better Performance    balanced-performance
  Best Performance      performance

Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
---
 .../ABI/testing/sysfs-platform_profile         | 18 +++++++++++-------
 drivers/acpi/platform_profile.c                |  1 +
 include/linux/platform_profile.h               |  1 +
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-platform_profile b/Documentation/ABI/testing/sysfs-platform_profile
index 9d6b89b66cca..dae9c8941905 100644
--- a/Documentation/ABI/testing/sysfs-platform_profile
+++ b/Documentation/ABI/testing/sysfs-platform_profile
@@ -5,13 +5,17 @@ Description:	This file contains a space-separated list of profiles supported for
 
 		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
-		============	============================================
+		====================	========================================
+		low-power		Low power consumption
+		cool			Cooler operation
+		quiet			Quieter operation
+		balanced		Balance between low power consumption
+					and performance
+		balanced-performance	Balance between performance and low
+					power consumption with a slight bias
+					towards performance
+		performance		High performance operation
+		====================	========================================
 
 		Userspace may expect drivers to offer more than one of these
 		standard profile names.
diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
index 4a59c5993bde..dd2fbf38e414 100644
--- a/drivers/acpi/platform_profile.c
+++ b/drivers/acpi/platform_profile.c
@@ -17,6 +17,7 @@ static const char * const profile_names[] = {
 	[PLATFORM_PROFILE_COOL] = "cool",
 	[PLATFORM_PROFILE_QUIET] = "quiet",
 	[PLATFORM_PROFILE_BALANCED] = "balanced",
+	[PLATFORM_PROFILE_BALANCED_PERFORMANCE] = "balanced-performance",
 	[PLATFORM_PROFILE_PERFORMANCE] = "performance",
 };
 static_assert(ARRAY_SIZE(profile_names) == PLATFORM_PROFILE_LAST);
diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
index b4794027e759..a6329003aee7 100644
--- a/include/linux/platform_profile.h
+++ b/include/linux/platform_profile.h
@@ -21,6 +21,7 @@ enum platform_profile_option {
 	PLATFORM_PROFILE_COOL,
 	PLATFORM_PROFILE_QUIET,
 	PLATFORM_PROFILE_BALANCED,
+	PLATFORM_PROFILE_BALANCED_PERFORMANCE,
 	PLATFORM_PROFILE_PERFORMANCE,
 	PLATFORM_PROFILE_LAST, /*must always be last */
 };
-- 
2.30.0


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

* [PATCH v2 4/4] platform/surface: Add platform profile driver
  2021-02-11 20:16 [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices Maximilian Luz
                   ` (2 preceding siblings ...)
  2021-02-11 20:17 ` [PATCH v2 3/4] ACPI: platform: Add balanced-performance platform profile Maximilian Luz
@ 2021-02-11 20:17 ` Maximilian Luz
  2021-03-04 11:01   ` Hans de Goede
  2021-02-12 15:29 ` [External] [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices Mark Pearson
  2021-02-15 14:34 ` Hans de Goede
  5 siblings, 1 reply; 15+ messages in thread
From: Maximilian Luz @ 2021-02-11 20:17 UTC (permalink / raw)
  To: Hans de Goede, Rafael J. Wysocki
  Cc: Maximilian Luz, Mark Gross, Len Brown, Mark Pearson, Jiaxun Yang,
	platform-driver-x86, linux-acpi, linux-kernel

Add a driver to provide platform profile support on 5th- and later
generation Microsoft Surface devices with a Surface System Aggregator
Module. On those devices, the platform profile can be used to influence
cooling behavior and power consumption.

For example, the default 'quiet' profile limits fan noise and in turn
sacrifices performance of the discrete GPU found on Surface Books. Its
full performance can only be unlocked on the 'performance' profile.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
---
 MAINTAINERS                                   |   6 +
 drivers/platform/surface/Kconfig              |  22 ++
 drivers/platform/surface/Makefile             |   1 +
 .../surface/surface_platform_profile.c        | 190 ++++++++++++++++++
 4 files changed, 219 insertions(+)
 create mode 100644 drivers/platform/surface/surface_platform_profile.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 000a82f59c76..a08d65f8f0df 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11811,6 +11811,12 @@ L:	platform-driver-x86@vger.kernel.org
 S:	Maintained
 F:	drivers/platform/surface/surface_hotplug.c
 
+MICROSOFT SURFACE PLATFORM PROFILE DRIVER
+M:	Maximilian Luz <luzmaximilian@gmail.com>
+L:	platform-driver-x86@vger.kernel.org
+S:	Maintained
+F:	drivers/platform/surface/surface_platform_profile.c
+
 MICROSOFT SURFACE PRO 3 BUTTON DRIVER
 M:	Chen Yu <yu.c.chen@intel.com>
 L:	platform-driver-x86@vger.kernel.org
diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig
index 179b8c93d7fd..a045425026ae 100644
--- a/drivers/platform/surface/Kconfig
+++ b/drivers/platform/surface/Kconfig
@@ -132,6 +132,28 @@ config SURFACE_HOTPLUG
 	  Select M or Y here, if you want to (fully) support hot-plugging of
 	  dGPU devices on the Surface Book 2 and/or 3 during D3cold.
 
+config SURFACE_PLATFORM_PROFILE
+	tristate "Surface Platform Profile Driver"
+	depends on SURFACE_AGGREGATOR_REGISTRY
+	select ACPI_PLATFORM_PROFILE
+	help
+	  Provides support for the ACPI platform profile on 5th- and later
+	  generation Microsoft Surface devices.
+
+	  More specifically, this driver provides ACPI platform profile support
+	  on Microsoft Surface devices with a Surface System Aggregator Module
+	  (SSAM) connected via the Surface Serial Hub (SSH / SAM-over-SSH). In
+	  other words, this driver provides platform profile support on the
+	  Surface Pro 5, Surface Book 2, Surface Laptop, Surface Laptop Go and
+	  later. On those devices, the platform profile can significantly
+	  influence cooling behavior, e.g. setting it to 'quiet' (default) or
+	  'low-power' can significantly limit performance of the discrete GPU on
+	  Surface Books, while in turn leading to lower power consumption and/or
+	  less fan noise.
+
+	  Select M or Y here, if you want to include ACPI platform profile
+	  support on the above mentioned devices.
+
 config SURFACE_PRO3_BUTTON
 	tristate "Power/home/volume buttons driver for Microsoft Surface Pro 3/4 tablet"
 	depends on INPUT
diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile
index 80035ee540bf..99372c427b73 100644
--- a/drivers/platform/surface/Makefile
+++ b/drivers/platform/surface/Makefile
@@ -13,4 +13,5 @@ obj-$(CONFIG_SURFACE_AGGREGATOR_CDEV)	+= surface_aggregator_cdev.o
 obj-$(CONFIG_SURFACE_AGGREGATOR_REGISTRY) += surface_aggregator_registry.o
 obj-$(CONFIG_SURFACE_GPE)		+= surface_gpe.o
 obj-$(CONFIG_SURFACE_HOTPLUG)		+= surface_hotplug.o
+obj-$(CONFIG_SURFACE_PLATFORM_PROFILE)	+= surface_platform_profile.o
 obj-$(CONFIG_SURFACE_PRO3_BUTTON)	+= surfacepro3_button.o
diff --git a/drivers/platform/surface/surface_platform_profile.c b/drivers/platform/surface/surface_platform_profile.c
new file mode 100644
index 000000000000..0081b01a5b0f
--- /dev/null
+++ b/drivers/platform/surface/surface_platform_profile.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Surface Platform Profile / Performance Mode driver for Surface System
+ * Aggregator Module (thermal subsystem).
+ *
+ * Copyright (C) 2021 Maximilian Luz <luzmaximilian@gmail.com>
+ */
+
+#include <asm/unaligned.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_profile.h>
+#include <linux/types.h>
+
+#include <linux/surface_aggregator/device.h>
+
+enum ssam_tmp_profile {
+	SSAM_TMP_PROFILE_NORMAL             = 1,
+	SSAM_TMP_PROFILE_BATTERY_SAVER      = 2,
+	SSAM_TMP_PROFILE_BETTER_PERFORMANCE = 3,
+	SSAM_TMP_PROFILE_BEST_PERFORMANCE   = 4,
+};
+
+struct ssam_tmp_profile_info {
+	__le32 profile;
+	__le16 unknown1;
+	__le16 unknown2;
+} __packed;
+
+struct ssam_tmp_profile_device {
+	struct ssam_device *sdev;
+	struct platform_profile_handler handler;
+};
+
+static SSAM_DEFINE_SYNC_REQUEST_CL_R(__ssam_tmp_profile_get, struct ssam_tmp_profile_info, {
+	.target_category = SSAM_SSH_TC_TMP,
+	.command_id      = 0x02,
+});
+
+static SSAM_DEFINE_SYNC_REQUEST_CL_W(__ssam_tmp_profile_set, __le32, {
+	.target_category = SSAM_SSH_TC_TMP,
+	.command_id      = 0x03,
+});
+
+static int ssam_tmp_profile_get(struct ssam_device *sdev, enum ssam_tmp_profile *p)
+{
+	struct ssam_tmp_profile_info info;
+	int status;
+
+	status = ssam_retry(__ssam_tmp_profile_get, sdev, &info);
+	if (status < 0)
+		return status;
+
+	*p = le32_to_cpu(info.profile);
+	return 0;
+}
+
+static int ssam_tmp_profile_set(struct ssam_device *sdev, enum ssam_tmp_profile p)
+{
+	__le32 profile_le = cpu_to_le32(p);
+
+	return ssam_retry(__ssam_tmp_profile_set, sdev, &profile_le);
+}
+
+static int convert_ssam_to_profile(struct ssam_device *sdev, enum ssam_tmp_profile p)
+{
+	switch (p) {
+	case SSAM_TMP_PROFILE_NORMAL:
+		return PLATFORM_PROFILE_BALANCED;
+
+	case SSAM_TMP_PROFILE_BATTERY_SAVER:
+		return PLATFORM_PROFILE_LOW_POWER;
+
+	case SSAM_TMP_PROFILE_BETTER_PERFORMANCE:
+		return PLATFORM_PROFILE_BALANCED_PERFORMANCE;
+
+	case SSAM_TMP_PROFILE_BEST_PERFORMANCE:
+		return PLATFORM_PROFILE_PERFORMANCE;
+
+	default:
+		dev_err(&sdev->dev, "invalid performance profile: %d", p);
+		return -EINVAL;
+	}
+}
+
+static int convert_profile_to_ssam(struct ssam_device *sdev, enum platform_profile_option p)
+{
+	switch (p) {
+	case PLATFORM_PROFILE_LOW_POWER:
+		return SSAM_TMP_PROFILE_BATTERY_SAVER;
+
+	case PLATFORM_PROFILE_BALANCED:
+		return SSAM_TMP_PROFILE_NORMAL;
+
+	case PLATFORM_PROFILE_BALANCED_PERFORMANCE:
+		return SSAM_TMP_PROFILE_BETTER_PERFORMANCE;
+
+	case PLATFORM_PROFILE_PERFORMANCE:
+		return SSAM_TMP_PROFILE_BEST_PERFORMANCE;
+
+	default:
+		/* This should have already been caught by platform_profile_store(). */
+		WARN(true, "unsupported platform profile");
+		return -EOPNOTSUPP;
+	}
+}
+
+static int ssam_platform_profile_get(struct platform_profile_handler *pprof,
+				     enum platform_profile_option *profile)
+{
+	struct ssam_tmp_profile_device *tpd;
+	enum ssam_tmp_profile tp;
+	int status;
+
+	tpd = container_of(pprof, struct ssam_tmp_profile_device, handler);
+
+	status = ssam_tmp_profile_get(tpd->sdev, &tp);
+	if (status)
+		return status;
+
+	status = convert_ssam_to_profile(tpd->sdev, tp);
+	if (status < 0)
+		return status;
+
+	*profile = status;
+	return 0;
+}
+
+static int ssam_platform_profile_set(struct platform_profile_handler *pprof,
+				     enum platform_profile_option profile)
+{
+	struct ssam_tmp_profile_device *tpd;
+	int tp;
+
+	tpd = container_of(pprof, struct ssam_tmp_profile_device, handler);
+
+	tp = convert_profile_to_ssam(tpd->sdev, profile);
+	if (tp < 0)
+		return tp;
+
+	return ssam_tmp_profile_set(tpd->sdev, tp);
+}
+
+static int surface_platform_profile_probe(struct ssam_device *sdev)
+{
+	struct ssam_tmp_profile_device *tpd;
+
+	tpd = devm_kzalloc(&sdev->dev, sizeof(*tpd), GFP_KERNEL);
+	if (!tpd)
+		return -ENOMEM;
+
+	tpd->sdev = sdev;
+
+	tpd->handler.profile_get = ssam_platform_profile_get;
+	tpd->handler.profile_set = ssam_platform_profile_set;
+
+	set_bit(PLATFORM_PROFILE_LOW_POWER, tpd->handler.choices);
+	set_bit(PLATFORM_PROFILE_BALANCED, tpd->handler.choices);
+	set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE, tpd->handler.choices);
+	set_bit(PLATFORM_PROFILE_PERFORMANCE, tpd->handler.choices);
+
+	platform_profile_register(&tpd->handler);
+	return 0;
+}
+
+static void surface_platform_profile_remove(struct ssam_device *sdev)
+{
+	platform_profile_remove();
+}
+
+static const struct ssam_device_id ssam_platform_profile_match[] = {
+	{ SSAM_SDEV(TMP, 0x01, 0x00, 0x01) },
+	{ },
+};
+MODULE_DEVICE_TABLE(ssam, ssam_platform_profile_match);
+
+static struct ssam_device_driver surface_platform_profile = {
+	.probe = surface_platform_profile_probe,
+	.remove = surface_platform_profile_remove,
+	.match_table = ssam_platform_profile_match,
+	.driver = {
+		.name = "surface_platform_profile",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
+};
+module_ssam_device_driver(surface_platform_profile);
+
+MODULE_AUTHOR("Maximilian Luz <luzmaximilian@gmail.com>");
+MODULE_DESCRIPTION("Platform Profile Support for Surface System Aggregator Module");
+MODULE_LICENSE("GPL");
-- 
2.30.0


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

* Re: [External] [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices
  2021-02-11 20:16 [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices Maximilian Luz
                   ` (3 preceding siblings ...)
  2021-02-11 20:17 ` [PATCH v2 4/4] platform/surface: Add platform profile driver Maximilian Luz
@ 2021-02-12 15:29 ` Mark Pearson
  2021-02-15 14:34 ` Hans de Goede
  5 siblings, 0 replies; 15+ messages in thread
From: Mark Pearson @ 2021-02-12 15:29 UTC (permalink / raw)
  To: Maximilian Luz, Hans de Goede, Rafael J. Wysocki
  Cc: Mark Gross, Len Brown, Jiaxun Yang, platform-driver-x86,
	linux-acpi, linux-kernel

On 11/02/2021 15:16, Maximilian Luz wrote:
> This series adds a driver to provide platform profile support on 5th-
> and later generation Microsoft Surface devices with a Surface System
> Aggregator Module. On those devices, the platform profile can be used to
> influence cooling behavior and power consumption.
> 
> To achieve this, a new platform profile is introduced: the
> 'balanced-performance' profile.
> 
> In addition, a couple of fix-ups are performed:
> - Hide CONFIG_ACPI_PLATFORM_PROFILE and change drivers so that it is
>   selected instead of depended on.
> - Fix some references to documentation in a comment.
> 
> Note: This series (or more specifically "platform/surface: Add platform
> profile driver") depends on the "platform/surface: Add Surface
> Aggregator device registry" series.
> 
> Changes in v2:
>  - Introduce new 'balanced-performance' platform profile and change
>    profile mapping in driver.
>  - Perform some fix-ups for the ACPI platform profile implementation:
>    - Fix some references to documentation in a comment.
>    - Hide CONFIG_ACPI_PLATFORM_PROFILE
> 
> Maximilian Luz (4):
>   ACPI: platform: Hide ACPI_PLATFORM_PROFILE option
>   ACPI: platform: Fix file references in comment
>   ACPI: platform: Add balanced-performance platform profile
>   platform/surface: Add platform profile driver
> 
>  .../ABI/testing/sysfs-platform_profile        |  18 +-
>  MAINTAINERS                                   |   6 +
>  drivers/acpi/Kconfig                          |  16 +-
>  drivers/acpi/platform_profile.c               |   1 +
>  drivers/platform/surface/Kconfig              |  22 ++
>  drivers/platform/surface/Makefile             |   1 +
>  .../surface/surface_platform_profile.c        | 190 ++++++++++++++++++
>  drivers/platform/x86/Kconfig                  |   4 +-
>  include/linux/platform_profile.h              |   6 +-
>  9 files changed, 237 insertions(+), 27 deletions(-)
>  create mode 100644 drivers/platform/surface/surface_platform_profile.c
> 
I looked through the patch series and it all looked good to me.
Glad the platform profile implementation is getting used in more places :)

Thanks
Mark

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

* Re: [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices
  2021-02-11 20:16 [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices Maximilian Luz
                   ` (4 preceding siblings ...)
  2021-02-12 15:29 ` [External] [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices Mark Pearson
@ 2021-02-15 14:34 ` Hans de Goede
  2021-02-15 14:54   ` Rafael J. Wysocki
  5 siblings, 1 reply; 15+ messages in thread
From: Hans de Goede @ 2021-02-15 14:34 UTC (permalink / raw)
  To: Maximilian Luz, Rafael J. Wysocki
  Cc: Mark Gross, Len Brown, Mark Pearson, Jiaxun Yang,
	platform-driver-x86, linux-acpi, linux-kernel

Hi,

On 2/11/21 9:16 PM, Maximilian Luz wrote:
> This series adds a driver to provide platform profile support on 5th-
> and later generation Microsoft Surface devices with a Surface System
> Aggregator Module. On those devices, the platform profile can be used to
> influence cooling behavior and power consumption.
> 
> To achieve this, a new platform profile is introduced: the
> 'balanced-performance' profile.
> 
> In addition, a couple of fix-ups are performed:
> - Hide CONFIG_ACPI_PLATFORM_PROFILE and change drivers so that it is
>   selected instead of depended on.
> - Fix some references to documentation in a comment.
> 
> Note: This series (or more specifically "platform/surface: Add platform
> profile driver") depends on the "platform/surface: Add Surface
> Aggregator device registry" series.
> 
> Changes in v2:
>  - Introduce new 'balanced-performance' platform profile and change
>    profile mapping in driver.
>  - Perform some fix-ups for the ACPI platform profile implementation:
>    - Fix some references to documentation in a comment.
>    - Hide CONFIG_ACPI_PLATFORM_PROFILE

Thanks, the entire series looks good to me, so for the series:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Rafael, can you (once 5.12-rc1 is out) pick 1-3/4 and then provide a
stable branch for me to merge?

Then I will pick up 4/4.

Regards,

Hans




> 
> Maximilian Luz (4):
>   ACPI: platform: Hide ACPI_PLATFORM_PROFILE option
>   ACPI: platform: Fix file references in comment
>   ACPI: platform: Add balanced-performance platform profile
>   platform/surface: Add platform profile driver
> 
>  .../ABI/testing/sysfs-platform_profile        |  18 +-
>  MAINTAINERS                                   |   6 +
>  drivers/acpi/Kconfig                          |  16 +-
>  drivers/acpi/platform_profile.c               |   1 +
>  drivers/platform/surface/Kconfig              |  22 ++
>  drivers/platform/surface/Makefile             |   1 +
>  .../surface/surface_platform_profile.c        | 190 ++++++++++++++++++
>  drivers/platform/x86/Kconfig                  |   4 +-
>  include/linux/platform_profile.h              |   6 +-
>  9 files changed, 237 insertions(+), 27 deletions(-)
>  create mode 100644 drivers/platform/surface/surface_platform_profile.c
> 


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

* Re: [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices
  2021-02-15 14:34 ` Hans de Goede
@ 2021-02-15 14:54   ` Rafael J. Wysocki
  2021-02-15 14:56     ` Hans de Goede
  2021-02-15 15:22     ` Hans de Goede
  0 siblings, 2 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2021-02-15 14:54 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Maximilian Luz, Rafael J. Wysocki, Mark Gross, Len Brown,
	Mark Pearson, Jiaxun Yang, Platform Driver,
	ACPI Devel Maling List, Linux Kernel Mailing List

On Mon, Feb 15, 2021 at 3:36 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 2/11/21 9:16 PM, Maximilian Luz wrote:
> > This series adds a driver to provide platform profile support on 5th-
> > and later generation Microsoft Surface devices with a Surface System
> > Aggregator Module. On those devices, the platform profile can be used to
> > influence cooling behavior and power consumption.
> >
> > To achieve this, a new platform profile is introduced: the
> > 'balanced-performance' profile.
> >
> > In addition, a couple of fix-ups are performed:
> > - Hide CONFIG_ACPI_PLATFORM_PROFILE and change drivers so that it is
> >   selected instead of depended on.
> > - Fix some references to documentation in a comment.
> >
> > Note: This series (or more specifically "platform/surface: Add platform
> > profile driver") depends on the "platform/surface: Add Surface
> > Aggregator device registry" series.
> >
> > Changes in v2:
> >  - Introduce new 'balanced-performance' platform profile and change
> >    profile mapping in driver.
> >  - Perform some fix-ups for the ACPI platform profile implementation:
> >    - Fix some references to documentation in a comment.
> >    - Hide CONFIG_ACPI_PLATFORM_PROFILE
>
> Thanks, the entire series looks good to me, so for the series:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>
> Rafael, can you (once 5.12-rc1 is out) pick 1-3/4 and then provide a
> stable branch for me to merge?

Since [1-3/4] appear to be uncontroversial, so IMO it would be better
to merge them during the merge window, so they are present in
5.12-rc1.

The extra stable branch wouldn't be necessary in that case.

> Then I will pick up 4/4.

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

* Re: [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices
  2021-02-15 14:54   ` Rafael J. Wysocki
@ 2021-02-15 14:56     ` Hans de Goede
  2021-02-15 15:22     ` Hans de Goede
  1 sibling, 0 replies; 15+ messages in thread
From: Hans de Goede @ 2021-02-15 14:56 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Maximilian Luz, Rafael J. Wysocki, Mark Gross, Len Brown,
	Mark Pearson, Jiaxun Yang, Platform Driver,
	ACPI Devel Maling List, Linux Kernel Mailing List

Hi,

On 2/15/21 3:54 PM, Rafael J. Wysocki wrote:
> On Mon, Feb 15, 2021 at 3:36 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 2/11/21 9:16 PM, Maximilian Luz wrote:
>>> This series adds a driver to provide platform profile support on 5th-
>>> and later generation Microsoft Surface devices with a Surface System
>>> Aggregator Module. On those devices, the platform profile can be used to
>>> influence cooling behavior and power consumption.
>>>
>>> To achieve this, a new platform profile is introduced: the
>>> 'balanced-performance' profile.
>>>
>>> In addition, a couple of fix-ups are performed:
>>> - Hide CONFIG_ACPI_PLATFORM_PROFILE and change drivers so that it is
>>>   selected instead of depended on.
>>> - Fix some references to documentation in a comment.
>>>
>>> Note: This series (or more specifically "platform/surface: Add platform
>>> profile driver") depends on the "platform/surface: Add Surface
>>> Aggregator device registry" series.
>>>
>>> Changes in v2:
>>>  - Introduce new 'balanced-performance' platform profile and change
>>>    profile mapping in driver.
>>>  - Perform some fix-ups for the ACPI platform profile implementation:
>>>    - Fix some references to documentation in a comment.
>>>    - Hide CONFIG_ACPI_PLATFORM_PROFILE
>>
>> Thanks, the entire series looks good to me, so for the series:
>>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>>
>> Rafael, can you (once 5.12-rc1 is out) pick 1-3/4 and then provide a
>> stable branch for me to merge?
> 
> Since [1-3/4] appear to be uncontroversial, so IMO it would be better
> to merge them during the merge window, so they are present in
> 5.12-rc1.
> 
> The extra stable branch wouldn't be necessary in that case.

That works for me, thanks.

Regards,

Hans




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

* Re: [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices
  2021-02-15 14:54   ` Rafael J. Wysocki
  2021-02-15 14:56     ` Hans de Goede
@ 2021-02-15 15:22     ` Hans de Goede
  2021-02-15 15:29       ` Rafael J. Wysocki
  1 sibling, 1 reply; 15+ messages in thread
From: Hans de Goede @ 2021-02-15 15:22 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Maximilian Luz, Rafael J. Wysocki, Mark Gross, Len Brown,
	Mark Pearson, Jiaxun Yang, Platform Driver,
	ACPI Devel Maling List, Linux Kernel Mailing List

Hi,

On 2/15/21 3:54 PM, Rafael J. Wysocki wrote:
> On Mon, Feb 15, 2021 at 3:36 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 2/11/21 9:16 PM, Maximilian Luz wrote:
>>> This series adds a driver to provide platform profile support on 5th-
>>> and later generation Microsoft Surface devices with a Surface System
>>> Aggregator Module. On those devices, the platform profile can be used to
>>> influence cooling behavior and power consumption.
>>>
>>> To achieve this, a new platform profile is introduced: the
>>> 'balanced-performance' profile.
>>>
>>> In addition, a couple of fix-ups are performed:
>>> - Hide CONFIG_ACPI_PLATFORM_PROFILE and change drivers so that it is
>>>   selected instead of depended on.
>>> - Fix some references to documentation in a comment.
>>>
>>> Note: This series (or more specifically "platform/surface: Add platform
>>> profile driver") depends on the "platform/surface: Add Surface
>>> Aggregator device registry" series.
>>>
>>> Changes in v2:
>>>  - Introduce new 'balanced-performance' platform profile and change
>>>    profile mapping in driver.
>>>  - Perform some fix-ups for the ACPI platform profile implementation:
>>>    - Fix some references to documentation in a comment.
>>>    - Hide CONFIG_ACPI_PLATFORM_PROFILE
>>
>> Thanks, the entire series looks good to me, so for the series:
>>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>>
>> Rafael, can you (once 5.12-rc1 is out) pick 1-3/4 and then provide a
>> stable branch for me to merge?
> 
> Since [1-3/4] appear to be uncontroversial, so IMO it would be better
> to merge them during the merge window, so they are present in
> 5.12-rc1.

So I just realized one problem with this plan, patch 1/4 depends
on (modifies) Kconfig bits which are only in my tree / my 5.12 pull-req
(which I send out earlier today).

One option here would be for me pickup 1-3/4 and send a second
pull-req during the merge-window, with your ack as these are
touching files under drivers/acpi ?

Regards,

Hans


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

* Re: [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices
  2021-02-15 15:22     ` Hans de Goede
@ 2021-02-15 15:29       ` Rafael J. Wysocki
  2021-02-15 16:36         ` Hans de Goede
  0 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2021-02-15 15:29 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J. Wysocki, Maximilian Luz, Rafael J. Wysocki, Mark Gross,
	Len Brown, Mark Pearson, Jiaxun Yang, Platform Driver,
	ACPI Devel Maling List, Linux Kernel Mailing List

On Mon, Feb 15, 2021 at 4:22 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 2/15/21 3:54 PM, Rafael J. Wysocki wrote:
> > On Mon, Feb 15, 2021 at 3:36 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi,
> >>
> >> On 2/11/21 9:16 PM, Maximilian Luz wrote:
> >>> This series adds a driver to provide platform profile support on 5th-
> >>> and later generation Microsoft Surface devices with a Surface System
> >>> Aggregator Module. On those devices, the platform profile can be used to
> >>> influence cooling behavior and power consumption.
> >>>
> >>> To achieve this, a new platform profile is introduced: the
> >>> 'balanced-performance' profile.
> >>>
> >>> In addition, a couple of fix-ups are performed:
> >>> - Hide CONFIG_ACPI_PLATFORM_PROFILE and change drivers so that it is
> >>>   selected instead of depended on.
> >>> - Fix some references to documentation in a comment.
> >>>
> >>> Note: This series (or more specifically "platform/surface: Add platform
> >>> profile driver") depends on the "platform/surface: Add Surface
> >>> Aggregator device registry" series.
> >>>
> >>> Changes in v2:
> >>>  - Introduce new 'balanced-performance' platform profile and change
> >>>    profile mapping in driver.
> >>>  - Perform some fix-ups for the ACPI platform profile implementation:
> >>>    - Fix some references to documentation in a comment.
> >>>    - Hide CONFIG_ACPI_PLATFORM_PROFILE
> >>
> >> Thanks, the entire series looks good to me, so for the series:
> >>
> >> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> >>
> >> Rafael, can you (once 5.12-rc1 is out) pick 1-3/4 and then provide a
> >> stable branch for me to merge?
> >
> > Since [1-3/4] appear to be uncontroversial, so IMO it would be better
> > to merge them during the merge window, so they are present in
> > 5.12-rc1.
>
> So I just realized one problem with this plan, patch 1/4 depends
> on (modifies) Kconfig bits which are only in my tree / my 5.12 pull-req
> (which I send out earlier today).

That should be fine.

I will be sending the first batch of pull requests tomorrow.  Then I
will wait for them to be merged and I will merge the mainline back at
that point.  The new patches will be applied on top of that merge, so
if your 5.12 material is included in it, they should build without
problems.

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

* Re: [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices
  2021-02-15 15:29       ` Rafael J. Wysocki
@ 2021-02-15 16:36         ` Hans de Goede
  2021-02-15 18:40           ` Rafael J. Wysocki
  2021-02-24 14:13           ` Rafael J. Wysocki
  0 siblings, 2 replies; 15+ messages in thread
From: Hans de Goede @ 2021-02-15 16:36 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Maximilian Luz, Rafael J. Wysocki, Mark Gross, Len Brown,
	Mark Pearson, Jiaxun Yang, Platform Driver,
	ACPI Devel Maling List, Linux Kernel Mailing List

Hi,

On 2/15/21 4:29 PM, Rafael J. Wysocki wrote:
> On Mon, Feb 15, 2021 at 4:22 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 2/15/21 3:54 PM, Rafael J. Wysocki wrote:
>>> On Mon, Feb 15, 2021 at 3:36 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 2/11/21 9:16 PM, Maximilian Luz wrote:
>>>>> This series adds a driver to provide platform profile support on 5th-
>>>>> and later generation Microsoft Surface devices with a Surface System
>>>>> Aggregator Module. On those devices, the platform profile can be used to
>>>>> influence cooling behavior and power consumption.
>>>>>
>>>>> To achieve this, a new platform profile is introduced: the
>>>>> 'balanced-performance' profile.
>>>>>
>>>>> In addition, a couple of fix-ups are performed:
>>>>> - Hide CONFIG_ACPI_PLATFORM_PROFILE and change drivers so that it is
>>>>>   selected instead of depended on.
>>>>> - Fix some references to documentation in a comment.
>>>>>
>>>>> Note: This series (or more specifically "platform/surface: Add platform
>>>>> profile driver") depends on the "platform/surface: Add Surface
>>>>> Aggregator device registry" series.
>>>>>
>>>>> Changes in v2:
>>>>>  - Introduce new 'balanced-performance' platform profile and change
>>>>>    profile mapping in driver.
>>>>>  - Perform some fix-ups for the ACPI platform profile implementation:
>>>>>    - Fix some references to documentation in a comment.
>>>>>    - Hide CONFIG_ACPI_PLATFORM_PROFILE
>>>>
>>>> Thanks, the entire series looks good to me, so for the series:
>>>>
>>>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>>>>
>>>> Rafael, can you (once 5.12-rc1 is out) pick 1-3/4 and then provide a
>>>> stable branch for me to merge?
>>>
>>> Since [1-3/4] appear to be uncontroversial, so IMO it would be better
>>> to merge them during the merge window, so they are present in
>>> 5.12-rc1.
>>
>> So I just realized one problem with this plan, patch 1/4 depends
>> on (modifies) Kconfig bits which are only in my tree / my 5.12 pull-req
>> (which I send out earlier today).
> 
> That should be fine.
> 
> I will be sending the first batch of pull requests tomorrow.  Then I
> will wait for them to be merged and I will merge the mainline back at
> that point.  The new patches will be applied on top of that merge, so
> if your 5.12 material is included in it, they should build without
> problems.

Ok, that sounds good to me.

Regards,

Hans


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

* Re: [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices
  2021-02-15 16:36         ` Hans de Goede
@ 2021-02-15 18:40           ` Rafael J. Wysocki
  2021-02-24 14:13           ` Rafael J. Wysocki
  1 sibling, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2021-02-15 18:40 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J. Wysocki, Maximilian Luz, Rafael J. Wysocki, Mark Gross,
	Len Brown, Mark Pearson, Jiaxun Yang, Platform Driver,
	ACPI Devel Maling List, Linux Kernel Mailing List

On Mon, Feb 15, 2021 at 5:36 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 2/15/21 4:29 PM, Rafael J. Wysocki wrote:
> > On Mon, Feb 15, 2021 at 4:22 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi,
> >>
> >> On 2/15/21 3:54 PM, Rafael J. Wysocki wrote:
> >>> On Mon, Feb 15, 2021 at 3:36 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 2/11/21 9:16 PM, Maximilian Luz wrote:
> >>>>> This series adds a driver to provide platform profile support on 5th-
> >>>>> and later generation Microsoft Surface devices with a Surface System
> >>>>> Aggregator Module. On those devices, the platform profile can be used to
> >>>>> influence cooling behavior and power consumption.
> >>>>>
> >>>>> To achieve this, a new platform profile is introduced: the
> >>>>> 'balanced-performance' profile.
> >>>>>
> >>>>> In addition, a couple of fix-ups are performed:
> >>>>> - Hide CONFIG_ACPI_PLATFORM_PROFILE and change drivers so that it is
> >>>>>   selected instead of depended on.
> >>>>> - Fix some references to documentation in a comment.
> >>>>>
> >>>>> Note: This series (or more specifically "platform/surface: Add platform
> >>>>> profile driver") depends on the "platform/surface: Add Surface
> >>>>> Aggregator device registry" series.
> >>>>>
> >>>>> Changes in v2:
> >>>>>  - Introduce new 'balanced-performance' platform profile and change
> >>>>>    profile mapping in driver.
> >>>>>  - Perform some fix-ups for the ACPI platform profile implementation:
> >>>>>    - Fix some references to documentation in a comment.
> >>>>>    - Hide CONFIG_ACPI_PLATFORM_PROFILE
> >>>>
> >>>> Thanks, the entire series looks good to me, so for the series:
> >>>>
> >>>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> >>>>
> >>>> Rafael, can you (once 5.12-rc1 is out) pick 1-3/4 and then provide a
> >>>> stable branch for me to merge?
> >>>
> >>> Since [1-3/4] appear to be uncontroversial, so IMO it would be better
> >>> to merge them during the merge window, so they are present in
> >>> 5.12-rc1.
> >>
> >> So I just realized one problem with this plan, patch 1/4 depends
> >> on (modifies) Kconfig bits which are only in my tree / my 5.12 pull-req
> >> (which I send out earlier today).
> >
> > That should be fine.
> >
> > I will be sending the first batch of pull requests tomorrow.  Then I
> > will wait for them to be merged and I will merge the mainline back at
> > that point.  The new patches will be applied on top of that merge, so
> > if your 5.12 material is included in it, they should build without
> > problems.
>
> Ok, that sounds good to me.

In fact, my pull requests are ready right now, so I will be sending
them shortly, but that doesn\t change the subsequent steps.

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

* Re: [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices
  2021-02-15 16:36         ` Hans de Goede
  2021-02-15 18:40           ` Rafael J. Wysocki
@ 2021-02-24 14:13           ` Rafael J. Wysocki
  1 sibling, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2021-02-24 14:13 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J. Wysocki, Maximilian Luz, Rafael J. Wysocki, Mark Gross,
	Len Brown, Mark Pearson, Jiaxun Yang, Platform Driver,
	ACPI Devel Maling List, Linux Kernel Mailing List

On Mon, Feb 15, 2021 at 5:36 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 2/15/21 4:29 PM, Rafael J. Wysocki wrote:
> > On Mon, Feb 15, 2021 at 4:22 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi,
> >>
> >> On 2/15/21 3:54 PM, Rafael J. Wysocki wrote:
> >>> On Mon, Feb 15, 2021 at 3:36 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 2/11/21 9:16 PM, Maximilian Luz wrote:
> >>>>> This series adds a driver to provide platform profile support on 5th-
> >>>>> and later generation Microsoft Surface devices with a Surface System
> >>>>> Aggregator Module. On those devices, the platform profile can be used to
> >>>>> influence cooling behavior and power consumption.
> >>>>>
> >>>>> To achieve this, a new platform profile is introduced: the
> >>>>> 'balanced-performance' profile.
> >>>>>
> >>>>> In addition, a couple of fix-ups are performed:
> >>>>> - Hide CONFIG_ACPI_PLATFORM_PROFILE and change drivers so that it is
> >>>>>   selected instead of depended on.
> >>>>> - Fix some references to documentation in a comment.
> >>>>>
> >>>>> Note: This series (or more specifically "platform/surface: Add platform
> >>>>> profile driver") depends on the "platform/surface: Add Surface
> >>>>> Aggregator device registry" series.
> >>>>>
> >>>>> Changes in v2:
> >>>>>  - Introduce new 'balanced-performance' platform profile and change
> >>>>>    profile mapping in driver.
> >>>>>  - Perform some fix-ups for the ACPI platform profile implementation:
> >>>>>    - Fix some references to documentation in a comment.
> >>>>>    - Hide CONFIG_ACPI_PLATFORM_PROFILE
> >>>>
> >>>> Thanks, the entire series looks good to me, so for the series:
> >>>>
> >>>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> >>>>
> >>>> Rafael, can you (once 5.12-rc1 is out) pick 1-3/4 and then provide a
> >>>> stable branch for me to merge?
> >>>
> >>> Since [1-3/4] appear to be uncontroversial, so IMO it would be better
> >>> to merge them during the merge window, so they are present in
> >>> 5.12-rc1.
> >>
> >> So I just realized one problem with this plan, patch 1/4 depends
> >> on (modifies) Kconfig bits which are only in my tree / my 5.12 pull-req
> >> (which I send out earlier today).
> >
> > That should be fine.
> >
> > I will be sending the first batch of pull requests tomorrow.  Then I
> > will wait for them to be merged and I will merge the mainline back at
> > that point.  The new patches will be applied on top of that merge, so
> > if your 5.12 material is included in it, they should build without
> > problems.
>
> Ok, that sounds good to me.

The [1-3/4] have just been applied (as 5.12-rc material), thanks!

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

* Re: [PATCH v2 4/4] platform/surface: Add platform profile driver
  2021-02-11 20:17 ` [PATCH v2 4/4] platform/surface: Add platform profile driver Maximilian Luz
@ 2021-03-04 11:01   ` Hans de Goede
  0 siblings, 0 replies; 15+ messages in thread
From: Hans de Goede @ 2021-03-04 11:01 UTC (permalink / raw)
  To: Maximilian Luz, Rafael J. Wysocki
  Cc: Mark Gross, Len Brown, Mark Pearson, Jiaxun Yang,
	platform-driver-x86, linux-acpi, linux-kernel

Hi,

On 2/11/21 9:17 PM, Maximilian Luz wrote:
> Add a driver to provide platform profile support on 5th- and later
> generation Microsoft Surface devices with a Surface System Aggregator
> Module. On those devices, the platform profile can be used to influence
> cooling behavior and power consumption.
> 
> For example, the default 'quiet' profile limits fan noise and in turn
> sacrifices performance of the discrete GPU found on Surface Books. Its
> full performance can only be unlocked on the 'performance' profile.
> 
> Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans



> ---
>  MAINTAINERS                                   |   6 +
>  drivers/platform/surface/Kconfig              |  22 ++
>  drivers/platform/surface/Makefile             |   1 +
>  .../surface/surface_platform_profile.c        | 190 ++++++++++++++++++
>  4 files changed, 219 insertions(+)
>  create mode 100644 drivers/platform/surface/surface_platform_profile.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 000a82f59c76..a08d65f8f0df 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11811,6 +11811,12 @@ L:	platform-driver-x86@vger.kernel.org
>  S:	Maintained
>  F:	drivers/platform/surface/surface_hotplug.c
>  
> +MICROSOFT SURFACE PLATFORM PROFILE DRIVER
> +M:	Maximilian Luz <luzmaximilian@gmail.com>
> +L:	platform-driver-x86@vger.kernel.org
> +S:	Maintained
> +F:	drivers/platform/surface/surface_platform_profile.c
> +
>  MICROSOFT SURFACE PRO 3 BUTTON DRIVER
>  M:	Chen Yu <yu.c.chen@intel.com>
>  L:	platform-driver-x86@vger.kernel.org
> diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig
> index 179b8c93d7fd..a045425026ae 100644
> --- a/drivers/platform/surface/Kconfig
> +++ b/drivers/platform/surface/Kconfig
> @@ -132,6 +132,28 @@ config SURFACE_HOTPLUG
>  	  Select M or Y here, if you want to (fully) support hot-plugging of
>  	  dGPU devices on the Surface Book 2 and/or 3 during D3cold.
>  
> +config SURFACE_PLATFORM_PROFILE
> +	tristate "Surface Platform Profile Driver"
> +	depends on SURFACE_AGGREGATOR_REGISTRY
> +	select ACPI_PLATFORM_PROFILE
> +	help
> +	  Provides support for the ACPI platform profile on 5th- and later
> +	  generation Microsoft Surface devices.
> +
> +	  More specifically, this driver provides ACPI platform profile support
> +	  on Microsoft Surface devices with a Surface System Aggregator Module
> +	  (SSAM) connected via the Surface Serial Hub (SSH / SAM-over-SSH). In
> +	  other words, this driver provides platform profile support on the
> +	  Surface Pro 5, Surface Book 2, Surface Laptop, Surface Laptop Go and
> +	  later. On those devices, the platform profile can significantly
> +	  influence cooling behavior, e.g. setting it to 'quiet' (default) or
> +	  'low-power' can significantly limit performance of the discrete GPU on
> +	  Surface Books, while in turn leading to lower power consumption and/or
> +	  less fan noise.
> +
> +	  Select M or Y here, if you want to include ACPI platform profile
> +	  support on the above mentioned devices.
> +
>  config SURFACE_PRO3_BUTTON
>  	tristate "Power/home/volume buttons driver for Microsoft Surface Pro 3/4 tablet"
>  	depends on INPUT
> diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile
> index 80035ee540bf..99372c427b73 100644
> --- a/drivers/platform/surface/Makefile
> +++ b/drivers/platform/surface/Makefile
> @@ -13,4 +13,5 @@ obj-$(CONFIG_SURFACE_AGGREGATOR_CDEV)	+= surface_aggregator_cdev.o
>  obj-$(CONFIG_SURFACE_AGGREGATOR_REGISTRY) += surface_aggregator_registry.o
>  obj-$(CONFIG_SURFACE_GPE)		+= surface_gpe.o
>  obj-$(CONFIG_SURFACE_HOTPLUG)		+= surface_hotplug.o
> +obj-$(CONFIG_SURFACE_PLATFORM_PROFILE)	+= surface_platform_profile.o
>  obj-$(CONFIG_SURFACE_PRO3_BUTTON)	+= surfacepro3_button.o
> diff --git a/drivers/platform/surface/surface_platform_profile.c b/drivers/platform/surface/surface_platform_profile.c
> new file mode 100644
> index 000000000000..0081b01a5b0f
> --- /dev/null
> +++ b/drivers/platform/surface/surface_platform_profile.c
> @@ -0,0 +1,190 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Surface Platform Profile / Performance Mode driver for Surface System
> + * Aggregator Module (thermal subsystem).
> + *
> + * Copyright (C) 2021 Maximilian Luz <luzmaximilian@gmail.com>
> + */
> +
> +#include <asm/unaligned.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_profile.h>
> +#include <linux/types.h>
> +
> +#include <linux/surface_aggregator/device.h>
> +
> +enum ssam_tmp_profile {
> +	SSAM_TMP_PROFILE_NORMAL             = 1,
> +	SSAM_TMP_PROFILE_BATTERY_SAVER      = 2,
> +	SSAM_TMP_PROFILE_BETTER_PERFORMANCE = 3,
> +	SSAM_TMP_PROFILE_BEST_PERFORMANCE   = 4,
> +};
> +
> +struct ssam_tmp_profile_info {
> +	__le32 profile;
> +	__le16 unknown1;
> +	__le16 unknown2;
> +} __packed;
> +
> +struct ssam_tmp_profile_device {
> +	struct ssam_device *sdev;
> +	struct platform_profile_handler handler;
> +};
> +
> +static SSAM_DEFINE_SYNC_REQUEST_CL_R(__ssam_tmp_profile_get, struct ssam_tmp_profile_info, {
> +	.target_category = SSAM_SSH_TC_TMP,
> +	.command_id      = 0x02,
> +});
> +
> +static SSAM_DEFINE_SYNC_REQUEST_CL_W(__ssam_tmp_profile_set, __le32, {
> +	.target_category = SSAM_SSH_TC_TMP,
> +	.command_id      = 0x03,
> +});
> +
> +static int ssam_tmp_profile_get(struct ssam_device *sdev, enum ssam_tmp_profile *p)
> +{
> +	struct ssam_tmp_profile_info info;
> +	int status;
> +
> +	status = ssam_retry(__ssam_tmp_profile_get, sdev, &info);
> +	if (status < 0)
> +		return status;
> +
> +	*p = le32_to_cpu(info.profile);
> +	return 0;
> +}
> +
> +static int ssam_tmp_profile_set(struct ssam_device *sdev, enum ssam_tmp_profile p)
> +{
> +	__le32 profile_le = cpu_to_le32(p);
> +
> +	return ssam_retry(__ssam_tmp_profile_set, sdev, &profile_le);
> +}
> +
> +static int convert_ssam_to_profile(struct ssam_device *sdev, enum ssam_tmp_profile p)
> +{
> +	switch (p) {
> +	case SSAM_TMP_PROFILE_NORMAL:
> +		return PLATFORM_PROFILE_BALANCED;
> +
> +	case SSAM_TMP_PROFILE_BATTERY_SAVER:
> +		return PLATFORM_PROFILE_LOW_POWER;
> +
> +	case SSAM_TMP_PROFILE_BETTER_PERFORMANCE:
> +		return PLATFORM_PROFILE_BALANCED_PERFORMANCE;
> +
> +	case SSAM_TMP_PROFILE_BEST_PERFORMANCE:
> +		return PLATFORM_PROFILE_PERFORMANCE;
> +
> +	default:
> +		dev_err(&sdev->dev, "invalid performance profile: %d", p);
> +		return -EINVAL;
> +	}
> +}
> +
> +static int convert_profile_to_ssam(struct ssam_device *sdev, enum platform_profile_option p)
> +{
> +	switch (p) {
> +	case PLATFORM_PROFILE_LOW_POWER:
> +		return SSAM_TMP_PROFILE_BATTERY_SAVER;
> +
> +	case PLATFORM_PROFILE_BALANCED:
> +		return SSAM_TMP_PROFILE_NORMAL;
> +
> +	case PLATFORM_PROFILE_BALANCED_PERFORMANCE:
> +		return SSAM_TMP_PROFILE_BETTER_PERFORMANCE;
> +
> +	case PLATFORM_PROFILE_PERFORMANCE:
> +		return SSAM_TMP_PROFILE_BEST_PERFORMANCE;
> +
> +	default:
> +		/* This should have already been caught by platform_profile_store(). */
> +		WARN(true, "unsupported platform profile");
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int ssam_platform_profile_get(struct platform_profile_handler *pprof,
> +				     enum platform_profile_option *profile)
> +{
> +	struct ssam_tmp_profile_device *tpd;
> +	enum ssam_tmp_profile tp;
> +	int status;
> +
> +	tpd = container_of(pprof, struct ssam_tmp_profile_device, handler);
> +
> +	status = ssam_tmp_profile_get(tpd->sdev, &tp);
> +	if (status)
> +		return status;
> +
> +	status = convert_ssam_to_profile(tpd->sdev, tp);
> +	if (status < 0)
> +		return status;
> +
> +	*profile = status;
> +	return 0;
> +}
> +
> +static int ssam_platform_profile_set(struct platform_profile_handler *pprof,
> +				     enum platform_profile_option profile)
> +{
> +	struct ssam_tmp_profile_device *tpd;
> +	int tp;
> +
> +	tpd = container_of(pprof, struct ssam_tmp_profile_device, handler);
> +
> +	tp = convert_profile_to_ssam(tpd->sdev, profile);
> +	if (tp < 0)
> +		return tp;
> +
> +	return ssam_tmp_profile_set(tpd->sdev, tp);
> +}
> +
> +static int surface_platform_profile_probe(struct ssam_device *sdev)
> +{
> +	struct ssam_tmp_profile_device *tpd;
> +
> +	tpd = devm_kzalloc(&sdev->dev, sizeof(*tpd), GFP_KERNEL);
> +	if (!tpd)
> +		return -ENOMEM;
> +
> +	tpd->sdev = sdev;
> +
> +	tpd->handler.profile_get = ssam_platform_profile_get;
> +	tpd->handler.profile_set = ssam_platform_profile_set;
> +
> +	set_bit(PLATFORM_PROFILE_LOW_POWER, tpd->handler.choices);
> +	set_bit(PLATFORM_PROFILE_BALANCED, tpd->handler.choices);
> +	set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE, tpd->handler.choices);
> +	set_bit(PLATFORM_PROFILE_PERFORMANCE, tpd->handler.choices);
> +
> +	platform_profile_register(&tpd->handler);
> +	return 0;
> +}
> +
> +static void surface_platform_profile_remove(struct ssam_device *sdev)
> +{
> +	platform_profile_remove();
> +}
> +
> +static const struct ssam_device_id ssam_platform_profile_match[] = {
> +	{ SSAM_SDEV(TMP, 0x01, 0x00, 0x01) },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(ssam, ssam_platform_profile_match);
> +
> +static struct ssam_device_driver surface_platform_profile = {
> +	.probe = surface_platform_profile_probe,
> +	.remove = surface_platform_profile_remove,
> +	.match_table = ssam_platform_profile_match,
> +	.driver = {
> +		.name = "surface_platform_profile",
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
> +	},
> +};
> +module_ssam_device_driver(surface_platform_profile);
> +
> +MODULE_AUTHOR("Maximilian Luz <luzmaximilian@gmail.com>");
> +MODULE_DESCRIPTION("Platform Profile Support for Surface System Aggregator Module");
> +MODULE_LICENSE("GPL");
> 


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

end of thread, other threads:[~2021-03-04 11:04 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-11 20:16 [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices Maximilian Luz
2021-02-11 20:17 ` [PATCH v2 1/4] ACPI: platform: Hide ACPI_PLATFORM_PROFILE option Maximilian Luz
2021-02-11 20:17 ` [PATCH v2 2/4] ACPI: platform: Fix file references in comment Maximilian Luz
2021-02-11 20:17 ` [PATCH v2 3/4] ACPI: platform: Add balanced-performance platform profile Maximilian Luz
2021-02-11 20:17 ` [PATCH v2 4/4] platform/surface: Add platform profile driver Maximilian Luz
2021-03-04 11:01   ` Hans de Goede
2021-02-12 15:29 ` [External] [PATCH v2 0/4] platform/surface: Add platform profile driver for Surface devices Mark Pearson
2021-02-15 14:34 ` Hans de Goede
2021-02-15 14:54   ` Rafael J. Wysocki
2021-02-15 14:56     ` Hans de Goede
2021-02-15 15:22     ` Hans de Goede
2021-02-15 15:29       ` Rafael J. Wysocki
2021-02-15 16:36         ` Hans de Goede
2021-02-15 18:40           ` Rafael J. Wysocki
2021-02-24 14:13           ` 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;
as well as URLs for NNTP newsgroup(s).