fix: prevent powertop autosuspend on touchpad for idle detection - #1333
Conversation
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR modifies system power management configuration for a NixOS host. It adds a udev rule to prevent autosuspend of the touchpad's I2C HID device and changes logind behavior to ignore lid-close events instead of suspending on battery power. Changes
Possibly related PRs
Poem
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a udev rule to the NixOS configuration for the 'matic' host to disable power management autosuspend for a specific i2c touchpad, ensuring idle detection works correctly on battery. A review comment identifies a likely issue with the device matching logic, noting that the current rule targets a child device name rather than the i2c device itself, and suggests a more robust rule using the kernel name and handling both 'add' and 'change' actions.
| KERNEL=="event*", ATTRS{name}=="keyd virtual keyboard", GROUP="input", MODE:="0660" | ||
| KERNEL=="event*", ATTRS{name}=="keyd virtual pointer", GROUP="input", MODE:="0660" | ||
| # Prevent powertop autosuspend on touchpad so idle detection works on battery | ||
| ACTION=="add", SUBSYSTEM=="i2c", DRIVERS=="i2c_hid_acpi", ATTRS{name}=="PIXA3854:00 093A:0274", ATTR{power/control}="on" |
There was a problem hiding this comment.
The udev rule as written is unlikely to match the intended device. ATTRS{name} searches the device and its parents, but "PIXA3854:00 093A:0274" is typically the name of the child input device, not the i2c device itself. To correctly target the i2c device (where power/control is located for powertop tuning), you should match the i2c device directly using its kernel name. Additionally, using ACTION=="add|change" is more robust to ensure the setting is applied if the device state is updated or if powertop runs later in the boot sequence.
ACTION=="add|change", SUBSYSTEM=="i2c", KERNEL=="i2c-PIXA3854:00", ATTR{power/control}="on"
There was a problem hiding this comment.
Pull request overview
This PR aims to keep the PIXA3854 touchpad from being autosuspended (as triggered by powertop --auto-tune) so Hyprland/hypridle continues receiving touchpad input events on battery and doesn’t falsely trigger idle actions.
Changes:
- Add a udev rule that sets the touchpad
power/controltoonfor the PIXA3854 i2c-hid device. - Change systemd-logind lid-close handling to ignore lid close (previously suspended on battery).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Prevent powertop autosuspend on touchpad so idle detection works on battery | ||
| ACTION=="add", SUBSYSTEM=="i2c", DRIVERS=="i2c_hid_acpi", ATTRS{name}=="PIXA3854:00 093A:0274", ATTR{power/control}="on" | ||
| ''; | ||
|
|
There was a problem hiding this comment.
This udev rule will only apply when the device is added. With powerManagement.powertop.enable = true, the powertop systemd unit typically runs later during boot and may set power/control back to auto, overriding this rule and leaving the original issue unfixed. Consider enforcing power/control=on after powertop runs (e.g., a dedicated oneshot systemd service After=powertop.service/WantedBy=multi-user.target, and optionally also on resume) or disabling powertop autotuning for this host if you rely on consistent input wake events.
| # Ensure touchpad stays out of autosuspend even after powertop autotuning | |
| systemd.services.touchpad-power-control = { | |
| description = "Keep PIXA3854 touchpad power/control set to on for reliable idle detection"; | |
| wantedBy = [ "multi-user.target" ]; | |
| after = [ "powertop.service" ]; | |
| serviceConfig = { | |
| Type = "oneshot"; | |
| ExecStart = '' | |
| ${pkgs.runtimeShell} -c 'for d in /sys/bus/i2c/devices/*PIXA3854:00*/power/control; do echo on > "$d" || true; done' | |
| ''; | |
| }; | |
| }; |
| # Ignore lid close on both battery and AC — let hypridle's idle timer handle suspension | ||
| services.logind.settings.Login.HandleLidSwitch = "ignore"; |
There was a problem hiding this comment.
This changes lid-close behavior on battery from suspend to ignore, which is a significant behavioral change (risk of the machine staying on in a bag) and isn’t mentioned in the PR description/root-cause writeup. If intentional, please document the rationale in the PR description (or an in-file comment explaining why this host should ignore lid close on battery) so reviewers/users understand the tradeoff.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Mesa DescriptionTL;DRAdds a udev rule to prevent What changed?
Description generated by Mesa. Update settings |
Mesa DescriptionTL;DRAdds udev rule to keep touchpad power control as What changed?
Description generated by Mesa. Update settings |
Summary
on, preventing powertop--auto-tunefrom enabling autosuspend on the PIXA3854 touchpadRoot cause
powertop --auto-tunesets touchpadpower/controltoauto, which allows the kernel to suspend the touchpad after brief inactivity on battery. When suspended, touchpad events don't reach Hyprland, so hypridle thinks the system is idle.Test plan
cat /sys/bus/i2c/devices/i2c-PIXA3854:00/power/controlshowsonafter rebootSummary by cubic
Prevents touchpad autosuspend so idle detection works on battery and stops false screen dim/lock. Also ignores lid-close events so
hypridlemanages suspend consistently.power/controlset toon, overridingpowertop --auto-tuneand restoring input events tohypridle.HandleLidSwitchtoignoreon battery and AC, relying onhypridletimers for suspend.Written for commit ba5c582. Summary will update on new commits.