Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2369593002:
ozone: Allow cursor requests to come in before the drm thread comes up. (Closed)
Descriptionozone: Allow cursor requests to come in before the drm thread comes up.
BUG=none
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
Committed: https://crrev.com/950c3c1b058cc222e862a5a12a6cb90cf2057b9c
Cr-Commit-Position: refs/heads/master@{#421288}
Patch Set 1 #Patch Set 2 : . #
Total comments: 3
Patch Set 3 : tot merge. #Patch Set 4 : todo #Messages
Total messages: 24 (17 generated)
Description was changed from ========== ozone: Allow cursor requests to come in before the drm thread comes up. BUG=none ========== to ========== ozone: Allow cursor requests to come in before the drm thread comes up. BUG=none 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 sadrul@chromium.org to run a CQ dry run
sadrul@chromium.org changed reviewers: + rjkroege@chromium.org
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.
https://codereview.chromium.org/2369593002/diff/20001/ui/ozone/platform/drm/o... File ui/ozone/platform/drm/ozone_platform_gbm.cc (right): https://codereview.chromium.org/2369593002/diff/20001/ui/ozone/platform/drm/o... ui/ozone/platform/drm/ozone_platform_gbm.cc:116: if (drm_thread_proxy_) Does the DCHECK actually fire in practice? This surprises me. Moreover: isn't mojo suppose to handle this? The pipe is already made and it should be accumulating the requests until the binding is made?
https://codereview.chromium.org/2369593002/diff/20001/ui/ozone/platform/drm/o... File ui/ozone/platform/drm/ozone_platform_gbm.cc (right): https://codereview.chromium.org/2369593002/diff/20001/ui/ozone/platform/drm/o... ui/ozone/platform/drm/ozone_platform_gbm.cc:116: if (drm_thread_proxy_) On 2016/09/25 19:09:43, rjkroege wrote: > Does the DCHECK actually fire in practice? This surprises me. > > Moreover: isn't mojo suppose to handle this? The pipe is already made and it > should be accumulating the requests until the binding is made? After https://codereview.chromium.org/2366623002/, the DCHECK does hit, since this code runs on the ui thread, and the gpu initialization happens asynchronously in the gpu thread (where |drm_thread_proxy_| is created). Once gpu process split happens, mojo will take care of it for us. But until then, we need to do this ourselves.
lgtm but please add TODO(). https://codereview.chromium.org/2369593002/diff/20001/ui/ozone/platform/drm/o... File ui/ozone/platform/drm/ozone_platform_gbm.cc (right): https://codereview.chromium.org/2369593002/diff/20001/ui/ozone/platform/drm/o... ui/ozone/platform/drm/ozone_platform_gbm.cc:116: if (drm_thread_proxy_) On 2016/09/25 20:44:35, sadrul wrote: > On 2016/09/25 19:09:43, rjkroege wrote: > > Does the DCHECK actually fire in practice? This surprises me. > > > > Moreover: isn't mojo suppose to handle this? The pipe is already made and it > > should be accumulating the requests until the binding is made? > > After https://codereview.chromium.org/2366623002/, the DCHECK does hit, since > this code runs on the ui thread, and the gpu initialization happens > asynchronously in the gpu thread (where |drm_thread_proxy_| is created). Once > gpu process split happens, mojo will take care of it for us. But until then, we > need to do this ourselves. ah. Can you add a TODO then.
The CQ bit was checked by sadrul@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.
The CQ bit was checked by sadrul@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.
The CQ bit was checked by sadrul@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rjkroege@chromium.org Link to the patchset: https://codereview.chromium.org/2369593002/#ps60001 (title: "todo")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== ozone: Allow cursor requests to come in before the drm thread comes up. BUG=none 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 ========== ozone: Allow cursor requests to come in before the drm thread comes up. BUG=none 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 Committed: https://crrev.com/950c3c1b058cc222e862a5a12a6cb90cf2057b9c Cr-Commit-Position: refs/heads/master@{#421288} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/950c3c1b058cc222e862a5a12a6cb90cf2057b9c Cr-Commit-Position: refs/heads/master@{#421288} |
