Re: [PATCH v2 4/4] platform/surface: Add platform profile driver

2021-03-04 Thread Hans de Goede
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 

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-...@vger.kernel.org
>  S:   Maintained
>  F:   drivers/platform/surface/surface_hotplug.c
>  
> +MICROSOFT SURFACE PLATFORM PROFILE DRIVER
> +M:   Maximilian Luz 
> +L:   platform-driver-...@vger.kernel.org
> +S:   Maintained
> +F:   drivers/platform/surface/surface_platform_profile.c
> +
>  MICROSOFT SURFACE PRO 3 BUTTON DRIVER
>  M:   Chen Yu 
>  L:   platform-driver-...@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 ..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 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +enum 

[PATCH v2 4/4] platform/surface: Add platform profile driver

2021-02-11 Thread Maximilian Luz
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 
---
 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-...@vger.kernel.org
 S: Maintained
 F: drivers/platform/surface/surface_hotplug.c
 
+MICROSOFT SURFACE PLATFORM PROFILE DRIVER
+M: Maximilian Luz 
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/surface/surface_platform_profile.c
+
 MICROSOFT SURFACE PRO 3 BUTTON DRIVER
 M: Chen Yu 
 L: platform-driver-...@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 ..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 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+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