[RFC PATCH v1 0/1] platform/x86: panasonic-laptop: add platform_profile support

[RFC PATCH v1 0/1] platform/x86: panasonic-laptop: add platform_profile support

[Alex Yeo]

This RFC patch adds support for mapping firmware thermal and CPU power
operating policy to platform_profile for Panasonic Let's Note laptops.
This originally started as a hwmon and thermal patch, but further
analysis had revealed that these are firmware power and thermal
policies as opposed to fan control.

Panasonic firmware exposes firmware operating policy through two
methods:
 - Fan cooling policy (active/passive)
 - Processor TDP limit (capped/uncapped)

At boot, firmware defaults to an active cooling policy with the
CPU TDP capped. Vendor drivers / DPTF normally unlocks this on
Windows.

These 2 methods map to 4 possible distinct states. Only states that
correspond to the platform_profile ABI are mapped and exposed as
choices. Unmapped states will be returned as "custom" by the driver.

I have tested the following models:
 - CF-RZ6 (2016)
 - CF-SV8 (2019)
 - CF-QV9 (2020)
 - CF-SR4 (2024)

State to platform_profile mapping:

 Profile                         | Fan Mode  | TDP
+--------------------------------+-----------+------------+
 PLATFORM_PROFILE_COOL (SR4 only)| Active    | Locked
 PLATFORM_PROFILE_BALANCED       | Active    | Locked
 PLATFORM_PROFILE_QUIET          | Passive   | Locked
 PLATFORM_PROFILE_PERFORMANCE    | Active    | Unlocked
 CUSTOM (unmapped on all)        | Passive   | Unlocked

Across all of them, the fan and TDP modes are toggled by the same ACPI
methods. However, there is a slight variation in its interpretation based
on testing results.

These methods are normally called by the DPTF/IPF _OSC handshake.
Depending on the capability bits, the firmware will alter its thermal
and power policy.

\_SB.IETM Implementation A:
 - CF-RZ6 (2016)
 - CF-SV8 (2019)

\_SB.IETM Implementation B:
 - CF-QV9 (2020)
 - CF-SR4 (2024)

Models belonging to the same group share what appears to be identical
AML code for the _OSC.

This patch only exposes the ability to read and set these firmware
toggles. The get() callback queries current firmware state and returns
the mapped platform_profile. The set() callback sets the target state
regardless of current state.

Proposed mappings in this patch are defined in the quirks struct.
Activation of platform_profile is gated by DMI checks.

For every model except the SR4, only balanced and performance profiles
are defined. This is because the aggressive fan paired with the locked
TDP on the SR4 more appropriately correspond to the cool profile.

Test results for the proposed mappings are below.

Test results:
================
(CPU PkgWatt: peak / sustained)
(Sysbench CPU: all available threads)

CF-RZ6: (i5-7Y57, Startup default: Balanced)

 Platform Profile |  CPU PkgWatt    |  Sysbench CPU  | Stress Temp (CPU)
+-----------------+-----------------+----------------+-------------------+
 BALANCED         |  10.0W / 9.0W   |    19712       |   57C
 PERFORMANCE      |  18.9W / 14.9W  |    29132       |   72C

CF-SV8: (i5-8365U, Startup default: Balanced)

 Platform Profile |  CPU PkgWatt     |  Sysbench CPU  | Stress Temp (CPU)
+-----------------+------------------+----------------+-------------------+
 BALANCED         |  9.9W  / 9.9W    |    44695       |   58C
 PERFORMANCE      |  29.0W / 19.9W   |    71747       |   81C

CF-QV9: (i5-10310U, Startup default: Balanced)

 Platform Profile |  CPU PkgWatt     |  Sysbench CPU  | Stress Temp (CPU)
+-----------------+------------------+----------------+-------------------+
 BALANCED         |  10.0W / 9.9W    |    46545       |   59C
 PERFORMANCE      |  18.9W / 14.9W   |    57416       |   70C

CF-SR4: (i5-1345U, Startup default: Cool)

 Platform Profile |  CPU PkgWatt     |  Sysbench CPU  | Stress Temp (CPU)
+-----------------+------------------+----------------+-------------------+
 COOL             |  12.0W / 12.0W   |    177139      |   66C
 QUIET            |  12.0W / 12.0W   |    176315      |   73C-75C
 PERFORMANCE      |  29.6W / 21.3W   |    247602      |   98C-100C

CF-SR4 Notes:
 - QUIET: downclock/throttle at 75C, no increase in package_throttle_count
 - PERFORMANCE: stable boost clock, package_throttle_count increase


Other notes:
===============

Additional notes about thermald:
During testing, I have observed that running thermald for the first time
since boot will trigger an _OSC negotiation that will unlock CPU TDP for
new models (IETM Implementation B). Once thermald is running, exits or
runs again, these bits were not observed to change again during testing.

Running thermald on older models (IETM Implementation A) does not touch
the fan curve or TDP mode setting.

Suspend: observed to not alter the values set prior to suspend

Thank you,
Alex

Alex Yeo (1):
  platform/x86: panasonic-laptop: add platform_profile support

 drivers/platform/x86/panasonic-laptop.c | 380 ++++++++++++++++++++++++
 1 file changed, 380 insertions(+)

-- 
2.55.0

[RFC PATCH v1 1/1] platform/x86: panasonic-laptop: add platform_profile support

[Alex Yeo]

Expose firmware thermal and power management policies via the
platform_profile interface.

Supported and tested models:
 - CF-RZ6 (2016)
 - CF-SV8 (2019)
 - CF-QV9 (2020)
 - CF-SR4 (2024)

Firmware fan operating mode and the TDP limit switch are mapped onto
the Linux platform_profile ABI.

This implementation queries the firmware for every get().
CUSTOM is returned when the current state is not recognized.
The set() callback writes the values regardless of what the
current fan mode and TDP limit is.

 - DMI quirks restrict support to tested models
 - Missing includes such as sysfs.h were added based on previous
   feedback.
 - Logging: pr_fmt added
 - When raising TDP, fan mode is switched first
 - When limiting TDP, TDP limit is applied first
 - ACPI handle for the EC device was used as the EC path varies
   slightly between models.

Signed-off-by: Alex Yeo <alexyeo362@gmail.com>
---
 drivers/platform/x86/panasonic-laptop.c | 380 ++++++++++++++++++++++++
 1 file changed, 380 insertions(+)

diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index 719add753cb3..eeb89892768b 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -119,10 +119,16 @@
  *		- v0.1  start from toshiba_acpi driver written by John Belmonte
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/acpi.h>
 #include <linux/backlight.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/ctype.h>
+#include <linux/dmi.h>
+#include <linux/err.h>
+#include <linux/errno.h>
 #include <linux/i8042.h>
 #include <linux/init.h>
 #include <linux/input.h>
@@ -130,13 +136,18 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/platform_profile.h>
+#include <linux/printk.h>
 #include <linux/seq_file.h>
 #include <linux/serio.h>
 #include <linux/slab.h>
+#include <linux/sysfs.h>
 #include <linux/types.h>
 #include <linux/uaccess.h>
 #include <acpi/video.h>
 
+DEFINE_FREE(acpi_dev_put, struct acpi_device *, if (_T) acpi_dev_put(_T))
+
 MODULE_AUTHOR("Hiroshi Miura <miura@da-cha.org>");
 MODULE_AUTHOR("David Bronaugh <dbronaugh@linuxboxen.org>");
 MODULE_AUTHOR("Harald Welte <laforge@gnumonks.org>");
@@ -158,12 +169,39 @@ MODULE_LICENSE("GPL");
 #define ECO_MODE_OFF		0x00
 #define ECO_MODE_ON		0x80
 
+#define PCC_ACPI_FAN_ACTIVE_MODE	0x00
+#define PCC_ACPI_FAN_PASSIVE_MODE	0x01
+#define PCC_ACPI_TDP_LIMIT_ON		0x01
+#define PCC_ACPI_TDP_LIMIT_OFF		0x00
+
 #define ACPI_PCC_DRIVER_NAME	"Panasonic Laptop Support"
 #define ACPI_PCC_DEVICE_NAME	"Hotkey"
 #define ACPI_PCC_CLASS		"pcc"
 
 #define ACPI_PCC_INPUT_PHYS	"panasonic/hkey0"
 
+enum pcc_fan_mode {
+	PCC_FAN_MODE_UNSET = 0,
+	PCC_FAN_MODE_ACTIVE,
+	PCC_FAN_MODE_PASSIVE,
+};
+
+enum pcc_tdp_mode {
+	PCC_TDP_MODE_UNSET = 0,
+	PCC_TDP_MODE_LOCKED,
+	PCC_TDP_MODE_UNLOCKED,
+};
+
+struct pcc_platform_profile {
+	enum pcc_fan_mode fan_mode;
+	enum pcc_tdp_mode tdp_mode;
+};
+
+struct pcc_quirk {
+	bool use_platform_profiles;
+	struct pcc_platform_profile platform_profiles[PLATFORM_PROFILE_LAST];
+};
+
 /* LCD_TYPEs: 0 = Normal, 1 = Semi-transparent
    ECO_MODEs: 0x03 = off, 0x83 = on
 */
@@ -239,6 +277,7 @@ static const struct key_entry panasonic_keymap[] = {
 
 struct pcc_acpi {
 	acpi_handle		handle;
+	acpi_handle		ec_handle;
 	unsigned long		num_sifr;
 	int			sticky_key;
 	int			eco_mode;
@@ -246,13 +285,111 @@ struct pcc_acpi {
 	int			ac_brightness;
 	int			dc_brightness;
 	int			current_brightness;
+	const struct pcc_quirk	*quirks;
 	struct acpi_device	*device;
 	struct input_dev	*input_dev;
 	struct backlight_device	*backlight;
 	struct platform_device	*platform;
+	struct device		*platform_profile_dev;
 	u32			sinf[] __counted_by(num_sifr);
 };
 
+static struct pcc_quirk quirk_cf_sr4 = {
+	.use_platform_profiles = true,
+	.platform_profiles = {
+		[PLATFORM_PROFILE_QUIET] =  {
+			.fan_mode = PCC_FAN_MODE_PASSIVE,
+			.tdp_mode = PCC_TDP_MODE_LOCKED,
+		},
+		[PLATFORM_PROFILE_COOL] =  {
+			.fan_mode = PCC_FAN_MODE_ACTIVE,
+			.tdp_mode = PCC_TDP_MODE_LOCKED,
+		},
+		[PLATFORM_PROFILE_PERFORMANCE] =  {
+			.fan_mode = PCC_FAN_MODE_ACTIVE,
+			.tdp_mode = PCC_TDP_MODE_UNLOCKED,
+		},
+	},
+};
+
+static struct pcc_quirk quirk_cf_qv9 = {
+	.use_platform_profiles = true,
+	.platform_profiles = {
+		[PLATFORM_PROFILE_BALANCED] =  {
+			.fan_mode = PCC_FAN_MODE_ACTIVE,
+			.tdp_mode = PCC_TDP_MODE_LOCKED,
+		},
+		[PLATFORM_PROFILE_PERFORMANCE] =  {
+			.fan_mode = PCC_FAN_MODE_ACTIVE,
+			.tdp_mode = PCC_TDP_MODE_UNLOCKED,
+		},
+	},
+};
+
+static struct pcc_quirk quirk_cf_sv8 = {
+	.use_platform_profiles = true,
+	.platform_profiles = {
+		[PLATFORM_PROFILE_BALANCED] =  {
+			.fan_mode = PCC_FAN_MODE_ACTIVE,
+			.tdp_mode = PCC_TDP_MODE_LOCKED,
+		},
+		[PLATFORM_PROFILE_PERFORMANCE] =  {
+			.fan_mode = PCC_FAN_MODE_ACTIVE,
+			.tdp_mode = PCC_TDP_MODE_UNLOCKED,
+		},
+	},
+};
+
+static struct pcc_quirk quirk_cf_rz6 = {
+	.use_platform_profiles = true,
+	.platform_profiles = {
+		[PLATFORM_PROFILE_BALANCED] =  {
+			.fan_mode = PCC_FAN_MODE_ACTIVE,
+			.tdp_mode = PCC_TDP_MODE_LOCKED,
+		},
+		[PLATFORM_PROFILE_PERFORMANCE] =  {
+			.fan_mode = PCC_FAN_MODE_ACTIVE,
+			.tdp_mode = PCC_TDP_MODE_UNLOCKED,
+		},
+	},
+};
+
+static const struct dmi_system_id pcc_quirks[] = {
+	{
+		.ident = "Panasonic Connect Co., Ltd. CFSR4-1",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Panasonic Connect Co., Ltd."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "CFSR4-1"),
+		},
+		.driver_data = &quirk_cf_sr4,
+	},
+	{
+		.ident = "Panasonic Corporation CFQV9-1",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Panasonic Corporation"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "CFQV9-1"),
+		},
+		.driver_data = &quirk_cf_qv9,
+	},
+	{
+		.ident = "Panasonic Corporation CFSV8-2",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Panasonic Corporation"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "CFSV8-2"),
+		},
+		.driver_data = &quirk_cf_sv8,
+	},
+	{
+		.ident = "Panasonic Corporation CFRZ6-2",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Panasonic Corporation"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "CFRZ6-2"),
+		},
+		.driver_data = &quirk_cf_rz6,
+	},
+	{},
+};
+
 /*
  * On some Panasonic models the volume up / down / mute keys send duplicate
  * keypress events over the PS/2 kbd interface, filter these out.
@@ -958,6 +1095,207 @@ static int acpi_pcc_init_input(struct pcc_acpi *pcc)
 	return error;
 }
 
+static int pcc_fan_mode_get(struct pcc_acpi *pcc, enum pcc_fan_mode *fan_mode)
+{
+	unsigned long long state;
+	acpi_status status;
+
+	status = acpi_evaluate_integer(pcc->ec_handle, "CEFM", NULL,
+				       &state);
+	if (ACPI_FAILURE(status)) {
+		pr_err("cannot get fan mode via CEFM\n");
+		return -EIO;
+	}
+
+	if (state == PCC_ACPI_FAN_ACTIVE_MODE)
+		*fan_mode = PCC_FAN_MODE_ACTIVE;
+	else if (state == PCC_ACPI_FAN_PASSIVE_MODE)
+		*fan_mode = PCC_FAN_MODE_PASSIVE;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+
+static int pcc_tdp_mode_get(struct pcc_acpi *pcc, enum pcc_tdp_mode *tdp_mode)
+{
+	unsigned long long state;
+	acpi_status status;
+
+	status = acpi_evaluate_integer(pcc->ec_handle, "EPLE", NULL,
+				       &state);
+	if (ACPI_FAILURE(status)) {
+		pr_err("cannot read power limit using EPLE\n");
+		return -EIO;
+	}
+
+	if (state == PCC_ACPI_TDP_LIMIT_ON)
+		*tdp_mode = PCC_TDP_MODE_LOCKED;
+	else if (state == PCC_ACPI_TDP_LIMIT_OFF)
+		*tdp_mode = PCC_TDP_MODE_UNLOCKED;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+
+static int pcc_fan_mode_set(struct pcc_acpi *pcc, enum pcc_fan_mode fan_mode)
+{
+	acpi_status status;
+
+	switch (fan_mode) {
+	case PCC_FAN_MODE_ACTIVE:
+		status = acpi_execute_simple_method(pcc->ec_handle,
+						    "SEFM",
+						    PCC_ACPI_FAN_ACTIVE_MODE);
+		break;
+	case PCC_FAN_MODE_PASSIVE:
+		status = acpi_execute_simple_method(pcc->ec_handle,
+						    "SEFM",
+						    PCC_ACPI_FAN_PASSIVE_MODE);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (ACPI_FAILURE(status)) {
+		pr_err("failed to set fan mode via SEFM\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int pcc_tdp_mode_set(struct pcc_acpi *pcc, enum pcc_tdp_mode tdp_mode)
+{
+	acpi_status status;
+
+	switch (tdp_mode) {
+	case PCC_TDP_MODE_LOCKED:
+		status = acpi_execute_simple_method(pcc->ec_handle,
+						    "SEPL",
+						    PCC_ACPI_TDP_LIMIT_ON);
+		break;
+	case PCC_TDP_MODE_UNLOCKED:
+		status = acpi_execute_simple_method(pcc->ec_handle,
+						    "SEPL",
+						    PCC_ACPI_TDP_LIMIT_OFF);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (ACPI_FAILURE(status)) {
+		pr_err("failed to set power mode via SEPL\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int pcc_platform_profile_get(struct device *dev, enum platform_profile_option *profile)
+{
+	struct pcc_acpi *pcc = dev_get_drvdata(dev);
+	enum pcc_fan_mode fan_mode;
+	enum pcc_tdp_mode tdp_mode;
+	int status;
+
+	status = pcc_fan_mode_get(pcc, &fan_mode);
+	if (status)
+		return status;
+
+	status = pcc_tdp_mode_get(pcc, &tdp_mode);
+	if (status)
+		return status;
+
+	for (enum platform_profile_option pp_opt = 0;
+	     pp_opt < PLATFORM_PROFILE_LAST;
+	     pp_opt++) {
+		enum pcc_fan_mode profile_fan_mode =
+			pcc->quirks->platform_profiles[pp_opt].fan_mode;
+		enum pcc_tdp_mode profile_tdp_mode =
+			pcc->quirks->platform_profiles[pp_opt].tdp_mode;
+
+		if (!(profile_fan_mode && profile_tdp_mode))
+			continue;
+
+		if (tdp_mode == profile_tdp_mode && fan_mode == profile_fan_mode) {
+			*profile = pp_opt;
+			return 0;
+		}
+	}
+
+	*profile = PLATFORM_PROFILE_CUSTOM;
+	return 0;
+}
+
+static int pcc_platform_profile_set_profile(struct pcc_acpi *pcc,
+					    enum pcc_fan_mode fan_mode,
+					    enum pcc_tdp_mode tdp_mode)
+{
+	int status;
+
+	switch (tdp_mode) {
+	case PCC_TDP_MODE_UNLOCKED:
+		status = pcc_fan_mode_set(pcc, fan_mode);
+		if (status)
+			return status;
+
+		return pcc_tdp_mode_set(pcc, tdp_mode);
+	case PCC_TDP_MODE_LOCKED:
+		status = pcc_tdp_mode_set(pcc, tdp_mode);
+		if (status)
+			return status;
+
+		return pcc_fan_mode_set(pcc, fan_mode);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int pcc_platform_profile_set(struct device *dev, enum platform_profile_option profile)
+{
+	struct pcc_acpi *pcc = dev_get_drvdata(dev);
+	struct pcc_platform_profile pcc_profile;
+
+	pcc_profile = pcc->quirks->platform_profiles[profile];
+	if (pcc_profile.fan_mode && pcc_profile.tdp_mode)
+		return pcc_platform_profile_set_profile(pcc,
+							pcc_profile.fan_mode,
+							pcc_profile.tdp_mode);
+
+	return -EINVAL;
+}
+
+static int pcc_platform_profile_probe(void *drvdata, unsigned long *choices)
+{
+	struct pcc_acpi *pcc = drvdata;
+
+	for (enum platform_profile_option pp_opt = 0;
+	     pp_opt < PLATFORM_PROFILE_LAST;
+	     pp_opt++) {
+		enum pcc_fan_mode fan_mode =
+			pcc->quirks->platform_profiles[pp_opt].fan_mode;
+		enum pcc_tdp_mode tdp_mode =
+			pcc->quirks->platform_profiles[pp_opt].tdp_mode;
+
+		if (fan_mode && tdp_mode) {
+			set_bit(pp_opt, choices);
+		} else if (fan_mode || tdp_mode) {
+			pr_err("error probing platform profiles: malformed quirk\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static const struct platform_profile_ops pcc_platform_profile_ops = {
+	.probe = pcc_platform_profile_probe,
+	.profile_get = pcc_platform_profile_get,
+	.profile_set = pcc_platform_profile_set,
+};
+
 /* kernel module interface */
 
 #ifdef CONFIG_PM_SLEEP
@@ -979,8 +1317,37 @@ static int acpi_pcc_hotkey_resume(struct device *dev)
 }
 #endif
 
+static int acpi_pcc_platform_profile_probe(struct pcc_acpi *pcc, struct platform_device *pdev)
+{
+	struct acpi_device *adev __free(acpi_dev_put) =
+		acpi_dev_get_first_match_dev("PNP0C09", NULL, -1);
+	int err;
+
+	if (!adev) {
+		pr_err("failed to find embedded controller path\n");
+		return -ENODEV;
+	}
+
+	pcc->ec_handle = acpi_device_handle(adev);
+
+	pcc->platform_profile_dev =
+		devm_platform_profile_register(&pdev->dev,
+					       "panasonic-laptop",
+					       pcc,
+					       &pcc_platform_profile_ops);
+	if (IS_ERR(pcc->platform_profile_dev)) {
+		err = PTR_ERR(pcc->platform_profile_dev);
+		pcc->platform_profile_dev = NULL;
+		return dev_err_probe(&pdev->dev, err,
+				     "failed to register platform profiles\n");
+	}
+
+	return 0;
+}
+
 static int acpi_pcc_hotkey_probe(struct platform_device *pdev)
 {
+	const struct dmi_system_id *dmi_id;
 	struct backlight_properties props;
 	struct acpi_device *device;
 	struct pcc_acpi *pcc;
@@ -1020,6 +1387,12 @@ static int acpi_pcc_hotkey_probe(struct platform_device *pdev)
 	strscpy(acpi_device_name(device), ACPI_PCC_DEVICE_NAME);
 	strscpy(acpi_device_class(device), ACPI_PCC_CLASS);
 
+	dmi_id = dmi_first_match(pcc_quirks);
+	if (dmi_id) {
+		pcc->quirks = dmi_id->driver_data;
+		pr_debug("quirk detect: enabled quirks for %s\n", dmi_id->ident);
+	}
+
 	result = acpi_pcc_init_input(pcc);
 	if (result) {
 		pr_err("Error installing keyinput handler\n");
@@ -1091,6 +1464,13 @@ static int acpi_pcc_hotkey_probe(struct platform_device *pdev)
 	}
 
 	i8042_install_filter(panasonic_i8042_filter, NULL);
+
+	if (pcc->quirks && pcc->quirks->use_platform_profiles) {
+		result = acpi_pcc_platform_profile_probe(pcc, pdev);
+		if (result)
+			pr_warn("error occurred setting up platform profiles\n");
+	}
+
 	return 0;
 
 out_platform:
-- 
2.55.0

Re: [RFC PATCH v1 1/1] platform/x86: panasonic-laptop: add platform_profile support

[Ilpo Järvinen]

On Thu, 6 Aug 2026, Alex Yeo wrote:

> Expose firmware thermal and power management policies via the
> platform_profile interface.
> 
> Supported and tested models:
>  - CF-RZ6 (2016)
>  - CF-SV8 (2019)
>  - CF-QV9 (2020)
>  - CF-SR4 (2024)
> 
> Firmware fan operating mode and the TDP limit switch are mapped onto
> the Linux platform_profile ABI.
> 
> This implementation queries the firmware for every get().
> CUSTOM is returned when the current state is not recognized.
> The set() callback writes the values regardless of what the
> current fan mode and TDP limit is.
> 
>  - DMI quirks restrict support to tested models
>  - Missing includes such as sysfs.h were added based on previous
>    feedback.
>  - Logging: pr_fmt added
>  - When raising TDP, fan mode is switched first
>  - When limiting TDP, TDP limit is applied first
>  - ACPI handle for the EC device was used as the EC path varies
>    slightly between models.
> 
> Signed-off-by: Alex Yeo <alexyeo362@gmail.com>
> ---
>  drivers/platform/x86/panasonic-laptop.c | 380 ++++++++++++++++++++++++
>  1 file changed, 380 insertions(+)
> 
> diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
> index 719add753cb3..eeb89892768b 100644
> --- a/drivers/platform/x86/panasonic-laptop.c
> +++ b/drivers/platform/x86/panasonic-laptop.c
> @@ -119,10 +119,16 @@
>   *		- v0.1  start from toshiba_acpi driver written by John Belmonte
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/acpi.h>
>  #include <linux/backlight.h>
>  #include <linux/bits.h>
> +#include <linux/cleanup.h>
>  #include <linux/ctype.h>
> +#include <linux/dmi.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
>  #include <linux/i8042.h>
>  #include <linux/init.h>
>  #include <linux/input.h>
> @@ -130,13 +136,18 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/platform_profile.h>
> +#include <linux/printk.h>
>  #include <linux/seq_file.h>
>  #include <linux/serio.h>
>  #include <linux/slab.h>
> +#include <linux/sysfs.h>
>  #include <linux/types.h>
>  #include <linux/uaccess.h>
>  #include <acpi/video.h>
>  
> +DEFINE_FREE(acpi_dev_put, struct acpi_device *, if (_T) acpi_dev_put(_T))
> +
>  MODULE_AUTHOR("Hiroshi Miura <miura@da-cha.org>");
>  MODULE_AUTHOR("David Bronaugh <dbronaugh@linuxboxen.org>");
>  MODULE_AUTHOR("Harald Welte <laforge@gnumonks.org>");
> @@ -158,12 +169,39 @@ MODULE_LICENSE("GPL");
>  #define ECO_MODE_OFF		0x00
>  #define ECO_MODE_ON		0x80
>  
> +#define PCC_ACPI_FAN_ACTIVE_MODE	0x00
> +#define PCC_ACPI_FAN_PASSIVE_MODE	0x01
> +#define PCC_ACPI_TDP_LIMIT_ON		0x01
> +#define PCC_ACPI_TDP_LIMIT_OFF		0x00
> +
>  #define ACPI_PCC_DRIVER_NAME	"Panasonic Laptop Support"
>  #define ACPI_PCC_DEVICE_NAME	"Hotkey"
>  #define ACPI_PCC_CLASS		"pcc"
>  
>  #define ACPI_PCC_INPUT_PHYS	"panasonic/hkey0"
>  
> +enum pcc_fan_mode {
> +	PCC_FAN_MODE_UNSET = 0,
> +	PCC_FAN_MODE_ACTIVE,
> +	PCC_FAN_MODE_PASSIVE,
> +};
> +
> +enum pcc_tdp_mode {
> +	PCC_TDP_MODE_UNSET = 0,
> +	PCC_TDP_MODE_LOCKED,
> +	PCC_TDP_MODE_UNLOCKED,
> +};
> +
> +struct pcc_platform_profile {
> +	enum pcc_fan_mode fan_mode;
> +	enum pcc_tdp_mode tdp_mode;
> +};
> +
> +struct pcc_quirk {
> +	bool use_platform_profiles;
> +	struct pcc_platform_profile platform_profiles[PLATFORM_PROFILE_LAST];
> +};
> +
>  /* LCD_TYPEs: 0 = Normal, 1 = Semi-transparent
>     ECO_MODEs: 0x03 = off, 0x83 = on
>  */
> @@ -239,6 +277,7 @@ static const struct key_entry panasonic_keymap[] = {
>  
>  struct pcc_acpi {
>  	acpi_handle		handle;
> +	acpi_handle		ec_handle;
>  	unsigned long		num_sifr;
>  	int			sticky_key;
>  	int			eco_mode;
> @@ -246,13 +285,111 @@ struct pcc_acpi {
>  	int			ac_brightness;
>  	int			dc_brightness;
>  	int			current_brightness;
> +	const struct pcc_quirk	*quirks;
>  	struct acpi_device	*device;
>  	struct input_dev	*input_dev;
>  	struct backlight_device	*backlight;
>  	struct platform_device	*platform;
> +	struct device		*platform_profile_dev;
>  	u32			sinf[] __counted_by(num_sifr);
>  };
>  
> +static struct pcc_quirk quirk_cf_sr4 = {
> +	.use_platform_profiles = true,
> +	.platform_profiles = {
> +		[PLATFORM_PROFILE_QUIET] =  {

There's extra space in all these.

> +			.fan_mode = PCC_FAN_MODE_PASSIVE,
> +			.tdp_mode = PCC_TDP_MODE_LOCKED,
> +		},
> +		[PLATFORM_PROFILE_COOL] =  {
> +			.fan_mode = PCC_FAN_MODE_ACTIVE,
> +			.tdp_mode = PCC_TDP_MODE_LOCKED,
> +		},
> +		[PLATFORM_PROFILE_PERFORMANCE] =  {
> +			.fan_mode = PCC_FAN_MODE_ACTIVE,
> +			.tdp_mode = PCC_TDP_MODE_UNLOCKED,
> +		},
> +	},
> +};
> +
> +static struct pcc_quirk quirk_cf_qv9 = {
> +	.use_platform_profiles = true,
> +	.platform_profiles = {
> +		[PLATFORM_PROFILE_BALANCED] =  {
> +			.fan_mode = PCC_FAN_MODE_ACTIVE,
> +			.tdp_mode = PCC_TDP_MODE_LOCKED,
> +		},
> +		[PLATFORM_PROFILE_PERFORMANCE] =  {
> +			.fan_mode = PCC_FAN_MODE_ACTIVE,
> +			.tdp_mode = PCC_TDP_MODE_UNLOCKED,
> +		},
> +	},
> +};
> +
> +static struct pcc_quirk quirk_cf_sv8 = {
> +	.use_platform_profiles = true,
> +	.platform_profiles = {
> +		[PLATFORM_PROFILE_BALANCED] =  {
> +			.fan_mode = PCC_FAN_MODE_ACTIVE,
> +			.tdp_mode = PCC_TDP_MODE_LOCKED,
> +		},
> +		[PLATFORM_PROFILE_PERFORMANCE] =  {
> +			.fan_mode = PCC_FAN_MODE_ACTIVE,
> +			.tdp_mode = PCC_TDP_MODE_UNLOCKED,
> +		},
> +	},
> +};
> +
> +static struct pcc_quirk quirk_cf_rz6 = {
> +	.use_platform_profiles = true,
> +	.platform_profiles = {
> +		[PLATFORM_PROFILE_BALANCED] =  {
> +			.fan_mode = PCC_FAN_MODE_ACTIVE,
> +			.tdp_mode = PCC_TDP_MODE_LOCKED,
> +		},
> +		[PLATFORM_PROFILE_PERFORMANCE] =  {
> +			.fan_mode = PCC_FAN_MODE_ACTIVE,
> +			.tdp_mode = PCC_TDP_MODE_UNLOCKED,
> +		},
> +	},
> +};
> +
> +static const struct dmi_system_id pcc_quirks[] = {
> +	{
> +		.ident = "Panasonic Connect Co., Ltd. CFSR4-1",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Panasonic Connect Co., Ltd."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "CFSR4-1"),
> +		},
> +		.driver_data = &quirk_cf_sr4,
> +	},
> +	{
> +		.ident = "Panasonic Corporation CFQV9-1",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Panasonic Corporation"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "CFQV9-1"),
> +		},
> +		.driver_data = &quirk_cf_qv9,
> +	},
> +	{
> +		.ident = "Panasonic Corporation CFSV8-2",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Panasonic Corporation"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "CFSV8-2"),
> +		},
> +		.driver_data = &quirk_cf_sv8,
> +	},
> +	{
> +		.ident = "Panasonic Corporation CFRZ6-2",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Panasonic Corporation"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "CFRZ6-2"),
> +		},
> +		.driver_data = &quirk_cf_rz6,
> +	},
> +	{},
> +};
> +
>  /*
>   * On some Panasonic models the volume up / down / mute keys send duplicate
>   * keypress events over the PS/2 kbd interface, filter these out.
> @@ -958,6 +1095,207 @@ static int acpi_pcc_init_input(struct pcc_acpi *pcc)
>  	return error;
>  }
>  
> +static int pcc_fan_mode_get(struct pcc_acpi *pcc, enum pcc_fan_mode *fan_mode)
> +{
> +	unsigned long long state;
> +	acpi_status status;
> +
> +	status = acpi_evaluate_integer(pcc->ec_handle, "CEFM", NULL,
> +				       &state);

Fits to one line.

> +	if (ACPI_FAILURE(status)) {
> +		pr_err("cannot get fan mode via CEFM\n");
> +		return -EIO;
> +	}
> +
> +	if (state == PCC_ACPI_FAN_ACTIVE_MODE)
> +		*fan_mode = PCC_FAN_MODE_ACTIVE;
> +	else if (state == PCC_ACPI_FAN_PASSIVE_MODE)
> +		*fan_mode = PCC_FAN_MODE_PASSIVE;
> +	else
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int pcc_tdp_mode_get(struct pcc_acpi *pcc, enum pcc_tdp_mode *tdp_mode)
> +{
> +	unsigned long long state;
> +	acpi_status status;
> +
> +	status = acpi_evaluate_integer(pcc->ec_handle, "EPLE", NULL,
> +				       &state);
> +	if (ACPI_FAILURE(status)) {
> +		pr_err("cannot read power limit using EPLE\n");
> +		return -EIO;
> +	}
> +
> +	if (state == PCC_ACPI_TDP_LIMIT_ON)
> +		*tdp_mode = PCC_TDP_MODE_LOCKED;
> +	else if (state == PCC_ACPI_TDP_LIMIT_OFF)
> +		*tdp_mode = PCC_TDP_MODE_UNLOCKED;
> +	else
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int pcc_fan_mode_set(struct pcc_acpi *pcc, enum pcc_fan_mode fan_mode)
> +{
> +	acpi_status status;
> +
> +	switch (fan_mode) {
> +	case PCC_FAN_MODE_ACTIVE:
> +		status = acpi_execute_simple_method(pcc->ec_handle,
> +						    "SEFM",
> +						    PCC_ACPI_FAN_ACTIVE_MODE);
> +		break;
> +	case PCC_FAN_MODE_PASSIVE:
> +		status = acpi_execute_simple_method(pcc->ec_handle,
> +						    "SEFM",
> +						    PCC_ACPI_FAN_PASSIVE_MODE);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (ACPI_FAILURE(status)) {
> +		pr_err("failed to set fan mode via SEFM\n");
> +		return -EIO;
> +	}

IMO, spreading stuff around like this just makes it harder to follow code 
flow. And the variation is just to pass a different argument to 
acpi_execute_simple_method() which would suggest a helper would be useful.

Once everything is done within the case, you can directly return from 
those cases for simplicity.

> +
> +	return 0;
> +}
> +
> +static int pcc_tdp_mode_set(struct pcc_acpi *pcc, enum pcc_tdp_mode tdp_mode)
> +{
> +	acpi_status status;
> +
> +	switch (tdp_mode) {
> +	case PCC_TDP_MODE_LOCKED:
> +		status = acpi_execute_simple_method(pcc->ec_handle,
> +						    "SEPL",
> +						    PCC_ACPI_TDP_LIMIT_ON);
> +		break;
> +	case PCC_TDP_MODE_UNLOCKED:
> +		status = acpi_execute_simple_method(pcc->ec_handle,
> +						    "SEPL",
> +						    PCC_ACPI_TDP_LIMIT_OFF);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (ACPI_FAILURE(status)) {
> +		pr_err("failed to set power mode via SEPL\n");
> +		return -EIO;
> +	}

Same problem here as above.

> +	return 0;
> +}
> +
> +static int pcc_platform_profile_get(struct device *dev, enum platform_profile_option *profile)
> +{
> +	struct pcc_acpi *pcc = dev_get_drvdata(dev);
> +	enum pcc_fan_mode fan_mode;
> +	enum pcc_tdp_mode tdp_mode;
> +	int status;
> +
> +	status = pcc_fan_mode_get(pcc, &fan_mode);
> +	if (status)
> +		return status;
> +
> +	status = pcc_tdp_mode_get(pcc, &tdp_mode);
> +	if (status)
> +		return status;

Please leave "status" for acpi_status and pick another name for the 
generic return variable (I personally prefer "ret" because it doesn't 
carry error connotation "err" does, but the latter seems to be already 
in use by this driver, among other variable names).

> +	for (enum platform_profile_option pp_opt = 0;

Move declaration to the beginning of the function.

> +	     pp_opt < PLATFORM_PROFILE_LAST;

Please use ARRAY_SIZE() + make sure you add the include for it.

> +	     pp_opt++) {
> +		enum pcc_fan_mode profile_fan_mode =
> +			pcc->quirks->platform_profiles[pp_opt].fan_mode;
> +		enum pcc_tdp_mode profile_tdp_mode =
> +			pcc->quirks->platform_profiles[pp_opt].tdp_mode;

Please make a local variable out of pcc->quirks->platform_profiles[pp_opt] 
instead (with a reasonably short name).

> +
> +		if (!(profile_fan_mode && profile_tdp_mode))

This code doesn't make sense for variables that are declared as enums 
(do not handle enums as truth values).

> +			continue;
> +
> +		if (tdp_mode == profile_tdp_mode && fan_mode == profile_fan_mode) {
> +			*profile = pp_opt;
> +			return 0;
> +		}
> +	}
> +
> +	*profile = PLATFORM_PROFILE_CUSTOM;
> +	return 0;
> +}
> +
> +static int pcc_platform_profile_set_profile(struct pcc_acpi *pcc,
> +					    enum pcc_fan_mode fan_mode,
> +					    enum pcc_tdp_mode tdp_mode)
> +{
> +	int status;

Change name.

> +
> +	switch (tdp_mode) {
> +	case PCC_TDP_MODE_UNLOCKED:
> +		status = pcc_fan_mode_set(pcc, fan_mode);
> +		if (status)
> +			return status;
> +
> +		return pcc_tdp_mode_set(pcc, tdp_mode);
> +	case PCC_TDP_MODE_LOCKED:
> +		status = pcc_tdp_mode_set(pcc, tdp_mode);
> +		if (status)
> +			return status;
> +
> +		return pcc_fan_mode_set(pcc, fan_mode);

This is structurally much easier to follow than pcc_fan_mode_set() above.

> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int pcc_platform_profile_set(struct device *dev, enum platform_profile_option profile)
> +{
> +	struct pcc_acpi *pcc = dev_get_drvdata(dev);
> +	struct pcc_platform_profile pcc_profile;

Why isn't this a pointer?

> +	pcc_profile = pcc->quirks->platform_profiles[profile];

I'd put the assignment to the declaration line (it'll be only 91 chars 
long and is quite boilerplately so fits well into the variable 
declarations block, IMO)

> +	if (pcc_profile.fan_mode && pcc_profile.tdp_mode)

Again, those are enums but you treat them as truth values which makes 
things harder to understand.

> +		return pcc_platform_profile_set_profile(pcc,
> +							pcc_profile.fan_mode,
> +							pcc_profile.tdp_mode);
> +
> +	return -EINVAL;
> +}
> +
> +static int pcc_platform_profile_probe(void *drvdata, unsigned long *choices)
> +{
> +	struct pcc_acpi *pcc = drvdata;
> +
> +	for (enum platform_profile_option pp_opt = 0;
> +	     pp_opt < PLATFORM_PROFILE_LAST;
> +	     pp_opt++) {

Declare the enum in the function variables and put this to single line.

> +		enum pcc_fan_mode fan_mode =
> +			pcc->quirks->platform_profiles[pp_opt].fan_mode;
> +		enum pcc_tdp_mode tdp_mode =
> +			pcc->quirks->platform_profiles[pp_opt].tdp_mode;
> +
> +		if (fan_mode && tdp_mode) {

Same comments as with the other code.

> +			set_bit(pp_opt, choices);
> +		} else if (fan_mode || tdp_mode) {
> +			pr_err("error probing platform profiles: malformed quirk\n");

This looks a clear developer error so WARN_ON() would be more appropriate 
than pr_err().

> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct platform_profile_ops pcc_platform_profile_ops = {
> +	.probe = pcc_platform_profile_probe,
> +	.profile_get = pcc_platform_profile_get,
> +	.profile_set = pcc_platform_profile_set,
> +};
> +
>  /* kernel module interface */
>  
>  #ifdef CONFIG_PM_SLEEP
> @@ -979,8 +1317,37 @@ static int acpi_pcc_hotkey_resume(struct device *dev)
>  }
>  #endif
>  
> +static int acpi_pcc_platform_profile_probe(struct pcc_acpi *pcc, struct platform_device *pdev)
> +{
> +	struct acpi_device *adev __free(acpi_dev_put) =
> +		acpi_dev_get_first_match_dev("PNP0C09", NULL, -1);
> +	int err;
> +
> +	if (!adev) {
> +		pr_err("failed to find embedded controller path\n");
> +		return -ENODEV;
> +	}
> +
> +	pcc->ec_handle = acpi_device_handle(adev);
> +
> +	pcc->platform_profile_dev =
> +		devm_platform_profile_register(&pdev->dev,
> +					       "panasonic-laptop",
> +					       pcc,
> +					       &pcc_platform_profile_ops);
> +	if (IS_ERR(pcc->platform_profile_dev)) {
> +		err = PTR_ERR(pcc->platform_profile_dev);
> +		pcc->platform_profile_dev = NULL;
> +		return dev_err_probe(&pdev->dev, err,
> +				     "failed to register platform profiles\n");
> +	}
> +
> +	return 0;
> +}
> +
>  static int acpi_pcc_hotkey_probe(struct platform_device *pdev)
>  {
> +	const struct dmi_system_id *dmi_id;
>  	struct backlight_properties props;
>  	struct acpi_device *device;
>  	struct pcc_acpi *pcc;
> @@ -1020,6 +1387,12 @@ static int acpi_pcc_hotkey_probe(struct platform_device *pdev)
>  	strscpy(acpi_device_name(device), ACPI_PCC_DEVICE_NAME);
>  	strscpy(acpi_device_class(device), ACPI_PCC_CLASS);
>  
> +	dmi_id = dmi_first_match(pcc_quirks);
> +	if (dmi_id) {
> +		pcc->quirks = dmi_id->driver_data;
> +		pr_debug("quirk detect: enabled quirks for %s\n", dmi_id->ident);
> +	}
> +
>  	result = acpi_pcc_init_input(pcc);
>  	if (result) {
>  		pr_err("Error installing keyinput handler\n");
> @@ -1091,6 +1464,13 @@ static int acpi_pcc_hotkey_probe(struct platform_device *pdev)
>  	}
>  
>  	i8042_install_filter(panasonic_i8042_filter, NULL);
> +
> +	if (pcc->quirks && pcc->quirks->use_platform_profiles) {
> +		result = acpi_pcc_platform_profile_probe(pcc, pdev);
> +		if (result)
> +			pr_warn("error occurred setting up platform profiles\n");
> +	}
> +
>  	return 0;
>  
>  out_platform:
> 

Also, this looked entirely independent of the existing code (?) so it 
looks as if it should be make a separate platform_driver instead of trying 
to klugde it into the existing probe. If there aren't cross references 
besides the sharing of the private data structure, I'd just introduce it 
as a separate struct platform_driver with a proper ID table and own probe, 
etc.

-- 
 i.