GNOME Bugzilla – Bug 688220
[info] System info tab for graphic info should use glxinfo -l 's result first
Last modified: 2017-01-04 14:58:58 UTC
Created attachment 228853 [details] [review] graphic info use glxinfo -l 's result first Open the system setting->Details->Graphics, find the Graphics Driver show VESA: XXXXXX strings , not the accurate information from glxinfo -l | grep "OpenGL renderer string" The "VESA:XXXX" string displayed in "System Settings-->Details->Graphics" is because gnome-control-center will find the driver information via: (1) parse /var/log/Xorg.0.log and find "VESA VBE OEM Product:" String (2) if (1) is not found, parse the result of "glxinfo -l" and find "OpenGL renderer string", and use this info as the string being shown. There're some of AMD's fglrx driver will show "VESA VBE OEM Product: XXXXX" string in /var/log/Xorg.0.log so this "VESA:XXXXX" string instead of the more accurate string from glxinfo will being shown in the panel. Use glxinfo's information (when available) first is more accurate. A proposed patch is attached.
Review of attachment 228853 [details] [review]: You just broke the case where nomodeset is passed to the kernel. See commit 2c1d6a829c246b33e4c054764ec41988646353f3
@Nocera Thanks for the info, already checked commit 2c1d6a829c246b33e4c054764ec41988646353f3, however, even when nomodeset is passed, it just indicated that we're not using kernel mode setting, this doesn't imply that we're running a VESA mode in Xorg. We can still run fglrx or nvidia driver after passing nomodeset. And more, glxinfo (if installed) can give the more accurate information than just parse the Xorg log. This also do not affect the system when glxinfo not installed. if you checked commit d734bbc7348021ae90d50ea7d1aef0f726c45d3b you'd see glxinfo is the first one to check and then lspci information. And the purpose of the patch in 2c1d6a829c246b33e4c054764ec41988646353f3 is try to replace lspci information. Thanks.
1) Split off the code that gathers the driver string 2) write a test suite to which you can feed glxinfo output and xorg logs 3) add the code from your patch and make sure nothing breaks (In reply to comment #2) > @Nocera > Thanks for the info, > already checked commit 2c1d6a829c246b33e4c054764ec41988646353f3, > > however, even when nomodeset is passed, > it just indicated that we're not using kernel mode setting, You realise that we don't actually check for nomodeset, right? > And more, glxinfo (if installed) can give the more accurate information than > just > parse the Xorg log. This also do not affect the system when glxinfo not > installed. if you checked commit d734bbc7348021ae90d50ea7d1aef0f726c45d3b > you'd see glxinfo is the first one to check and then lspci information. > And the purpose of the patch in 2c1d6a829c246b33e4c054764ec41988646353f3 is try > to replace lspci information. glxinfo is a run-time requirement.
@Nocera I'm not quite sure what you want and the concern here, could you explain more clearly? The patch just change the output priority of the current code. (glxinfo first then xorg, instead of xorg log first then glxinfo), I didn't add anything not exist in the current codebase, if glxinfo not exists, xorg log will still being parsed. Thanks.
(In reply to comment #4) > @Nocera > I'm not quite sure what you want and the concern here, > could you explain more clearly? Your code is invasive for this particular feature. We don't want to cause regressions and the best way is through using a test suite. There's plenty of other uses of test suites in the gnome-control-center (there's a test for the hostname setting for example). > The patch just change the output priority of the current code. (glxinfo first > then xorg, instead of xorg log first then glxinfo), > I didn't add anything not exist in the current codebase, > if glxinfo not exists, xorg log will still being parsed. But it will change the way the function behaves, and I want to be able to test it.
Created attachment 228954 [details] A proposed test suite for the bug #688220 code change. @Nocera Thanks, I understand your concerns, I ref to test-hostname.c and write a test suite in attachment, I'm not sure if it helps, could you help to review it and advise?
(In reply to comment #6) > Created an attachment (id=228954) [details] > A proposed test suite for the bug #688220 code change. > > @Nocera > Thanks, I understand your concerns, I ref to test-hostname.c and write a test > suite in attachment, I'm not sure if it helps, could you help to review it and > advise? I'm afraid it doesn't really help. The code is copy-pasted from the original file. The code you want to test needs to be made available for use from a third-party and actually used by this test application.
This code has changed quite a bit in recent times. The renderer string for the main adapter is exported from gnome-session, additional renderer strings are gathered from gnome-session provided helpers. If there are still problems, please test against the latest version of gnome-session and file a bug there.