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.

Sign up for GitHub

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

nixos/nvidia: various fixes #337289

Merged
merged 3 commits into from
Aug 25, 2024
Merged

nixos/nvidia: various fixes #337289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b52dae
linuxPackages.nvidiaPackages.*: don't package unused openssl 1.1 comp…
Kiskae Aug 25, 2024
20c5d0a
nixos/nvidia: make the nvidia driver variant a mandatory user choice
Kiskae Aug 25, 2024
ef3b6ad
nixos/nvidia: allow the gsp to be toggled independently from open sou…
Kiskae Aug 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions nixos/doc/manual/release-notes/rl-2411.section.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@
Processes also now run as a dynamically allocated user by default instead of
root.

- The nvidia driver no longer defaults to the proprietary driver starting with version 560. You will need to manually set `hardware.nvidia.open` to select the proprietary or open driver.

- `singularity-tools` have the `storeDir` argument removed from its override interface and use `builtins.storeDir` instead.

- Two build helpers in `singularity-tools`, i.e., `mkLayer` and `shellScript`, are deprecated, as they are no longer involved in image-building. Maintainers will remove them in future releases.
Expand Down
36 changes: 29 additions & 7 deletions nixos/modules/hardware/video/nvidia.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,21 @@ in
'';
};

open = lib.mkEnableOption ''
the open source NVIDIA kernel module
open = lib.mkOption {
example = true;
description = "Whether to enable the open source NVIDIA kernel module.";
type = lib.types.bool;
Copy link
Member

@Mic92 Mic92 Aug 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Since this affect quite a few people, we might want to introduce some guiding message here. If we also accept "unset" here, then an assertion could reproduce the changelog above.

Copy link
Member

Choose a reason for hiding this comment

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

You can see how people will get confused here: NixOS/nixos-hardware#1092 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

If we also accept "unset" here, then an assertion could reproduce the changelog above.

That's the original version of this pr, accepting null. But then we would have to cast nullOr types.bool to types.bool on every place referencing cfg.open, or adding a pretty ugly isOpen indirection. I would rather prefer improving the error message for all mandatory options as in #338362

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this affect quite a few people, we might want to introduce some guiding message here. If we also accept "unset" here, then an assertion could reproduce the changelog above.

Yep, just helped someone that updated their flake and got this issue. Would love to see a PR to make this message cleaner, at least for the short term.

defaultText = lib.literalExpression ''
lib.mkIf (lib.versionOlder config.hardware.nvidia.package.version "560") false
'';
};

gsp.enable = lib.mkEnableOption ''
the GPU System Processor (GSP) on the video card
'' // {
defaultText = lib.literalExpression ''lib.versionAtLeast config.hardware.nvidia.package.version "560"'';
defaultText = lib.literalExpression ''
config.hardware.nvidia.open || lib.versionAtLeast config.hardware.nvidia.package.version "555"
'';
};
};
};
Expand Down Expand Up @@ -308,7 +319,8 @@ in
};
environment.systemPackages = [ nvidia_x11.bin ];

hardware.nvidia.open = lib.mkDefault (lib.versionAtLeast nvidia_x11.version "560");
hardware.nvidia.open = lib.mkIf (lib.versionOlder nvidia_x11.version "560") (lib.mkDefault false);
hardware.nvidia.gsp.enable = lib.mkDefault (cfg.open || lib.versionAtLeast nvidia_x11.version "555");
})

# X11
Expand Down Expand Up @@ -367,8 +379,18 @@ in
}

{
assertion = cfg.open -> (cfg.package ? open && cfg.package ? firmware);
message = "This version of NVIDIA driver does not provide a corresponding opensource kernel driver";
assertion = cfg.gsp.enable -> (cfg.package ? firmware);
message = "This version of NVIDIA driver does not provide a GSP firmware.";
}

{
assertion = cfg.open -> (cfg.package ? open);
message = "This version of NVIDIA driver does not provide a corresponding opensource kernel driver.";
}

{
assertion = cfg.open -> cfg.gsp.enable;
message = "The GSP cannot be disabled when using the opensource kernel driver.";
}

{
Expand Down Expand Up @@ -555,7 +577,7 @@ in

services.dbus.packages = lib.optional cfg.dynamicBoost.enable nvidia_x11.bin;

hardware.firmware = lib.optional (cfg.open || lib.versionAtLeast nvidia_x11.version "555") nvidia_x11.firmware;
hardware.firmware = lib.optional cfg.gsp.enable nvidia_x11.firmware;

systemd.tmpfiles.rules =
[
Expand Down
3 changes: 3 additions & 0 deletions pkgs/os-specific/linux/nvidia-x11/builder.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ installPhase() {
mkdir $i/lib/vdpau
mv $i/lib/libvdpau* $i/lib/vdpau

# Compatibility with openssl 1.1, unused
rm -f $i/lib/libnvidia-pkcs11.so*

# Install ICDs, make absolute paths.
# Be careful not to modify any original files because this runs twice.

Expand Down