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

Enable dxvknvapi on more (DLSS) titles I've verified as functioning & stable #6227

Closed
wants to merge 29 commits into from

Conversation

adamnv
Copy link
Contributor

@adamnv adamnv commented Oct 12, 2022

Expanding the verified list introduced in #6120 . This time, also, targeted at experimental_7.0. Thanks.

rbernon and others added 23 commits September 12, 2022 22:29
For CI / bleeding-edge automation.
…recognized.

From ValveSoftware#6079

Edited by Paul Gofman:
- fixed behaviour for Vulkan instance type;
- stylistic changes.
…egistered_t() if win_func is NULL.

CW-Bug-Id: #21313
@adamnv
Copy link
Contributor Author

adamnv commented Oct 13, 2022

I'd appreciate your thoughts on a discovery, @ivyl :
Red Dead Redemption 2 requires PROTON_ENABLE_NVAPI=1 and also PROTON_HIDE_NVIDIA_GPU=0 for DLSS to be correctly detected (even under its default Vulkan renderer, yeah!).

Since I don't think PROTON_HIDE_NVIDIA_GPU=1 is the default for all apps, I assume it's being explicitly set in a game profile somewhere, but after grepping proton I can't seem to detect where; any clues?

Or maybe PROTON_HIDE_NVIDIA_GPU=1 is actually the default for all apps and I should just explicitly add PROTON_HIDE_NVIDIA_GPU=0 for RDR2. Or PROTON_HIDE_NVIDIA_GPU=0 should just be implied for titles needing the "enablenvapi" quirk in the proton launch script.

edit: a data point from testing which I forgot to add - just PROTON_HIDE_NVIDIA_GPU=0 on its own without PROTON_ENABLE_NVAPI=1 will cause a launch failure. I'm guessing that's the reason PROTON_HIDE_NVIDIA_GPU=1 was forced for this title before dxvknvapi was part of Proton - the title gets really confused if it thinks it detects nvidia but nvapi fails to initialize.

@ivyl
Copy link
Collaborator

ivyl commented Oct 14, 2022

I'd appreciate your thoughts on a discovery, @ivyl : Red Dead Redemption 2 requires PROTON_ENABLE_NVAPI=1 and also PROTON_HIDE_NVIDIA_GPU=0 for DLSS to be correctly detected (even under its default Vulkan renderer, yeah!).

Since I don't think PROTON_HIDE_NVIDIA_GPU=1 is the default for all apps, I assume it's being explicitly set in a game profile somewhere, but after grepping proton I can't seem to detect where; any clues?

Ah, it's another flavor of the nvapihack we had to add.

If you look into proton script you can see that PROTON_HIDE_NVIDIA_GPU gets converted into hidenvgpu compat option.

Steam provides us with compat config options using SteamPlay 2.0 Manifest which indeed has this option set for a few games:

1174180/comment | Red Dead Redemption 2
1174180/config | hidenvgpu,nativevulkanloader

997070/comment | Marvel's Avengers
997070/config | hidenvgpu

1404210/comment | Red Dead Online
1404210/config | hidenvgpu,nativevulkanloader

Or maybe PROTON_HIDE_NVIDIA_GPU=1 is actually the default for all apps and I should just explicitly add PROTON_HIDE_NVIDIA_GPU=0 for RDR2. Or PROTON_HIDE_NVIDIA_GPU=0 should just be implied for titles needing the "enablenvapi" quirk in the proton launch script.

I think enablenvapi would need to override hidenvgpu.

edit: a data point from testing which I forgot to add - just PROTON_HIDE_NVIDIA_GPU=0 on its own without PROTON_ENABLE_NVAPI=1 will cause a launch failure. I'm guessing that's the reason PROTON_HIDE_NVIDIA_GPU=1 was forced for this title before dxvknvapi was part of Proton - the title gets really confused if it thinks it detects nvidia but nvapi fails to initialize.

We have dxgi nvapi hack for the longest time. Even when we were using wined3d's dxgi we made changes to dxvk so all the logic and overrides are a part of dxvk_config.dll and we used that for overrides on our end.

If memory serves me well I think that RDR2 was using also some other API (Vulkan?) and didn't like either the mismatch or tried to load nvapi if it saw Nvidia's vendor id there.

@adamnv
Copy link
Contributor Author

adamnv commented Oct 14, 2022

I think enablenvapi would need to override hidenvgpu

Thanks! I'll go for that angle I reckon.

@Bitwolfies
Copy link
Contributor

Bitwolfies commented Oct 19, 2022

The Uncharted Legacy of Thieves collection could also be added to this list in my testing, it works on all settings, no crashes.

proton Outdated
@@ -1420,6 +1422,8 @@ class Session:

if "enablenvapi" in self.compat_config:
self.env["DXVK_ENABLE_NVAPI"] = "1"
# enablenvapi beats hidenvgpu
self.env["PROTON_HIDE_NVIDIA_GPU"] = "0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you tested that it works? It doesn't look like it does.

in init_session() we have a bunch of checks in the form:

self.check_environment("PROTON_HIDE_NVIDIA_GPU", "hidenvgpu")
self.check_environment("PROTON_ENABLE_NVAPI", "enablenvapi")

This converts the environment variables to entries in self.compat_config.

If you look a bit down hidenvidiagpu is handled as follows:

if "hidenvgpu" in self.compat_config:
    self.env["WINE_HIDE_NVIDIA_GPU"] = "1" 

Converting the compat_config value in WINE_ prefixed variable that's then interpreted by Wine.

That's a few lines above this line, so it is too late to affect the outcome.

Something like this (untested) should make more sense:

diff --git a/proton b/proton
index 47e51897..46edef0a 100755
--- a/proton
+++ b/proton
@@ -1317,6 +1317,12 @@ class Session:
         self.check_environment("PROTON_HEAP_DELAY_FREE", "heapdelayfree")
         self.check_environment("PROTON_ENABLE_NVAPI", "enablenvapi")
 
+        if "enablenvapi" in self.compat_config:
+            self.env["DXVK_ENABLE_NVAPI"] = "1"
+            # enablenvapi beats hidenvgpu
+            if "hidenvgpu" in self.compat_config:
+                self.compat_config.remove("hidenvgpu")
+
         if "noesync" in self.compat_config:
             self.env.pop("WINEESYNC", "")
         else:

ivyl pushed a commit that referenced this pull request Oct 19, 2022
ivyl pushed a commit that referenced this pull request Oct 19, 2022
ivyl pushed a commit that referenced this pull request Oct 19, 2022
@ivyl ivyl force-pushed the experimental_7.0 branch from 5339633 to 082faef Compare October 19, 2022 21:30
@ivyl
Copy link
Collaborator

ivyl commented Oct 19, 2022

I've pushed all but the RDR2 commits to experimental. It should appear in bleeding-edge soon. Thanks!

@adamnv
Copy link
Contributor Author

adamnv commented Oct 20, 2022

Have you tested that it works? It doesn't look like it does.

Yeah well spotted! I did test that it works, but I had a testing error (I'd accidentally left the env var in the launch options too...)

I've updated the RDR2 change again to override the right env var; would you like to pick it up from this branch or should I open a new PR?
Thanks.

ivyl pushed a commit that referenced this pull request Oct 28, 2022
ivyl pushed a commit that referenced this pull request Oct 28, 2022
ivyl pushed a commit that referenced this pull request Oct 28, 2022
@ivyl ivyl force-pushed the experimental_7.0 branch from 082faef to a1524f7 Compare October 28, 2022 17:26
@Bitwolfies
Copy link
Contributor

Sackboy is good too!

@ivyl
Copy link
Collaborator

ivyl commented Nov 1, 2022

This is now merged to experimental and should appear in the next experimental release.

It's already in the bleeding-edge: https://github.com/ValveSoftware/Proton/commits/experimental-bleeding-edge-7.0-28117-20221101-pd31d72-w9c8813-d0b9f78-v4df366

I've taken a slightly different approach to overriding hidenvgpu.

Feel free to open more PRs with more games.

@ivyl ivyl closed this Nov 1, 2022
ivyl added a commit that referenced this pull request Nov 16, 2022
ivyl pushed a commit that referenced this pull request Nov 16, 2022
ivyl pushed a commit that referenced this pull request Dec 22, 2022
Link: #6120
Link: #6227

(squashed a bunch of commits)
ivyl added a commit that referenced this pull request Dec 22, 2022
ivyl pushed a commit that referenced this pull request Dec 22, 2022
ivyl pushed a commit that referenced this pull request Apr 17, 2023
Link: #6120
Link: #6227

(squashed a bunch of commits)
ivyl added a commit that referenced this pull request Apr 17, 2023
ivyl pushed a commit that referenced this pull request Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants