ozone: drm: Return page flip status right after submission

Rather than waiting for the page flip event to return the status, return
it right after it's determined, which is immediately after submitting the
page flip to the kernel. This requires splitting the page flip callback
into two callbacks, one that takes the status and one that takes the
presentation feedback. This should be familiar as the GLSurface API has
exactly the same separation.

This does not actually change when GbmSurfaceless passes the ACK along,
so there should be no visible change. Changing when the ACK is returned
from GbmSurfacless can be done but would also require changes to
viz::BufferQueue.

Bug: 851997
Test: unit tests, CrOS still renders

Change-Id: Ided422ca3065e431ca8c9a5909b3faae72d97cc5
Reviewed-on: https://chromium-review.googlesource.com/1104863Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Commit-Queue: Michael Spang <spang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570067}

ui/ozone/platform/drm/gpu/drm_thread.cc

@@ -201,27 +201,33 @@ void DrmThread::GetScanoutFormats(
   display_manager_->GetScanoutFormats(widget, scanout_formats);
 }
 
-void DrmThread::SchedulePageFlip(gfx::AcceleratedWidget widget,
-                                 std::vector<DrmOverlayPlane> planes,
-                                 SwapCompletionOnceCallback callback) {
+void DrmThread::SchedulePageFlip(
+    gfx::AcceleratedWidget widget,
+    std::vector<DrmOverlayPlane> planes,
+    SwapCompletionOnceCallback submission_callback,
+    PresentationOnceCallback presentation_callback) {
   scoped_refptr<ui::DrmDevice> drm_device =
       device_manager_->GetDrmDevice(widget);
 
   drm_device->plane_manager()->RequestPlanesReadyCallback(
       std::move(planes), base::BindOnce(&DrmThread::OnPlanesReadyForPageFlip,
                                         weak_ptr_factory_.GetWeakPtr(), widget,
-                                        std::move(callback)));
+                                        std::move(submission_callback),
+                                        std::move(presentation_callback)));
 }
 
-void DrmThread::OnPlanesReadyForPageFlip(gfx::AcceleratedWidget widget,
-                                         SwapCompletionOnceCallback callback,
-                                         std::vector<DrmOverlayPlane> planes) {
+void DrmThread::OnPlanesReadyForPageFlip(
+    gfx::AcceleratedWidget widget,
+    SwapCompletionOnceCallback submission_callback,
+    PresentationOnceCallback presentation_callback,
+    std::vector<DrmOverlayPlane> planes) {
   DrmWindow* window = screen_manager_->GetWindow(widget);
   if (window) {
-    window->SchedulePageFlip(std::move(planes), std::move(callback));
+    window->SchedulePageFlip(std::move(planes), std::move(submission_callback),
+                             std::move(presentation_callback));
   } else {
-    std::move(callback).Run(gfx::SwapResult::SWAP_ACK,
-                            gfx::PresentationFeedback::Failure());
+    std::move(submission_callback).Run(gfx::SwapResult::SWAP_ACK);
+    std::move(presentation_callback).Run(gfx::PresentationFeedback::Failure());
   }
 }
 

ui/ozone/platform/drm/gpu/drm_thread.h

@@ -85,7 +85,8 @@ class DrmThread : public base::Thread,
   // DrmWindowProxy (on GPU thread) is the client for these methods.
   void SchedulePageFlip(gfx::AcceleratedWidget widget,
                         std::vector<DrmOverlayPlane> planes,
-                        SwapCompletionOnceCallback callback);
+                        SwapCompletionOnceCallback submission_callback,
+                        PresentationOnceCallback presentation_callback);
   void GetVSyncParameters(
       gfx::AcceleratedWidget widget,
       const gfx::VSyncProvider::UpdateVSyncCallback& callback);
@@ -143,7 +144,8 @@ class DrmThread : public base::Thread,
 
  private:
   void OnPlanesReadyForPageFlip(gfx::AcceleratedWidget widget,
-                                SwapCompletionOnceCallback callback,
+                                SwapCompletionOnceCallback submission_callback,
+                                PresentationOnceCallback presentation_callback,
                                 std::vector<DrmOverlayPlane> planes);
 
   std::unique_ptr<DrmDeviceManager> device_manager_;

ui/ozone/platform/drm/gpu/drm_window.cc

@@ -114,8 +114,10 @@ void DrmWindow::MoveCursor(const gfx::Point& location) {
     controller_->MoveCursor(location);
 }
 
-void DrmWindow::SchedulePageFlip(std::vector<DrmOverlayPlane> planes,
-                                 SwapCompletionOnceCallback callback) {
+void DrmWindow::SchedulePageFlip(
+    std::vector<DrmOverlayPlane> planes,
+    SwapCompletionOnceCallback submission_callback,
+    PresentationOnceCallback presentation_callback) {
   if (controller_) {
     const DrmDevice* drm = controller_->GetDrmDevice().get();
     for (const auto& plane : planes) {
@@ -131,20 +133,23 @@ void DrmWindow::SchedulePageFlip(std::vector<DrmOverlayPlane> planes,
 
   if (force_buffer_reallocation_) {
     force_buffer_reallocation_ = false;
-    std::move(callback).Run(gfx::SwapResult::SWAP_NAK_RECREATE_BUFFERS,
-                            gfx::PresentationFeedback::Failure());
+    std::move(submission_callback)
+        .Run(gfx::SwapResult::SWAP_NAK_RECREATE_BUFFERS);
+    std::move(presentation_callback).Run(gfx::PresentationFeedback::Failure());
     return;
   }
 
   last_submitted_planes_ = DrmOverlayPlane::Clone(planes);
 
   if (!controller_) {
-    std::move(callback).Run(gfx::SwapResult::SWAP_ACK,
-                            gfx::PresentationFeedback::Failure());
+    std::move(submission_callback).Run(gfx::SwapResult::SWAP_ACK);
+    std::move(presentation_callback).Run(gfx::PresentationFeedback::Failure());
     return;
   }
 
-  controller_->SchedulePageFlip(std::move(planes), std::move(callback));
+  controller_->SchedulePageFlip(std::move(planes),
+                                std::move(submission_callback),
+                                std::move(presentation_callback));
 }
 
 std::vector<OverlayCheckReturn_Params> DrmWindow::TestPageFlip(

ui/ozone/platform/drm/gpu/drm_window.h

@@ -86,7 +86,8 @@ class DrmWindow {
   void MoveCursor(const gfx::Point& location);
 
   void SchedulePageFlip(std::vector<DrmOverlayPlane> planes,
-                        SwapCompletionOnceCallback callback);
+                        SwapCompletionOnceCallback submission_callback,
+                        PresentationOnceCallback presentation_callback);
   std::vector<OverlayCheckReturn_Params> TestPageFlip(
       const std::vector<OverlayCheck_Params>& overlay_params);
 

ui/ozone/platform/drm/gpu/drm_window_proxy.cc

@@ -18,14 +18,17 @@ DrmWindowProxy::DrmWindowProxy(gfx::AcceleratedWidget widget,
 
 DrmWindowProxy::~DrmWindowProxy() {}
 
-void DrmWindowProxy::SchedulePageFlip(std::vector<DrmOverlayPlane> planes,
-                                      SwapCompletionOnceCallback callback) {
-  auto safe_callback = CreateSafeOnceCallback(std::move(callback));
+void DrmWindowProxy::SchedulePageFlip(
+    std::vector<DrmOverlayPlane> planes,
+    SwapCompletionOnceCallback submission_callback,
+    PresentationOnceCallback presentation_callback) {
   drm_thread_->task_runner()->PostTask(
       FROM_HERE,
       base::BindOnce(&DrmThread::SchedulePageFlip,
                      base::Unretained(drm_thread_), widget_,
-                     base::Passed(&planes), std::move(safe_callback)));
+                     base::Passed(&planes),
+                     CreateSafeOnceCallback(std::move(submission_callback)),
+                     CreateSafeOnceCallback(std::move(presentation_callback))));
 }
 
 void DrmWindowProxy::GetVSyncParameters(

ui/ozone/platform/drm/gpu/drm_window_proxy.h

@@ -25,7 +25,8 @@ class DrmWindowProxy {
   gfx::AcceleratedWidget widget() const { return widget_; }
 
   void SchedulePageFlip(std::vector<DrmOverlayPlane> planes,
-                        SwapCompletionOnceCallback callback);
+                        SwapCompletionOnceCallback submission_callback,
+                        PresentationOnceCallback presentation_callback);
 
   void GetVSyncParameters(
       const gfx::VSyncProvider::UpdateVSyncCallback& callback);

ui/ozone/platform/drm/gpu/drm_window_unittest.cc

@@ -72,10 +72,12 @@ class DrmWindowTest : public testing::Test {
   void SetUp() override;
   void TearDown() override;
 
-  void OnSwapBuffers(gfx::SwapResult result,
-                     const gfx::PresentationFeedback& feedback) {
-    on_swap_buffers_count_++;
+  void OnSubmission(gfx::SwapResult result) {
     last_swap_buffers_result_ = result;
+  }
+
+  void OnPresentation(const gfx::PresentationFeedback& feedback) {
+    on_swap_buffers_count_++;
     last_presentation_feedback_ = feedback;
   }
 
@@ -188,7 +190,8 @@ TEST_F(DrmWindowTest, CheckDeathOnFailedSwap) {
   // Window was re-sized, so the expectation is to re-create the buffers first.
   window->SchedulePageFlip(
       ui::DrmOverlayPlane::Clone(planes),
-      base::BindOnce(&DrmWindowTest::OnSwapBuffers, base::Unretained(this)));
+      base::BindOnce(&DrmWindowTest::OnSubmission, base::Unretained(this)),
+      base::BindOnce(&DrmWindowTest::OnPresentation, base::Unretained(this)));
   EXPECT_EQ(1, on_swap_buffers_count_);
   EXPECT_EQ(gfx::SwapResult::SWAP_NAK_RECREATE_BUFFERS,
             last_swap_buffers_result_);
@@ -196,8 +199,10 @@ TEST_F(DrmWindowTest, CheckDeathOnFailedSwap) {
             last_presentation_feedback_.flags);
 
   EXPECT_DEATH_IF_SUPPORTED(
-      window->SchedulePageFlip(ui::DrmOverlayPlane::Clone(planes),
-                               base::BindOnce(&DrmWindowTest::OnSwapBuffers,
-                                              base::Unretained(this))),
+      window->SchedulePageFlip(
+          ui::DrmOverlayPlane::Clone(planes),
+          base::BindOnce(&DrmWindowTest::OnSubmission, base::Unretained(this)),
+          base::BindOnce(&DrmWindowTest::OnPresentation,
+                         base::Unretained(this))),
       "SchedulePageFlip failed");
 }

ui/ozone/platform/drm/gpu/gbm_surfaceless.cc

@@ -234,25 +234,24 @@ void GbmSurfaceless::PendingFrame::Flush() {
 void GbmSurfaceless::SubmitFrame() {
   DCHECK(!unsubmitted_frames_.empty());
 
-  if (unsubmitted_frames_.front()->ready && !swap_buffers_pending_) {
-    std::unique_ptr<PendingFrame> frame(std::move(unsubmitted_frames_.front()));
+  if (unsubmitted_frames_.front()->ready && !submitted_frame_) {
+    submitted_frame_ = std::move(unsubmitted_frames_.front());
     unsubmitted_frames_.erase(unsubmitted_frames_.begin());
-    swap_buffers_pending_ = true;
 
-    bool schedule_planes_succeeded = frame->ScheduleOverlayPlanes(widget_);
-    auto callback = base::BindOnce(&GbmSurfaceless::SwapCompleted,
-                                   weak_factory_.GetWeakPtr(),
-                                   base::Passed(std::move(frame)));
+    bool schedule_planes_succeeded =
+        submitted_frame_->ScheduleOverlayPlanes(widget_);
 
     if (!schedule_planes_succeeded) {
-      // |callback| is a wrapper for SwapCompleted(). Call it to properly
-      // propagate the failed state.
-      std::move(callback).Run(gfx::SwapResult::SWAP_FAILED,
-                              gfx::PresentationFeedback::Failure());
+      OnSubmission(gfx::SwapResult::SWAP_FAILED);
+      OnPresentation(gfx::PresentationFeedback::Failure());
       return;
     }
 
-    window_->SchedulePageFlip(std::move(planes_), std::move(callback));
+    window_->SchedulePageFlip(std::move(planes_),
+                              base::BindOnce(&GbmSurfaceless::OnSubmission,
+                                             weak_factory_.GetWeakPtr()),
+                              base::BindOnce(&GbmSurfaceless::OnPresentation,
+                                             weak_factory_.GetWeakPtr()));
     planes_.clear();
   }
 }
@@ -270,14 +269,19 @@ void GbmSurfaceless::FenceRetired(PendingFrame* frame) {
   SubmitFrame();
 }
 
-void GbmSurfaceless::SwapCompleted(std::unique_ptr<PendingFrame> frame,
-                                   gfx::SwapResult result,
-                                   const gfx::PresentationFeedback& feedback) {
+void GbmSurfaceless::OnSubmission(gfx::SwapResult result) {
+  submitted_frame_->swap_result = result;
+}
+
+void GbmSurfaceless::OnPresentation(const gfx::PresentationFeedback& feedback) {
   // Explicitly destroy overlays to free resources (e.g., fences) early.
-  frame->overlays.clear();
-  frame->completion_callback.Run(result);
-  frame->presentation_callback.Run(feedback);
-  swap_buffers_pending_ = false;
+  submitted_frame_->overlays.clear();
+
+  gfx::SwapResult result = submitted_frame_->swap_result;
+  std::move(submitted_frame_->completion_callback).Run(result);
+  std::move(submitted_frame_->presentation_callback).Run(feedback);
+  submitted_frame_.reset();
+
   if (result == gfx::SwapResult::SWAP_FAILED) {
     last_swap_buffers_result_ = false;
     return;

ui/ozone/platform/drm/gpu/gbm_surfaceless.h

@@ -83,6 +83,7 @@ class GbmSurfaceless : public gl::SurfacelessEGL {
     void Flush();
 
     bool ready = false;
+    gfx::SwapResult swap_result = gfx::SwapResult::SWAP_FAILED;
     std::vector<gl::GLSurfaceOverlay> overlays;
     SwapCompletionCallback completion_callback;
     PresentationCallback presentation_callback;
@@ -93,9 +94,8 @@ class GbmSurfaceless : public gl::SurfacelessEGL {
   EGLSyncKHR InsertFence(bool implicit);
   void FenceRetired(PendingFrame* frame);
 
-  void SwapCompleted(std::unique_ptr<PendingFrame> pending_frame,
-                     gfx::SwapResult result,
-                     const gfx::PresentationFeedback& feedback);
+  void OnSubmission(gfx::SwapResult result);
+  void OnPresentation(const gfx::PresentationFeedback& feedback);
 
   GbmSurfaceFactory* surface_factory_;
   std::unique_ptr<DrmWindowProxy> window_;
@@ -105,9 +105,9 @@ class GbmSurfaceless : public gl::SurfacelessEGL {
   gfx::AcceleratedWidget widget_;
   std::unique_ptr<gfx::VSyncProvider> vsync_provider_;
   std::vector<std::unique_ptr<PendingFrame>> unsubmitted_frames_;
+  std::unique_ptr<PendingFrame> submitted_frame_;
   bool has_implicit_external_sync_;
   bool last_swap_buffers_result_ = true;
-  bool swap_buffers_pending_ = false;
   bool use_egl_fence_sync_ = true;
   // Conservatively assume we begin on a device that requires
   // explicit synchronization.

ui/ozone/platform/drm/gpu/hardware_display_controller.cc

@@ -30,15 +30,14 @@ namespace {
 
 void CompletePageFlip(
     base::WeakPtr<HardwareDisplayController> hardware_display_controller_,
-    SwapCompletionOnceCallback callback,
+    PresentationOnceCallback callback,
     DrmOverlayPlaneList plane_list,
-    gfx::SwapResult swap_result,
     const gfx::PresentationFeedback& presentation_feedback) {
   if (hardware_display_controller_) {
     hardware_display_controller_->OnPageFlipComplete(std::move(plane_list),
                                                      presentation_feedback);
   }
-  std::move(callback).Run(swap_result, presentation_feedback);
+  std::move(callback).Run(presentation_feedback);
 }
 
 }  // namespace
@@ -96,7 +95,8 @@ void HardwareDisplayController::Disable() {
 
 void HardwareDisplayController::SchedulePageFlip(
     DrmOverlayPlaneList plane_list,
-    SwapCompletionOnceCallback callback) {
+    SwapCompletionOnceCallback submission_callback,
+    PresentationOnceCallback presentation_callback) {
   DCHECK(!page_flip_request_);
   scoped_refptr<PageFlipRequest> page_flip_request =
       base::MakeRefCounted<PageFlipRequest>(GetRefreshInterval());
@@ -109,15 +109,17 @@ void HardwareDisplayController::SchedulePageFlip(
     // able to happen but both CrtcController::AssignOverlayPlanes and
     // HardwareDisplayPlaneManagerLegacy::Commit appear to have cases
     // where we ACK without actually scheduling a page flip.
-    std::move(callback).Run(gfx::SwapResult::SWAP_ACK,
-                            gfx::PresentationFeedback::Failure());
+    std::move(submission_callback).Run(gfx::SwapResult::SWAP_ACK);
+    std::move(presentation_callback).Run(gfx::PresentationFeedback::Failure());
     return;
   }
 
+  std::move(submission_callback).Run(gfx::SwapResult::SWAP_ACK);
+
   // Everything was submitted successfully, wait for asynchronous completion.
-  page_flip_request->TakeCallback(
-      base::BindOnce(&CompletePageFlip, weak_ptr_factory_.GetWeakPtr(),
-                     base::Passed(&callback), base::Passed(&plane_list)));
+  page_flip_request->TakeCallback(base::BindOnce(
+      &CompletePageFlip, weak_ptr_factory_.GetWeakPtr(),
+      base::Passed(&presentation_callback), base::Passed(&plane_list)));
   page_flip_request_ = std::move(page_flip_request);
 }
 

ui/ozone/platform/drm/gpu/hardware_display_controller.h

@@ -117,7 +117,8 @@ class HardwareDisplayController {
   // Note that this function does not block. Also, this function should not be
   // called again before the page flip occurrs.
   void SchedulePageFlip(DrmOverlayPlaneList plane_list,
-                        SwapCompletionOnceCallback callback);
+                        SwapCompletionOnceCallback submission_callback,
+                        PresentationOnceCallback presentation_callback);
 
   // Returns true if the page flip with the |plane_list| would succeed. This
   // doesn't change any state.

ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc

@@ -47,8 +47,8 @@ class HardwareDisplayControllerTest : public testing::Test {
 
   void InitializeDrmDevice(bool use_atomic);
   void SchedulePageFlip(ui::DrmOverlayPlaneList planes);
-  void PageFlipCallback(gfx::SwapResult result,
-                        const gfx::PresentationFeedback& feedback);
+  void OnSubmission(gfx::SwapResult swap_result);
+  void OnPresentation(const gfx::PresentationFeedback& feedback);
 
  protected:
   std::unique_ptr<ui::HardwareDisplayController> controller_;
@@ -138,15 +138,19 @@ void HardwareDisplayControllerTest::SchedulePageFlip(
     ui::DrmOverlayPlaneList planes) {
   controller_->SchedulePageFlip(
       std::move(planes),
-      base::BindOnce(&HardwareDisplayControllerTest::PageFlipCallback,
+      base::BindOnce(&HardwareDisplayControllerTest::OnSubmission,
+                     base::Unretained(this)),
+      base::BindOnce(&HardwareDisplayControllerTest::OnPresentation,
                      base::Unretained(this)));
 }
 
-void HardwareDisplayControllerTest::PageFlipCallback(
-    gfx::SwapResult result,
+void HardwareDisplayControllerTest::OnSubmission(gfx::SwapResult result) {
+  last_swap_result_ = result;
+}
+
+void HardwareDisplayControllerTest::OnPresentation(
     const gfx::PresentationFeedback& feedback) {
   page_flips_++;
-  last_swap_result_ = result;
   last_presentation_feedback_ = feedback;
 }
 
@@ -430,7 +434,9 @@ TEST_F(HardwareDisplayControllerTest, PlaneStateAfterAddCrtc) {
   primary_crtc_plane->set_owning_crtc(0);
   hdc_controller->SchedulePageFlip(
       ui::DrmOverlayPlane::Clone(planes),
-      base::BindOnce(&HardwareDisplayControllerTest::PageFlipCallback,
+      base::BindOnce(&HardwareDisplayControllerTest::OnSubmission,
+                     base::Unretained(this)),
+      base::BindOnce(&HardwareDisplayControllerTest::OnPresentation,
                      base::Unretained(this)));
   drm_->RunCallbacks();
   EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);

ui/ozone/platform/drm/gpu/page_flip_request.cc

@@ -15,7 +15,7 @@ PageFlipRequest::PageFlipRequest(const base::TimeDelta& refresh_interval)
 PageFlipRequest::~PageFlipRequest() {
 }
 
-void PageFlipRequest::TakeCallback(SwapCompletionOnceCallback callback) {
+void PageFlipRequest::TakeCallback(PresentationOnceCallback callback) {
   DCHECK(!callback_);
   callback_ = std::move(callback);
 }
@@ -36,7 +36,7 @@ void PageFlipRequest::Signal(unsigned int frame, base::TimeTicks timestamp) {
                           gfx::PresentationFeedback::Flags::kHWClock |
                           gfx::PresentationFeedback::Flags::kHWCompletion;
   gfx::PresentationFeedback feedback(timestamp, refresh_interval_, kFlags);
-  std::move(callback_).Run(gfx::SwapResult::SWAP_ACK, feedback);
+  std::move(callback_).Run(feedback);
 }
 
 }  // namespace ui

ui/ozone/platform/drm/gpu/page_flip_request.h

@@ -28,7 +28,7 @@ class PageFlipRequest : public base::RefCounted<PageFlipRequest> {
   //
   // This is only called once all of the individual flips have have been
   // successfully submitted.
-  void TakeCallback(SwapCompletionOnceCallback callback);
+  void TakeCallback(PresentationOnceCallback callback);
 
   // Add a page flip to this request. Once all page flips return, the
   // overall callback is made with the timestamp from the page flip event
@@ -43,7 +43,7 @@ class PageFlipRequest : public base::RefCounted<PageFlipRequest> {
   friend class base::RefCounted<PageFlipRequest>;
   ~PageFlipRequest();
 
-  SwapCompletionOnceCallback callback_;
+  PresentationOnceCallback callback_;
   int page_flip_count_ = 0;
   base::TimeDelta refresh_interval_;
   gfx::SwapResult result_ = gfx::SwapResult::SWAP_ACK;

ui/ozone/platform/drm/gpu/screen_manager_unittest.cc

@@ -509,7 +509,8 @@ TEST_F(ScreenManagerTest, EnableControllerWhenWindowHasBuffer) {
       drm_, DRM_FORMAT_XRGB8888, {}, GetPrimaryBounds().size());
   ui::DrmOverlayPlaneList planes;
   planes.push_back(ui::DrmOverlayPlane(buffer, nullptr));
-  window->SchedulePageFlip(std::move(planes), base::DoNothing());
+  window->SchedulePageFlip(std::move(planes), base::DoNothing(),
+                           base::DoNothing());
   screen_manager_->AddWindow(1, std::move(window));
 
   screen_manager_->AddDisplayController(drm_, kPrimaryCrtc, kPrimaryConnector);
@@ -535,7 +536,8 @@ TEST_F(ScreenManagerTest, RejectBufferWithIncompatibleModifiers) {
 
   ui::DrmOverlayPlaneList planes;
   planes.push_back(ui::DrmOverlayPlane(buffer, nullptr));
-  window->SchedulePageFlip(std::move(planes), base::DoNothing());
+  window->SchedulePageFlip(std::move(planes), base::DoNothing(),
+                           base::DoNothing());
   screen_manager_->AddWindow(1, std::move(window));
 
   screen_manager_->AddDisplayController(drm_, kPrimaryCrtc, kPrimaryConnector);

ui/ozone/public/swap_completion_callback.h

@@ -14,8 +14,10 @@ struct PresentationFeedback;
 
 namespace ui {
 
-using SwapCompletionOnceCallback =
-    base::OnceCallback<void(gfx::SwapResult, const gfx::PresentationFeedback&)>;
+using SwapCompletionOnceCallback = base::OnceCallback<void(gfx::SwapResult)>;
+
+using PresentationOnceCallback =
+    base::OnceCallback<void(const gfx::PresentationFeedback&)>;
 
 }  // namespace ui