Closed 1ace closed 2 days ago
1ace
commented
1 year ago Been using this code since December but I kept forgetting to upstream it :see_no_evil:
The only thing I have really tested is the charge threshold, so it may contain bugs in the rest of the code.
linrunner
commented
1 year ago Hi, thank you very much for this. I have merged it here for now: https://github.com/linrunner/TLP/tree/pr/685-applesmc
Some questions:
applesmc kernel driver: which driver version do you use? The driver in mainline does not support the feature yet. I'm aware of https://github.com/c---/applesmc-next, but afaik it is not yet submitted for mainline - do you know more about it?
threshold value ranges:
Don't you want to be mentioned in the copyright statement?
I am interested in what is actually left of TLP's settings on the architecture, could you post the full output of sudo tlp-stat?
1ace
commented
1 year ago @linrunner sorry for the delay, I forgot to reply :facepalm:
I hadn't realized, but indeed the power supply driver isn't upstream yet, so this PR shouldn't be merged yet either (I just marked as draft to avoid mistakenly merging it). The version of the kernel used on Asahi distros currently lives in the @AsahiLinux fork at https://github.com/AsahiLinux/linux/, where all the Apple Silicon work is done before being upstreamed. The mscsmc driver specifically is available at https://github.com/AsahiLinux/linux/blob/asahi/drivers/power/supply/macsmc_power.c
I didn't see the post you linked (I'm not on twitter anymore); I guess I need to change the is_within_bounds checks from 0–100 to 50–100 (done locally, will be part of the next push).
As for 0, looking at the kernel code it is simply ignored:
#define MIN_CHARGE_START 50
#define MAX_CHARGE_START 100
[...]
case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
if (val->intval < MIN_CHARGE_START || val->intval > MAX_CHARGE_START)
return -EINVAL;I don't really care about copyright, and I don't think there's anything in here that could be "claimable" anyway (it's just a copy of your template with a few strings and numbers adapted), so I'm happy to leave it to you :)
sure :)
--- TLP 1.5.0 --------------------------------------------
+++ Configured Settings:
defaults.conf L0004: TLP_ENABLE="1"
defaults.conf L0005: TLP_WARN_LEVEL="3"
defaults.conf L0006: TLP_PERSISTENT_DEFAULT="0"
defaults.conf L0007: DISK_IDLE_SECS_ON_AC="0"
defaults.conf L0008: DISK_IDLE_SECS_ON_BAT="2"
defaults.conf L0009: MAX_LOST_WORK_SECS_ON_AC="15"
defaults.conf L0010: MAX_LOST_WORK_SECS_ON_BAT="60"
defaults.conf L0011: CPU_ENERGY_PERF_POLICY_ON_AC="balance_performance"
defaults.conf L0012: CPU_ENERGY_PERF_POLICY_ON_BAT="balance_power"
defaults.conf L0013: SCHED_POWERSAVE_ON_AC="0"
defaults.conf L0014: SCHED_POWERSAVE_ON_BAT="1"
defaults.conf L0015: NMI_WATCHDOG="0"
defaults.conf L0016: DISK_DEVICES="nvme0n1 sda"
defaults.conf L0017: DISK_APM_LEVEL_ON_AC="254 254"
defaults.conf L0018: DISK_APM_LEVEL_ON_BAT="128 128"
defaults.conf L0019: DISK_APM_CLASS_DENYLIST="usb ieee1394"
defaults.conf L0020: DISK_IOSCHED="keep keep"
defaults.conf L0021: SATA_LINKPWR_ON_AC="med_power_with_dipm max_performance"
defaults.conf L0022: SATA_LINKPWR_ON_BAT="med_power_with_dipm min_power"
defaults.conf L0023: AHCI_RUNTIME_PM_ON_AC="on"
defaults.conf L0024: AHCI_RUNTIME_PM_ON_BAT="auto"
defaults.conf L0025: AHCI_RUNTIME_PM_TIMEOUT="15"
defaults.conf L0026: PCIE_ASPM_ON_AC="default"
defaults.conf L0027: PCIE_ASPM_ON_BAT="default"
defaults.conf L0028: RADEON_DPM_PERF_LEVEL_ON_AC="auto"
defaults.conf L0029: RADEON_DPM_PERF_LEVEL_ON_BAT="auto"
defaults.conf L0030: RADEON_POWER_PROFILE_ON_AC="default"
defaults.conf L0031: RADEON_POWER_PROFILE_ON_BAT="default"
defaults.conf L0032: WIFI_PWR_ON_AC="off"
defaults.conf L0033: WIFI_PWR_ON_BAT="on"
defaults.conf L0034: WOL_DISABLE="Y"
defaults.conf L0035: SOUND_POWER_SAVE_ON_AC="1"
defaults.conf L0036: SOUND_POWER_SAVE_ON_BAT="1"
defaults.conf L0037: SOUND_POWER_SAVE_CONTROLLER="Y"
defaults.conf L0038: BAY_POWEROFF_ON_AC="0"
defaults.conf L0039: BAY_POWEROFF_ON_BAT="0"
defaults.conf L0040: BAY_DEVICE="sr0"
defaults.conf L0041: RUNTIME_PM_ON_AC="on"
defaults.conf L0042: RUNTIME_PM_ON_BAT="auto"
defaults.conf L0043: RUNTIME_PM_DRIVER_DENYLIST="mei_me nouveau radeon"
defaults.conf L0044: USB_AUTOSUSPEND="1"
defaults.conf L0045: USB_EXCLUDE_AUDIO="1"
defaults.conf L0046: USB_EXCLUDE_BTUSB="0"
defaults.conf L0047: USB_EXCLUDE_PHONE="0"
defaults.conf L0048: USB_EXCLUDE_PRINTER="1"
defaults.conf L0049: USB_EXCLUDE_WWAN="0"
defaults.conf L0050: USB_AUTOSUSPEND_DISABLE_ON_SHUTDOWN="0"
defaults.conf L0051: RESTORE_DEVICE_STATE_ON_STARTUP="0"
/etc/tlp.d/99-eric.conf L0027: RESTORE_THRESHOLDS_ON_BAT="1"
defaults.conf L0053: NATACPI_ENABLE="1"
defaults.conf L0054: TPACPI_ENABLE="1"
defaults.conf L0055: TPSMAPI_ENABLE="1"
/etc/tlp.d/99-eric.conf L0021: START_CHARGE_THRESH_BAT0="70"
/etc/tlp.d/99-eric.conf L0022: STOP_CHARGE_THRESH_BAT0="80"
+++ System Info
System =
BIOS =
OS Release = Arch Linux ARM
Kernel = 6.2.0-asahi-12-1-edge-ARCH #2 SMP PREEMPT_DYNAMIC Tue, 28 Mar 2023 09:12:05 +0000 aarch64
/proc/cmdline = initrd=\initramfs-linux-asahi-edge.img cryptdevice=UUID=54ed86af-b980-47eb-ae4b-48043d9f1ba9:luks root=/dev/mapper/vg0-root rw
Init system = systemd
Boot mode = UEFI
+++ TLP Status
State = enabled
RDW state = not installed
Last run = 03:05:01 PM, 172 sec(s) ago
Mode = battery
Power source = battery
+++ Processor
/sys/devices/system/cpu/cpu0/cpufreq/scaling_driver = apple-cpufreq
/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor = schedutil
/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors = conservative ondemand userspace powersave performance schedutil
/sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq = 600000 [kHz]
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq = 2424000 [kHz]
/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_frequencies = 600000 912000 1284000 1752000 2004000 2256000 2424000 [kHz]
/sys/devices/system/cpu/cpu1..cpu7: omitted for clarity, use -v to show all
/sys/devices/system/cpu/cpufreq/boost = (not available)
/sys/module/workqueue/parameters/power_efficient = N
/proc/sys/kernel/nmi_watchdog = 0
+++ Platform Profile
/sys/firmware/acpi/platform_profile = (not available)
/sys/firmware/acpi/platform_profile_choices = (not available)
+++ Temperatures
Fan speed = (not available)
+++ File System
/proc/sys/vm/laptop_mode = 2
/proc/sys/vm/dirty_writeback_centisecs = 6000
/proc/sys/vm/dirty_expire_centisecs = 6000
/proc/sys/vm/dirty_ratio = 20
/proc/sys/vm/dirty_background_ratio = 10
+++ Disks
Devices = nvme0n1 sda
/dev/nvme0n1:
Type = NVMe
Disk ID = nvme-APPLE_SSD_AP1024Z_0ba018e421a0e63e_1
Model = APPLE SSD AP1024Z
Firmware = 874.100.
Scheduler = none [mq-deadline] kyber bfq (multi queue)
Runtime PM:
/sys/block/nvme0n1/device/power/control = auto, autosuspend_delay_ms = (not available)
SMART info:
Critical Warning: 0x00
Temperature: 32 Celsius
Available Spare: 100%
Available Spare Threshold: 99%
Percentage Used: 0%
Data Units Written: 4,301,899 [2.20 TB]
Power Cycles: 532
Power On Hours: 24
Unsafe Shutdowns: 58
Media and Data Integrity Errors: 0
/dev/sda: not present.
+++ Other Graphics
/sys/class/drm/card1/device/driver = asahi
+++ Other Graphics
/sys/class/drm/card2/device/driver = apple-drm
+++ Wireless
bluetooth = on
nfc = none (no device)
wifi = on
wwan = none (no device)
hci0(hci_bcm4377) : bluetooth, not connected
wlan0(brcmfmac) : wifi, connected, power management = on
+++ Audio
+++ PCIe Active State Power Management
/sys/module/pcie_aspm/parameters/policy = [default] performance powersave powersupersave
+++ PCIe Runtime Power Management
Enable devices = (disabled)
Disable devices = (disabled)
Device denylist = (disabled)
Driver denylist = mei_me nouveau radeon
/sys/bus/pci/devices/0000:00:00.0/power/control = auto (0x060400, PCI bridge, pcieport)
/sys/bus/pci/devices/0000:01:00.0/power/control = auto (0x028000, Network controller, brcmfmac)
/sys/bus/pci/devices/0000:01:00.1/power/control = auto (0x028000, Network controller, hci_bcm4377)
+++ USB
Autosuspend = enabled
Device allowlist = (not configured)
Device denylist = (not configured)
Exclude audio = enabled
Exclude bluetooth = disabled
Exclude phones = disabled
Exclude printers = enabled
Exclude WWAN = disabled
+++ Battery Care
Plugin: applesmc
Supported features: charge thresholds
Driver usage:
* natacpi (APPLESMC_MOD) = active (charge thresholds)
Parameter value ranges:
* START_CHARGE_THRESH_BAT0: 0(off)..100
* STOP_CHARGE_THRESH_BAT0: 0..100(default)
+++ Battery Status: macsmc-battery
/sys/class/power_supply/macsmc-battery/manufacturer = (not available)
/sys/class/power_supply/macsmc-battery/model_name = bq40z651
/sys/class/power_supply/macsmc-battery/cycle_count = 37
/sys/class/power_supply/macsmc-battery/charge_full_design = 4563 [mAh]
/sys/class/power_supply/macsmc-battery/charge_full = 4366 [mAh]
/sys/class/power_supply/macsmc-battery/charge_now = 3136 [mAh]
/sys/class/power_supply/macsmc-battery/current_now = -1092 [mA]
/sys/class/power_supply/macsmc-battery/status = Discharging
/sys/class/power_supply/macsmc-battery/charge_control_start_threshold = 70 [%]
/sys/class/power_supply/macsmc-battery/charge_control_end_threshold = 80 [%]
Charge = 71.8 [%]
Capacity = 95.7 [%]
+++ Recommendations
* Install ethtool to disable Wake-on-LANAlso, marcan recently posted something about the EC firmware only supports 80% or 100% and that he might rewrite the threshold code to map to these two values so that the kernel doesn't have to be involved, which means the threshold would work even when the computer is suspended or even off, with the downside that it wouldn't support other values anymore, but these other values don't necessarily provide much anyway.
(I'm saying all this from memory, I'll try to find the post again to link to it instead, so don't believe me too much :P)
1ace
commented
1 year ago Found @marcan's post: https://social.treehouse.systems/@marcan/110219038891519815
For this PR, I guess the difference is simply that the is_within_bounds "$new_stop" 50 100 (and corresponding start) checks becomes "$new_stop" = 80 || "$new_stop" = 100 (and I'll have to check for the start but I expect 75 & 95).
linrunner
commented
1 year ago (1) I suppose all those who use Linux on Apple Silicon already use the Asahi Linux kernel. I can therefore imagine to merge your PR. What I don't like to do to myself anymore are external kernel modules as a prerequisite. I'm tired of the ones need by ThinkPads.
(2) OK
(3) Fair enough :-)
(4) Interesting. More devices than I thought. PCIe too.
For this PR, I guess the difference is simply that the is_within_bounds "$new_stop" 50 100 (and corresponding start) checks becomes "$new_stop" = 80 || "$new_stop" = 100 (and I'll have to check for the start but I expect 75 & 95).
The question is whether we should let the user configure the start threshold at all, or whether we should calculate it from the stop threshold instead.
1ace
commented
1 year ago @marcan chimed in on IRC to say that since none of this is specific to this power driver, there shouldn't be any need to PRs like this one, and I agree.
@linrunner do you think TLP could be reworked to enumerate power interfaces, instead of having to copy a boilerplate file just to change a couple of values in it? That way it will be automatically be compatible with all future devices that follow the spec :)
linrunner
commented
1 year ago @1ace The generic specification - i.e. charge_control_start/stop_threshold - is undoubtedly a great advance. Unfortunately, the user learns nothing from it about allowed value ranges and special cases of the respective hardware. In fact, there are a lot of variants, see https://linrunner.de/tlp/settings/bc-vendors.html.
I deliberately chose the design with the vendor-specific plugins to spare the users frustrating trial and error which values really work.
An approach to handle as many vendors as possible in a generic plugin with countless case distinctions was also discarded for reliability reasons.
I would be happy to finish writing the plugin and then ask you to test it.
linrunner
commented
11 months ago @1ace I finally found the time to revise your plugin. It's now available in the main branch. May I ask you to test?
linrunner
commented
10 months ago @ALL: Testing Instructions
Install Packages are available here. If there are none that match your distribution:
Test Automated
Install the tool clitest from your distribution's official repository Clone the main repo
git clone https://github.com/linrunner/TLP.git1.1 Execute the test script and post all output
_**Prerquisite: Connect the charger.**_cd TLP/cd unit-tests
clitest charge-thresholds_macbook
sudo tlp-stat -s -bManual
2.1 Execute the following test cases in a terminal and post all output
Prerquisite: Connect the charger.
sudo tlp-stat -s -b
sudo tlp setcharge 75 80
sudo tlp-stat -b2.2 Run on battery until charge level falls below 70%
sudo tlp-stat -b2.3 Connect the charger, check if charging stops at 80%.
sudo tlp-stat -b2.4 Revert to stop threshold 100%
sudo tlp fullcharge2.5 Check if charging commences and reaches 100%
sudo tlp-stat -b@ALL: Code improved once more. I'm still looking for a volunteer who owns the hardware and wants to test it!
ZerdoX-x
commented
7 months ago I'm still looking for a volunteer who owns the hardware and wants to test it!
I am on M1 laptop, gentoo linux. I have no experience with TLP, but I can try it ;)
UPD: oops, I found test instructions above. I'll post output today.
ZerdoX-x
commented
7 months ago LMK if i did something wrong. also, my laptop was full charged (acpi shows Full, 99%) when i launched tests, and with charger cable connected, as stated in test requirements
linrunner
commented
7 months ago @ZerdoX-x Thank you. I can see that the code and the test script need to be revised. The 1st battery is called macsmc-battery and not BAT0. That's what happens when you don't have the hardware yourself.
Stay tuned please.
ZerdoX-x
commented
7 months ago Sure. Just ping me and I'll provide results ASAP 👀
linrunner
commented
7 months ago @ZerdoX-x I noticed a concept problem during troubleshooting, which is why it took a while.
Please update the plugin and test again with the (also updated) test script. Show the output of tlp-stat -s -b too.
ZerdoX-x
commented
7 months ago 83% capacity :sob:
linrunner
commented
7 months ago @ZerdoX-x you didn't test with the latest, corrected plugin nor with the corrected script.
In the meantime I had to edit history. Use git pull --rebase to resync your local copy.
IMPORTANT: please test with the charger connected. The script is designed for AC mode only.
tlp-stat -s -btoo.
System = BIOS =
DMI data about the laptop is missing. May I see the output of
ls -l /sys/class/dmi/idOops. I didn't save the file properly and forgot to connect the charger. Now only #34 fails
DMI data about the laptop is missing. May I see the output of
ls -l /sys/class/dmi/id
I do not have a dmi folder
Most of them - drwxr-xr-x 2 root root 0 Jan 1 1970, all with the same permissions and owners
afair this is intended for now in asahi linux, I guess https://github.com/AsahiLinux/m1n1/pull/378 should fix it?
linrunner
commented
7 months ago Very well. In #34, the expected result in the test script is incorrect. Otherwise perfect. Thank you very much for your contribution so far.
What remains is to test whether the thresholds really stop the charging process. I suggest you carry out the manual test from above.
I will add a "(not available)" for the missing DMI information.
ZerdoX-x
commented
1 month ago UPD: nvm, i'll report a bit later about manual tests.
Set this in tlp.conf. The rest is defaults
START_CHARGE_THRESH_BAT0=75
STOP_CHARGE_THRESH_BAT0=80But now seems like I cannot charge above 75%
$ acpi
Battery 0: Not charging, 75%Shouldn't my laptop get charged to 80 first?
linrunner
commented
1 month ago @ZerdoX-x This may well be correct, as the definition of the start charge threshold is "battery charge level below which charging will begin when connecting the charger". Have you tried discharging to 74% first?
Incidentally, the output of any third-party tool is completely unsuitable for diagnosis. I always need to have tlp-stat -s -b.
ZerdoX-x
commented
1 month ago Yes. Seems like everything is working for me correctly. I was confused about laptop not charging but it was due to some software compiling on machine at that time
linrunner
commented
1 month ago Can I still see the current state, please?
tlp-stat -s -bI thought it would stop charging at 80, drop to 75 and start charging till 80 again
linrunner
commented
1 month ago ZerdoX-x
commented
1 month ago Oh my bad. It should be like that. So is everything alright?
linrunner
commented
1 month ago Yes. Thanks for testing.
dkwo
commented
1 month ago I wonder how this PR compares with https://github.com/AsahiLinux/asahi-scripts , which recently introduced udev rules and a systemd service for battery charge (see macsmc-battery folder). If I had to run a daemon, I'd prefer to just use tlp, which is battle-proven and I'm more familiar with. Cc @marcan @jannau
linrunner
commented
1 month ago @dkwo oh well, yet another conflict ... :-(
jannau
commented
1 month ago I don't see a conflict. https://github.com/AsahiLinux/asahi-scripts/pull/49 simply monitors charge_control_end_threshold (the only user controllable setting) using systemd (probably running anyway) and restores the last written value on driver load using udev (most certainly running). This should be fully transparent to other solutions except that the initial charge_control_end_threshold might differ from 100.
linrunner
commented
1 month ago @jannau Two different tools writing the same kernel tunable means unpredictable results = conflict. There can only be one.
jannau
commented
1 month ago I don't see much risk. Udev will in most cases just write to it on system boot probably before other daemons managing are running. In situations where that's not the case it will only write the last written value so nothing should notice this.
linrunner
commented
1 month ago @jannau If incorrect values suddenly appear in the charge thresholds you never know for sure which tool caused it. I don't think that's desirable for support (to put it mildly). Fortunately, the value range is quite small :-).
Btw: Which file contains the configuration for the udev rule?
linrunner
commented
3 weeks ago Hi @ALL : TLP 1.7 Beta 1 is out with this feature. More testers wanted -> https://github.com/linrunner/TLP/issues/760
Closes https://github.com/linrunner/TLP/issues/665