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

Add ForcedGpu option #206

Merged
merged 34 commits into from
Oct 24, 2023
Merged

Add ForcedGpu option #206

merged 34 commits into from
Oct 24, 2023

Conversation

jrelvas-ipc
Copy link
Contributor

This PR adds two new options which improve the functionality of Vinegar for laptop users (and users with exotic 3+ graphics card machines). The actual env variables are abstracted away, making this work with the gpus of all three vendors.

Wine's vulkan implementation chooses the dgpu by default, but this isn't the case with its opengl implementation. This change makes opengl use the dgpu by default, matching vulkan's behavior. It also lets users disable prime much more easily, without needing to tinker with env variables which may vary depending on their hardware.

Mostly complete, but it still needs some code cleanup and to prevent launching if the user is using OpenGL and has more than two GPUs (there's no way for us or gl to know the right one, so the user must explictly define it.)

@jrelvas-ipc jrelvas-ipc marked this pull request as draft October 15, 2023 05:17
Also did some refactoring to share validation logic across binaries
In these circumstances, there's no way for us to determine the right gpu
...so abort and prompt the user to explictly choose it...
...or to try the vulkan renderer
@jrelvas-ipc
Copy link
Contributor Author

PR is finished and ready for review!

Overview of functionality and changes:

  • ForcedGpu (toml:"gpu") option in binary cfg.

  • Accepted values:

    • "" (empty): skip prime logic, same as existing behavior.
    • "integrated": do not use prime offload on systems where it's supported.
    • "prime-discrete": use prime offload, if supported. (default)
    • (VID:NID): use gpu with the matching vid:nid (ex: 10de:27ba). Automatically lowercased and with 0x excluded.
    • (number): use gpu with the matching index.
  • Value is validated in the config module, warning users during editing if it doesn't match one of the above.

  • Prime offload logic is skipped if the system only has a single card.

  • Prime offload logic is skipped if the system has three or more cards (exotic configs like workstation desktops or laptops with egpus)

    • Additionally, if the OpenGL renderer is being used, launching is aborted and the user will be prompted to choose the card. This must be done as OpenGL isn't capable of choosing the GPU based on its capabilities, unlike Vulkan.
  • Prime offload logic is skipped if card0 has no eDP (If card0 has no eDP connection, the device is not a laptop)

  • Environment Variables are set based on the gpu chosen by the prime offload logic or by the user's explicit choice.

    • Env vars are not modified, only defined. This ensures the user's and the underlying system's choices are not accidentally overridden.
    • MESA_VK_DEVICE_SELECT_FORCE_DEFAULT_DEVICE is set to 1 and DRI_PRIME is set to the chosen card, ensuring only the intended card can be seen or used by OpenGL and Vulkan.
    • An extra step is required for nvidia GPUs to prevent GLX (used by Wine's OpenGL implementation) from farting itself. If the chosen card's vendor is nvidia, the driver it's currently using is probed;
      • If it's the nvidia proprietary driver, then __GLX_VENDOR_LIBRARY_NAME is set to nvidia.
      • Otherwise, it's set to mesa (nouveau).
  • With the exception of the ForcedGpu option validation in config.go , all logic is entirely inside of the new prime.go module. No other changes were made.

@jrelvas-ipc jrelvas-ipc marked this pull request as ready for review October 15, 2023 18:26
@jrelvas-ipc jrelvas-ipc changed the title Add PrimeOffload and ForcedGpuId options Add ForcedGpu option Oct 15, 2023
It's actually unnecessary to check the card's vendor.
Instead, just check for its driver. If it's nvidia, apply the...
...GLX workaround.
Now gpu works entirely around indexes
@jrelvas-ipc jrelvas-ipc marked this pull request as draft October 16, 2023 14:49
@jrelvas-ipc jrelvas-ipc marked this pull request as ready for review October 16, 2023 18:04
I just noticed this and it was really bothering me. It had to be fixed.
@jrelvas-ipc
Copy link
Contributor Author

@apprehensions Remember when I said I wouldn't be making any more changes? sorry i lied lol

I noticed this and felt an uncontrollable urge to fix it

the code is now one line more polished

@jrelvas-ipc
Copy link
Contributor Author

@apprehensions Branch updated to 8b061c5

Will be making the requested adjustments soon

Hopefully the last refactor of this code
@jrelvas-ipc
Copy link
Contributor Author

Refactor around sysinfo is done.

internal/config/cardpick.go Outdated Show resolved Hide resolved
internal/config/cardpick.go Outdated Show resolved Hide resolved
internal/config/config.go Outdated Show resolved Hide resolved
internal/config/env.go Outdated Show resolved Hide resolved
Comment on lines 37 to 104
func pickCard(opt string, env Environment, isVulkan bool) error {
if opt == "" { //Default value for opt
opt = "none"
}

var cIndex int
var indexStr string

prime := false

switch opt {
//Handle PRIME options
case "integrated":
cIndex = 0
prime = true
case "prime-discrete":
cIndex = 1
prime = true
//Skip pickCard logic option
case "none":
return nil
//Otherwise, interpret opt as a card index
default:
var err error
cIndex, err = strconv.Atoi(opt)
if err != nil {
return err
}
}

if cIndex < 0 {
return errors.New("card index cannot be negative")
}

indexStr = strconv.Itoa(cIndex)

//PRIME Validation
if prime {
allowed, err := primeIsAllowed(isVulkan)
if err != nil {
return err
}
if !allowed {
return nil
}
}

if len(sysinfo.Cards) < cIndex+1 {
return errors.New("gpu not found")
}

c := sysinfo.Cards[cIndex]

env.SetIfUndefined("MESA_VK_DEVICE_SELECT_FORCE_DEFAULT_DEVICE", "1")
env.SetIfUndefined("DRI_PRIME", indexStr)

if strings.HasSuffix(c.Driver, "nvidia") { //Workaround for OpenGL in nvidia GPUs
env.SetIfUndefined("__GLX_VENDOR_LIBRARY_NAME", "nvidia")
} else {
env.SetIfUndefined("__GLX_VENDOR_LIBRARY_NAME", "mesa")
}

log.Printf("Chose card %s (%s).", c.Path, indexStr)

env.Setenv()

return nil
}
Copy link
Member

Choose a reason for hiding this comment

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

No

internal/config/env.go Outdated Show resolved Hide resolved
Co-authored-by: sewn <[email protected]>
Signed-off-by: Jrelvas <[email protected]>
internal/config/cardpick.go Outdated Show resolved Hide resolved
@jrelvas-ipc
Copy link
Contributor Author

oops that has to be fixed

internal/config/cardpick.go Outdated Show resolved Hide resolved
internal/config/cardpick.go Outdated Show resolved Hide resolved
internal/config/env.go Outdated Show resolved Hide resolved
internal/config/config.go Show resolved Hide resolved
internal/config/cardpick.go Outdated Show resolved Hide resolved
@jrelvas-ipc
Copy link
Contributor Author

Tested. Nothing appears to be broken!

b.Env.Set("__GLX_VENDOR_LIBRARY_NAME", "mesa")
}

b.Env.Setenv()
Copy link
Member

Choose a reason for hiding this comment

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

Move to setup

@apprehensions
Copy link
Member

Awesome work and patience

@apprehensions apprehensions merged commit 28fadcb into vinegarhq:master Oct 24, 2023
1 check passed
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.

2 participants