TLP icon indicating copy to clipboard operation
TLP copied to clipboard

[T440p Coreboot] False battery readings

Open karstenes opened this issue 3 years ago • 22 comments

Describe the bug

TLP does not detect my original thinkpad T440p battery on open-rc artix on coreboot. I have made sure acpi_call and thinkpad_acpi are running and I am able to control the fans through thinkpad_acpi. TLP continues to use the generic driver for the battery however. I have tried running linux-lts (linux 5.15.74-1) but no luck with that either.

Expected behavior

TLP detects the battery and uses the modern thinkpad driver.

To Reproduce

The problem occurs on both battery and AC. I am not sure how to reproduce, but my system is the latest artix linux openrc edition. I am running this on the latest skulls coreboot release as well.

Here is my tlp-stat: https://gist.github.com/karstenes/f8e1b6f220c1d6d12df2587338814bee

karstenes avatar Oct 25 '22 15:10 karstenes

Hi, your Gist link doesn't work.

linrunner avatar Oct 26 '22 18:10 linrunner

Fixed the link

karstenes avatar Oct 26 '22 18:10 karstenes

Coreboot does not return the correct model string as the original BIOS via /sys/class/dmi/id/product_version.

As a workaround you can try to configure:

X_SIMULATE_MODEL="ThinkPad T440p"

Then show tlp-stat -s -b again please.

This information here is also wrong:

/sys/class/power_supply/BAT0/charge_full_design             =   5616 [mAh]
/sys/class/power_supply/BAT0/charge_full                    =   4561 [mAh]
/sys/class/power_supply/BAT0/charge_now                     =   1682 [mAh]
/sys/class/power_supply/BAT0/current_now                    =   2090 [mA]

These are not mA(h) values but mW(h) values missing the last digit (I can conclude that because 56,000 mWh is a typical battery size for the T440p). See https://github.com/linrunner/TLP/issues/626#issuecomment-1068818111 .

You should create a Coreboot bug report for both of your issues.

linrunner avatar Oct 26 '22 18:10 linrunner

Yes, the battery is 57 Wh. Here is the tlp-stat on ac: https://gist.github.com/karstenes/0b7eba367c1243a804762bcfc04ef920 and bat: https://gist.github.com/karstenes/159be9ff50c1799662cec6dbcada21d2

The env variable did work to convince it it is a t440p.

Here's the battery variables you said were wrong (so TLP was just detecting the wrong unit?)

❯ cat /sys/class/power_supply/BAT0/charge_full_design 5616000

❯ cat /sys/class/power_supply/BAT0/charge_full 4561000

❯ cat /sys/class/power_supply/BAT0/charge_now 3210000

❯ cat /sys/class/power_supply/BAT0/current_now 2088000

karstenes avatar Oct 26 '22 18:10 karstenes

No, the kernel returns wrong values and I suppose it gets them from the BIOS via ACPI.

Actually there are two sets of battery sysfiles, one for W units and one for A units (I have added dots as thousand separators for clarity):

(1) W units scheme -- energy = J = Wh This is what all stock ThinkPad BIOSes provide, example for a 57 Wh battery:

/sys/class/power_supply/BAT0/energy_full_design: 57.000.000 [µWh]
/sys/class/power_supply/BAT0/energy_full: 53.700.000 [µWh]
/sys/class/power_supply/BAT0/energy_now: 37.410.000 [µWh]
/sys/class/power_supply/BAT0/power_now: 0 [µW]

(2) A units scheme -- charge = C = Ah This is what Coreboot provides for your T440p:

/sys/class/power_supply/BAT0/charge_full_design: 5.616.000 [µAh]
/sys/class/power_supply/BAT0/charge_full: 4.561.000 [µAh]
/sys/class/power_supply/BAT0/charge_now: 3.210.000 [µAh]
/sys/class/power_supply/BAT0/current_now: 2.088.000 [µA]

But your values are not plausible: the 6-cell with 57 Wh has a nominal voltage of ~11,5V resulting in ~4,96 Ah not 5,61 Ah.

The correct kernel response would be in your case: /sys/class/power_supply/BAT0/energy_full_design: 56.160.000 [µWh]

So my conclusion based on your values is that Coreboot provides the battery values to the kernel in the wrong scheme.

TLP accepts only the W units scheme for ThinkPads and thus doesn't display the A units sysfiles. Including a workaround in TLP for Coreboot wouldn't make sense, because the A unit values provided are incorrect anyway.

linrunner avatar Oct 27 '22 15:10 linrunner

Btw: you don't need acpi_call in conjunction with kernel 6.0 because it supports charge thresholds and recalibration natively. Unfortunately Coreboot does not support recalibration (refer to #626).

linrunner avatar Oct 27 '22 16:10 linrunner

@karstenes : Any news on this? Did you file a Coreboot bug yet?

linrunner avatar Nov 09 '22 17:11 linrunner

@linrunner sorry, been busy and forgot to. Anything else you want in the bug report other than what has been discussed?

karstenes avatar Nov 09 '22 18:11 karstenes

@karstenes My key points are all in https://github.com/linrunner/TLP/issues/657#issuecomment-1293730559

linrunner avatar Nov 10 '22 06:11 linrunner

@karstenes Anything new to report here?

linrunner avatar Jan 09 '23 07:01 linrunner

@karstenes Anything new to report here?

Nothing as of now. The coreboot issue has not been responded to as of now.

karstenes avatar Feb 23 '23 19:02 karstenes

Do you mind posting the link to the coreboot issue here?

linrunner avatar Feb 23 '23 19:02 linrunner

Coreboot Issue

karstenes avatar Feb 23 '23 19:02 karstenes

Libreboot shows the same symptoms: https://thinkpad-forum.de/threads/projektvorstellung-tlp-%E2%80%93-linux-stromsparen.82441/post-2349536

linrunner avatar May 04 '23 06:05 linrunner

apparently this was addressed some time ago? https://review.coreboot.org/c/coreboot/+/23178 maybe there was a breaking change or maybe this is different than what is being described see also: https://www.coreboot.org/Board:lenovo/x200#Recalibrate_batteries

diabl0w avatar Aug 06 '23 00:08 diabl0w

@diabl0w The world does not only consist of charge thresholds. If you look at the output of @karstenes, you will see that the battery as well as the charge thresholds are recognized and thus should work.

/sys/class/power_supply/BAT0/charge_control_start_threshold = 0 [%] /sys/class/power_supply/BAT0/charge_control_end_threshold = 100 [%]

The issue here is that false battery readings are provided by Coreboot. See my exhaustive explanation.

If you are affected by this, please contribute by updating the coreboot bug or creating your own. Just asking here does not bring us closer to a solution.

linrunner avatar Aug 07 '23 08:08 linrunner

@diabl0w The world does not only consist of charge thresholds. If you look at the output of @karstenes, you will see that the battery as well as the charge thresholds are recognized and thus should work.

/sys/class/power_supply/BAT0/charge_control_start_threshold = 0 [%] /sys/class/power_supply/BAT0/charge_control_end_threshold = 100 [%]

The issue here is that false battery readings are provided by Coreboot. See my exhaustive explanation.

If you are affected by this, please contribute by updating the coreboot bug or creating your own. Just asking here does not bring us closer to a solution.

interesting, i guess i thought that was the only thing in the world. or more likely i made an innocemt error as i was naviagting a few things at once. who knows... anyways, i apologize for the false positive and i appreciate you looking into the feedback. i will go ahead and ping the current open coreboot report to being attention to it

diabl0w avatar Aug 16 '23 21:08 diabl0w

@linrunner, in fact, i do recall where that link came from. it came from Patrick Rudolph, a coreboot maintainer, who linked to that commit as a fix for the battery recalibration issue. perhaps he thinks thresholds are the only thing in the world as well: https://ticket.coreboot.org/issues/156

and, i obtained that link from the link i posted in my original comment which linked here

diabl0w avatar Aug 16 '23 21:08 diabl0w

@karstenes @all Since no one from the Core- or Libreboot developers feels obliged to fix the problem, I changed my mind and added a workaround for the missing battery data in tlp-stat -b.

Packages are available here: https://download.linrunner.de/packages/

Please show the output of

  sudo tlp-stat -s -b

linrunner avatar Nov 09 '23 17:11 linrunner

Initial testing happened here.

More tester are welcome!

linrunner avatar Nov 23 '23 16:11 linrunner

Added to the FAQ: https://linrunner.de/tlp/faq/battery.html#are-thinkpads-with-coreboot-supported

linrunner avatar Aug 26 '24 17:08 linrunner

Hi @karstenes @diabl0w : TLP 1.7 Beta 1 is out with the corresponding change. Please verify -> https://github.com/linrunner/TLP/issues/760

linrunner avatar Sep 05 '24 08:09 linrunner