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

steam/fhsenv: reorder and fix VK_ICD_FILENAMES #126435

Closed
wants to merge 1 commit into from

Conversation

critbase
Copy link
Contributor

Motivation for this change

some of the changes introduced with the pressure-vessel commit have caused some pain when using proton/vulkan-enabled games.

this pr makes the environment variable VK_ICD_FILENAMES use the user defined nvidia package (through
config.hardware.nvidia.package), in addition to reordering the list to
make it more friendly to setups with multiple GPUs (Optimus hybrid
laptops, mainly). The order now prioritizes discrete GPUs first (nvidia, amd/radeon), and intel GPUs second (since these are all integrated).

This solves some issues as seen in issue #126428 and in a few comments in #108598 (comment)

As i do not have an amd device at all, it would be nice for someone with an optimus amd laptop to test this pr, though it shouldn't change their current experience.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.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.

@Flakebi
Copy link
Member

Flakebi commented Jun 10, 2021

While you’re at it, could you also add amd_icd64.json and amd_icd32.json for the amdvlk driver?

@Flakebi
Copy link
Member

Flakebi commented Jun 10, 2021

Thanks!

I just thought about it, this makes the nvidia and amd drivers a dependency of steam, so people with only an intel GPU will still need to download them (and both are not exactly tiny).

I am a bit surprised that setting VK_ICD_FILENAMES is necessary at all, especially since /usr/share/vulkan/icd.d should be the standard path where the Vulkan loader looks for driver icds (unless the patched nix version is running inside the fhsenv and is only looking in /run/opengl-driver/share/vulkan/icd.d?).

On a sidenote, I often set VK_ICD_FILENAMES when starting steam to test driver changes and that doesn’t work anymore as it now gets overwritten by the steam environment. (Admittedly, that is more of an edge case than normal usage.)

@critbase
Copy link
Contributor Author

critbase commented Jun 10, 2021

Hmm, that's true. Maybe it's possible to conditionally set VK_ICD_FILENAMES depending on the contents of config.services.xserver.videoDrivers?

I'm not sure why VK_ICD_FILENAMES is necessary, but I'm assuming it's for the pressure-vessel fixes, in which case I don't want to mess with it too much.

EDIT:
Also, I moved the config specifically regarding nvidia packages to the config.programs.steam module; with this change, using steam from something like a nix-shell will work as it has before, but if a user strays from the default linux kernel/nvidia package (like me), they should use the steam module, as this will properly configure VK_ICD_FILENAMES.
Maybe it's also worth moving the amdvlk stuff to the module, and as mentioned, conditionally setting VK_ICD_FILENAMES depending on whether nvidia/amdvlk is present.

@critbase
Copy link
Contributor Author

A weird side effect I just noticed when trying to play Hades (one of the only Vulkan-native games i own), it seems to use my nvidia gpu regardless of the nvidia prime environment variables (running on an optimus laptop).
I can't tell if this is an issue due to my hardware or the environment variable, but it's not an issue (since generally if a game uses vulkan, i'd want to run it with my discrete gpu), just a quirk to be aware of.

@jonringer
Copy link
Contributor

Hmm, that's true. Maybe it's possible to conditionally set VK_ICD_FILENAMES depending on the contents of config.services.xserver.videoDrivers?

I'm not sure why VK_ICD_FILENAMES is necessary, but I'm assuming it's for the pressure-vessel fixes, in which case I don't want to mess with it too much.

Yes, that's what I was thinking, and just start from most specific to least specific. Just reording the items may cause issues for other users. I'll try to cook something up after work

cc @nh2

@davidak
Copy link
Member

davidak commented Jul 20, 2021

Result of nixpkgs-review pr 126435 run on x86_64-linux 1

3 packages built:
  • kodiPackages.steam-launcher
  • lutris
  • steam

@babbaj
Copy link
Contributor

babbaj commented Aug 19, 2021

This pr fixes the problem I have where for some reason my nvidia icd files aren't present in /usr/share/vulkan/icd.d at all which causes some games to not work at all

@@ -44,7 +44,14 @@ in {

hardware.steam-hardware.enable = true;

environment.systemPackages = [ steam steam.run ];
environment.systemPackages = [
Copy link
Member

Choose a reason for hiding this comment

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

Why not just inline this overriding into the derivation for the FHSEnv?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's because (at least, i think) the config attributes (like config.hardware.nvidia.package) can't be referred to from within packages

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, you can't do that, but if the ICD files are similar/identical for all Nvidia driver packages, then you could use a generic one in the FHS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think the ICD files are similar enough to do that; my reasoning is a bit anecdotal as im not entirely sure of how this works, but using a different version of the nvidia driver (nvidiaBeta or nvidiaVulkanBeta) caused proton/dxvk to throw errors and not work for me.

Copy link
Member

Choose a reason for hiding this comment

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

I just checked, and yeah, they look similar but they point to different store paths, so it wouldn't work.

Copy link
Member

Choose a reason for hiding this comment

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

What happens if I'm not using Nvidia on my system? Will this then pull in Nvidia drivers even though I don't use them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

im actually not sure how the fhsenv gets the ICD files to use. If i knew that, i think i could come up with a better/cleaner solution.
as for pulling in nvidia, i think it will... i'm unsure how else to handle it other than a manual switch to toggle it on, but that then adds another toggle on top of enabling the steam module for nvidia users, or a switch to toggle it off for non-nvidia users.

Copy link
Member

Choose a reason for hiding this comment

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

From what I can tell, the icd files you have are driver-specific, and are currently in /run/opengl-driver/share/....
I took a look at this, and in the FHS that is made, we still have access to this folder, so you might be able to just add all the icd files in /run/opengl-driver/share/vulkan/icd.d/ to VK_ICD_FILENAMES for Steam. I am also not sure where the current icd files in /usr/ come from.

In essence, taking the existing VK_ICD_FILENAMES, and appending to it itself, but with /usr replaced with /run/opengl-driver, might make it all work magically.

Copy link
Contributor

@babbaj babbaj Aug 19, 2021

Choose a reason for hiding this comment

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

most of the icd files come from the mesa.drivers package except for the nvidia files which i have no clue where they're supposed to come from.

@@ -282,7 +283,7 @@ in buildFHSUserEnv rec {

export STEAM_RUNTIME=${if nativeOnly then "0" else "/steamrt"}

export VK_ICD_FILENAMES=/usr/share/vulkan/icd.d/intel_icd.x86_64.json:/usr/share/vulkan/icd.d/intel_icd.i686.json:/usr/share/vulkan/icd.d/lvp_icd.x86_64.json:/usr/share/vulkan/icd.d/lvp_icd.i686.json:/usr/share/vulkan/icd.d/nvidia_icd.json:/usr/share/vulkan/icd.d/nvidia_icd32.json:/usr/share/vulkan/icd.d/radeon_icd.x86_64.json:/usr/share/vulkan/icd.d/radeon_icd.i686.json
export VK_ICD_FILENAMES=/usr/share/vulkan/icd.d/nvidia_icd.json:/usr/share/vulkan/icd.d/nvidia_icd32.json:/usr/share/vulkan/icd.d/radeon_icd.x86_64.json:/usr/share/vulkan/icd.d/radeon_icd.i686.json:${amdvlk}/share/vulkan/icd.d/amd_icd64.json:${driversi686Linux.amdvlk}/share/vulkan/icd.d/amd_icd32.json:/usr/share/vulkan/icd.d/intel_icd.x86_64.json:/usr/share/vulkan/icd.d/intel_icd.i686.json:/usr/share/vulkan/icd.d/lvp_icd.x86_64.json:/usr/share/vulkan/icd.d/lvp_icd.i686.json
Copy link
Member

Choose a reason for hiding this comment

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

I think this would hard code the driver used, which isn't desirable. You likely want to do the same approach for the other drivers as for the Nvidia one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll take a look at doing the same for intel (which might not even need it), but i don't want to change stuff around for AMD given that i can't test it at all. i don't want to make changes that may break an already working system.
also, reordering the ICD files may end up being unnecessary, but i'll test different configurations a bit more to make sure.

@critbase
Copy link
Contributor Author

critbase commented Aug 20, 2021

Ok, this is really weird, but I'm not getting any issues with vulkan anymore, even with shuffling around the nvidia drivers im using and rebooting. It seems the ICD files in the FHS env are taken from /run/opengl-driver and /run/opengl-driver-32 (please correct me if i'm wrong). For whatever reason, back when i first opened this PR i had issues but... no longer.
Note that i'm talking strictly about nvidia GPUs; i think users of AMD gpus that use the amdvlk driver will still have something to gain from this PR.

If there are actually no issues with nvidia GPUs, and other people aren't having any issues, I think i'll turn this PR solely into adding in support for amdvlk drivers, though im unsure if ICD files for those drivers also show up in /run/opengl-driver and/or /usr/share/vulkan/icd.d in the FHS env.

EDIT: just noticed #134556 after doing a cursory search for the term "steam". I think what I can make this PR into is changing the VK_ICD_FILENAMES line in the fhsenv itself to just use /run/opengl-driver directly. Depending on if the amdvlk driver makes its files available through /run/opengl-driver as well (i don't have an amd machine to check, and am unsure of how to check myself if possible), we could just add that path to the environment variable, and maybe even avoid pulling in amdvlk as a dependency.

@TredwellGit
Copy link
Member

For your information, most AMD users do not use AMDVLK. Mesa comes with RADV out of the box and is typically preferred.

@babbaj
Copy link
Contributor

babbaj commented Aug 20, 2021

@critbase I just tested my system with an amd gpu and with the amdvlk driver added it does show up in /run/opengl-driver

[babbaj@nixos:~]$ ls /run/opengl-driver/share/vulkan/icd.d/
amd_icd64.json         lvp_icd.x86_64.json
intel_icd.x86_64.json  radeon_icd.x86_64.json

[babbaj@nixos:~]$ cat /run/opengl-driver/share/vulkan/icd.d/amd_icd64.json 
{
  "file_format_version": "1.0.0",
  "ICD": {
    "library_path": "/nix/store/vq25p9l8ynlhjag6kzkacypivdwr8svr-amdvlk-2021.Q3.2/lib/amdvlk64.so",
    "api_version": "1.2.182"
  },
  "layer": {
    "name": "VK_LAYER_AMD_switchable_graphics_64",
    "type": "GLOBAL",
    "library_path": "/nix/store/vq25p9l8ynlhjag6kzkacypivdwr8svr-amdvlk-2021.Q3.2/lib/amdvlk64.so",
    "api_version": "1.2.182",
    "implementation_version": "1",
    "description": "AMD switchable graphics layer",
    "functions": {
      "vkGetInstanceProcAddr": "vk_icdGetInstanceProcAddrSG",
      "vkGetDeviceProcAddr": "vk_icdGetDeviceProcAddrSG"
    },
    "disable_environment": {
      "DISABLE_LAYER_AMD_SWITCHABLE_GRAPHICS_1": "1"
    }
  }
}

I think /run/opengl-driver might even be good enough to replace /usr/share/vulkan

@critbase
Copy link
Contributor Author

@babbaj nice, thank you. I'll go ahead and change this PR to just changing the line in the FHS env where VK_ICD_FILENAMES is set, which should hopefully be a somewhat elegant solution.

to test the change i'll make, it should be enough to just paste the following into environment.systemPackages or similar:

(steam.override {                                                                                                                                     
  extraProfile = ''                                                                                                                                   
    unset VK_ICD_FILENAMES                                                                                                                            
    export VK_ICD_FILENAMES=/run/opengl-driver/share/vulkan/icd.d/intel_icd.x86_64.json:/run/opengl-driver-32/share/vulkan/icd.d/intel_icd.i686.json:\
    /run/opengl-driver/share/vulkan/icd.d/radeon_icd.x86_64.json:/run/opengl-driver-32/share/vulkan/icd.d/radeon_icd.i686.json:\                      
    /run/opengl-driver/share/vulkan/icd.d/amd_icd64.json:/run/opengl-driver-32/share/vulkan/icd.d/amd_icd32.json:\                                    
    /run/opengl-driver/share/vulkan/icd.d/nvidia_icd.json:/run/opengl-driver-32/share/vulkan/icd.d/nvidia_icd.json:\                                  
    /run/opengl-driver/share/vulkan/icd.d/lvp_icd.x86_64.json:/run/opengl-driver-32/share/vulkan/icd.d/lvp_icd.i686.json                              
  '';                                                                                                                                                 
})                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              

primarily by using /run/opengl-driver directly to ensure the correct
vulkan ICD file is picked up.
@babbaj
Copy link
Contributor

babbaj commented Aug 20, 2021

if [ -f $out/usr/share/vulkan/icd.d/nvidia_icd.json ]; then

this code can be removed or reused if it's still useful

@critbase
Copy link
Contributor Author

Good catch; I'll try and find out if it's necessary.
Though, i think #135024 might end up being a nicer way to do this

@stale
Copy link

stale bot commented Apr 18, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 18, 2022
@jonringer
Copy link
Contributor

This has been fixed with use of XDG_DATA_DIRS providing /run/opengl-driver/share as a default.

@jonringer jonringer closed this Apr 19, 2022
@jonringer
Copy link
Contributor

For x-reference #161576

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: steam 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants