ThinkPad battery charge control

2024-04-25 Thread Malte Dehling
Hi,

I've added some things to the thinkpad acpi driver to allow modifying
battery charging behavior: keep charge within a range, force discharge,
disable charging on AC.  I think something like this would be good to
have in NetBSD, but not necessarily how I implemented it.  So this is
mainly a request for discussion :)

I've attached patches for reference and in case anyone wants to use them
-- tested on my old X230 and W530 but based on my reading and studying
acpidumps the same should work on most newer models.

On Linux and FreeBSD people just make acpi calls from userspace it
seems, usually hidden away in tools like TLP.

OpenBSD recently added sysctls hw.battery.{chargestart,chargestop,
chargemode} to provide this functionality (supported currently only by
the thinkpad and apple silicon drivers.)  The approach is simple, but
not as flexible as I would like:
- It doesn't let you control charging behavior of individual batteries.
- There are reasons to want to control charge_inhibit and
  force_discharge modes separately.

Finally, a question: The ACPI standard has for a long time had
(optional) _BMD & _BMC methods to control charging of control method
batteries.  Has anyone seen these implemented in the wild?

Cheers,
-- 
Malte Dehling
From 43e122db83d0fdd55817477446b46892b080c4da Mon Sep 17 00:00:00 2001
From: Malte Dehling 
Date: Tue, 23 Apr 2024 15:48:25 -0700
Subject: [PATCH 1/2] thinkpad(4): cosmetic changes

---
 sys/dev/acpi/thinkpad_acpi.c | 53 ++--
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/sys/dev/acpi/thinkpad_acpi.c b/sys/dev/acpi/thinkpad_acpi.c
index 395bbb65c03..0ef5e9d4653 100644
--- a/sys/dev/acpi/thinkpad_acpi.c
+++ b/sys/dev/acpi/thinkpad_acpi.c
@@ -138,8 +138,8 @@ typedef struct thinkpad_softc {
 #define THINKPAD_WWAN_RADIOSSW		0x02
 #define THINKPAD_WWAN_RESUMECTRL	0x04
 
-#define THINKPAD_UWB_HWPRESENT	0x01
-#define THINKPAD_UWB_RADIOSSW	0x02
+#define THINKPAD_UWB_HWPRESENT		0x01
+#define THINKPAD_UWB_RADIOSSW		0x02
 
 #define THINKPAD_RFK_BLUETOOTH		0
 #define THINKPAD_RFK_WWAN		1
@@ -165,7 +165,7 @@ static void	thinkpad_bluetooth_toggle(thinkpad_softc_t *);
 static bool	thinkpad_resume(device_t, const pmf_qual_t *);
 static void	thinkpad_brightness_up(device_t);
 static void	thinkpad_brightness_down(device_t);
-static uint8_t	thinkpad_brightness_read(thinkpad_softc_t *sc);
+static uint8_t	thinkpad_brightness_read(thinkpad_softc_t *);
 static void	thinkpad_cmos(thinkpad_softc_t *, uint8_t);
 
 CFATTACH_DECL3_NEW(thinkpad, sizeof(thinkpad_softc_t),
@@ -230,7 +230,7 @@ thinkpad_attach(device_t parent, device_t self, void *opaque)
 
 	sc->sc_ecdev = NULL;
 	for (curdev = deviter_first(&di, DEVITER_F_ROOT_FIRST);
-	 curdev != NULL; curdev = deviter_next(&di))
+	curdev != NULL; curdev = deviter_next(&di))
 		if (device_is_a(curdev, "acpiecdt") ||
 		device_is_a(curdev, "acpiec")) {
 			sc->sc_ecdev = curdev;
@@ -330,29 +330,30 @@ thinkpad_attach(device_t parent, device_t self, void *opaque)
 #endif
 	for (i = TP_PSW_DISPLAY_CYCLE; i < TP_PSW_LAST; i++)
 		sc->sc_smpsw[i].smpsw_type = PSWITCH_TYPE_HOTKEY;
-	psw[TP_PSW_DISPLAY_CYCLE].smpsw_name = PSWITCH_HK_DISPLAY_CYCLE;
-	psw[TP_PSW_LOCK_SCREEN].smpsw_name = PSWITCH_HK_LOCK_SCREEN;
-	psw[TP_PSW_BATTERY_INFO].smpsw_name = PSWITCH_HK_BATTERY_INFO;
-	psw[TP_PSW_EJECT_BUTTON].smpsw_name = PSWITCH_HK_EJECT_BUTTON;
-	psw[TP_PSW_ZOOM_BUTTON].smpsw_name = PSWITCH_HK_ZOOM_BUTTON;
-	psw[TP_PSW_VENDOR_BUTTON].smpsw_name = PSWITCH_HK_VENDOR_BUTTON;
+
+	psw[TP_PSW_DISPLAY_CYCLE].smpsw_name	= PSWITCH_HK_DISPLAY_CYCLE;
+	psw[TP_PSW_LOCK_SCREEN].smpsw_name	= PSWITCH_HK_LOCK_SCREEN;
+	psw[TP_PSW_BATTERY_INFO].smpsw_name	= PSWITCH_HK_BATTERY_INFO;
+	psw[TP_PSW_EJECT_BUTTON].smpsw_name	= PSWITCH_HK_EJECT_BUTTON;
+	psw[TP_PSW_ZOOM_BUTTON].smpsw_name	= PSWITCH_HK_ZOOM_BUTTON;
+	psw[TP_PSW_VENDOR_BUTTON].smpsw_name	= PSWITCH_HK_VENDOR_BUTTON;
 #ifndef THINKPAD_NORMAL_HOTKEYS
-	psw[TP_PSW_FNF1_BUTTON].smpsw_name = PSWITCH_HK_FNF1_BUTTON;
-	psw[TP_PSW_WIRELESS_BUTTON].smpsw_name = PSWITCH_HK_WIRELESS_BUTTON;
-	psw[TP_PSW_WWAN_BUTTON].smpsw_name = PSWITCH_HK_WWAN_BUTTON;
-	psw[TP_PSW_POINTER_BUTTON].smpsw_name  = PSWITCH_HK_POINTER_BUTTON;
-	psw[TP_PSW_FNF10_BUTTON].smpsw_name= PSWITCH_HK_FNF10_BUTTON;
-	psw[TP_PSW_FNF11_BUTTON].smpsw_name= PSWITCH_HK_FNF11_BUTTON;
-	psw[TP_PSW_BRIGHTNESS_UP].smpsw_name   = PSWITCH_HK_BRIGHTNESS_UP;
-	psw[TP_PSW_BRIGHTNESS_DOWN].smpsw_name = PSWITCH_HK_BRIGHTNESS_DOWN;
-	psw[TP_PSW_THINKLIGHT].smpsw_name  = PSWITCH_HK_THINKLIGHT;
-	psw[TP_PSW_VOLUME_UP].smpsw_name   = PSWITCH_HK_VOLUME_UP;
-	psw[TP_PSW_VOLUME_DOWN].smpsw_name = PSWITCH_HK_VOLUME_DOWN;
-	psw[TP_PSW_VOLUME_MUTE].smpsw_name = PSWITCH_HK_VOLUME_MUTE;
-	psw[TP_PSW_STAR_BUTTON].smpsw_name = PSWITCH_HK_STAR_BUTTON;
-	psw[TP_PSW_SCISSORS_BUTTON].smpsw_name = PSWITCH_HK_SCISSORS_BUTTON;
-	psw[TP_PSW_BLUETOOTH_BUTTON].smpsw_name = PSWITCH_HK_BLUETOOTH_BUTTON;
-	psw[TP

Re: ThinkPad battery charge control

2024-04-27 Thread Dave Tyson
On Thu, 2024-04-25 at 21:54 -0400, Malte Dehling wrote:
> Hi,
> 
> I've added some things to the thinkpad acpi driver to allow modifying
> battery charging behavior: keep charge within a range, force
> discharge,
> disable charging on AC.  I think something like this would be good to
> have in NetBSD, but not necessarily how I implemented it.  So this is
> mainly a request for discussion :)
> 
> I've attached patches for reference and in case anyone wants to use
> them
> -- tested on my old X230 and W530 but based on my reading and
> studying
> acpidumps the same should work on most newer models.
> 
> On Linux and FreeBSD people just make acpi calls from userspace it
> seems, usually hidden away in tools like TLP.
> 
> OpenBSD recently added sysctls hw.battery.{chargestart,chargestop,
> chargemode} to provide this functionality (supported currently only
> by
> the thinkpad and apple silicon drivers.)  The approach is simple, but
> not as flexible as I would like:
> - It doesn't let you control charging behavior of individual
> batteries.
> - There are reasons to want to control charge_inhibit and
>   force_discharge modes separately.
> 
> Finally, a question: The ACPI standard has for a long time had
> (optional) _BMD & _BMC methods to control charging of control method
> batteries.  Has anyone seen these implemented in the wild?
> 
> Cheers,

Hi Malte,

I applied your patch to NetBSD-10.0_STABLE successfully and tested the
kernel on a thinkpad T420s. I set charge_stop=85 and charge_start=15. 
With the laptop on mains it stopped charging the battery at the 85%
value. I switched force_discharge=1 and ran a build release to put some
load on, the battery level dropped down and sailed past 15% towards
zero - the thinkpad alarm alerted me and a quick unplug/replug of the
power lead started recharging it. I had vaguely hoped that as the 15%
level was breached the charging of the battery might restart
automatically, i.e force_discharge would reset to zero :-)

I think this is a really useful feature as there seems to be a view
that charging li-ion cells to 100% reduces their life and it is best to
avoid discharging them below 15%. These figures are what Samsung
recommend on mobiles phones. 

Cheers,
Dave


Re: Re: ThinkPad battery charge control

2024-04-28 Thread Malte Dehling
Hi Dave!

On Sat, Apr 27, 2024 at 09:57:41PM +0100, Dave Tyson wrote:
> On Thu, 2024-04-25 at 21:54 -0400, Malte Dehling wrote:
> > I've added some things to the thinkpad acpi driver to allow
> > modifying battery charging behavior: keep charge within a range,
> > force discharge, disable charging on AC.  I think something like
> > this would be good to have in NetBSD, but not necessarily how I
> > implemented it.  So this is mainly a request for discussion :)
> >
> > I've attached patches for reference and in case anyone wants to use
> > them -- tested on my old X230 and W530 but based on my reading and
> > studying acpidumps the same should work on most newer models.
>
> I applied your patch to NetBSD-10.0_STABLE successfully and tested the
> kernel on a thinkpad T420s. I set charge_stop=85 and charge_start=15.
> With the laptop on mains it stopped charging the battery at the 85%
> value. I switched force_discharge=1 and ran a build release to put
> some load on, the battery level dropped down and sailed past 15%
> towards zero - the thinkpad alarm alerted me and a quick unplug/replug
> of the power lead started recharging it. I had vaguely hoped that as
> the 15% level was breached the charging of the battery might restart
> automatically, i.e force_discharge would reset to zero :-)
>
> I think this is a really useful feature as there seems to be a view
> that charging li-ion cells to 100% reduces their life and it is best
> to avoid discharging them below 15%. These figures are what Samsung
> recommend on mobiles phones.

Thank you for taking the time to test!  As you noticed, the force
discharge setting will completely empty the battery.  This should not
trigger an emergency shutdown since your AC adapter remains connected,
so at least in that way it's harmless.  The only reason I can think of
to use force_discharge is for calibration purposes.

Some more observations from my systems (the behavior may vary a bit
between models):

- force_discharge is disabled when the AC adapter is disconnected or the
  battery is fully drained.
- enabling charge_inhibit seems to affect both batteries though
  disabling it must be done for each battery individually?  my systems
  only have one battery so I can't _really_ test this.
- charge_inhibit is _not_ reset when AC power is removed or the battery
  is drained, so be careful with it -- especially in conjunction with
  force_discharge.

Note that these are not things I programmed, it's just how the embedded
controller handles things.

In normal operation my thinkpad sits on its docking station and I have
charge_start=70 and charge_stop=85 set.  This way it will charge to 85%
and then over time slowly lose charge until it hits 70% at which point
it will charge back up to 85%.  I found a lot of discussion online over
which numbers are best, but ultimately it's a trade-off.

Cheers,
-- 
Malte Dehling


signature.asc
Description: PGP signature