From: "Kurt Borja" <kuurtb@gmail.com>
To: "Derek John Clark" <derekjohn.clark@gmail.com>,
"Kurt Borja" <kuurtb@gmail.com>
Cc: "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>,
platform-driver-x86@vger.kernel.org
Subject: Re: Report: lenovo_wmi_other FW attributes always read 0
Date: Tue, 10 Feb 2026 18:38:52 -0500 [thread overview]
Message-ID: <DGBOF2O43IUT.1EJ7X7L00993R@gmail.com> (raw)
In-Reply-To: <CAFqHKTkHhx14rM7QTATLTq-ry1z53+D5h1y982gaBd+RbMNJAQ@mail.gmail.com>
On Fri Feb 6, 2026 at 2:11 AM -05, Derek John Clark wrote:
> On Thu, Feb 5, 2026 at 9:40 AM Kurt Borja <kuurtb@gmail.com> wrote:
>>
>> On Wed Feb 4, 2026 at 10:13 PM -05, Derek John Clark wrote:
>> > On Wed, Feb 4, 2026 at 4:04 PM Derek John Clark
>> > <derekjohn.clark@gmail.com> wrote:
>> >>
>> >> On February 4, 2026 3:33:34 PM PST, Kurt Borja <kuurtb@gmail.com> wrote:
>> >> >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 attached the Legion Go 2 acpidump
>> >>
>> >> >>
>> >> >> 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.
>> >>
>> >> I think doing it programmatically would be more effective if we can
>> >> get it to work. I'd imagine there are lots of models with random
>> >> attributes that behave this way.
>> >>
>> >> >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;
>> >> > }
>> >>
>> >> That could work, though I'd do it like this to avoid infinite recursion:
>> >>
>> >> 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) {
>> >> mode = 0;
>> >> goto zero_fallback;
>> >>
>> >> if (value == 0) {
>> >> return -EINVAL;
>> >> }
>> >>
>> >> I'll push something to that effect to my tree and we'll see how that
>> >> works. The downside is traversing the list multiple times, and the
>> >> need to put this in multiple _show functions. As a proof of concept
>> >> I'm fine with it though.
>> >>
>> >> - Derek
>> >>
>> >>
>> >
>> > Kurt,
>> >
>> > After adding the workaround as described I got some interesting
>> > results. The attribute I have issues with in particular is cpu_temp
>> > (01040000). This attribute has capdata for custom mode & shows full
>> > set/get support, but is hard coded to return 0 in the dsdt. That means
>> > this isn't a "silver bullet" to work around a poorly implemented BIOS
>> > it seems.
>>
>> In my system FPPT is also hardcoded to zero :/
>>
>> >
>> > I did go back and recheck the documentation, and all these attributes
>> > do have a valid 00 mode in the docs that I overlooked before. It looks
>> > like an older version of the spec possibly. The notes are fairly
>> > sparse on it.
>>
>> Is the 0x00 mode actually a mode or just something like no-mode?
>>
>> I checked the acpidump you sent the SPPT tunable accepts valid modes and
>> also the 0x00 mode when reading... that's interesting.
>>
>> >
>> > I updated by branch and also included a few lazy "info" debug
>> > statements to help figure out what is going on. Let me know if that
>> > has better results for you.
>>
>> It has! Now all attributes are readable. There quite a few attributes
>> with 0 min/max/step values. I guess that's just poorly implemented BIOS
>> again.
>>
>> Here are the results
>>
>> $ fwupdmgr get-bios-settings
>>
>> gpu_oc_stat:
>> Setting type: Integer
>> Current Value: 17
>> 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: 103
>> 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: 55
>> 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: 10
>> 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: 87
>> 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: 70
>> 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: 16
>> 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: 65
>> 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: 125
>> 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: 56
>> 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: 15
>> 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: 65
>> Description: Set the GPU configurable total graphics power
>> Read Only: False
>> Minimum value: 0
>> Maximum value: 0
>> Scalar Increment: 0
>
> That is encouraging, at least we're on the right path.
>
>> I still can't write to them though. Every write gets ignored without
>> failing
>>
>> $ cat /sys/class/firmware-attributes/lenovo-wmi-other-0/attributes/ppt_pl2_sppt/current_value
>> 125
>>
>> $ echo 126 | sudo tee /sys/class/firmware-attributes/lenovo-wmi-other-0/attributes/ppt_pl2_sppt/current_value
>> 126
>>
>> $ cat /sys/class/firmware-attributes/lenovo-wmi-other-0/attributes/ppt_pl2_sppt/current_value
>> 125
>>
>> I tried writting to them using acpi_call and it works (0x7e = 126)
>>
>> $ echo '\_SB_.GZFD.WMAE 0 0x12 {0x00, 0x00, 0x01, 0x01, 0x7e, 0x00, 0x00, 0x00}' | sudo tee /proc/acpi/call
>> \_SB_.GZFD.WMAE 0 0x12 {0x00, 0x00, 0x01, 0x01, 0x7e, 0x00, 0x00, 0x00}
>>
>> $ sudo cat /proc/acpi/call
>> 0x0
>>
>> $ cat /sys/class/firmware-attributes/lenovo-wmi-other-0/attributes/ppt_pl2_sppt/current_value
>> 126
>
> This is very annoying. Presumably that means your device is providing
> good capdata for custom mode on some attributes? If that is the case
> then it will bee very difficult to auto-discover the no-mode.
>
>> Maybe we can check if the tunable returns zero at the discovery phase
>> and add some kind of flag there `.no_mode = 1`. Then we can check that
>> flag when writting the attribute.
>>
>> It's getting tricky because legion go 2 does enforce 0xFF mode when
>> writting, while my device enforces 0x00. Even worse, if I pass 0xFF it
>> returns 0, like it was successful...
>
> Indeed, I confirmed my device does the inverse.
>
>
>> $ echo '\_SB_.GZFD.WMAE 0 0x12 {0x00, 0xFF, 0x01, 0x01, 0x7e, 0x00, 0x00, 0x00}' | sudo tee /proc/acpi/call
>> \_SB_.GZFD.WMAE 0 0x12 {0x00, 0xFF, 0x01, 0x01, 0x7e, 0x00, 0x00, 0x00}
>>
>> $ sudo cat /proc/acpi/call
>> 0x0
>>
>> IMO we should check this no_mode stuff at the discovery phase.
>
> I updated my branch to try and discover the GET and SET per attribute
> during the is_supported phase by doing the fail loop there, once for
> each. I store the modes that worked on new members of the tunable_attr
> to reuse if either 0xff or 0x00 worked, and report -EOPNOTSUPP if
> nothing worked. If that happens it won't create the attribute at all,
> which eliminates showing attributes with botched implementations such
> as the gpu_temp on Go 2. That will add a lot of wmi calls on init (up
> to 2 per attribute in the config), but that might be preferable as it
> eliminates the loops in the _store and _show functions for all the RO
> and RW file descriptors. I check the capdata first at least to early
> exit if the attribute isn't supported at all.
>
> To summarize the flow in is_suported is now:
> - Check if there is a capdata block for the attribute under custom mode
> - If that fails, check if there is a capdata block for the attribute
> under no mode
> - If both fail then return
> - If one of them succeed, check if the attribute is supported in the
> capdata block, return -EOPNOTSUPP if not.
> - set the tunable_attr->get_mode_id to the successful mode.
> - reset the mode var to custom, get the current value, and check if
> the return value is 0
> - if that fails, try again with no mode
> - if both fail, return -EOPNOTSUPP
> - If either pass, set the tunable_attr->set_mode_id to the successful mode.
> - return capdata->supported
>
> Then in the _show & _store functions I can just reuse the previously
> discovered mode values. when getting the capdata or setting/getting
> the current value itself.
>
> There is some hope perhaps in using the WMI interface version for a
> quirk as well. 0x000c0000 "Get BIOS WMI Version" returns a u32 with
> bytes 015 being minor version and 16-31 being the major version. If we
> were to quirk this then presumably that would have a discernible
> pattern. It would require a large sample size to figure out which
> devices do what exactly though, which would be less than ideal.
>
> In any case, I've updated my branch with the changes. If that works
> for you I'll clean it up a bit and start getting ready to submit for
> review. I do have a couple more features to add on top of this issue
> so my timeline for submitting has slid right about a week.
>
> Thanks,
> Derek
Hi Derek,
The procedure above does fix reading and modifying tunables :)
I had a bit of hope the patch would be short enough for stable inclusion
but I think that may not be possible.
Thanks for working on this!
--
Thanks,
~ Kurt
prev parent reply other threads:[~2026-02-10 23:38 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
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 [this message]
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=DGBOF2O43IUT.1EJ7X7L00993R@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.