All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kurt Borja" <kuurtb@gmail.com>
To: "Derek J. Clark" <derekjohn.clark@gmail.com>,
	"Kurt Borja" <kuurtb@gmail.com>, "Rong Zhang" <i@rong.moe>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Armin Wolf" <W_Armin@gmx.de>,
	"Mark Pearson" <mpearson-lenovo@squebb.ca>
Cc: <platform-driver-x86@vger.kernel.org>
Subject: Re: Report: lenovo_wmi_other FW attributes always read 0
Date: Wed, 04 Feb 2026 18:33:34 -0500	[thread overview]
Message-ID: <DG6KJR1FONK7.2TL5QSPZ2A4GA@gmail.com> (raw)
In-Reply-To: <4C43FF8D-A529-4645-93E8-DF5E6734328A@gmail.com>

On Wed Feb 4, 2026 at 12:45 PM -05, Derek J. Clark wrote:
> On February 4, 2026 8:43:56 AM PST, Kurt Borja <kuurtb@gmail.com> wrote:
>>On Wed Feb 4, 2026 at 3:36 AM -05, Derek J. Clark wrote:
>>> On February 4, 2026 12:00:12 AM PST, Kurt Borja <kuurtb@gmail.com> wrote:
>>>>Hi all,
>>>
>>> Hi Kurt.
>>
>>Hi Derek,
>>
>>>
>>>>In my system (83KY Legion 7 16IAX1) the current_value of any of the
>>>>firmware attributes exposed by this driver always reads 0
>>>>
>>>>	$ ls /sys/class/firmware-attributes/lenovo-wmi-other-0/attributes/
>>>>	ppt_pl1_spl  ppt_pl2_sppt  ppt_pl3_fppt
>>>>
>>>>	$ cat /sys/class/firmware-attributes/lenovo-wmi-other-0/attributes/*/current_value
>>>>	0
>>>>	0
>>>>	0
>>>>
>>>>After investigating my acpidump [1] I found that the argument passed to
>>>>the WMI method Arg2 is matched as a whole integer instead of individual
>>>>bytes (L: 49203)
>>>>
>>>>	If ((ToInteger (Arg2) == 0x01010000))
>>>>	{
>>>>		Return (^^PC00.LPCB.EC0.F5E0) /* \_SB_.PC00.LPCB.EC0_.F5E0 */
>>>>	}
>>>>
>>>>	If ((ToInteger (Arg2) == 0x01020000))
>>>>	{
>>>>		Return (^^PC00.LPCB.EC0.CCP1) /* \_SB_.PC00.LPCB.EC0_.CCP1 */
>>>>	}
>>>>
>>>>	If ((ToInteger (Arg2) == 0x01030000))
>>>>	{
>>>>		/* This case (CPU FPPT) is actually just zero... */
>>>>		Return (Zero)
>>>>	}
>>>>
>>>>...and the driver always sets the second byte of Arg2 to the current
>>>>profile (mode), which is never zero!
>>>>
>>>>	attribute_id =
>>>>		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
>>>>		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
>>>>	-->	FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) |
>>>>		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
>>>>
>>>>So it never actually matches the attribute and falls back to zero :/
>>>>
>>>>This is however, not the case for all tunables. As you can see in the
>>>>acpidump, just bellow the block above, these are matched depending on
>>>>the current mode (second byte)
>>>
>>> This typically means that the method is stubbed so that it returns 0 on is_supported. During development I forgot to go back and add a check for this when adding the attributes. Per the spec, there is no profile that corresponds to 0x00, so this is working as expected when showing an unsupported attribute. If you add a debug print to the capdata driver when it is querying the attributes you can see the entries are empty. 
>>
>>Hmm it's weird because if I read
>>
>>	\_SB_.PC00.LPCB.EC0_.F5E0
>>	\_SB_.PC00.LPCB.EC0_.CCP1
>>
>>directly, they show the correct SPPT and SPL values shown in the windows
>>Legion Space app (under Custom mode).
>>
>>It seems like these are supported?
>
> Oh, that is strange, yeah. It also explains why some things just aren't working like they should. Once again the documentation is not completely accurate for this interface, which is quite frustrating... I think I have a device with this problem as well that I haven't yet troubleshot in the new series, so I can work it locally.
>
> It sounds like I'll need to check if the current mode entry is present in the cached data, and if not check if there is a  "no mode" present. Traversing the list twice seems tedious & slow, so I'll probably refactor the capdata side to check for a match in bytes 0, 1, and 3, and if byte 2 is 0 in the data then return that, but if it is missing continue to search for the match? They have historically been in ascending order (though, at this point I don't know if that can be trusted) I'm curious if any devices have both a 00 entry and mode specific entries, because that pattern would possibly break them. I could perhaps save the index of the 00 entry separately when it is seen, then keep looking for the exact match and, if not found, fall back to the saved index. That guarantees a maximum O(n) search every time, which isn't great either.

I checked the captada buffers in my device and the "mode" byte is never
zero :/ In fact every tunable is listed once for each mode, which I
think is just normal behavior.

Can you send me an acpidump of a device which enforces the mode byte for
each tunable? I want to see how other devices do it.

>
> I'd like to hear everyone else's thoughts, perhaps there's a more clever way we can do this efficiently and accurately.

I think my firmware's behavior is just non-compliant with the
specification. In that case a quirk approach should work? The only
problem would be the tunables that DO enforce the mode byte in my
system.

Another possibility is that there are some tunables that do not enforce
modes (but don't necessarily reject them either).

Maybe a workaround for non-compliant attributes could work. Something
dirty like

zero_fallback:
	attribute_id = LWMI_ATTR_ID(tunable_attr->device_id, tunable_attr->feature_id,
				    mode, tunable_attr->type_id);
	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
	if (ret)
		return ret;
	
	...

	if (value == 0) {
		mode = 0;
		goto zero_fallback;
	}

I wonder how Legion Space deals with my device...

>
> I assume these values are also tunable in Legion Space in Windows and not just viewable?
>
> Thanks,
> Derek
>
>>
>>>
>>> Coincidentally, I'm planning on submitting a series this week that adds the missing check for is_supported and also adds the not yet implemented attributes. If you want to take an early preview, it can be found here:
>>>
>>> https://github.com/pastaq/linux/tree/pastaq/6.18/lenovo/wmi-next
>>>
>>> With this you should only see attributes that are supported.
>>
>>Same problem, see bellow
>>
>>	$ fwupdmgr get-bios-settings
>>
>>	gpu_oc_stat:
>>	  Setting type:         Integer
>>	  Current Value:        0
>>	  Description:          Set the GPU overclocking status
>>	  Read Only:            False
>>	  Minimum value:        0
>>	  Maximum value:        0
>>	  Scalar Increment:     0
>>
>>	cpu_temp:
>>	  Setting type:         Integer
>>	  Current Value:        0
>>	  Description:          Set the CPU thermal load limit
>>	  Read Only:            False
>>	  Minimum value:        85
>>	  Maximum value:        105
>>	  Scalar Increment:     1
>>
>>	gpu_nv_ac_offset:
>>	  Setting type:         Integer
>>	  Current Value:        0
>>	  Description:          Set the Nvidia GPU AC total processing power baseline offset
>>	  Read Only:            False
>>	  Minimum value:        10
>>	  Maximum value:        80
>>	  Scalar Increment:     1
>>
>>	gpu_nv_cpu_boost:
>>	  Setting type:         Integer
>>	  Current Value:        0
>>	  Description:          Set the Nvidia GPU to CPU dynamic boost limit
>>	  Read Only:            False
>>	  Minimum value:        0
>>	  Maximum value:        0
>>	  Scalar Increment:     0
>>
>>	gpu_temp:
>>	  Setting type:         Integer
>>	  Current Value:        0
>>	  Description:          Set the GPU thermal load limit
>>	  Read Only:            False
>>	  Minimum value:        75
>>	  Maximum value:        87
>>	  Scalar Increment:     1
>>
>>	ppt_pl1_spl:
>>	  Setting type:         Integer
>>	  Current Value:        0
>>	  Description:          Set the CPU sustained power limit
>>	  Read Only:            False
>>	  Minimum value:        50
>>	  Maximum value:        110
>>	  Scalar Increment:     1
>>
>>	cpu_oc_stat:
>>	  Setting type:         Integer
>>	  Current Value:        0
>>	  Description:          Set the CPU overclocking status
>>	  Read Only:            False
>>	  Minimum value:        0
>>	  Maximum value:        0
>>	  Scalar Increment:     0
>>
>>	ppt_cpu_cl:
>>	  Setting type:         Integer
>>	  Current Value:        0
>>	  Description:          Set the CPU cross loading power limit
>>	  Read Only:            False
>>	  Minimum value:        30
>>	  Maximum value:        75
>>	  Scalar Increment:     1
>>
>>	ppt_pl2_sppt:
>>	  Setting type:         Integer
>>	  Current Value:        0
>>	  Description:          Set the CPU slow package power tracking limit
>>	  Read Only:            False
>>	  Minimum value:        60
>>	  Maximum value:        168
>>	  Scalar Increment:     1
>>
>>	ppt_pl1_tau:
>>	  Setting type:         Integer
>>	  Current Value:        0
>>	  Description:          Set the CPU sustained power limit exceed duration
>>	  Read Only:            False
>>	  Minimum value:        0
>>	  Maximum value:        0
>>	  Scalar Increment:     0
>>
>>	gpu_nv_ppab:
>>	  Setting type:         Integer
>>	  Current Value:        0
>>	  Description:          Set the Nvidia GPU power performance aware boost limit
>>	  Read Only:            False
>>	  Minimum value:        0
>>	  Maximum value:        0
>>	  Scalar Increment:     0
>>
>>	gpu_nv_ctgp:
>>	  Setting type:         Integer
>>	  Current Value:        0
>>	  Description:          Set the GPU configurable total graphics power
>>	  Read Only:            False
>>	  Minimum value:        0
>>	  Maximum value:        0
>>	  Scalar Increment:     0
>>
>>Also it still shows SPPT and SPL as supported (which is coherent with
>>Legion Space).
>>

-- 
Thanks,
 ~ Kurt

  reply	other threads:[~2026-02-04 23:33 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-04  8:00 Report: lenovo_wmi_other FW attributes always read 0 Kurt Borja
2026-02-04  8:36 ` Derek J. Clark
2026-02-04 16:43   ` Kurt Borja
2026-02-04 17:45     ` Derek J. Clark
2026-02-04 23:33       ` Kurt Borja [this message]
2026-02-05  0:04         ` Derek John Clark
2026-02-05  3:13           ` Derek John Clark
2026-02-05 17:40             ` Kurt Borja
2026-02-06  7:11               ` Derek John Clark
2026-02-06 18:01                 ` Rong Zhang
2026-02-06 23:31                   ` Derek J. Clark
2026-02-17 14:07                     ` Rong Zhang
2026-02-10 23:38                 ` Kurt Borja

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=DG6KJR1FONK7.2TL5QSPZ2A4GA@gmail.com \
    --to=kuurtb@gmail.com \
    --cc=W_Armin@gmx.de \
    --cc=derekjohn.clark@gmail.com \
    --cc=i@rong.moe \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=mpearson-lenovo@squebb.ca \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.