-
-
Notifications
You must be signed in to change notification settings - Fork 4
Add Force Integrated GPU option #53
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
Conversation
Just at a cursory glance, it looks pretty good and enabling the setting doesn't apparently cause issue on a desktop. You did manage to copy the style of help text pretty well! The actual code implem looks like it follows styling just fine. Because the ui file is a mess, I'll just put my "review" here of what I noticed:
On a tangential note, would it be possible to actually check if the user has a dual graphics setup? My thinking is that the toggle should be not enabled (i.e. grayed out, unselectable) for desktops, or if the current system doesn't actually have a second GPU - if it is possible, we could check that when Nero Manager itself starts, and then pass it along as a param when creating Prefix Settings windows (since it would be wasteful to have to check for every settings window invocation) - we'd just have to add it to |
Thanks for the detailed review! As for the checking, I have some ideas that I'll try out but just as a quick guess it might require an additional |
Also a side note: I've never used a dual gpu desktop myself but from my understanding of how gpus work, they have to be connected to the display to be able to output visuals but can be seen by the system as a device even if not. Which means, on desktops you can't use the iGPU for anything unless you physically plug in a cable. Since it is something I'm assuming someone who uses linux on a desktop would know, and if this does mess up the "convenient variable" I mentioned, it might not be possible to make this work properly for all desktop configurations. And as an additional hurdle, laptops with the primary display gpu set to the dGPU could complicate things even more. (Although this is the one case I don't want to plan around as it doesn't make sense in the first place) |
Mrgrgr. So that complicates things. I think it's okay then to leave it as-is as a dumb toggle. If there comes a method to somewhat intelligently figure out single vs. low-power+performance GPU setups, this could be added later--but it's just a nice-to-have rather than a requirement. On a tangential note, as a recent development, I do now have access to a dual GPU laptop in the form of a MacbookPro8,2 (Intel 3000+AMD 6750M), however this GPU combo does not have Vulkan support on either end so it might not be useful (as DXVK basically only works with NV Maxwell/Pascal+ or AMD RX 500-series+, and the requirements even higher for VKD3D). If I have time in the future, I could toy around with this more independently just for shits and giggles, but it's not necessarily a priority of mine to begin with and might be better suited to a separate commit.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few nits at this point. :)
Also as a note for self if I decide to implement disabling the toggle: checking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me from here!
Added new option to force usage of iGPU. Closes #51
This was my attempt at copying how other ui widgets were implemented, more than likely has some bad code conventions, so the
neroprefixsettings.ui
file needs a thorough review.For the actual feature, the environment variable
MESA_VK_DEVICE_SELECT_FORCE_DEFAULT_DEVICE
seems to work the best out of the ones I've tried, working with intel/nvidia, amd/nvidia and amd/amd systems. Should still be built and tested to see if it has any issues in other system configurations.