mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Mark Pearson" <mpearson-lenovo@squebb.ca>
To: "Derek J . Clark" <derekjohn.clark@gmail.com>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Hans de Goede" <hansg@kernel.org>
Cc: "Armin Wolf" <W_Armin@gmx.de>, "Jonathan Corbet" <corbet@lwn.net>,
	"Rong Zhang" <i@rong.moe>, "Kurt Borja" <kuurtb@gmail.com>,
	"platform-driver-x86@vger.kernel.org"
	<platform-driver-x86@vger.kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 11/16] platform/x86: lenovo-wmi-other: Add missing CPU tunable attributes
Date: Sat, 04 Apr 2026 20:48:19 -0400	[thread overview]
Message-ID: <22b180a6-c6ee-4785-9cee-577b9f55d48b@app.fastmail.com> (raw)
In-Reply-To: <20260402032424.678528-12-derekjohn.clark@gmail.com>

Hi Derek

On Wed, Apr 1, 2026, at 11:24 PM, Derek J. Clark wrote:
> Use an enum for all device ID's and CPU attribute feature ID's,
> add missing CPU attributes.
>
> Reviewed-by: Rong Zhang <i@rong.moe>
> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> ---
> v7:
>   - Fix typo in documentation.
> v4:
>   - Align type ID defines.
>   - Align CPU feature enum values.
>   - remove cpu_oc_stat from Documentation.
> v3:
>   - Remove cpu_oc_stat.
> ---
>  .../wmi/devices/lenovo-wmi-other.rst          |  9 ++
>  drivers/platform/x86/lenovo/wmi-capdata.h     |  5 +-
>  drivers/platform/x86/lenovo/wmi-other.c       | 99 ++++++++++++++++++-
>  3 files changed, 107 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/wmi/devices/lenovo-wmi-other.rst 
> b/Documentation/wmi/devices/lenovo-wmi-other.rst
> index 01d471156738..189dd4d31926 100644
> --- a/Documentation/wmi/devices/lenovo-wmi-other.rst
> +++ b/Documentation/wmi/devices/lenovo-wmi-other.rst
> @@ -68,9 +68,18 @@ Each attribute has the following properties:
>   - type
> 
>  The following firmware-attributes are implemented:
> + - cpu_temp: CPU Thermal Load Limit
> + - ppt_cpu_cl: CPU Cross Loading Power Limit
> + - ppt_pl1_apu_spl: Platform Profile Tracking APU Sustained Power Limit
>   - ppt_pl1_spl: Platform Profile Tracking Sustained Power Limit
> + - ppt_pl1_spl_cl: Platform Profile Tracking Cross Loading Sustained 
> Power Limit
> + - ppt_pl1_tau: Exceed Duration for Platform Profile Tracking 
> Sustained Power Limit
>   - ppt_pl2_sppt: Platform Profile Tracking Slow Package Power Tracking
> + - ppt_pl2_sppt_cl: Platform Profile Tracking Cross Loading Slow 
> Package Tracking
>   - ppt_pl3_fppt: Platform Profile Tracking Fast Package Power Tracking
> + - ppt_pl3_fppt_cl: Platform Profile Tracking Cross Loading Fast 
> Package Power Tracking
> + - ppt_pl4_ipl: Platform Profile Tracking Instantaneous Power Limit
> + - ppt_pl4_ipl_cl: Platform Profile Tracking Cross Loading 
> Instantaneous Power Limit
> 

Just wanted to flag that the naming for sppt and fppt seems wrong to me. Wanted to make sure it was deliberate.

I have, for Feature ID's:
1 = CPU Short Term Power Limit (you have Slow package Power Tracking)
3 = CPU Peak Power Limit (you have as Fast Package Power Tracking)

>  LENOVO_FAN_TEST_DATA
>  -------------------------
> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h 
> b/drivers/platform/x86/lenovo/wmi-capdata.h
> index 1388eaf4ab4a..f2d45cd7a188 100644
> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
> @@ -18,7 +18,10 @@
>  #define LWMI_ATTR_MODE_ID_MASK	GENMASK(15, 8)
>  #define LWMI_ATTR_TYPE_ID_MASK	GENMASK(7, 0)
> 
> -#define LWMI_DEVICE_ID_FAN	0x04
> +enum lwmi_device_id {
> +	LWMI_DEVICE_ID_CPU = 0x01,
> +	LWMI_DEVICE_ID_FAN = 0x04,
> +};
> 
>  #define LWMI_TYPE_ID_NONE 0x00
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c 
> b/drivers/platform/x86/lenovo/wmi-other.c
> index 19a072bea2d4..56c8b84f306d 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -50,14 +50,21 @@
> 
>  #define LENOVO_OTHER_MODE_GUID "DC2A8805-3A8C-41BA-A6F7-092E0089CD3B"
> 
> -#define LWMI_DEVICE_ID_CPU 0x01
> -
> -#define LWMI_FEATURE_ID_CPU_SPPT 0x01
> -#define LWMI_FEATURE_ID_CPU_SPL 0x02
> -#define LWMI_FEATURE_ID_CPU_FPPT 0x03
> +enum lwmi_feature_id_cpu {
> +	LWMI_FEATURE_ID_CPU_SPPT =	0x01,
> +	LWMI_FEATURE_ID_CPU_SPL =	0x02,
> +	LWMI_FEATURE_ID_CPU_FPPT =	0x03,
> +	LWMI_FEATURE_ID_CPU_TEMP =	0x04,
> +	LWMI_FEATURE_ID_CPU_APU =	0x05,
> +	LWMI_FEATURE_ID_CPU_CL =	0x06,
> +	LWMI_FEATURE_ID_CPU_TAU =	0x07,
> +	LWMI_FEATURE_ID_CPU_IPL =	0x09,
> +};
> 
>  #define LWMI_FEATURE_ID_FAN_RPM 0x03
> 
> +#define LWMI_TYPE_ID_CROSSLOAD	0x01
> +
>  #define LWMI_FEATURE_VALUE_GET 17
>  #define LWMI_FEATURE_VALUE_SET 18
> 
> @@ -551,18 +558,72 @@ static struct tunable_attr_01 ppt_pl1_spl = {
>  	.type_id = LWMI_TYPE_ID_NONE,
>  };
> 
> +static struct tunable_attr_01 ppt_pl1_spl_cl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_SPL,
> +	.type_id = LWMI_TYPE_ID_CROSSLOAD,
> +};
> +
>  static struct tunable_attr_01 ppt_pl2_sppt = {
>  	.device_id = LWMI_DEVICE_ID_CPU,
>  	.feature_id = LWMI_FEATURE_ID_CPU_SPPT,
>  	.type_id = LWMI_TYPE_ID_NONE,
>  };
> 
> +static struct tunable_attr_01 ppt_pl2_sppt_cl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_SPPT,
> +	.type_id = LWMI_TYPE_ID_CROSSLOAD,
> +};
> +
>  static struct tunable_attr_01 ppt_pl3_fppt = {
>  	.device_id = LWMI_DEVICE_ID_CPU,
>  	.feature_id = LWMI_FEATURE_ID_CPU_FPPT,
>  	.type_id = LWMI_TYPE_ID_NONE,
>  };
> 
> +static struct tunable_attr_01 ppt_pl3_fppt_cl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_FPPT,
> +	.type_id = LWMI_TYPE_ID_CROSSLOAD,
> +};
> +
> +static struct tunable_attr_01 cpu_temp = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_TEMP,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 ppt_pl1_apu_spl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_APU,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 ppt_cpu_cl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_CL,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 ppt_pl1_tau = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_TAU,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 ppt_pl4_ipl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_IPL,
> +	.type_id = LWMI_TYPE_ID_NONE,
> +};
> +
> +static struct tunable_attr_01 ppt_pl4_ipl_cl = {
> +	.device_id = LWMI_DEVICE_ID_CPU,
> +	.feature_id = LWMI_FEATURE_ID_CPU_IPL,
> +	.type_id = LWMI_TYPE_ID_CROSSLOAD,
> +};
> +
>  struct capdata01_attr_group {
>  	const struct attribute_group *attr_group;
>  	struct tunable_attr_01 *tunable_attr;
> @@ -898,17 +959,45 @@ static bool lwmi_attr_01_is_supported(struct 
> tunable_attr_01 *tunable_attr)
>  		.name = _fsname, .attrs = _attrname##_attrs               \
>  	}
> 
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(cpu_temp, "cpu_temp",
> +			      "Set the CPU thermal load limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_cpu_cl, "ppt_cpu_cl",
> +			      "Set the CPU cross loading power limit");

> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl1_apu_spl, "ppt_pl1_apu_spl",
> +			      "Set the APU sustained power limit");
>  LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl1_spl, "ppt_pl1_spl",
>  			      "Set the CPU sustained power limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl1_spl_cl, "ppt_pl1_spl_cl",
> +			      "Set the CPU cross loading sustained power limit");
>  LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl2_sppt, "ppt_pl2_sppt",
>  			      "Set the CPU slow package power tracking limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl2_sppt_cl, "ppt_pl2_sppt_cl",
> +			      "Set the CPU cross loading slow package power tracking limit");
>  LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl3_fppt, "ppt_pl3_fppt",
>  			      "Set the CPU fast package power tracking limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl3_fppt_cl, "ppt_pl3_fppt_cl",
> +			      "Set the CPU cross loading fast package power tracking limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl1_tau, "ppt_pl1_tau",
> +			      "Set the CPU sustained power limit exceed duration");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl4_ipl, "ppt_pl4_ipl",
> +			      "Set the CPU instantaneous power limit");
> +LWMI_ATTR_GROUP_TUNABLE_CAP01(ppt_pl4_ipl_cl, "ppt_pl4_ipl_cl",
> +			      "Set the CPU cross loading instantaneous power limit");
> +

Depending on what you think above, these might need updating too

> 
>  static struct capdata01_attr_group cd01_attr_groups[] = {
> +	{ &cpu_temp_attr_group, &cpu_temp },
> +	{ &ppt_cpu_cl_attr_group, &ppt_cpu_cl },
> +	{ &ppt_pl1_apu_spl_attr_group, &ppt_pl1_apu_spl },
>  	{ &ppt_pl1_spl_attr_group, &ppt_pl1_spl },
> +	{ &ppt_pl1_spl_cl_attr_group, &ppt_pl1_spl_cl },
> +	{ &ppt_pl1_tau_attr_group, &ppt_pl1_tau },
>  	{ &ppt_pl2_sppt_attr_group, &ppt_pl2_sppt },
> +	{ &ppt_pl2_sppt_cl_attr_group, &ppt_pl2_sppt_cl },
>  	{ &ppt_pl3_fppt_attr_group, &ppt_pl3_fppt },
> +	{ &ppt_pl3_fppt_cl_attr_group, &ppt_pl3_fppt_cl },
> +	{ &ppt_pl4_ipl_attr_group, &ppt_pl4_ipl },
> +	{ &ppt_pl4_ipl_cl_attr_group, &ppt_pl4_ipl_cl },
>  	{},
>  };
> 
> -- 
> 2.53.0

  reply	other threads:[~2026-04-05  0:48 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-02  3:24 [PATCH v7 00/16] platform-x86: lenovo-wmi: Add fixes and enhancement Derek J. Clark
2026-04-02  3:24 ` [PATCH v7 01/16] platform/x86: lenovo-wmi-helpers: Fix memory leak in lwmi_dev_evaluate_int() Derek J. Clark
2026-04-02  3:24 ` [PATCH v7 02/16] platform/x86: lenovo-wmi-other: Balance IDA id allocation and free Derek J. Clark
2026-04-02  3:24 ` [PATCH v7 03/16] platform/x86: lenovo-wmi-other: Balance component bind and unbind Derek J. Clark
2026-04-02  3:24 ` [PATCH v7 04/16] platform/x86: lenovo-wmi-other: Zero initialize WMI arguments Derek J. Clark
2026-04-02  3:24 ` [PATCH v7 05/16] platform/x86: lenovo-wmi-other: Fix tunable_attr_01 struct members Derek J. Clark
2026-04-02  3:24 ` [PATCH v7 06/16] platform/x86: lenovo-wmi-other: Limit adding attributes to supported devices Derek J. Clark
2026-04-04 21:51   ` Mark Pearson
2026-04-06 18:40     ` Derek John Clark
2026-04-02  3:24 ` [PATCH v7 07/16] platform/x86: lenovo: Decouple lenovo-wmi-gamezone and lenovo-wmi-other Derek J. Clark
2026-04-02  3:24 ` [PATCH v7 08/16] platform/x86: lenovo-wmi-helpers: Move gamezone enums to wmi-helpers Derek J. Clark
2026-04-05  0:03   ` Mark Pearson
2026-04-06 18:42     ` Derek John Clark
2026-04-02  3:24 ` [PATCH v7 09/16] platform/x86: lenovo-wmi-other: Move LWMI_FAN_DIV Derek J. Clark
2026-04-02  3:24 ` [PATCH v7 10/16] platform/x86: lenovo-wmi-other: Add lwmi_attr_id() function Derek J. Clark
2026-04-05  0:16   ` Mark Pearson
2026-04-06 18:45     ` Derek John Clark
2026-04-06 18:50       ` Mark Pearson
2026-04-02  3:24 ` [PATCH v7 11/16] platform/x86: lenovo-wmi-other: Add missing CPU tunable attributes Derek J. Clark
2026-04-05  0:48   ` Mark Pearson [this message]
2026-04-06 19:14     ` Derek John Clark
2026-04-06 19:23       ` Mark Pearson
2026-04-02  3:24 ` [PATCH v7 12/16] platform/x86: lenovo-wmi-other: Add GPU " Derek J. Clark
2026-04-05  0:54   ` Mark Pearson
2026-04-06 19:22     ` Derek John Clark
2026-04-02  3:24 ` [PATCH v7 13/16] platform/x86: lenovo-wmi-other: Rename LWMI_OM_FW_ATTR_BASE_PATH Derek J. Clark
2026-04-02  3:24 ` [PATCH v7 14/16] platform/x86: lenovo-wmi-other: Add WMI battery charge limiting Derek J. Clark
2026-04-02  3:24 ` [PATCH v7 15/16] platform/x86: lenovo-wmi-helpers: Add helper for creating per-device debugfs dir Derek J. Clark
2026-04-02  3:24 ` [PATCH v7 16/16] platform/x86: lenovo-wmi-capdata: Add debugfs file for dumping capdata Derek J. Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=22b180a6-c6ee-4785-9cee-577b9f55d48b@app.fastmail.com \
    --to=mpearson-lenovo@squebb.ca \
    --cc=W_Armin@gmx.de \
    --cc=corbet@lwn.net \
    --cc=derekjohn.clark@gmail.com \
    --cc=hansg@kernel.org \
    --cc=i@rong.moe \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=kuurtb@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®