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

tuxedo-drivers: init at 4.6.1 #293017

Closed

Conversation

CutestNekoAqua
Copy link
Contributor

@CutestNekoAqua CutestNekoAqua commented Mar 3, 2024

Description of changes

The currently-offered tuxedo-keyboard driver has been deprecated for a long time - see here.

The new driver is called tuxedo-drivers and is located here; also has a GitHub read-only mirror here.

Closes #291669

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@CutestNekoAqua
Copy link
Contributor Author

I do not have a tuxedo laptop. Maybe someone with a tuxedo laptop should test the builds?

@CutestNekoAqua
Copy link
Contributor Author

I am not sure how we should go on with the hardware/tuxedo-keyboard module. Either we deprecate it and introduce a new hardware/tuxedo-hardware, or we rename it.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100 labels Mar 3, 2024
@RaitoBezarius
Copy link
Member

@blitz ?

@CutestNekoAqua
Copy link
Contributor Author

did some thinking and I think its best if we just rename the old module and do some adjustments to properly also integrate the tuxedo control center or something?

@blanky0230
Copy link
Member

did some thinking and I think its best if we just rename the old module and do some adjustments to properly also integrate the tuxedo control center or something?

Maybe. 🤷 But I suppose a more avid nix on tuxedo user should provide their points. As I mentioned at #291669 (comment) I rarely use mine anymore, so I'm also a bit out of the loop regarding their releases and changes.

@liketechnik
Copy link
Member

to properly also integrate the tuxedo control center

This is probably still difficult to do, due to:


I'm currently rebuilding my config using tuxedo-drivers instead of tuxedo-keyboard and come back here when I got results.

@liketechnik
Copy link
Member

liketechnik commented Mar 4, 2024

So, together with the 6.6.17-xanmod1 kernel on a Pulse 15 Gen 1, using the tuxedo-drivers package from this PR instead of tuxedo-keyboard works flawlessly, afaict (i.e. hardware works, tuxedo control center works, no errors in dmesg).

@CutestNekoAqua
Copy link
Contributor Author

CutestNekoAqua commented Mar 6, 2024

So, together with the 6.6.17-xanmod1 kernel on a Pulse 15 Gen 1, using the tuxedo-drivers package from this PR instead of tuxedo-keyboard works flawlessly, afaict (i.e. hardware works, tuxedo control center works, no errors in dmesg).

👍 nice. As no-one else took a stance, I think its best if we keep the old nix module and just introduce a new one? If no-one opposes that, I will do that for now.

Edit: rethought, as no one did have a concrete opinion on this, I will just rename the old module, as that prevents evaluation clutter, and its a very common thing in nixpkgs.

@blitz
Copy link
Contributor

blitz commented Mar 6, 2024

@RaitoBezarius I moved on to a Framework laptop, so I'm not particularly useful to test this anymore. :(

@blitz
Copy link
Contributor

blitz commented Mar 6, 2024

So do I understand this correctly and TCC is not required anymore, if you use these drivers?

@CutestNekoAqua
Copy link
Contributor Author

So do I understand this correctly and TCC is not required anymore, if you use these drivers?

Not sure, but I think it makes TCC optional.

@CutestNekoAqua
Copy link
Contributor Author

But most importantly: tuxedo-keyboard got merged into tuxedo-drivers.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Mar 7, 2024
@CutestNekoAqua
Copy link
Contributor Author

Okay I think thats all needed. If nobody opposes anything, feel free to merge from my side :)

@CutestNekoAqua CutestNekoAqua requested a review from blitz March 7, 2024 10:02
@CutestNekoAqua
Copy link
Contributor Author

Had to remove the deprecation notice, as I forgot thats not how to deprecate things in nixpkgs 😅

should be good to go now..

@CutestNekoAqua
Copy link
Contributor Author

I guess its ready to merge?

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Mar 19, 2024
@xaverdh
Copy link
Contributor

xaverdh commented Aug 10, 2024

ah, I think this is upstream commit bdcebd30983c3f7b2d15e8e26441d33bc5777dfe, which started passing through the MAKEFLAGS

@xaverdh
Copy link
Contributor

xaverdh commented Aug 10, 2024

You can fix that with the following:

-  makeFlags = kernel.makeFlags ++ [
-    "KERNELRELEASE=${kernel.modDirVersion}"
-    "KDIR=${kernel.dev}/lib/modules/${kernel.modDirVersion}/build"
-    "INSTALL_MOD_PATH=$(out)"
-  ];
+  makeFlags = lib.filter
+    (flag: lib.head (lib.strings.splitString "=" flag) != "O")
+    (kernel.makeFlags ++ [
+      "KERNELRELEASE=${kernel.modDirVersion}"
+      "KDIR=${kernel.dev}/lib/modules/${kernel.modDirVersion}/build"
+      "INSTALL_MOD_PATH=${placeholder "out"}"
+    ]);

@gabyx
Copy link
Contributor

gabyx commented Aug 17, 2024

@CutestNekoAqua : How is this PR best tested with a flake.nix? Can I load this PR as input with:

    # Test tuxedo-drivers on this PR: https://github.com/NixOS/nixpkgs/pull/293017
    tuxedo.url = "github:CutestNekoAqua/nixpkgs?ref=76427d3992c7a2a3293485d8322b98730afd20db";

and then I am abit stuck...

@gabyx
Copy link
Contributor

gabyx commented Aug 17, 2024

@CutestNekoAqua : I found out:
in flake.nix add

tuxedo.url = "github:CutestNekoAqua/nixpkgs?ref=76427d3992c7a2a3293485d8322b98730afd20db"

in configuration.nix

disabledModules = [
    "hardware/tuxedo-keyboard.nix"
  ];
import = [
"${inputs.tuxedo}/nixos/modules/hardware/tuxedo-drivers.nix"
...
];

@@ -490,6 +490,8 @@ in {

rust-out-of-tree-module = if lib.versionAtLeast kernel.version "6.7" then callPackage ../os-specific/linux/rust-out-of-tree-module { } else null;

tuxedo-drivers = if lib.versionAtLeast kernel.version "4.14" then callPackage ../os-specific/linux/tuxedo-drivers { } else null;

tuxedo-keyboard = if lib.versionAtLeast kernel.version "4.14" then callPackage ../os-specific/linux/tuxedo-keyboard { } else null;
Copy link
Contributor

Choose a reason for hiding this comment

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

the tuxedo-keyboard kernel module was not removed from this file, is this planned for the next step?

@dasj19
Copy link
Contributor

dasj19 commented Aug 25, 2024

I also tried to package this today, only now I am seeing this thread...

I ended up with a working package using:

{ lib, stdenv, fetchFromGitLab, linuxHeaders, pkgs, kernel, pahole }:

stdenv.mkDerivation (finalAttrs: {
  pname = "tuxedo-drivers-${kernel.version}";
  version = "4.6.2";

  src = fetchFromGitLab {
    owner = "tuxedocomputers";
    repo = "development/packages/tuxedo-drivers";
    rev = "v${finalAttrs.version}";
    hash = "sha256-HS/KGhgFW0vO2SSFYSdseQBhAZC70eAx9JvRgR6e6qs=";
  };

  buildInputs = [
    pahole
    linuxHeaders
  ];

  buildFlags = [ "all" ];

  makeFlags = [ "KDIR=${kernel.dev}/lib/modules/${kernel.modDirVersion}/build" ];

  installPhase = ''
    runHook preInstall

    mkdir -p "$out/lib/modules/${kernel.modDirVersion}"

    for module in clevo_acpi.ko clevo_wmi.ko \
        ite_8291/ite_8291.ko ite_8291_lb/ite_8291_lb.ko \
        ite_8297/ite_8297.ko ite_829x/ite_829x.ko \
        tuxedo_compatibility_check/tuxedo_compatibility_check.ko \
        tuxedo_io/tuxedo_io.ko \
        tuxedo_keyboard.ko \
        tuxedo_nb02_nvidia_power_ctrl/tuxedo_nb02_nvidia_power_ctrl.ko \
        tuxedo_nb04/tuxedo_nb04_kbd_backlight.ko \
        tuxedo_nb04/tuxedo_nb04_keyboard.ko \
        tuxedo_nb04/tuxedo_nb04_power_profiles.ko \
        tuxedo_nb04/tuxedo_nb04_sensors.ko \
        tuxedo_nb04/tuxedo_nb04_wmi_ab.ko \
        tuxedo_nb04/tuxedo_nb04_wmi_bs.ko \
        tuxedo_nb05/tuxedo_nb05_ec.ko \
        tuxedo_nb05/tuxedo_nb05_fan_control.ko \
        tuxedo_nb05/tuxedo_nb05_kbd_backlight.ko \
        tuxedo_nb05/tuxedo_nb05_keyboard.ko \
        tuxedo_nb05/tuxedo_nb05_power_profiles.ko \
        tuxedo_nb05/tuxedo_nb05_sensors.ko \
        uniwill_wmi.ko; \
      do
        mv src/$module $out/lib/modules/${kernel.modDirVersion}
    done

    runHook postInstall
  '';

meta = with lib; {
    description = "tuxedo-drivers kernel module";
    homepage = "https://gitlab.com/tuxedocomputers/development/packages/tuxedo-drivers";
    license = licenses.gpl3Plus;
    maintainers = with maintainers; [ dasj19 ];
};

Maybe we should merge our efforts and have this merged in the nixpkgs.
I have a Tuxedo Book XA15 to test on.

@gabyx
Copy link
Contributor

gabyx commented Aug 27, 2024

  disabledModules = [
    "hardware/tuxedo-keyboard.nix"
    "services/hardware/tuxedo-rs.nix"
  ];

  imports = [
    # Testing PR 293017
    "${inputs.tuxedo}/nixos/modules/hardware/tuxedo-drivers.nix"
    "${inputs.tuxedo}/nixos/modules/services/hardware/tuxedo-rs.nix"
...

and then somewhere I do:

hardware.tuxedo-drivers.enable = true;
  hardware.tuxedo-rs = {
    enable = true;
    tailor-gui.enable = true;
  };

@CutestNekoAqua : I still have problems using this, how do you enable this modification over NixOS?

      at /nix/store/ch5lqh5klkjnf7iiik2hvnvx3qy3bgbw-source/nixos/modules/hardware/tuxedo-drivers.nix:7:20:
┃
┃             6|   cfg = config.hardware.tuxedo-drivers;
┃             7|   tuxedo-drivers = config.boot.kernelPackages.tuxedo-drivers;
┃              |                    ^
┃             8| in

How do I make this config.boot.kernelPackages.tuxedo-drivers also available?

@Artturin
Copy link
Member

  disabledModules = [
    "hardware/tuxedo-keyboard.nix"
    "services/hardware/tuxedo-rs.nix"
  ];

  imports = [
    # Testing PR 293017
    "${inputs.tuxedo}/nixos/modules/hardware/tuxedo-drivers.nix"
    "${inputs.tuxedo}/nixos/modules/services/hardware/tuxedo-rs.nix"
...

and then somewhere I do:

hardware.tuxedo-drivers.enable = true;
  hardware.tuxedo-rs = {
    enable = true;
    tailor-gui.enable = true;
  };

@CutestNekoAqua : I still have problems using this, how do you enable this modification over NixOS?

      at /nix/store/ch5lqh5klkjnf7iiik2hvnvx3qy3bgbw-source/nixos/modules/hardware/tuxedo-drivers.nix:7:20:
┃
┃             6|   cfg = config.hardware.tuxedo-drivers;
┃             7|   tuxedo-drivers = config.boot.kernelPackages.tuxedo-drivers;
┃              |                    ^
┃             8| in

How do I make this config.boot.kernelPackages.tuxedo-drivers also available?

  boot.kernelPackages = pkgs.linuxKernel.packages.linux_X_X.extend (final: prev: {
    tuxedo-drivers = inputs.tuxedo.legacyPackages.x86_64-linux.linuxKernel.packages.linux_X_X.tuxedo-drivers;
    # OR `callPackage` it to download less packages
    tuxedo-drivers = prev.callPackage (inputs.tuxedo + "/pkgs/os-specific/linux/tuxedo-keyboard/default.nix") { };
  });

@CutestNekoAqua
Copy link
Contributor Author

I also tried to package this today, only now I am seeing this thread...

I ended up with a working package using:

{ lib, stdenv, fetchFromGitLab, linuxHeaders, pkgs, kernel, pahole }:

stdenv.mkDerivation (finalAttrs: {
  pname = "tuxedo-drivers-${kernel.version}";
  version = "4.6.2";

  src = fetchFromGitLab {
    owner = "tuxedocomputers";
    repo = "development/packages/tuxedo-drivers";
    rev = "v${finalAttrs.version}";
    hash = "sha256-HS/KGhgFW0vO2SSFYSdseQBhAZC70eAx9JvRgR6e6qs=";
  };

  buildInputs = [
    pahole
    linuxHeaders
  ];

  buildFlags = [ "all" ];

  makeFlags = [ "KDIR=${kernel.dev}/lib/modules/${kernel.modDirVersion}/build" ];

  installPhase = ''
    runHook preInstall

    mkdir -p "$out/lib/modules/${kernel.modDirVersion}"

    for module in clevo_acpi.ko clevo_wmi.ko \
        ite_8291/ite_8291.ko ite_8291_lb/ite_8291_lb.ko \
        ite_8297/ite_8297.ko ite_829x/ite_829x.ko \
        tuxedo_compatibility_check/tuxedo_compatibility_check.ko \
        tuxedo_io/tuxedo_io.ko \
        tuxedo_keyboard.ko \
        tuxedo_nb02_nvidia_power_ctrl/tuxedo_nb02_nvidia_power_ctrl.ko \
        tuxedo_nb04/tuxedo_nb04_kbd_backlight.ko \
        tuxedo_nb04/tuxedo_nb04_keyboard.ko \
        tuxedo_nb04/tuxedo_nb04_power_profiles.ko \
        tuxedo_nb04/tuxedo_nb04_sensors.ko \
        tuxedo_nb04/tuxedo_nb04_wmi_ab.ko \
        tuxedo_nb04/tuxedo_nb04_wmi_bs.ko \
        tuxedo_nb05/tuxedo_nb05_ec.ko \
        tuxedo_nb05/tuxedo_nb05_fan_control.ko \
        tuxedo_nb05/tuxedo_nb05_kbd_backlight.ko \
        tuxedo_nb05/tuxedo_nb05_keyboard.ko \
        tuxedo_nb05/tuxedo_nb05_power_profiles.ko \
        tuxedo_nb05/tuxedo_nb05_sensors.ko \
        uniwill_wmi.ko; \
      do
        mv src/$module $out/lib/modules/${kernel.modDirVersion}
    done

    runHook postInstall
  '';

meta = with lib; {
    description = "tuxedo-drivers kernel module";
    homepage = "https://gitlab.com/tuxedocomputers/development/packages/tuxedo-drivers";
    license = licenses.gpl3Plus;
    maintainers = with maintainers; [ dasj19 ];
};

Maybe we should merge our efforts and have this merged in the nixpkgs.
I have a Tuxedo Book XA15 to test on.

Imo this way is way to complicated. My way works fine :)

@CutestNekoAqua
Copy link
Contributor Author

You can fix that with the following:

-  makeFlags = kernel.makeFlags ++ [
-    "KERNELRELEASE=${kernel.modDirVersion}"
-    "KDIR=${kernel.dev}/lib/modules/${kernel.modDirVersion}/build"
-    "INSTALL_MOD_PATH=$(out)"
-  ];
+  makeFlags = lib.filter
+    (flag: lib.head (lib.strings.splitString "=" flag) != "O")
+    (kernel.makeFlags ++ [
+      "KERNELRELEASE=${kernel.modDirVersion}"
+      "KDIR=${kernel.dev}/lib/modules/${kernel.modDirVersion}/build"
+      "INSTALL_MOD_PATH=${placeholder "out"}"
+    ]);

Will fix

@dasj19
Copy link
Contributor

dasj19 commented Sep 1, 2024

@CutestNekoAqua I cloned your fork, switched to the tuxedo-drivers-init then merged in the latest upstream/master locally (Fixing conflicts)
Afterwards I applied the change proposed by @xaverdh and only then the build succeeded. I'll keep my system on this branch to see if I uncover any bugs.

Regarding the keyboard backlight: Gnome does support brightness level and on/off, but not change of color. Tailor does not support setting the backlight at all. The Fn hotkeys for on/off also work, together with the brightness adjustment, I don't know if there is a hotkey for changing the color. Maybe the only way to do it with this setup is using kernel params: https://nixos.wiki/wiki/TUXEDO_Devices

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Sep 7, 2024
gabyx added a commit to gabyx/dotfiles that referenced this pull request Sep 8, 2024
gabyx added a commit to gabyx/dotfiles that referenced this pull request Sep 8, 2024
gabyx added a commit to gabyx/dotfiles that referenced this pull request Sep 8, 2024
@gabyx
Copy link
Contributor

gabyx commented Sep 8, 2024

Seams to work. But have not tested custom settings.

@Green-Sky
Copy link
Contributor

You can fix that with the following:

-  makeFlags = kernel.makeFlags ++ [
-    "KERNELRELEASE=${kernel.modDirVersion}"
-    "KDIR=${kernel.dev}/lib/modules/${kernel.modDirVersion}/build"
-    "INSTALL_MOD_PATH=$(out)"
-  ];
+  makeFlags = lib.filter
+    (flag: lib.head (lib.strings.splitString "=" flag) != "O")
+    (kernel.makeFlags ++ [
+      "KERNELRELEASE=${kernel.modDirVersion}"
+      "KDIR=${kernel.dev}/lib/modules/${kernel.modDirVersion}/build"
+      "INSTALL_MOD_PATH=${placeholder "out"}"
+    ]);

Will fix

Im also running into this issue.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 10, 2024
@SebRut
Copy link
Contributor

SebRut commented Sep 21, 2024

anything new? I'd like to switch to this to use the latest kernel as 6.9 is EOL.

@nagymathev
Copy link

same goes here @SebRut, waiting

@xaverdh xaverdh mentioned this pull request Sep 21, 2024
13 tasks
@xaverdh
Copy link
Contributor

xaverdh commented Sep 21, 2024

updated version is here

@papanito papanito mentioned this pull request Sep 28, 2024
13 tasks
Keksgesicht added a commit to Keksgesicht/nixpkgs that referenced this pull request Oct 7, 2024
Co-Authored-By: April John <[email protected]>
Co-Authored-By: Dominik Xaver Hörl <[email protected]>

thanks to the work of:
- NixOS#293017
- NixOS#343483
Keksgesicht added a commit to Keksgesicht/nixpkgs that referenced this pull request Oct 7, 2024
Co-authored-by: April John <[email protected]>
Co-authored-by: Dominik Xaver Hörl <[email protected]>

thanks to the work of:
- NixOS#293017
- NixOS#343483
Keksgesicht added a commit to Keksgesicht/nixpkgs that referenced this pull request Nov 4, 2024
Co-Authored-By: April John <[email protected]>
Co-Authored-By: Dominik Xaver Hörl <[email protected]>

thanks to the work of:
- NixOS#293017
- NixOS#343483
github-actions bot pushed a commit to Mic92/nixpkgs that referenced this pull request Nov 10, 2024
Co-Authored-By: April John <[email protected]>
Co-Authored-By: Dominik Xaver Hörl <[email protected]>

thanks to the work of:
- NixOS#293017
- NixOS#343483
@xaverdh
Copy link
Contributor

xaverdh commented Nov 10, 2024

thank you for your work on this @CutestNekoAqua
#336633 got merged, which also contained some of your work here!

@xaverdh xaverdh closed this Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (new) This PR adds a module in `nixos/` 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package request: tuxedo-drivers