Change GPUInfo to handle multiple GPUs.

Change GPUInfo to handle multiple GPUs.

At the moment we always select one GPU as primary and the others as secondary.  In preliminary GPU info collection (without creating a GL context and collect GL VENDOR/RENDERER strings), we actually don't know which GPU is in use.  The current logic is that if one GPU is Intel, we assume the other is primary.

I agree a better logic is needed.  However, at the moment, logging/crash reports all requires ONE GPU vendor_id/device_id, so even though we don't know which GPU is active, we still need to randomly select one.

This needs more thinking and design, so I think it's reasonable to leave it to a possible future CL.

Also, the logic on collecting GPU info through libpci is changed to collect multiple GPUs.  If one is Intel and one is NVIDIA, we assume it's optimus and we disable GPU features.  This logic is hardwired in Chrome in this CL.  The plan is to push the logic to blacklist, but I'll leave it to a seperate CL.

BUG=126307, 75220
TEST=about:gpu page on a Linux system with optimus GPU
R=kbr
TBR=jam
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=136429

[Zhenyao Mo, 2012-05-10 00:02:11]

Ken, please have a look.

Note that in this CL I only changed the linux port code to log multiple GPUs in
GPUInfo.  I'll do the Mac port in another CL.

[Ken Russell (switch to Gerrit), 2012-05-10 01:11:08]

Couple of questions.

https://chromiumcodereview.appspot.com/10389051/diff/2004/content/browser/gpu...
File content/browser/gpu/gpu_data_manager_impl.h (left):

https://chromiumcodereview.appspot.com/10389051/diff/2004/content/browser/gpu...
content/browser/gpu/gpu_data_manager_impl.h:91: static bool
Merge(content::GPUInfo* object, const content::GPUInfo& other);
Was removing this method intentional? If the GPU process fails to collect GPU
information and manages to send back an empty GPUInfo to the browser process, it
looks like the browser process will always overwrite its possibly better
information. Is that what was happening on Optimus machines before this CL? Is
the new behavior better or worse than the earlier behavior?

https://chromiumcodereview.appspot.com/10389051/diff/2004/content/test/gpu/gp...
File content/test/gpu/gpu_test_config_unittest.cc (right):

https://chromiumcodereview.appspot.com/10389051/diff/2004/content/test/gpu/gp...
content/test/gpu/gpu_test_config_unittest.cc:220: 
Are any new unit tests needed or possible?

[Zhenyao Mo, 2012-05-10 21:55:32]

Revised, please have another look.

Also, I changed the about:gpu behavior to list GPUs as GPU0, GPU1, ... instead
of primary GPU, secondary GPUs.

The intention is for GPU0 always to be the active one if we can tell which one
is active.  But this is yet to be implemented.

http://codereview.chromium.org/10389051/diff/2004/content/browser/gpu/gpu_dat...
File content/browser/gpu/gpu_data_manager_impl.h (left):

http://codereview.chromium.org/10389051/diff/2004/content/browser/gpu/gpu_dat...
content/browser/gpu/gpu_data_manager_impl.h:91: static bool
Merge(content::GPUInfo* object, const content::GPUInfo& other);
On 2012/05/10 01:11:09, kbr wrote:
> Was removing this method intentional? If the GPU process fails to collect GPU
> information and manages to send back an empty GPUInfo to the browser process,
it
> looks like the browser process will always overwrite its possibly better
> information. Is that what was happening on Optimus machines before this CL? Is
> the new behavior better or worse than the earlier behavior?

The removal of this Merge function is on purpose.  It's very easy to miss this
function when we add new fields to the GPUInfo, and the optimization we lose
without Merge is ignorable (we always re-run blacklist logic when we launch GPU
process).

However, you are right, I failed to consider the case where GPU process side
info collection fails.  Fixed in the new CL.

http://codereview.chromium.org/10389051/diff/2004/content/test/gpu/gpu_test_c...
File content/test/gpu/gpu_test_config_unittest.cc (right):

http://codereview.chromium.org/10389051/diff/2004/content/test/gpu/gpu_test_c...
content/test/gpu/gpu_test_config_unittest.cc:220: 
On 2012/05/10 01:11:09, kbr wrote:
> Are any new unit tests needed or possible?

Not really.  Test bot config only needs to deal with primary GPU, and the
behavior of that is not changing with this CL and is already tested.

[Ken Russell (switch to Gerrit), 2012-05-10 22:22:40]

Sounds good. LGTM