Identify the active GPU using GL strings in multiple GPU situation.

Identify the active GPU using GL strings in multiple GPU situation.

Also, now we switch the multi_gpu_category setting for each entry to "active"
gpu as default (before was "primary" as default). It's more intuitive as mostly
we want to apply entries to the GPU that's in use.

This fixes a few bugs where we incorrectly apply the blacklist decisions
and workarounds to the GPU that's not in use.

BUG=547025, 413004
TEST=gpu_unittests

Committed: https://crrev.com/7c7ee3e822e34dd970ea38cb8db4b18da9ef61bb
Cr-Commit-Position: refs/heads/master@{#364192}

[Zhenyao Mo, 2015-12-08 18:33:09]

Description was changed from

==========
Identify the active GPU using GL strings in multiple GPU situation.

This fixes a few bugs where we incorrectly apply the blacklist decisions
and workarounds to the GPU that's not in use.

BUG=547025,413004
==========

to

==========
Identify the active GPU using GL strings in multiple GPU situation.

Also, now we switch the multi_gpu_category setting for each entry to "active"
gpu as default (before was "primary" as default). It's more intuitive as mostly
we want to apply entries to the GPU that's in use.

This fixes a few bugs where we incorrectly apply the blacklist decisions
and workarounds to the GPU that's not in use.

BUG=547025,413004
==========

[Zhenyao Mo, 2015-12-08 18:33:54]

Patchset #2 (id:20001) has been deleted

[Zhenyao Mo, 2015-12-08 18:34:04]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[Zhenyao Mo, 2015-12-08 18:34:17]

Description was changed from

==========
Identify the active GPU using GL strings in multiple GPU situation.

Also, now we switch the multi_gpu_category setting for each entry to "active"
gpu as default (before was "primary" as default). It's more intuitive as mostly
we want to apply entries to the GPU that's in use.

This fixes a few bugs where we incorrectly apply the blacklist decisions
and workarounds to the GPU that's not in use.

BUG=547025,413004
==========

to

==========
Identify the active GPU using GL strings in multiple GPU situation.

Also, now we switch the multi_gpu_category setting for each entry to "active"
gpu as default (before was "primary" as default). It's more intuitive as mostly
we want to apply entries to the GPU that's in use.

This fixes a few bugs where we incorrectly apply the blacklist decisions
and workarounds to the GPU that's not in use.

BUG=547025,413004
TEST=gpu_unittests
==========

[Zhenyao Mo, 2015-12-08 18:35:15]

zmo@chromium.org changed reviewers:
+ kbr@chromium.org

[Zhenyao Mo, 2015-12-08 18:35:16]

kbr: Please review.
vmiura, dyen: FYI (feel free to review)

[commit-bot: I haz the power, 2015-12-08 18:36:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1503223005/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1503223005/1

[commit-bot: I haz the power, 2015-12-08 19:01:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-08 19:01:23]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Zhenyao Mo, 2015-12-08 22:17:48]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-08 22:22:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1503223005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1503223005/40001

[Zhenyao Mo, 2015-12-08 22:23:18]

On 2015/12/08 22:22:01, commit-bot: I haz the power wrote:
> Dry run: CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1503223005/40001
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1503223005/40001

This CL works (tested on Optimus)

We still need to change crash reports / fingerprint / etc. to use the "active"
gpu, but that's all over the places, so I will do it in a separate CL.

Please review.

[commit-bot: I haz the power, 2015-12-08 22:54:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-08 22:54:05]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ken Russell (switch to Gerrit), 2015-12-08 22:58:48]

This looks good overall but one comment.

https://codereview.chromium.org/1503223005/diff/40001/gpu/config/gpu_info_col...
File gpu/config/gpu_info_collector.cc (right):

https://codereview.chromium.org/1503223005/diff/40001/gpu/config/gpu_info_col...
gpu/config/gpu_info_collector.cc:166:
context_gpu_info.secondary_gpus[ii].active;
Are the secondary GPUs guaranteed to be in the same order?

https://codereview.chromium.org/1503223005/diff/40001/gpu/config/gpu_info_col...
gpu/config/gpu_info_collector.cc:228: active_vendor_id = kVendorIDs[ii];
We need to be very careful here. "ati" is a substring of "corporation" and I
recall NVIDIA reports "NVIDIA Corporation". I've run into this problem before
trying to parse GL strings. I think you should tokenize the renderer and vendor
strings and look for the lowercase tokens being exactly equal to each of the
above names. Please try to add a test for this case.

https://codereview.chromium.org/1503223005/diff/40001/gpu/config/gpu_info_col...
File gpu/config/gpu_info_collector.h (right):

https://codereview.chromium.org/1503223005/diff/40001/gpu/config/gpu_info_col...
gpu/config/gpu_info_collector.h:49: // If more than one GPUs are identified, GL
strings are available,
"identified, GL" -> "identified, and GL"

[Zhenyao Mo, 2015-12-09 18:35:39]

Patchset #3 (id:60001) has been deleted

[Zhenyao Mo, 2015-12-09 18:36:53]

Revised, please review again.

https://codereview.chromium.org/1503223005/diff/40001/gpu/config/gpu_info_col...
File gpu/config/gpu_info_collector.cc (right):

https://codereview.chromium.org/1503223005/diff/40001/gpu/config/gpu_info_col...
gpu/config/gpu_info_collector.cc:166:
context_gpu_info.secondary_gpus[ii].active;
On 2015/12/08 22:58:48, Ken Russell wrote:
> Are the secondary GPUs guaranteed to be in the same order?

I don't think the order will change because it's the same collecting code plus
the context stuff, but let's just copy over all the GPUs,
to be on the safe side.

https://codereview.chromium.org/1503223005/diff/40001/gpu/config/gpu_info_col...
gpu/config/gpu_info_collector.cc:228: active_vendor_id = kVendorIDs[ii];
On 2015/12/08 22:58:48, Ken Russell wrote:
> We need to be very careful here. "ati" is a substring of "corporation" and I
> recall NVIDIA reports "NVIDIA Corporation". I've run into this problem before
> trying to parse GL strings. I think you should tokenize the renderer and
vendor
> strings and look for the lowercase tokens being exactly equal to each of the
> above names. Please try to add a test for this case.

Done.

https://codereview.chromium.org/1503223005/diff/40001/gpu/config/gpu_info_col...
File gpu/config/gpu_info_collector.h (right):

https://codereview.chromium.org/1503223005/diff/40001/gpu/config/gpu_info_col...
gpu/config/gpu_info_collector.h:49: // If more than one GPUs are identified, GL
strings are available,
On 2015/12/08 22:58:48, Ken Russell wrote:
> "identified, GL" -> "identified, and GL"

Done.

[Zhenyao Mo, 2015-12-09 18:36:59]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-09 18:39:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1503223005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1503223005/80001

[Ken Russell (switch to Gerrit), 2015-12-09 21:05:12]

Thanks for updating. LGTM

https://codereview.chromium.org/1503223005/diff/80001/gpu/config/gpu_info_col...
File gpu/config/gpu_info_collector.cc (right):

https://codereview.chromium.org/1503223005/diff/80001/gpu/config/gpu_info_col...
gpu/config/gpu_info_collector.cc:250: // We fail to idendify the GPU vendor
through GL_VENDOR/GL_RENDERER.
typo: identify

[commit-bot: I haz the power, 2015-12-09 21:17:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-09 21:17:06]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Zhenyao Mo, 2015-12-09 21:18:27]

The CQ bit was checked by zmo@chromium.org

[Zhenyao Mo, 2015-12-09 21:18:27]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org
Link to the patchset: https://codereview.chromium.org/1503223005/#ps100001
(title: " ")

[commit-bot: I haz the power, 2015-12-09 21:21:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1503223005/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1503223005/100001

[commit-bot: I haz the power, 2015-12-09 23:00:38]

Description was changed from

==========
Identify the active GPU using GL strings in multiple GPU situation.

Also, now we switch the multi_gpu_category setting for each entry to "active"
gpu as default (before was "primary" as default). It's more intuitive as mostly
we want to apply entries to the GPU that's in use.

This fixes a few bugs where we incorrectly apply the blacklist decisions
and workarounds to the GPU that's not in use.

BUG=547025,413004
TEST=gpu_unittests
==========

to

==========
Identify the active GPU using GL strings in multiple GPU situation.

Also, now we switch the multi_gpu_category setting for each entry to "active"
gpu as default (before was "primary" as default). It's more intuitive as mostly
we want to apply entries to the GPU that's in use.

This fixes a few bugs where we incorrectly apply the blacklist decisions
and workarounds to the GPU that's not in use.

BUG=547025,413004
TEST=gpu_unittests
==========

[commit-bot: I haz the power, 2015-12-09 23:00:39]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2015-12-09 23:02:15]

Description was changed from

==========
Identify the active GPU using GL strings in multiple GPU situation.

Also, now we switch the multi_gpu_category setting for each entry to "active"
gpu as default (before was "primary" as default). It's more intuitive as mostly
we want to apply entries to the GPU that's in use.

This fixes a few bugs where we incorrectly apply the blacklist decisions
and workarounds to the GPU that's not in use.

BUG=547025,413004
TEST=gpu_unittests
==========

to

==========
Identify the active GPU using GL strings in multiple GPU situation.

Also, now we switch the multi_gpu_category setting for each entry to "active"
gpu as default (before was "primary" as default). It's more intuitive as mostly
we want to apply entries to the GPU that's in use.

This fixes a few bugs where we incorrectly apply the blacklist decisions
and workarounds to the GPU that's not in use.

BUG=547025,413004
TEST=gpu_unittests

Committed: https://crrev.com/7c7ee3e822e34dd970ea38cb8db4b18da9ef61bb
Cr-Commit-Position: refs/heads/master@{#364192}
==========

[commit-bot: I haz the power, 2015-12-09 23:02:20]

Patchset 4 (id:??) landed as
https://crrev.com/7c7ee3e822e34dd970ea38cb8db4b18da9ef61bb
Cr-Commit-Position: refs/heads/master@{#364192}