[ozone] Keep the queue of surfaceless buffers inside gl_surface_ozone

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/ozone/platform/drm/gpu/hardware_display_controller.h"
 
 #include <drm.h>
 #include <string.h>
 #include <xf86drm.h>
 
@@ skipping 15 matching lines @@
     bool is_sync,
     const base::Closure& callback)
     : planes(planes), is_sync(is_sync), callback(callback) {
 }
 
 HardwareDisplayController::PageFlipRequest::~PageFlipRequest() {
 }
 
 HardwareDisplayController::HardwareDisplayController(
     scoped_ptr<CrtcController> controller)
-    : is_disabled_(true) {
+    : is_disabled_(true), ignore_page_flip_event_(false) {
   memset(&mode_, 0, sizeof(mode_));
   AddCrtc(controller.Pass());
 }
 
 HardwareDisplayController::~HardwareDisplayController() {
   // Reset the cursor.
   UnsetCursor();
-  ClearPendingRequests();
+  ProcessPageFlipRequest();
 }
 
 bool HardwareDisplayController::Modeset(const OverlayPlane& primary,
                                         drmModeModeInfo mode) {
   TRACE_EVENT0("drm", "HDC::Modeset");
   DCHECK(primary.buffer.get());
   bool status = true;
   for (size_t i = 0; i < crtc_controllers_.size(); ++i)
     status &= crtc_controllers_[i]->Modeset(primary, mode);
 
   is_disabled_ = false;
   mode_ = mode;
 
-  current_planes_ = std::vector<OverlayPlane>(1, primary);
-  ClearPendingRequests();
+  ProcessPageFlipRequest();
 
   // Because a page flip is pending we need to leave some state for the

[dnicoara, 2015/04/21 18:50:42]

Could you please update the comment.

-  // callback. We use the modeset state since it is the only valid state.
+  // callback.
   if (HasPendingPageFlips())
-    requests_.push_back(
-        PageFlipRequest(current_planes_, false, base::Bind(&base::DoNothing)));
+    ignore_page_flip_event_ = true;

[dnicoara, 2015/04/21 18:50:42]

nit: Simplity it to 'ignore_page_flip_event_ = HasPendingPageFlips();'

[alexst (slow to review), 2015/04/21 19:19:55]

Done.

[spang, 2015/04/21 19:39:36]

This is funky.

Could we pass a callback w/ weak pointer to the PageFlipRequest instead? Then
cancellation will be transparent.

[alexst (slow to review), 2015/04/21 19:46:17]

Yes, that's a good idea.

 
   return status;
 }
 
 void HardwareDisplayController::Disable() {
   TRACE_EVENT0("drm", "HDC::Disable");
   for (size_t i = 0; i < crtc_controllers_.size(); ++i)
     crtc_controllers_[i]->Disable();
 
   // Don't hold onto any requests because we don't track fron buffer while
   // disabled.
-  while (requests_.size())
-    ProcessPageFlipRequest();
-
-  current_planes_.clear();
+  ProcessPageFlipRequest();
 
   is_disabled_ = true;
 }
 
 bool HardwareDisplayController::SchedulePageFlip(
     const OverlayPlaneList& plane_list,
     bool is_sync,
     const base::Closure& callback) {
   TRACE_EVENT0("drm", "HDC::SchedulePageFlip");
 
+  // A request is being serviced right now.

[dnicoara, 2015/04/21 18:50:42]

Shouldn't this be reversed? When we schedule the page flip, there shouldn't be
any pending request, right?

[alexst (slow to review), 2015/04/21 19:19:55]

of course!

+  DCHECK(HasPendingPageFlips());
+  DCHECK(page_flip_request_.get());
+
   // Ignore requests with no planes to schedule.
   if (plane_list.empty()) {
     callback.Run();
     return true;
   }
 
-  requests_.push_back(PageFlipRequest(plane_list, is_sync, callback));
+  page_flip_request_.reset(new PageFlipRequest(plane_list, is_sync, callback));
 
-  // A request is being serviced right now.
-  if (HasPendingPageFlips())
-    return true;
-
-  bool status = ActualSchedulePageFlip();
-
-  // No page flip event on failure so discard failed request.
-  if (!status)
-    requests_.pop_front();
-
-  return status;
+  return ActualSchedulePageFlip();
 }
 
 bool HardwareDisplayController::SetCursor(
     const scoped_refptr<ScanoutBuffer>& buffer) {
   bool status = true;
 
   if (is_disabled_)
     return true;
 
   for (size_t i = 0; i < crtc_controllers_.size(); ++i)
@@ skipping 106 matching lines @@
   //       for the next request. In this case we expect that |requests_| isn't
   //       empty.
   //  2) A CRTC was added while it was page flipping. In this case a modeset
   //     must be performed. Modesetting clears all pending requests, however the
   //     CRTCs will honor the scheduled page flip. Thus we need to handle page
   //     flip events with no requests.
 
   if (HasPendingPageFlips())
     return;
 
-  if (!requests_.empty())
-    ProcessPageFlipRequest();
+  if (ignore_page_flip_event_) {
+    ignore_page_flip_event_ = false;
+    return;
+  }
 
-  // ProcessPageFlipRequest() consumes a request.
-  if (requests_.empty())
-    return;
-
-  // At this point we still have requests pending, so schedule the next request.
-  bool status = ActualSchedulePageFlip();
-  if (!status) {
-    PageFlipRequest request = requests_.front();
-    requests_.pop_front();
-
-    // Normally the caller would handle the error call, but because we're in a
-    // delayed schedule the initial SchedulePageFlip() already returned true,
-    // thus we need to run the callback.
-    request.callback.Run();
-  }
+  ProcessPageFlipRequest();
 }
 
 scoped_refptr<DrmDevice> HardwareDisplayController::GetAllocationDrmDevice()
     const {
   DCHECK(!crtc_controllers_.empty());
   // TODO(dnicoara) When we support mirroring across DRM devices, figure out
   // which device should be used for allocations.
   return crtc_controllers_[0]->drm();
 }
 
 bool HardwareDisplayController::HasPendingPageFlips() const {
   for (size_t i = 0; i < crtc_controllers_.size(); ++i)
     if (crtc_controllers_[i]->page_flip_pending())
       return true;
 
   return false;
 }
 
 bool HardwareDisplayController::ActualSchedulePageFlip() {
   TRACE_EVENT0("drm", "HDC::ActualSchedulePageFlip");
-  DCHECK(!requests_.empty());
+  DCHECK(page_flip_request_.get());
+  DCHECK(!is_disabled_);
 
-  if (is_disabled_) {
-    ProcessPageFlipRequest();
-    return true;
-  }
-
-  OverlayPlaneList pending_planes = requests_.front().planes;
+  OverlayPlaneList& pending_planes = page_flip_request_->planes;
   std::sort(pending_planes.begin(), pending_planes.end(),
             [](const OverlayPlane& l, const OverlayPlane& r) {
               return l.z_order < r.z_order;
             });
 
   bool status = true;
   for (size_t i = 0; i < crtc_controllers_.size(); ++i) {
     status &= crtc_controllers_[i]->SchedulePageFlip(
         owned_hardware_planes_.get(crtc_controllers_[i]->drm().get()),
         pending_planes);
   }
 
-  bool is_sync = requests_.front().is_sync;
+  bool is_sync = page_flip_request_->is_sync;
   for (const auto& planes : owned_hardware_planes_) {
     if (!planes.first->plane_manager()->Commit(planes.second, is_sync)) {
       status = false;
     }
   }
 
   return status;
 }
 
 void HardwareDisplayController::ProcessPageFlipRequest() {
-  DCHECK(!requests_.empty());
-  PageFlipRequest request = requests_.front();
-  requests_.pop_front();
-
-  current_planes_.swap(request.planes);
-  request.callback.Run();
-}
-
-void HardwareDisplayController::ClearPendingRequests() {
-  while (!requests_.empty()) {
-    PageFlipRequest request = requests_.front();
-    requests_.pop_front();
-    request.callback.Run();
+  if (page_flip_request_.get()) {
+    scoped_ptr<PageFlipRequest> last_request;
+    // We are done with the last request, so clear it before we execute the
+    // callback since it may schedule a new frame.
+    swap(last_request, page_flip_request_);
+    last_request->callback.Run();
   }
 }
 
 }  // namespace ui