Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asusctl: init at 4.0.6 #147786

Closed
wants to merge 4 commits into from
Closed

asusctl: init at 4.0.6 #147786

wants to merge 4 commits into from

Conversation

Cogitri
Copy link
Contributor

@Cogitri Cogitri commented Nov 29, 2021

Motivation for this change

Add new package asusctl to improve support for ASUS Laptops.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

Copy link
Contributor

@legendofmiracles legendofmiracles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work (by the looks of it, can't test it)! I tried my hands at it a while back but gave up very quickly after some complications...

maintainers/maintainer-list.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/asusctl/default.nix Show resolved Hide resolved
@Cogitri
Copy link
Contributor Author

Cogitri commented Nov 29, 2021

Hm, for some reason the systemd system unit service isn't copied to share/systemd while the user services are

@figsoda
Copy link
Member

figsoda commented Nov 29, 2021

should probably have more patching since asusctl tries to edit config files (example)

would be nice to have a module as well

nixos/modules/services/misc/asusctl.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/asusctl/default.nix Outdated Show resolved Hide resolved
@leo60228
Copy link
Member

leo60228 commented Dec 8, 2021

I could be wrong, but I think the convention is to have separate asusctl: init at 4.0.6 and nixos/asusctl: init commits.

@Cogitri
Copy link
Contributor Author

Cogitri commented Dec 12, 2021

Fixed 👍

Copy link
Member

@leo60228 leo60228 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I won't be able to test this for a few weeks, but it looks good.

@Madouura
Copy link
Contributor

For the most part, it seems to be working fine.
Just a few problems, at least on my configuration.

supergfxd:

sudo systemctl status supergfxd
● supergfxd.service - SUPERGFX
     Loaded: loaded (/etc/systemd/system/supergfxd.service; linked; vendor preset: enabled)
     Active: active (running) since Thu 2021-12-16 10:29:19 CST; 18min ago
   Main PID: 4725 (supergfxd)
         IP: 0B in, 0B out
         IO: 0B read, 20.0K written
      Tasks: 1 (limit: 18444)
     Memory: 284.0K
        CPU: 0
     CGroup: /system.slice/supergfxd.service
             └─4725 /nix/store/ysk35xrh97hb2lrw67h2ndhkk5wzv0mg-supergfxctl-2.0.4/bin/supergfxd

Dec 16 10:29:21 tsuki supergfxd[4725]: INFO: GFX: 0000:07:00.4: Function for 0000:07:00.0
Dec 16 10:29:21 tsuki supergfxd[4725]: INFO: GFX: 0000:07:00.2: Function for 0000:07:00.0
Dec 16 10:29:21 tsuki supergfxd[4725]: INFO: GFX: 0000:07:00.0: Function for 0000:07:00.0
Dec 16 10:29:21 tsuki supergfxd[4725]: INFO: GFX: 0000:07:00.5: Function for 0000:07:00.0
Dec 16 10:29:21 tsuki supergfxd[4725]: INFO: GFX: 0000:07:00.3: Function for 0000:07:00.0
Dec 16 10:29:21 tsuki supergfxd[4725]: INFO: GFX: 0000:07:00.1: Function for 0000:07:00.0
Dec 16 10:29:21 tsuki supergfxd[4725]: INFO: GFX: 0000:01:00.0: NVIDIA graphics, setting PM to auto
Dec 16 10:29:21 tsuki supergfxd[4725]: INFO: GFX: Writing /etc/X11/xorg.conf.d/90-nvidia-primary.conf
Dec 16 10:29:21 tsuki supergfxd[4725]: INFO: GFX: Writing /etc/modprobe.d/supergfxd.conf
Dec 16 10:29:21 tsuki supergfxd[4725]: ERROR: Gfx controller: Command exec error: "modprobe" "nvidia_drm": No such file or directory (os error 2)

asusd:

● asusd.service - ASUS Notebook Control
     Loaded: loaded (/etc/systemd/system/asusd.service; linked; vendor preset: enabled)
     Active: active (running) since Thu 2021-12-16 10:43:10 CST; 4s ago
    Process: 11348 ExecStartPre=/nix/store/fvprxgcxf4px865gdjd81fbwnxcjrg41-coreutils-9.0/bin/sleep 2 (code=exited, status=0/SUCCESS)
   Main PID: 11350 (asusd)
         IP: 0B in, 0B out
         IO: 3.3M read, 12.0K written
      Tasks: 4 (limit: 18444)
     Memory: 2.4M
        CPU: 0
     CGroup: /system.slice/asusd.service
             └─11350 /nix/store/rfdn03vqb8s2kjc5llqi9hmbcrbmnrrn-asusctl-4.0.6/bin/asusd

Dec 16 10:43:10 tsuki asusd[11350]: ERROR: rog_bios_control: Path /sys/firmware/efi/efivars/AsusPostLogoSound-607005d5-3f75-4b2e-98f0-85ba66797a3e: No such fil>
Dec 16 10:43:10 tsuki asusd[11350]: INFO: Battery charge limit: 80
Dec 16 10:43:10 tsuki asusd[11350]: INFO: Device has profile control available
Dec 16 10:43:10 tsuki asusd[11350]: ERROR: AniMe control: Missing functionality: ASUS AniMe device node not found
Dec 16 10:43:10 tsuki asusd[11350]: INFO: Matched to ROG Zephyrus G15 GA503Q
Dec 16 10:43:10 tsuki asusd[11350]: WARN: led_node: Missing functionality: ASUS LED device node not found
Dec 16 10:43:10 tsuki asusd[11350]: WARN: led_node: Missing functionality: ASUS LED device node not found
Dec 16 10:43:10 tsuki asusd[11350]: WARN: led_node: Missing functionality: ASUS LED device node not found
Dec 16 10:43:10 tsuki asusd[11350]: INFO: Using device at: "/dev/hidraw5" for LED control
Dec 16 10:43:10 tsuki systemd[1]: Started ASUS Notebook Control.

supergfx can't seem to modprobe, for some reason.
asusd can't seem to read an efivar, which I have confirmed is there.
asusd won't start without a rebuild, and it goes without mentioning, doesn't start on boot, so I have to nixos-rebuild and then start it manually.

@Madouura
Copy link
Contributor

Madouura commented Dec 18, 2021

Also, supergfxd creates /etc/supergfxd.conf.
This should be controlled by the nix configuration in nixos/supergfxctl.nix preferably.

Known options:

{
  "gfx_mode": "Hybrid",
  "gfx_managed": true,
  "gfx_vfio_enable": false
}

Comment on lines +33 to +35
# Use default phases since the build scripts install systemd services and udev rules too
buildPhase = "buildPhase";
installPhase = "installPhase";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Use default phases since the build scripts install systemd services and udev rules too
buildPhase = "buildPhase";
installPhase = "installPhase";

Is that not working?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because then cargo build is run instead of make.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely want to run our cargo build phase because it is very different from standard cargo build process. We can run anything else in post phases.
Also we already handle stripping and vendoring outself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still needs to be fixed properly

@mattywillo
Copy link

mattywillo commented May 14, 2022

Gave this a quick try, something I noticed: supergfxctrl creates it's own modprobe config at /etc/modprobe.d/supergfxd.conf, pretty unexpected to turn on the service, run supergfxctrl once, then turn off the service and still have the nvidia kernel module blacklisted on next reboot.

Not a showstopper but might throw some people for a loop, not sure the best fix either, since it seems to rely on doing this for any of its features that require a reboot. Ideally the file would be cleaned up if the service is disabled but I don't believe there is a clean way to do that, maybe a patch to prevent it from writing to modprobe.d, and a config option to enable/disable the patch, so the user is aware of non-declarative changes to the system?

@Cogitri
Copy link
Contributor Author

Cogitri commented May 24, 2022

Gave this a quick try, something I noticed: supergfxctrl creates it's own modprobe config at /etc/modprobe.d/supergfxd.conf, pretty unexpected to turn on the service, run supergfxctrl once, then turn off the service and still have the nvidia kernel module blacklisted on next reboot.

Not a showstopper but might throw some people for a loop, not sure the best fix either, since it seems to rely on doing this for any of its features that require a reboot. Ideally the file would be cleaned up if the service is disabled but I don't believe there is a clean way to do that, maybe a patch to prevent it from writing to modprobe.d, and a config option to enable/disable the patch, so the user is aware of non-declarative changes to the system?

I'd rather get this merged first before pouring more work into this PR

@mattywillo
Copy link

mattywillo commented May 25, 2022

I'd rather get this merged first before pouring more work into this PR

For sure, its a fairly niche situation in an already niche pkg that's only a portion of the PR, with no obviously correct solution, I agree on getting it merged as it is, just wanted to note it down

@Madouura
Copy link
Contributor

cc @SuperSandro2000
Could we get this merged please?

@Madouura
Copy link
Contributor

Didn't notice the branch conflict.
@Cogitri Could you please fix it?

@Cogitri
Copy link
Contributor Author

Cogitri commented Jun 28, 2022

Done ✔️

Comment on lines +9 to +23
+StartLimitInterval=0
StartLimitBurst=2
Before=multi-user.target
+After=supergfxd.service
+Requires=supergfxd.service

[Service]
Environment=IS_SERVICE=1
-ExecStartPre=/bin/sleep 2
ExecStart=/usr/bin/asusd
+StateDirectory=asusd
Restart=on-failure
Restart=always
-RestartSec=1
+RestartSec=4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather overwrite this within the nixos service than patch it.

Comment on lines +48 to +49
buildPhase = "buildPhase";
installPhase = "installPhase";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs a proper solution.

@onny
Copy link
Contributor

onny commented Sep 6, 2022

Please rebase

@andOrlando
Copy link

andOrlando commented Sep 18, 2022

Why do we want /etc/supergfxd.conf managed by nix? Would you, in some cases, want ot be able to switch graphics modes on the fly without having to rebuild your system? Also it might be worth updating supergfxctl seeing as it's currently on 4.0.5

@devurandom
Copy link

devurandom commented Sep 24, 2022

It would be great to have some variant of asusctl in NixOS, even if it had some quirks. I would like to at least be able to change from flashing keyboard backlight to a static colour or disable the AniMe Matrix display.

@SuperSandro2000
Copy link
Member

Why do we want /etc/supergfxd.conf managed by nix?

Because NixOS tries to be mostly stateless and most system wide settings are managed by it.

Would you, in some cases, want ot be able to switch graphics modes on the fly without having to rebuild your system?

Do we? I don't know but a specialization could fix that.

@K900
Copy link
Contributor

K900 commented Dec 2, 2022

Superseded by #201081

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet