Revert "Ozone/DRM: Eliminate tracking of owned planes in HardwareDisplayController"

This reverts commit 7edc6542.

Reason for revert: broke ARC++ graphics/video b/148349017
Bug: b/148349017

Original change's description:
> Ozone/DRM: Eliminate tracking of owned planes in HardwareDisplayController
> 
> Don't keep track of owned planes directly in HardwareDisplayController.
> We already know the set of owned CRTCs and we can ask the plane manager
> to find the planes corresponding to those CRTCs. By removing
> |owned_hardware_planes_| from HardwareDisplayController, we reduce
> the amount of state that has to be protected by a lock if we wish to
> perform pageflip testing on another thread (see the bug for more
> context).
> 
> TEST=Built and ran on eve and chell. Connected and disconnected an
> external monitor. There is also plenty of existing unit tests that cover
> this code.
> 
> Bug: 1034559
> Change-Id: I16fccc549c272c8659d392996a163877ac7bd7ba
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2003561
> Commit-Queue: Saman Sami <samans@chromium.org>
> Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#733746}

TBR=spang@chromium.org,achaulk@chromium.org,dnicoara@chromium.org,samans@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1034559
Change-Id: I9b746ae555e875d6648ea68cac9ee642ac3fc3b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2028307Reviewed-by: Kazuhiro Inaba <kinaba@chromium.org>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Kazuhiro Inaba <kinaba@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Auto-Submit: Kazuhiro Inaba <kinaba@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736395}

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

@@ -102,7 +102,7 @@ void HardwareDisplayController::Disable() {
     controller->Disable();
 
   bool ret = GetDrmDevice()->plane_manager()->DisableOverlayPlanes(
-      GetAllOwnedPlanes());
+      &owned_hardware_planes_);
   LOG_IF(ERROR, !ret) << "Can't disable overlays when disabling HDC.";
 
   is_disabled_ = true;
@@ -162,19 +162,16 @@ bool HardwareDisplayController::ScheduleOrTestPageFlip(
             [](const DrmOverlayPlane& l, const DrmOverlayPlane& r) {
               return l.z_order < r.z_order;
             });
-
-  HardwareDisplayPlaneList hardware_planes;
-  hardware_planes.old_plane_list = GetAllOwnedPlanes();
-
-  GetDrmDevice()->plane_manager()->BeginFrame(hardware_planes);
+  GetDrmDevice()->plane_manager()->BeginFrame(&owned_hardware_planes_);
 
   bool status = true;
   for (const auto& controller : crtc_controllers_) {
-    status &= controller->AssignOverlayPlanes(&hardware_planes, pending_planes);
+    status &= controller->AssignOverlayPlanes(&owned_hardware_planes_,
+                                              pending_planes);
   }
 
   status &= GetDrmDevice()->plane_manager()->Commit(
-      hardware_planes, page_flip_request, out_fence);
+      &owned_hardware_planes_, page_flip_request, out_fence);
 
   return status;
 }
@@ -236,7 +233,18 @@ void HardwareDisplayController::SetCursor(SkBitmap bitmap) {
 
 void HardwareDisplayController::AddCrtc(
     std::unique_ptr<CrtcController> controller) {
-  DCHECK(crtc_controllers_.empty() || controller->drm() == GetDrmDevice());
+  scoped_refptr<DrmDevice> drm = controller->drm();
+  DCHECK(crtc_controllers_.empty() || drm == GetDrmDevice());
+
+  // Check if this controller owns any planes and ensure we keep track of them.
+  const std::vector<std::unique_ptr<HardwareDisplayPlane>>& all_planes =
+      drm->plane_manager()->planes();
+  uint32_t crtc = controller->crtc();
+  for (const auto& plane : all_planes) {
+    if (plane->in_use() && (plane->owning_crtc() == crtc))
+      owned_hardware_planes_.old_plane_list.push_back(plane.get());
+  }
+
   crtc_controllers_.push_back(std::move(controller));
 }
 
@@ -254,9 +262,26 @@ std::unique_ptr<CrtcController> HardwareDisplayController::RemoveCrtc(
   std::unique_ptr<CrtcController> controller(std::move(*controller_it));
   crtc_controllers_.erase(controller_it);
 
-  std::vector<HardwareDisplayPlane*> planes =
-      drm->plane_manager()->GetOwnedPlanesForCrtcs({crtc});
-  drm->plane_manager()->DisableOverlayPlanes(planes);
+  // Move all the planes that have been committed in the last pageflip for this
+  // CRTC at the end of the collection.
+  auto first_plane_to_disable_it =
+      std::partition(owned_hardware_planes_.old_plane_list.begin(),
+                     owned_hardware_planes_.old_plane_list.end(),
+                     [crtc](const HardwareDisplayPlane* plane) {
+                       return plane->owning_crtc() != crtc;
+                     });
+
+  // Disable the planes enabled with the last commit on |crtc|, otherwise
+  // the planes will be visible if the crtc is reassigned to another connector.
+  HardwareDisplayPlaneList hardware_plane_list;
+  std::copy(first_plane_to_disable_it,
+            owned_hardware_planes_.old_plane_list.end(),
+            std::back_inserter(hardware_plane_list.old_plane_list));
+  drm->plane_manager()->DisableOverlayPlanes(&hardware_plane_list);
+
+  // Remove the planes assigned to |crtc|.
+  owned_hardware_planes_.old_plane_list.erase(
+      first_plane_to_disable_it, owned_hardware_planes_.old_plane_list.end());
 
   return controller;
 }
@@ -373,12 +398,4 @@ void HardwareDisplayController::ResetCursor() {
   UpdateCursorImage();
 }
 
-std::vector<HardwareDisplayPlane*>
-HardwareDisplayController::GetAllOwnedPlanes() {
-  std::vector<uint32_t> crtcs;
-  for (const auto& controller : crtc_controllers_)
-    crtcs.push_back(controller->crtc());
-  return GetDrmDevice()->plane_manager()->GetOwnedPlanesForCrtcs(crtcs);
-}
-
 }  // namespace ui

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

@@ -179,7 +179,8 @@ class HardwareDisplayController {
   void UpdateCursorLocation();
   void ResetCursor();
   void DisableCursor();
-  std::vector<HardwareDisplayPlane*> GetAllOwnedPlanes();
+
+  HardwareDisplayPlaneList owned_hardware_planes_;
 
   // Stores the CRTC configuration. This is used to identify monitors and
   // configure them.

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

@@ -136,16 +136,20 @@ void HardwareDisplayPlaneManager::PopulateSupportedFormats() {
 }
 
 void HardwareDisplayPlaneManager::ResetCurrentPlaneList(
-    const HardwareDisplayPlaneList& plane_list) const {
-  for (auto* hardware_plane : plane_list.plane_list) {
+    HardwareDisplayPlaneList* plane_list) const {
+  for (auto* hardware_plane : plane_list->plane_list) {
     hardware_plane->set_in_use(false);
     hardware_plane->set_owning_crtc(0);
   }
+
+  plane_list->plane_list.clear();
+  plane_list->legacy_page_flips.clear();
+  plane_list->atomic_property_set.reset(drmModeAtomicAlloc());
 }
 
 void HardwareDisplayPlaneManager::BeginFrame(
-    const HardwareDisplayPlaneList& plane_list) {
-  for (auto* plane : plane_list.old_plane_list) {
+    HardwareDisplayPlaneList* plane_list) {
+  for (auto* plane : plane_list->old_plane_list) {
     plane->set_in_use(false);
   }
 }
@@ -166,7 +170,7 @@ bool HardwareDisplayPlaneManager::AssignOverlayPlanes(
         FindNextUnusedPlane(&plane_idx, crtc_index, plane);
     if (!hw_plane) {
       LOG(ERROR) << "Failed to find a free plane for crtc " << crtc_id;
-      ResetCurrentPlaneList(*plane_list);
+      ResetCurrentPlaneList(plane_list);
       return false;
     }
 
@@ -187,7 +191,7 @@ bool HardwareDisplayPlaneManager::AssignOverlayPlanes(
     }
 
     if (!SetPlaneData(plane_list, hw_plane, plane, crtc_id, fixed_point_rect)) {
-      ResetCurrentPlaneList(*plane_list);
+      ResetCurrentPlaneList(plane_list);
       return false;
     }
 
@@ -218,19 +222,6 @@ std::vector<uint64_t> HardwareDisplayPlaneManager::GetFormatModifiers(
   return std::vector<uint64_t>();
 }
 
-std::vector<HardwareDisplayPlane*>
-HardwareDisplayPlaneManager::GetOwnedPlanesForCrtcs(
-    const std::vector<uint32_t>& crtcs) {
-  std::vector<HardwareDisplayPlane*> result;
-  for (const auto& plane : planes()) {
-    if (std::find(crtcs.begin(), crtcs.end(), plane->owning_crtc()) !=
-        crtcs.end()) {
-      result.push_back(plane.get());
-    }
-  }
-  return result;
-}
-
 bool HardwareDisplayPlaneManager::SetColorMatrix(
     uint32_t crtc_id,
     const std::vector<float>& color_matrix) {

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

@@ -65,7 +65,7 @@ class HardwareDisplayPlaneManager {
 
   // Clears old frame state out. Must be called before any AssignOverlayPlanes
   // calls.
-  void BeginFrame(const HardwareDisplayPlaneList& plane_list);
+  void BeginFrame(HardwareDisplayPlaneList* plane_list);
 
   // Sets the color transform matrix (a 3x3 matrix represented in vector form)
   // on the CRTC with ID |crtc_id|.
@@ -95,13 +95,13 @@ class HardwareDisplayPlaneManager {
   // out buffers are replaced, and not when the buffers are scheduled with
   // |page_flip_request|. Note that the returned fence may be a nullptr
   // if the system doesn't support out fences.
-  virtual bool Commit(const HardwareDisplayPlaneList& plane_list,
+  virtual bool Commit(HardwareDisplayPlaneList* plane_list,
                       scoped_refptr<PageFlipRequest> page_flip_request,
                       std::unique_ptr<gfx::GpuFence>* out_fence) = 0;
 
-  // Disable all the overlay planes in |plane_list|.
-  virtual bool DisableOverlayPlanes(
-      const std::vector<HardwareDisplayPlane*>& plane_list) = 0;
+  // Disable all the overlay planes previously submitted and now stored in
+  // plane_list->old_plane_list.
+  virtual bool DisableOverlayPlanes(HardwareDisplayPlaneList* plane_list) = 0;
 
   // Set the drm_color_ctm contained in |ctm_blob_data| to all planes' KMS
   // states
@@ -132,9 +132,6 @@ class HardwareDisplayPlaneManager {
   std::vector<uint64_t> GetFormatModifiers(uint32_t crtc_id,
                                            uint32_t format) const;
 
-  std::vector<HardwareDisplayPlane*> GetOwnedPlanesForCrtcs(
-      const std::vector<uint32_t>& crtcs);
-
  protected:
   struct CrtcProperties {
     // Unique identifier for the CRTC. This must be greater than 0 to be valid.
@@ -194,7 +191,7 @@ class HardwareDisplayPlaneManager {
                             const DrmOverlayPlane& overlay,
                             uint32_t crtc_index) const;
 
-  void ResetCurrentPlaneList(const HardwareDisplayPlaneList& plane_list) const;
+  void ResetCurrentPlaneList(HardwareDisplayPlaneList* plane_list) const;
 
   // Populates scanout formats supported by all planes.
   void PopulateSupportedFormats();

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

@@ -61,31 +61,31 @@ HardwareDisplayPlaneManagerAtomic::~HardwareDisplayPlaneManagerAtomic() {
 }
 
 bool HardwareDisplayPlaneManagerAtomic::Commit(
-    const HardwareDisplayPlaneList& plane_list,
+    HardwareDisplayPlaneList* plane_list,
     scoped_refptr<PageFlipRequest> page_flip_request,
     std::unique_ptr<gfx::GpuFence>* out_fence) {
   bool test_only = !page_flip_request;
-  for (HardwareDisplayPlane* plane : plane_list.old_plane_list) {
-    if (!base::Contains(plane_list.plane_list, plane)) {
+  for (HardwareDisplayPlane* plane : plane_list->old_plane_list) {
+    if (!base::Contains(plane_list->plane_list, plane)) {
       // This plane is being released, so we need to zero it.
       plane->set_in_use(false);
       HardwareDisplayPlaneAtomic* atomic_plane =
           static_cast<HardwareDisplayPlaneAtomic*>(plane);
       atomic_plane->SetPlaneData(
-          plane_list.atomic_property_set.get(), 0, 0, gfx::Rect(), gfx::Rect(),
+          plane_list->atomic_property_set.get(), 0, 0, gfx::Rect(), gfx::Rect(),
           gfx::OVERLAY_TRANSFORM_NONE, base::kInvalidPlatformFile);
     }
   }
 
   std::vector<uint32_t> crtcs;
-  for (HardwareDisplayPlane* plane : plane_list.plane_list) {
+  for (HardwareDisplayPlane* plane : plane_list->plane_list) {
     HardwareDisplayPlaneAtomic* atomic_plane =
         static_cast<HardwareDisplayPlaneAtomic*>(plane);
     if (crtcs.empty() || crtcs.back() != atomic_plane->crtc_id())
       crtcs.push_back(atomic_plane->crtc_id());
   }
 
-  drmModeAtomicReqPtr request = plane_list.atomic_property_set.get();
+  drmModeAtomicReqPtr request = plane_list->atomic_property_set.get();
   for (uint32_t crtc : crtcs) {
     int idx = LookupCrtcIndex(crtc);
 
@@ -104,9 +104,11 @@ bool HardwareDisplayPlaneManagerAtomic::Commit(
   }
 
   if (test_only) {
-    for (HardwareDisplayPlane* plane : plane_list.plane_list) {
+    for (HardwareDisplayPlane* plane : plane_list->plane_list) {
       plane->set_in_use(false);
     }
+  } else {
+    plane_list->plane_list.swap(plane_list->old_plane_list);
   }
 
   uint32_t flags = 0;
@@ -127,14 +129,15 @@ bool HardwareDisplayPlaneManagerAtomic::Commit(
   {
     std::vector<base::ScopedFD::Receiver> out_fence_fd_receivers;
     if (out_fence) {
-      if (!AddOutFencePtrProperties(plane_list.atomic_property_set.get(), crtcs,
-                                    &out_fence_fds, &out_fence_fd_receivers)) {
+      if (!AddOutFencePtrProperties(plane_list->atomic_property_set.get(),
+                                    crtcs, &out_fence_fds,
+                                    &out_fence_fd_receivers)) {
         ResetCurrentPlaneList(plane_list);
         return false;
       }
     }
 
-    if (!drm_->CommitProperties(plane_list.atomic_property_set.get(), flags,
+    if (!drm_->CommitProperties(plane_list->atomic_property_set.get(), flags,
                                 crtcs.size(), page_flip_request)) {
       if (!test_only) {
         PLOG(ERROR) << "Failed to commit properties for page flip.";
@@ -150,13 +153,14 @@ bool HardwareDisplayPlaneManagerAtomic::Commit(
   if (out_fence)
     *out_fence = CreateMergedGpuFenceFromFDs(std::move(out_fence_fds));
 
+  plane_list->plane_list.clear();
+  plane_list->atomic_property_set.reset(drmModeAtomicAlloc());
   return true;
 }
 
 bool HardwareDisplayPlaneManagerAtomic::DisableOverlayPlanes(
-    const std::vector<HardwareDisplayPlane*>& plane_list) {
-  ScopedDrmAtomicReqPtr atomic_property_set(drmModeAtomicAlloc());
-  for (HardwareDisplayPlane* plane : plane_list) {
+    HardwareDisplayPlaneList* plane_list) {
+  for (HardwareDisplayPlane* plane : plane_list->old_plane_list) {
     if (plane->type() != HardwareDisplayPlane::kOverlay)
       continue;
     plane->set_in_use(false);
@@ -164,14 +168,15 @@ bool HardwareDisplayPlaneManagerAtomic::DisableOverlayPlanes(
 
     HardwareDisplayPlaneAtomic* atomic_plane =
         static_cast<HardwareDisplayPlaneAtomic*>(plane);
-    atomic_plane->SetPlaneData(atomic_property_set.get(), 0, 0, gfx::Rect(),
-                               gfx::Rect(), gfx::OVERLAY_TRANSFORM_NONE,
-                               base::kInvalidPlatformFile);
+    atomic_plane->SetPlaneData(
+        plane_list->atomic_property_set.get(), 0, 0, gfx::Rect(), gfx::Rect(),
+        gfx::OVERLAY_TRANSFORM_NONE, base::kInvalidPlatformFile);
   }
-  bool ret = drm_->CommitProperties(atomic_property_set.get(),
+  bool ret = drm_->CommitProperties(plane_list->atomic_property_set.get(),
                                     DRM_MODE_ATOMIC_NONBLOCK, 0, nullptr);
   PLOG_IF(ERROR, !ret) << "Failed to commit properties for page flip.";
 
+  plane_list->atomic_property_set.reset(drmModeAtomicAlloc());
   return ret;
 }
 

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

@@ -19,11 +19,10 @@ class HardwareDisplayPlaneManagerAtomic : public HardwareDisplayPlaneManager {
   ~HardwareDisplayPlaneManagerAtomic() override;
 
   // HardwareDisplayPlaneManager:
-  bool Commit(const HardwareDisplayPlaneList& plane_list,
+  bool Commit(HardwareDisplayPlaneList* plane_list,
               scoped_refptr<PageFlipRequest> page_flip_request,
               std::unique_ptr<gfx::GpuFence>* out_fence) override;
-  bool DisableOverlayPlanes(
-      const std::vector<HardwareDisplayPlane*>& plane_list) override;
+  bool DisableOverlayPlanes(HardwareDisplayPlaneList* plane_list) override;
 
   bool SetColorCorrectionOnAllCrtcPlanes(
       uint32_t crtc_id,

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

@@ -46,21 +46,23 @@ HardwareDisplayPlaneManagerLegacy::~HardwareDisplayPlaneManagerLegacy() {
 }
 
 bool HardwareDisplayPlaneManagerLegacy::Commit(
-    const HardwareDisplayPlaneList& plane_list,
+    HardwareDisplayPlaneList* plane_list,
     scoped_refptr<PageFlipRequest> page_flip_request,
     std::unique_ptr<gfx::GpuFence>* out_fence) {
   bool test_only = !page_flip_request;
   if (test_only) {
-    for (HardwareDisplayPlane* plane : plane_list.plane_list) {
+    for (HardwareDisplayPlane* plane : plane_list->plane_list) {
       plane->set_in_use(false);
     }
+    plane_list->plane_list.clear();
+    plane_list->legacy_page_flips.clear();
     return true;
   }
-  if (plane_list.plane_list.empty())  // No assigned planes, nothing to do.
+  if (plane_list->plane_list.empty())  // No assigned planes, nothing to do.
     return true;
 
   bool ret = true;
-  for (const auto& flip : plane_list.legacy_page_flips) {
+  for (const auto& flip : plane_list->legacy_page_flips) {
     if (!drm_->PageFlip(flip.crtc_id, flip.framebuffer, page_flip_request)) {
       // 1) Permission Denied is a legitimate error.
       // 2) EBUSY or ENODEV are possible if we're page flipping a disconnected
@@ -78,19 +80,25 @@ bool HardwareDisplayPlaneManagerLegacy::Commit(
     }
   }
 
-  if (!ret)
+  if (ret) {
+    plane_list->plane_list.swap(plane_list->old_plane_list);
+    plane_list->plane_list.clear();
+    plane_list->legacy_page_flips.clear();
+  } else {
     ResetCurrentPlaneList(plane_list);
+  }
 
   return ret;
 }
 
 bool HardwareDisplayPlaneManagerLegacy::DisableOverlayPlanes(
-    const std::vector<HardwareDisplayPlane*>& plane_list) {
+    HardwareDisplayPlaneList* plane_list) {
   // We're never going to ship legacy pageflip with overlays enabled.
-  DCHECK(std::find_if(plane_list.begin(), plane_list.end(),
+  DCHECK(std::find_if(plane_list->old_plane_list.begin(),
+                      plane_list->old_plane_list.end(),
                       [](HardwareDisplayPlane* plane) {
                         return plane->type() == HardwareDisplayPlane::kOverlay;
-                      }) == plane_list.end());
+                      }) == plane_list->old_plane_list.end());
   return true;
 }
 

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

@@ -19,11 +19,10 @@ class HardwareDisplayPlaneManagerLegacy : public HardwareDisplayPlaneManager {
   ~HardwareDisplayPlaneManagerLegacy() override;
 
   // HardwareDisplayPlaneManager:
-  bool Commit(const HardwareDisplayPlaneList& plane_list,
+  bool Commit(HardwareDisplayPlaneList* plane_list,
               scoped_refptr<PageFlipRequest> page_flip_request,
               std::unique_ptr<gfx::GpuFence>* out_fence) override;
-  bool DisableOverlayPlanes(
-      const std::vector<HardwareDisplayPlane*>& plane_list) override;
+  bool DisableOverlayPlanes(HardwareDisplayPlaneList* plane_list) override;
 
   bool SetColorCorrectionOnAllCrtcPlanes(
       uint32_t crtc_id,