|
|
Chromium Code Reviews
DescriptionRestrict GPU Raster on CrOS to Intel
We are starting our Canary/Dev testing with Intel Chromebooks only
BUG=684094
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Review-Url: https://codereview.chromium.org/2729963002
Cr-Commit-Position: refs/heads/master@{#455857}
Committed: https://chromium.googlesource.com/chromium/src/+/f0383828a203d0661de71a5d19a743813a773bac
Patch Set 1 #
Total comments: 2
Patch Set 2 : Restrict to 7th Gen #Patch Set 3 : no blacklisting #Messages
Total messages: 49 (36 generated)
Description was changed from ========== Restrict GPU Raster on CrOS to Intel + GLES3 ARM hasn't been tested yet, and GLES3+ should restrict us to modern hardware (not the CR48 :D). BUG=684094 ========== to ========== Restrict GPU Raster on CrOS to Intel + GLES3 ARM hasn't been tested yet, and GLES3+ should restrict us to modern hardware (not the CR48 :D). BUG=684094 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was checked by ericrk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
The CQ bit was unchecked by ericrk@chromium.org
The CQ bit was checked by ericrk@chromium.org
The CQ bit was checked by ericrk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
ericrk@chromium.org changed reviewers: + vmiura@chromium.org
https://codereview.chromium.org/2729963002/diff/1/gpu/config/software_renderi... File gpu/config/software_rendering_list_json.cc (right): https://codereview.chromium.org/2729963002/diff/1/gpu/config/software_renderi... gpu/config/software_rendering_list_json.cc:1512: "pixel_shader_version": { Is there a reason to use "pixel_shader_versionn" > 3 instead of "gl_version" > 3?
Description was changed from ========== Restrict GPU Raster on CrOS to Intel + GLES3 ARM hasn't been tested yet, and GLES3+ should restrict us to modern hardware (not the CR48 :D). BUG=684094 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Restrict GPU Raster on CrOS to Intel 7th Gen+ ARM hasn't been tested yet, and 6th Gen and below Intel are likely underpowered. BUG=684094 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Description was changed from ========== Restrict GPU Raster on CrOS to Intel 7th Gen+ ARM hasn't been tested yet, and 6th Gen and below Intel are likely underpowered. BUG=684094 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Restrict GPU Raster on CrOS to Intel 7th Gen+ ARM hasn't been tested yet, and 6th Gen and below Intel are likely underpowered. BUG=684094 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Patchset #2 (id:20001) has been deleted
The CQ bit was checked by ericrk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Restrict GPU Raster on CrOS to Intel 7th Gen+ ARM hasn't been tested yet, and 6th Gen and below Intel are likely underpowered. BUG=684094 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Restrict GPU Raster on CrOS to Intel 7th Gen+ ARM hasn't been tested yet, and 6th Gen and below Intel are likely underpowered. BUG=684094 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
https://codereview.chromium.org/2729963002/diff/1/gpu/config/software_renderi... File gpu/config/software_rendering_list_json.cc (right): https://codereview.chromium.org/2729963002/diff/1/gpu/config/software_renderi... gpu/config/software_rendering_list_json.cc:1512: "pixel_shader_version": { On 2017/03/04 02:04:47, vmiura wrote: > Is there a reason to use "pixel_shader_versionn" > 3 instead of "gl_version" > > 3? Nope. However, decided to move to an explicit blacklist, as it turns out this filter won't exclude SandyBridge.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)
Patchset #2 (id:40001) has been deleted
The CQ bit was checked by ericrk@chromium.org
The CQ bit was unchecked by ericrk@chromium.org
The CQ bit was checked by ericrk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
Description was changed from ========== Restrict GPU Raster on CrOS to Intel 7th Gen+ ARM hasn't been tested yet, and 6th Gen and below Intel are likely underpowered. BUG=684094 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Restrict GPU Raster on CrOS to Intel We are starting our Canary/Dev testing with Intel Chromebooks only BUG=684094 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
ericrk@chromium.org changed reviewers: + marcheu@chromium.org
Discussed with marcheu@. It sounds like we'd ideally enable this on all CrOS devices, without maintaining a blacklist - especially as we were only going to blacklist on a limited number of SandyBridge devices. We can always add blacklist entries later if it looks like these devices are causing problems. We will re-evaluate before pushing to Beta.
On 2017/03/07 18:22:17, ericrk wrote: > Discussed with marcheu@. It sounds like we'd ideally enable this on all CrOS > devices, without maintaining a blacklist - especially as we were only going to > blacklist on a limited number of SandyBridge devices. > > We can always add blacklist entries later if it looks like these devices are > causing problems. We will re-evaluate before pushing to Beta. sgtm. Have we tested Sandy Bridge specifically on CrOS?
On 2017/03/07 18:24:10, vmiura wrote: > On 2017/03/07 18:22:17, ericrk wrote: > > Discussed with marcheu@. It sounds like we'd ideally enable this on all CrOS > > devices, without maintaining a blacklist - especially as we were only going to > > blacklist on a limited number of SandyBridge devices. > > > > We can always add blacklist entries later if it looks like these devices are > > causing problems. We will re-evaluate before pushing to Beta. > > sgtm. Have we tested Sandy Bridge specifically on CrOS? Not yet - I hope to track down a sandy-bridge device (or see if marcheu@ can) and test this week. I'll hold off on launching the finch trial until we've done a basic sanity-check.
On 2017/03/07 18:38:45, ericrk wrote: > On 2017/03/07 18:24:10, vmiura wrote: > > On 2017/03/07 18:22:17, ericrk wrote: > > > Discussed with marcheu@. It sounds like we'd ideally enable this on all CrOS > > > devices, without maintaining a blacklist - especially as we were only going > to > > > blacklist on a limited number of SandyBridge devices. > > > > > > We can always add blacklist entries later if it looks like these devices are > > > causing problems. We will re-evaluate before pushing to Beta. > > > > sgtm. Have we tested Sandy Bridge specifically on CrOS? > > Not yet - I hope to track down a sandy-bridge device (or see if marcheu@ can) > and test this week. I'll hold off on launching the finch trial until we've done > a basic sanity-check. It turns out that marcheu@ has already done a spot check of these devices. I think we're good for canary. We can do a more thorough telemetry and MotionMark run before making the go/no-go call for Sandybridge beta.
The CQ bit was checked by ericrk@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/03/07 20:53:42, ericrk wrote: > On 2017/03/07 18:38:45, ericrk wrote: > > On 2017/03/07 18:24:10, vmiura wrote: > > > On 2017/03/07 18:22:17, ericrk wrote: > > > > Discussed with marcheu@. It sounds like we'd ideally enable this on all > CrOS > > > > devices, without maintaining a blacklist - especially as we were only > going > > to > > > > blacklist on a limited number of SandyBridge devices. > > > > > > > > We can always add blacklist entries later if it looks like these devices > are > > > > causing problems. We will re-evaluate before pushing to Beta. > > > > > > sgtm. Have we tested Sandy Bridge specifically on CrOS? > > > > Not yet - I hope to track down a sandy-bridge device (or see if marcheu@ can) > > and test this week. I'll hold off on launching the finch trial until we've > done > > a basic sanity-check. > > It turns out that marcheu@ has already done a spot check of these devices. I > think we're good > for canary. We can do a more thorough telemetry and MotionMark run before making > > the go/no-go call for Sandybridge beta. friendly ping. - Victor, does this sound OK? Marcheu@'s now done some spot checks and motion mark, and things are looking good.
On 2017/03/09 19:26:36, ericrk wrote: > On 2017/03/07 20:53:42, ericrk wrote: > > On 2017/03/07 18:38:45, ericrk wrote: > > > On 2017/03/07 18:24:10, vmiura wrote: > > > > On 2017/03/07 18:22:17, ericrk wrote: > > > > > Discussed with marcheu@. It sounds like we'd ideally enable this on all > > CrOS > > > > > devices, without maintaining a blacklist - especially as we were only > > going > > > to > > > > > blacklist on a limited number of SandyBridge devices. > > > > > > > > > > We can always add blacklist entries later if it looks like these devices > > are > > > > > causing problems. We will re-evaluate before pushing to Beta. > > > > > > > > sgtm. Have we tested Sandy Bridge specifically on CrOS? > > > > > > Not yet - I hope to track down a sandy-bridge device (or see if marcheu@ > can) > > > and test this week. I'll hold off on launching the finch trial until we've > > done > > > a basic sanity-check. > > > > It turns out that marcheu@ has already done a spot check of these devices. I > > think we're good > > for canary. We can do a more thorough telemetry and MotionMark run before > making > > > > the go/no-go call for Sandybridge beta. > > friendly ping. - Victor, does this sound OK? Marcheu@'s now done some spot > checks and motion mark, and things are looking good. I spot checked, and I also ran motion mark. I didn't find the time to run telemetry top25 (doing perf right now), but motion mark looks pretty good, it doesn't have any regressions, and some nice improvements.
lgtm
On 2017/03/09 19:35:53, marcheu wrote: > lgtm lgtm
The CQ bit was checked by ericrk@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1489088578269000,
"parent_rev": "5f0d1c533d0cec061943fa9d2b8470bb8c0975d9", "commit_rev":
"f0383828a203d0661de71a5d19a743813a773bac"}
Message was sent while issue was closed.
Description was changed from ========== Restrict GPU Raster on CrOS to Intel We are starting our Canary/Dev testing with Intel Chromebooks only BUG=684094 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Restrict GPU Raster on CrOS to Intel We are starting our Canary/Dev testing with Intel Chromebooks only BUG=684094 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2729963002 Cr-Commit-Position: refs/heads/master@{#455857} Committed: https://chromium.googlesource.com/chromium/src/+/f0383828a203d0661de71a5d19a7... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:80001) as https://chromium.googlesource.com/chromium/src/+/f0383828a203d0661de71a5d19a7... |
