GNOME Bugzilla – Bug 773117
Add a way to launch an app on the discrete GPU
Last modified: 2016-10-21 19:12:10 UTC
.
Created attachment 337882 [details] [review] WIP: Add a way to launch an app on the discrete GPU Unfinished, untested.
Review of attachment 337882 [details] [review]: ::: js/ui/appDisplay.js @@ +1905,3 @@ + if (discreteGpuAvailable && + actions.indexOf('activate-discrete-gpu') == -1) { + this._newWindowMenuItem = this._appendMenuItem(_("Launch on discrete GPU")); Overrides the existing _newWindowMenuItem @@ +1910,3 @@ + this._source.animateLaunch(); + + this._source.app.open_new_window(-1); Do you mean this._source.app.launch(0, -1, true); ?
Created attachment 338032 [details] [review] shell-app: Add "discrete_gpu" option when launching apps And adapt existing callers to the new API. This will allow us to implement a way to launch applications on the discrete GPU for systems where an "Optimus" system exists.
Created attachment 338033 [details] [review] appDisplay: Add a menu item to launch on the discrete GPU
I split up the patch in two. Now the "launch on discrete GPU" menu item does show up, but the FALSE is being passed to the shell_app_launch(), which I guess might be because it's taking a different path. If you want to test this, you can "mock" the switcheroo daemon. You need to stub out the majority of the code in main(): https://github.com/hadess/switcheroo-control/blob/master/src/switcheroo-control.c#L177 and set "->available = TRUE" No need to install it, just run as root. My test application is: https://github.com/hadess/glarea-example It's ebassi's demo, with a .desktop file added, and the window title set to the renderer (which isn't going to help you). When mocked, simply grep for PRIME: grep PRIME `pidof glarea`
Created attachment 338102 [details] [review] appDisplay: Add a menu item to launch on the discrete GPU It will only show up when a discrete GPU is available (detected through the switcheroo-control D-Bus service), and the application hasn't alreayd been launched. Note that this will not currently work for D-Bus activated applications, eg. the menu item will be available, but the environment variable will not be passed through to D-Bus to use when launching the application.
Created attachment 338103 [details] screenshot
(In reply to Bastien Nocera from comment #6) > Note that this will not currently work for D-Bus activated applications, > eg. the menu item will be available, but the environment variable will > not be passed through to D-Bus to use when launching the application. hmm.. Some sucky ideas: 1) hide the menu item in dbus activated cases 2) don't rely on DRI_PRIME but instead write out ~/.drirc to couple the application with a particular device_id. Of course then it would get launched on that gpu from then on out. 3) Add systemd user session service files for all dbus activatable applications, then write Environment=DRI_PRIME=1 into a service.d directory in XDG_RUNTIME_DIR 4) Call UpdateActivationEnvironment in a racy way to temporarily change DRI_PRIME before doing the activation. 5) add wayland protocol (alongside startup notification) for the app to ask the compositor which card it should use Probably 1 is our best near term option and 5 is our best longer term option.
another idea, is the menu item could map to a desktop action (like we have "new-window" currently), say, 'launch-on-gpu' then the device to use could be tucked away in the platform data passed at the time the action is activated
Review of attachment 338032 [details] [review]: ::: js/misc/util.js @@ +96,2 @@ let context = global.create_app_launch_context(0, -1); + app.launch([], context, false); This is g_app_info_launch(), not shell_app_launch() ::: js/ui/calendar.js @@ +903,3 @@ if (app.get_id() == 'evolution.desktop') app = Gio.DesktopAppInfo.new('evolution-calendar.desktop'); + app.launch([], global.create_app_launch_context(0, -1), false); Dto. ::: js/ui/components/autorunManager.js @@ +66,3 @@ retval = app.launch(files, + global.create_app_launch_context(0, -1), + false) Dto. ::: js/ui/remoteSearch.js @@ +298,3 @@ // the app itself but warn so we can catch the error in logs log('Search provider ' + this.appInfo.get_id() + ' does not implement LaunchSearch'); + this.appInfo.launch([], global.create_app_launch_context(0, -1), false); Dto.
Review of attachment 338102 [details] [review]: ::: js/ui/appDisplay.js @@ +1901,3 @@ + if (discreteGpuAvailable && + this._source.app.state == Shell.AppState.STOPPED && Maybe add !this._source.app.app_info.get_boolean('DBusActivatable') until we figure out how to make those work @@ +1902,3 @@ + if (discreteGpuAvailable && + this._source.app.state == Shell.AppState.STOPPED && + actions.indexOf('activate-discrete-gpu') == -1) { Is this used in practice? If not, maybe leave it out until we have an action name to settle on? @@ +1905,3 @@ + this._onDiscreteGpuMenuItem = this._appendMenuItem(_("Launch on discrete GPU")); + this._onDiscreteGpuMenuItem.connect('activate', Lang.bind(this, function() { + if (this._source.app.state == Shell.AppState.STOPPED) If you want to handle the case where the app has changed state between opening the menu and activating the action, shouldn't this be something like: if (this._source.app.state != Shell.AppState.STOPPED) return; this._source.animateLaunch();
Created attachment 338173 [details] [review] shell-app: Add "discrete_gpu" option when launching apps And adapt existing callers to the new API. This will allow us to implement a way to launch applications on the discrete GPU for systems where an "Optimus" system exists.
Created attachment 338175 [details] [review] appDisplay: Add a menu item to launch on the discrete GPU It will only show up when a discrete GPU is available (detected through the switcheroo-control D-Bus service), and the application hasn't alreayd been launched. Note that this will not currently work for D-Bus activated applications, eg. the menu item will be available, but the environment variable will not be passed through to D-Bus to use when launching the application.
Created attachment 338176 [details] [review] appDisplay: Add a menu item to launch on the discrete GPU It will only show up when a discrete GPU is available (detected through the switcheroo-control D-Bus service), and the application hasn't alreayd been launched. Note that this will not currently work for D-Bus activated applications, eg. the menu item will be not available, as we don't have a way to pass the environment variable through to D-Bus to use to launch the application.
Created attachment 338181 [details] [review] [f25] shell-app: Add shell_app_launch_context() helper API This will allow us to implement a way to launch applications on the discrete GPU for systems where an "Optimus" system exists, through the caller setting an environment variable.
Created attachment 338182 [details] [review] [f25] appDisplay: Add a menu item to launch on the discrete GPU It will only show up when a discrete GPU is available (detected through the switcheroo-control D-Bus service), and the application hasn't alreayd been launched. Note that this will not currently work for D-Bus activated applications, eg. the menu item will be not available, as we don't have a way to pass the environment variable through to D-Bus to use to launch the application.
"discrete GPU" sounds a bit technical to me, but if that's the established terminology and users will be familiar with it, fair enough. A less technical menu item could read "Launch using Discrete Graphics Card" or "Launch using Secondary Graphics Card".
Agreed. The problem with those suggestions though is that it lacks the "launch on more powerful graphics card" angle. Which the user would know if they knew they had 2 graphics cards (integrated vs discrete). There must be a better term though. Secondary makes it seems like the "backup" card.
Disclaimer: I did not test the patches (yet), just reviewed the code.
Review of attachment 338176 [details] [review]: Is "will be not available" correct? "will not be available" sounds better to my non-native ears ...
Review of attachment 338173 [details] [review]: LGTM
Review of attachment 338181 [details] [review]: LGTM
Review of attachment 338182 [details] [review]: OK
(In reply to Bastien Nocera from comment #18) > Agreed. The problem with those suggestions though is that it lacks the > "launch on more powerful graphics card" angle. Which the user would know if > they knew they had 2 graphics cards (integrated vs discrete). There must be > a better term though. Secondary makes it seems like the "backup" card. Are we actually sure that DRI_PRIME=1 is always discrete graphics? I'm pretty sure some machines let you choose which graphics card the bios uses, and I think in those cases DRI_PRIME=1 will always be "not the one the bios uses". In other words, I think in some situations DRI_PRIME=1 will actually pick integrated graphics, but i'm not 100% sure.
(In reply to Ray Strode [halfline] from comment #24) > (In reply to Bastien Nocera from comment #18) > > Agreed. The problem with those suggestions though is that it lacks the > > "launch on more powerful graphics card" angle. Which the user would know if > > they knew they had 2 graphics cards (integrated vs discrete). There must be > > a better term though. Secondary makes it seems like the "backup" card. > > Are we actually sure that DRI_PRIME=1 is always discrete graphics? I'm > pretty sure some machines let you choose which graphics card the bios uses, > and I think in those cases DRI_PRIME=1 will always be "not the one the bios > uses". > > In other words, I think in some situations DRI_PRIME=1 will actually pick > integrated graphics, but i'm not 100% sure. No, I'm not certain, but: - switcheroo-control forces the integrated card to be used - I expect the lower layers to "do the right thing" The devices I have don't allow changing the discrete GPU to be used as the default one. One doesn't have a BIOS to make that change (MacBook Pro), and the other just allows disabling the discrete card (T430s).
If there is a machine with this capability, I expect to be able to say which one is which through the switcheroo interface, and export that in switcheroo-control. That would be a couple more lines of code, depending on how we want to integrate this use case[1]. [1]: which is contrary to the "use cases" we set out at: https://wiki.gnome.org/Design/OS/DualGPU
Created attachment 338200 [details] [review] [f25] appDisplay: Add a menu item to launch on the discrete GPU It will only show up when a discrete GPU is available (detected through the switcheroo-control D-Bus service), and the application hasn't alreayd been launched. Note that this will not currently work for D-Bus activated applications, eg. the menu item will be not available, as we don't have a way to pass the environment variable through to D-Bus to use to launch the application.
Review of attachment 338200 [details] [review]: ::: js/ui/appDisplay.js @@ +1903,3 @@ + this._source.app.state == Shell.AppState.STOPPED && + !this._source.app.app_info.get_boolean('DBusActivatable')) { + this._onDiscreteGpuMenuItem = this._appendMenuItem(_("Launch using Dedicated Graphics Card")); Assuming this is the only change to the previous version, LGTM
(In reply to Florian Müllner from comment #28) > Review of attachment 338200 [details] [review] [review]: > > ::: js/ui/appDisplay.js > @@ +1903,3 @@ > + this._source.app.state == Shell.AppState.STOPPED && > + !this._source.app.app_info.get_boolean('DBusActivatable')) { > + this._onDiscreteGpuMenuItem = > this._appendMenuItem(_("Launch using Dedicated Graphics Card")); > > Assuming this is the only change to the previous version, LGTM It was, yes.
Review of attachment 338181 [details] [review]: ::: src/shell-app.c @@ +1239,3 @@ + GError **error) +{ + g_return_if_fail (app->app_info != NULL, FALSE); This should have read: g_return_val_if_fail(app->info != NULL, FALSE);
Review of attachment 338200 [details] [review]: Marking both "f25" patches as reviewed. I've pushed a Fedora 25 build for those, which will have to wait for switcheroo-control to be packaged before being pushed. Patches at: http://pkgs.fedoraproject.org/cgit/rpms/gnome-shell.git/tree/?h=f25 for the interested parties (note the string break)
Attachment 338173 [details] pushed as 39a840e - shell-app: Add "discrete_gpu" option when launching apps Attachment 338176 [details] pushed as 009d021 - appDisplay: Add a menu item to launch on the discrete GPU
*** Bug 704387 has been marked as a duplicate of this bug. ***