Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1874643003:
Do not lose secondary gpus and make sure to have an active gpu on multiple gpu configurations (Closed)
|
Created:
4 years, 8 months ago by Julien Isorce Samsung Modified:
4 years, 8 months ago CC:
chromium-reviews, darin-cc_chromium.org, jam, piman+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDo not lose secondary gpus and make sure to have an active gpu on multiple gpu configurations
Currently on chrome://gpu the user will always see
GPU0: VENDOR = 0x10de, DEVICE= 0x0de1
First problem is that "*ACTIVE*" marker is missing.
Second problem is that on multiple gpu configuration it will not show GPUi gpus
and no *ACTIVE* as well (is active gpu is not the primary gpu)
When merging GpuInfo received from the GPU process the secondary_gpus
vector is always cleared.
Indeed the GPU process always sends an empty secondary_gpus vector.
This CL also adds identification of active GPU that uses the "nouveau" open source graphic driver.
This CL also add a 4 new browser tests IdentifyActiveGpuPageN that check
the active gpu is properly listed on chrome://gpu page as well as the
secondary gpus.
R=kbr@chromium.org, piman@chromium.org, zmo@chromium.org
BUG=547025
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
Committed: https://crrev.com/74cae740e2e5bb6ee994888200f3b70af8431d88
Cr-Commit-Position: refs/heads/master@{#389546}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Add kGpuSecondaryVendorIDs and kGpuSecondaryDeviceIDs and more tests #Patch Set 3 : Add utility function gpu::ParseSecondaryGpuDevicesFromCommandLine (and rebase) #
Total comments: 10
Patch Set 4 : Added use_testing_switches param to ParseSecondaryGpuDevicesFromCommandLine and added IdentifyActiv… #
Total comments: 4
Patch Set 5 : Add unit test and remove use_testing_switches boolean (TODO: add kGpuActiveDeviceId) #Patch Set 6 : Add kGpuActiveDeviceID #Patch Set 7 : Add more unit tests #Patch Set 8 : Enable new browser tests on osx because they should work thanks to the new kGpuActiveDeviceID switc… #
Total comments: 2
Patch Set 9 : Add kGpuActiveVendorID and fix clang build errors #Patch Set 10 : Last Patch Set was missing small local build fix #Patch Set 11 : Rebase #Patch Set 12 : Added back line removed by mistake and just rebase #
Messages
Total messages: 46 (16 generated)
Description was changed from ========== Do not lose secondary gpus and make sure to have an active gpu on multiple gpu configurations Currently on chrome://gpu the user will always see GPU0: VENDOR = 0x10de, DEVICE= 0x0de1 First problem is that "*ACTIVE*" marker is missing. Second problen is that on multiple gpu configuration it will not show GPUi gpus and no *ACTIVE* as well (is active gpu is not the primary gpu) When merging GpuInfo received from the GPU process the secondary_gpus vector is always cleared. Indeed the GPU process always sends an empty secondary_gpus vector. This CL also fixes identification of active GPU that uses opensource driver nouveau or radeon. This CL also add a new browser test IdentifyActiveGpuPage that checks the active gpu is properly listed on chrome://gpu page as well as the secondary gpus. R=kbr@chromium.org, zmo@chromium.org BUG=547025 ========== to ========== Do not lose secondary gpus and make sure to have an active gpu on multiple gpu configurations Currently on chrome://gpu the user will always see GPU0: VENDOR = 0x10de, DEVICE= 0x0de1 First problem is that "*ACTIVE*" marker is missing. Second problen is that on multiple gpu configuration it will not show GPUi gpus and no *ACTIVE* as well (is active gpu is not the primary gpu) When merging GpuInfo received from the GPU process the secondary_gpus vector is always cleared. Indeed the GPU process always sends an empty secondary_gpus vector. This CL also fixes identification of active GPU that uses opensource driver nouveau or radeon. This CL also add a new browser test IdentifyActiveGpuPage that checks the active gpu is properly listed on chrome://gpu page as well as the secondary gpus. R=kbr@chromium.org, zmo@chromium.org BUG=547025 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Note that solving the TODO I added: https://codereview.chromium.org/1874643003/patch/1/10001 would solve everything. Also it would avoid what I added in this CL, the call to gpu::IdentifyActiveGPU(&gpu_info_) in gpu_data_manager_impl_private.cc and the "if (!context_gpu_info.secondary_gpus.empty())" in gpu_info_collector.cc. But how to send them properly to the gpu process ? The new kGpuSecondaryVendorID would contain either just the first secondary gpu (if would solve most of the multiple gpu configuration I suppose). Or ids separated by ";", like: kGpuSecondaryVendorID: "0x1002;0x10de"
Thanks for putting this together. It looks pretty good overall but Mo should review it too. https://codereview.chromium.org/1874643003/diff/1/content/test/gpu/page_sets/... File content/test/gpu/page_sets/gpu_process_tests.py (right): https://codereview.chromium.org/1874643003/diff/1/content/test/gpu/page_sets/... content/test/gpu/page_sets/gpu_process_tests.py:220: options.AppendExtraBrowserArgs('--gpu-testing-secondary-device-id=0x0de1') A machine with both AMD and NVIDIA GPUs isn't a realistic scenario. Could you change the fake data to define an Intel GPU and a discrete NVIDIA GPU? Can you test more scenarios, like multiple cases for the GL vendor causing the integrated and discrete GPU to be active?
On 2016/04/08 16:50:23, capOM wrote: > Note that solving the TODO I added: > https://codereview.chromium.org/1874643003/patch/1/10001 would solve everything. > Also it would avoid what I added in this CL, the call to > gpu::IdentifyActiveGPU(&gpu_info_) in gpu_data_manager_impl_private.cc and the > "if (!context_gpu_info.secondary_gpus.empty())" in gpu_info_collector.cc. > > But how to send them properly to the gpu process ? The new kGpuSecondaryVendorID > would contain either just the first secondary gpu (if would solve most of the > multiple gpu configuration I suppose). Or ids separated by ";", like: > kGpuSecondaryVendorID: "0x1002;0x10de" That sounds fine. Maybe call the new switches "kGpuSecondaryVendorIDs" and "kGpuSecondaryDeviceIDs" (plural) and pass them down in a semicolon separated list as you suggest.
>Thanks for putting this together. It looks pretty good >overall but Mo should review it too. Thx for the review. zmo@ ? > Maybe call the new switches "kGpuSecondaryVendorIDs" and > "kGpuSecondaryDeviceIDs" (plural) and pass them down in a > semicolon separated list as you suggest. All right. https://codereview.chromium.org/1874643003/diff/1/content/test/gpu/page_sets/... File content/test/gpu/page_sets/gpu_process_tests.py (right): https://codereview.chromium.org/1874643003/diff/1/content/test/gpu/page_sets/... content/test/gpu/page_sets/gpu_process_tests.py:220: options.AppendExtraBrowserArgs('--gpu-testing-secondary-device-id=0x0de1') On 2016/04/14 01:54:59, Ken Russell wrote: > A machine with both AMD and NVIDIA GPUs isn't a realistic scenario. Could you > change the fake data to define an Intel GPU and a discrete NVIDIA GPU? Can you > test more scenarios, like multiple cases for the GL vendor causing the > integrated and discrete GPU to be active? Make sense I do not remember why I put AMD instead of Intel since on my laptop I have what you said: Intel GPU and NVIDIA GPU as discrete. Sure I can add tests for more combinations.
On 2016/04/14 18:02:47, j.isorce wrote: > >Thanks for putting this together. It looks pretty good > >overall but Mo should review it too. > > Thx for the review. zmo@ ? > > > Maybe call the new switches "kGpuSecondaryVendorIDs" and > > "kGpuSecondaryDeviceIDs" (plural) and pass them down in a > > semicolon separated list as you suggest. > > All right. > > https://codereview.chromium.org/1874643003/diff/1/content/test/gpu/page_sets/... > File content/test/gpu/page_sets/gpu_process_tests.py (right): > > https://codereview.chromium.org/1874643003/diff/1/content/test/gpu/page_sets/... > content/test/gpu/page_sets/gpu_process_tests.py:220: > options.AppendExtraBrowserArgs('--gpu-testing-secondary-device-id=0x0de1') > On 2016/04/14 01:54:59, Ken Russell wrote: > > A machine with both AMD and NVIDIA GPUs isn't a realistic scenario. Could you > > change the fake data to define an Intel GPU and a discrete NVIDIA GPU? Can you > > test more scenarios, like multiple cases for the GL vendor causing the > > integrated and discrete GPU to be active? > > Make sense I do not remember why I put AMD instead of Intel since on my laptop I > have what you said: Intel GPU and NVIDIA GPU as discrete. > Sure I can add tests for more combinations. Done, please have a look.
> On 2016/04/14 18:02:47, j.isorce wrote: > Thanks for putting this together. It looks pretty good > overall but Mo should review it too. ping ?
Mostly looks good. https://codereview.chromium.org/1874643003/diff/40001/content/gpu/gpu_main.cc File content/gpu/gpu_main.cc (right): https://codereview.chromium.org/1874643003/diff/40001/content/gpu/gpu_main.cc... content/gpu/gpu_main.cc:445: gpu_info.gpu.active = true; Why assume the primary GPU is active? Wouldn't this be incorrect in some situations? https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_info_col... File gpu/config/gpu_info_collector.cc (right): https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_info_col... gpu/config/gpu_info_collector.cc:238: const std::string kRadeonName = "radeon"; Really? We have drivers with "Radeon" as vendor string? https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_util.cc File gpu/config/gpu_util.cc (right): https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_util.cc#... gpu/config/gpu_util.cc:49: // |str| is in the format of "0x040a,0x10de,...,hex32_N". not ',', but ';' https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_util.h File gpu/config/gpu_util.h (right): https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_util.h#n... gpu/config/gpu_util.h:41: const std::string& secondary_vendor_ids_key, You really don't need to pass in the two keys here. You can just directly use them in the function. We never call them with two keys as empty string. https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_util.h#n... gpu/config/gpu_util.h:43: GPUInfo* gpu_info); Also, can you add a unit test in gpu_util_unittest.cc for this function?
Thx for the review, here are my replies. https://codereview.chromium.org/1874643003/diff/40001/content/gpu/gpu_main.cc File content/gpu/gpu_main.cc (right): https://codereview.chromium.org/1874643003/diff/40001/content/gpu/gpu_main.cc... content/gpu/gpu_main.cc:445: gpu_info.gpu.active = true; On 2016/04/19 21:05:27, Zhenyao Mo wrote: > Why assume the primary GPU is active? Wouldn't this be incorrect in some > situations? You are right. When there is least one secondary gpu the active gpu will be identified by gpu::IdentifyActiveGPU (assuming that function succeeds). gpu::IdentifyActiveGPU will be called from "CollectGraphicsInfo" (some lines above, i.e. l307) But when there is no secondary gpu, gpu::IdentifyActiveGPU does an early return so gpu_info.gpu.active is false (default value). I do not think it is a problem for black list or workarounds when there is only one gpu but that's is disturbing that active is false. I'll change this to set it to true only if gpu_info.secondary_gpus.size() == 0 https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_info_col... File gpu/config/gpu_info_collector.cc (right): https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_info_col... gpu/config/gpu_info_collector.cc:238: const std::string kRadeonName = "radeon"; On 2016/04/19 21:05:27, Zhenyao Mo wrote: > Really? We have drivers with "Radeon" as vendor string? You are right it is wrong, I double checked in Mesa source code and for the open source radeon driver we cannot rely on the vendor name since it will be either "X.Org" or "X.Org R300 Project". But we can rely on the renderer string which would contain either ATI or AMD so existing code is fine for AMD hardware. But "nouveau" still required. (see https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/radeon/r600_p...) So I'll remove kRadeonName. https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_util.cc File gpu/config/gpu_util.cc (right): https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_util.cc#... gpu/config/gpu_util.cc:49: // |str| is in the format of "0x040a,0x10de,...,hex32_N". On 2016/04/19 21:05:27, Zhenyao Mo wrote: > not ',', but ';' Thx https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_util.h File gpu/config/gpu_util.h (right): https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_util.h#n... gpu/config/gpu_util.h:41: const std::string& secondary_vendor_ids_key, On 2016/04/19 21:05:27, Zhenyao Mo wrote: > You really don't need to pass in the two keys here. You can just directly use > them in the function. We never call them with two keys as empty string. Yup but there is the "testing" version of the key. I need to figure out how to do it then. https://codereview.chromium.org/1874643003/diff/40001/gpu/config/gpu_util.h#n... gpu/config/gpu_util.h:43: GPUInfo* gpu_info); On 2016/04/19 21:05:27, Zhenyao Mo wrote: > Also, can you add a unit test in gpu_util_unittest.cc for this function? Oki
In patch set 4 I addressed all remarks but still missing the c++ unit test in gpu_util_unittest.cc. Before to do it let me know what you think of the new version of "ParseSecondaryGpuDevicesFromCommandLine". Thx
Mostly look good. Please address the "active" bit and add the unit tests. https://codereview.chromium.org/1874643003/diff/60001/content/gpu/gpu_main.cc File content/gpu/gpu_main.cc (right): https://codereview.chromium.org/1874643003/diff/60001/content/gpu/gpu_main.cc... content/gpu/gpu_main.cc:447: gpu_info.gpu.active = true; This is better, but still doesn't address the problem when multiple GPUs are passed from browser process to GPU process, which one is active. I suggest in multiple GPU situation, you pass another switch to identify which GPU is active. If you decide to pass all the information including secondary GPUs, it doesn't make sense you leave out the "active" bit. https://codereview.chromium.org/1874643003/diff/60001/gpu/config/gpu_util.cc File gpu/config/gpu_util.cc (right): https://codereview.chromium.org/1874643003/diff/60001/gpu/config/gpu_util.cc#... gpu/config/gpu_util.cc:113: bool use_testing_switches, I don't think you need this bool. If testing switches exist, they should always overwrite the other switches.
https://codereview.chromium.org/1874643003/diff/60001/content/gpu/gpu_main.cc File content/gpu/gpu_main.cc (right): https://codereview.chromium.org/1874643003/diff/60001/content/gpu/gpu_main.cc... content/gpu/gpu_main.cc:447: gpu_info.gpu.active = true; On 2016/04/21 17:03:37, Zhenyao Mo wrote: > This is better, but still doesn't address the problem when multiple GPUs are > passed from browser process to GPU process, which one is active. I suggest in > multiple GPU situation, you pass another switch to identify which GPU is active. > If you decide to pass all the information including secondary GPUs, it doesn't > make sense you leave out the "active" bit. Done, I added kGpuActiveDeviceID. https://codereview.chromium.org/1874643003/diff/60001/gpu/config/gpu_util.cc File gpu/config/gpu_util.cc (right): https://codereview.chromium.org/1874643003/diff/60001/gpu/config/gpu_util.cc#... gpu/config/gpu_util.cc:113: bool use_testing_switches, On 2016/04/21 17:03:37, Zhenyao Mo wrote: > I don't think you need this bool. If testing switches exist, they should always > overwrite the other switches. Done.
On 2016/04/21 17:03:38, Zhenyao Mo wrote: > Mostly look good. Please address the "active" bit and add the unit tests. Thx for the review. Done: unit test added from Patch Set 5, and "active" bit in Patch Set 6.
On 2016/04/21 23:29:22, j.isorce wrote: > On 2016/04/21 17:03:38, Zhenyao Mo wrote: > > Mostly look good. Please address the "active" bit and add the unit tests. > > Thx for the review. Done: unit test added from Patch Set 5, and "active" bit in > Patch Set 6. I also added more unit tests and enable new browser tests on osx (Patch Set 7 and 8). PTAL.
Description was changed from ========== Do not lose secondary gpus and make sure to have an active gpu on multiple gpu configurations Currently on chrome://gpu the user will always see GPU0: VENDOR = 0x10de, DEVICE= 0x0de1 First problem is that "*ACTIVE*" marker is missing. Second problen is that on multiple gpu configuration it will not show GPUi gpus and no *ACTIVE* as well (is active gpu is not the primary gpu) When merging GpuInfo received from the GPU process the secondary_gpus vector is always cleared. Indeed the GPU process always sends an empty secondary_gpus vector. This CL also fixes identification of active GPU that uses opensource driver nouveau or radeon. This CL also add a new browser test IdentifyActiveGpuPage that checks the active gpu is properly listed on chrome://gpu page as well as the secondary gpus. R=kbr@chromium.org, zmo@chromium.org BUG=547025 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Do not lose secondary gpus and make sure to have an active gpu on multiple gpu configurations Currently on chrome://gpu the user will always see GPU0: VENDOR = 0x10de, DEVICE= 0x0de1 First problem is that "*ACTIVE*" marker is missing. Second problem is that on multiple gpu configuration it will not show GPUi gpus and no *ACTIVE* as well (is active gpu is not the primary gpu) When merging GpuInfo received from the GPU process the secondary_gpus vector is always cleared. Indeed the GPU process always sends an empty secondary_gpus vector. This CL also adds identification of active GPU that uses the "nouveau" open source graphic driver. This CL also add a 4 new browser tests IdentifyActiveGpuPageN that check the active gpu is properly listed on chrome://gpu page as well as the secondary gpus. R=kbr@chromium.org, zmo@chromium.org BUG=547025 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
https://codereview.chromium.org/1874643003/diff/140001/content/browser/gpu/gp... File content/browser/gpu/gpu_data_manager_impl_private.cc (right): https://codereview.chromium.org/1874643003/diff/140001/content/browser/gpu/gp... content/browser/gpu/gpu_data_manager_impl_private.cc:776: base::StringPrintf("0x%04x", maybe_active_gpu_device.device_id)); You should use vendor_id + devcie_id to identify a GPU.
The CQ bit was checked by zmo@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1874643003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1874643003/140001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
Done in Patch Set 9. Thx https://codereview.chromium.org/1874643003/diff/140001/content/browser/gpu/gp... File content/browser/gpu/gpu_data_manager_impl_private.cc (right): https://codereview.chromium.org/1874643003/diff/140001/content/browser/gpu/gp... content/browser/gpu/gpu_data_manager_impl_private.cc:776: base::StringPrintf("0x%04x", maybe_active_gpu_device.device_id)); On 2016/04/22 17:17:53, Zhenyao Mo wrote: > You should use vendor_id + devcie_id to identify a GPU. You are entirely right only the pair vendor_id,device_id is unique.
LGTM if bots are green
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1874643003/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1874643003/180001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1874643003/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1874643003/200001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by j.isorce@samsung.com
The patchset sent to the CQ was uploaded after l-g-t-m from zmo@chromium.org Link to the patchset: https://codereview.chromium.org/1874643003/#ps200001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1874643003/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1874643003/200001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Description was changed from ========== Do not lose secondary gpus and make sure to have an active gpu on multiple gpu configurations Currently on chrome://gpu the user will always see GPU0: VENDOR = 0x10de, DEVICE= 0x0de1 First problem is that "*ACTIVE*" marker is missing. Second problem is that on multiple gpu configuration it will not show GPUi gpus and no *ACTIVE* as well (is active gpu is not the primary gpu) When merging GpuInfo received from the GPU process the secondary_gpus vector is always cleared. Indeed the GPU process always sends an empty secondary_gpus vector. This CL also adds identification of active GPU that uses the "nouveau" open source graphic driver. This CL also add a 4 new browser tests IdentifyActiveGpuPageN that check the active gpu is properly listed on chrome://gpu page as well as the secondary gpus. R=kbr@chromium.org, zmo@chromium.org BUG=547025 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Do not lose secondary gpus and make sure to have an active gpu on multiple gpu configurations Currently on chrome://gpu the user will always see GPU0: VENDOR = 0x10de, DEVICE= 0x0de1 First problem is that "*ACTIVE*" marker is missing. Second problem is that on multiple gpu configuration it will not show GPUi gpus and no *ACTIVE* as well (is active gpu is not the primary gpu) When merging GpuInfo received from the GPU process the secondary_gpus vector is always cleared. Indeed the GPU process always sends an empty secondary_gpus vector. This CL also adds identification of active GPU that uses the "nouveau" open source graphic driver. This CL also add a 4 new browser tests IdentifyActiveGpuPageN that check the active gpu is properly listed on chrome://gpu page as well as the secondary gpus. R=kbr@chromium.org, piman@chromium.org, zmo@chromium.org BUG=547025 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
j.isorce@samsung.com changed reviewers: + piman@chromium.org
On 2016/04/25 10:38:01, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...) ** Presubmit ERRORS ** Missing LGTM from an OWNER for these files: content/gpu/gpu_main.cc Hi Antoine, please have a look to this CL, thx.
lgtm
The CQ bit was checked by j.isorce@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1874643003/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1874643003/200001
Message was sent while issue was closed.
Description was changed from ========== Do not lose secondary gpus and make sure to have an active gpu on multiple gpu configurations Currently on chrome://gpu the user will always see GPU0: VENDOR = 0x10de, DEVICE= 0x0de1 First problem is that "*ACTIVE*" marker is missing. Second problem is that on multiple gpu configuration it will not show GPUi gpus and no *ACTIVE* as well (is active gpu is not the primary gpu) When merging GpuInfo received from the GPU process the secondary_gpus vector is always cleared. Indeed the GPU process always sends an empty secondary_gpus vector. This CL also adds identification of active GPU that uses the "nouveau" open source graphic driver. This CL also add a 4 new browser tests IdentifyActiveGpuPageN that check the active gpu is properly listed on chrome://gpu page as well as the secondary gpus. R=kbr@chromium.org, piman@chromium.org, zmo@chromium.org BUG=547025 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Do not lose secondary gpus and make sure to have an active gpu on multiple gpu configurations Currently on chrome://gpu the user will always see GPU0: VENDOR = 0x10de, DEVICE= 0x0de1 First problem is that "*ACTIVE*" marker is missing. Second problem is that on multiple gpu configuration it will not show GPUi gpus and no *ACTIVE* as well (is active gpu is not the primary gpu) When merging GpuInfo received from the GPU process the secondary_gpus vector is always cleared. Indeed the GPU process always sends an empty secondary_gpus vector. This CL also adds identification of active GPU that uses the "nouveau" open source graphic driver. This CL also add a 4 new browser tests IdentifyActiveGpuPageN that check the active gpu is properly listed on chrome://gpu page as well as the secondary gpus. R=kbr@chromium.org, piman@chromium.org, zmo@chromium.org BUG=547025 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Message was sent while issue was closed.
Committed patchset #11 (id:200001)
Message was sent while issue was closed.
Description was changed from ========== Do not lose secondary gpus and make sure to have an active gpu on multiple gpu configurations Currently on chrome://gpu the user will always see GPU0: VENDOR = 0x10de, DEVICE= 0x0de1 First problem is that "*ACTIVE*" marker is missing. Second problem is that on multiple gpu configuration it will not show GPUi gpus and no *ACTIVE* as well (is active gpu is not the primary gpu) When merging GpuInfo received from the GPU process the secondary_gpus vector is always cleared. Indeed the GPU process always sends an empty secondary_gpus vector. This CL also adds identification of active GPU that uses the "nouveau" open source graphic driver. This CL also add a 4 new browser tests IdentifyActiveGpuPageN that check the active gpu is properly listed on chrome://gpu page as well as the secondary gpus. R=kbr@chromium.org, piman@chromium.org, zmo@chromium.org BUG=547025 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Do not lose secondary gpus and make sure to have an active gpu on multiple gpu configurations Currently on chrome://gpu the user will always see GPU0: VENDOR = 0x10de, DEVICE= 0x0de1 First problem is that "*ACTIVE*" marker is missing. Second problem is that on multiple gpu configuration it will not show GPUi gpus and no *ACTIVE* as well (is active gpu is not the primary gpu) When merging GpuInfo received from the GPU process the secondary_gpus vector is always cleared. Indeed the GPU process always sends an empty secondary_gpus vector. This CL also adds identification of active GPU that uses the "nouveau" open source graphic driver. This CL also add a 4 new browser tests IdentifyActiveGpuPageN that check the active gpu is properly listed on chrome://gpu page as well as the secondary gpus. R=kbr@chromium.org, piman@chromium.org, zmo@chromium.org BUG=547025 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/74cae740e2e5bb6ee994888200f3b70af8431d88 Cr-Commit-Position: refs/heads/master@{#389546} ==========
Message was sent while issue was closed.
Patchset 11 (id:??) landed as https://crrev.com/74cae740e2e5bb6ee994888200f3b70af8431d88 Cr-Commit-Position: refs/heads/master@{#389546}
Message was sent while issue was closed.
On 2016/04/25 20:03:03, commit-bot: I haz the power wrote: > Patchset 11 (id:??) landed as > https://crrev.com/74cae740e2e5bb6ee994888200f3b70af8431d88 > Cr-Commit-Position: refs/heads/master@{#389546} Oups, "Patch Set 12" has nothing to do with this CL. (Only patch set 11 has landed) |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
