ozone/drm: Restore plane->in_use when necessary

HardwareDisplayPlaneManager::BeginFrame currently iterates all the
planes active on a specific crtc and set in_use to false, so that they
can be used when building up the next frame to commit.

Not all the path of HDPM::AssignOverlayPlanes and HDPM::Commit end up
committing the planes though, this means a pageflip test, or a failing
pageflip, might end up leaving |in_use| set to false, while the crtc is
actually still using those planes.

Bug: 1127487
Test: HardwareDisplayPlaneManagerAtomicTest.{AssignPlanesRestoresInUse,PageflipTestRestoresInUse}

Change-Id: Iee7f6b87af7e38f9ac7c6dea0687cc37dc4c6493
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2446831
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Mark Yacoub <markyacoub@google.com>
Cr-Commit-Position: refs/heads/master@{#813807}

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

@@ -152,6 +152,19 @@ void HardwareDisplayPlaneManager::ResetCurrentPlaneList(
   plane_list->atomic_property_set.reset(drmModeAtomicAlloc());
 }
 
+void HardwareDisplayPlaneManager::RestoreCurrentPlaneList(
+    HardwareDisplayPlaneList* plane_list) const {
+  for (auto* plane : plane_list->plane_list) {
+    plane->set_in_use(false);
+  }
+  for (auto* plane : plane_list->old_plane_list) {
+    plane->set_in_use(true);
+  }
+  plane_list->plane_list.clear();
+  plane_list->legacy_page_flips.clear();
+  plane_list->atomic_property_set.reset(drmModeAtomicAlloc());
+}
+
 void HardwareDisplayPlaneManager::BeginFrame(
     HardwareDisplayPlaneList* plane_list) {
   for (auto* plane : plane_list->old_plane_list) {
@@ -174,8 +187,7 @@ bool HardwareDisplayPlaneManager::AssignOverlayPlanes(
     HardwareDisplayPlane* hw_plane =
         FindNextUnusedPlane(&plane_idx, crtc_index, plane);
     if (!hw_plane) {
-      LOG(ERROR) << "Failed to find a free plane for crtc " << crtc_id;
-      ResetCurrentPlaneList(plane_list);
+      RestoreCurrentPlaneList(plane_list);
       return false;
     }
 
@@ -191,7 +203,7 @@ bool HardwareDisplayPlaneManager::AssignOverlayPlanes(
                   crop_rect.width() << 16, crop_rect.height() << 16);
 
     if (!SetPlaneData(plane_list, hw_plane, plane, crtc_id, fixed_point_rect)) {
-      ResetCurrentPlaneList(plane_list);
+      RestoreCurrentPlaneList(plane_list);
       return false;
     }
 

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

@@ -223,7 +223,13 @@ class HardwareDisplayPlaneManager {
                             const DrmOverlayPlane& overlay,
                             uint32_t crtc_index) const;
 
+  // Resets |plane_list| setting all planes to unused.
+  // Frees any temporary data structure in |plane_list| used for pageflipping.
   void ResetCurrentPlaneList(HardwareDisplayPlaneList* plane_list) const;
+  // Restores |plane_list| planes |in_use| flag to what it was before
+  // BeginFrame was called.
+  // Frees any temporary data structure in |plane_list| used for pageflipping.
+  void RestoreCurrentPlaneList(HardwareDisplayPlaneList* plane_list) const;
 
   // Populates scanout formats supported by all planes.
   void PopulateSupportedFormats();

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

@@ -168,9 +168,12 @@ bool HardwareDisplayPlaneManagerAtomic::Commit(
   }
 
   if (test_only) {
-    for (HardwareDisplayPlane* plane : plane_list->plane_list) {
+    for (auto* plane : plane_list->plane_list) {
       plane->set_in_use(false);
     }
+    for (auto* plane : plane_list->old_plane_list) {
+      plane->set_in_use(true);
+    }
   } else {
     plane_list->plane_list.swap(plane_list->old_plane_list);
   }

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

@@ -619,6 +619,72 @@ TEST_P(HardwareDisplayPlaneManagerAtomicTest, UnusedPlanesAreReleased) {
   EXPECT_EQ(0u, GetPlanePropertyValue(kPlaneOffset + 1, "FB_ID"));
 }
 
+TEST_P(HardwareDisplayPlaneManagerAtomicTest, AssignPlanesRestoresInUse) {
+  InitializeDrmState(/*crtc_count=*/2, /*planes_per_crtc=*/2);
+  fake_drm_->InitializeState(crtc_properties_, connector_properties_,
+                             plane_properties_, property_names_, use_atomic_);
+
+  ui::DrmOverlayPlaneList assigns;
+  scoped_refptr<ui::DrmFramebuffer> primary_buffer =
+      CreateBuffer(kDefaultBufferSize);
+  scoped_refptr<ui::DrmFramebuffer> overlay_buffer =
+      CreateBuffer(gfx::Size(1, 1));
+  assigns.push_back(ui::DrmOverlayPlane(primary_buffer, nullptr));
+  assigns.push_back(ui::DrmOverlayPlane(overlay_buffer, nullptr));
+  ui::HardwareDisplayPlaneList hdpl;
+
+  scoped_refptr<ui::PageFlipRequest> page_flip_request =
+      base::MakeRefCounted<ui::PageFlipRequest>(base::TimeDelta());
+  fake_drm_->plane_manager()->BeginFrame(&hdpl);
+  EXPECT_TRUE(fake_drm_->plane_manager()->AssignOverlayPlanes(
+      &hdpl, assigns, crtc_properties_[0].id));
+  EXPECT_TRUE(fake_drm_->plane_manager()->Commit(
+      &hdpl, /*should_modeset*/ false, page_flip_request, nullptr));
+  EXPECT_TRUE(fake_drm_->plane_manager()->planes().front()->in_use());
+  assigns.push_back(ui::DrmOverlayPlane(overlay_buffer, nullptr));
+
+  fake_drm_->plane_manager()->BeginFrame(&hdpl);
+  // Assign overlay planes will fail since there aren't enough planes.
+  EXPECT_FALSE(fake_drm_->plane_manager()->AssignOverlayPlanes(
+      &hdpl, assigns, crtc_properties_[0].id));
+
+  // The primary plane should still be in use since we failed to assign
+  // planes and did not commit a new configuration.
+  EXPECT_TRUE(fake_drm_->plane_manager()->planes().front()->in_use());
+}
+
+TEST_P(HardwareDisplayPlaneManagerAtomicTest, PageflipTestRestoresInUse) {
+  InitializeDrmState(/*crtc_count=*/2, /*planes_per_crtc=*/2);
+  fake_drm_->InitializeState(crtc_properties_, connector_properties_,
+                             plane_properties_, property_names_, use_atomic_);
+
+  ui::DrmOverlayPlaneList assigns;
+  scoped_refptr<ui::DrmFramebuffer> primary_buffer =
+      CreateBuffer(kDefaultBufferSize);
+  scoped_refptr<ui::DrmFramebuffer> overlay_buffer =
+      CreateBuffer(gfx::Size(1, 1));
+  assigns.push_back(ui::DrmOverlayPlane(primary_buffer, nullptr));
+  assigns.push_back(ui::DrmOverlayPlane(overlay_buffer, nullptr));
+  ui::HardwareDisplayPlaneList hdpl;
+
+  scoped_refptr<ui::PageFlipRequest> page_flip_request =
+      base::MakeRefCounted<ui::PageFlipRequest>(base::TimeDelta());
+  fake_drm_->plane_manager()->BeginFrame(&hdpl);
+  EXPECT_TRUE(fake_drm_->plane_manager()->AssignOverlayPlanes(
+      &hdpl, assigns, crtc_properties_[0].id));
+  EXPECT_TRUE(fake_drm_->plane_manager()->Commit(
+      &hdpl, /*should_modeset*/ false, page_flip_request, nullptr));
+  assigns.clear();
+  fake_drm_->plane_manager()->BeginFrame(&hdpl);
+  EXPECT_TRUE(fake_drm_->plane_manager()->AssignOverlayPlanes(
+      &hdpl, assigns, crtc_properties_[0].id));
+  EXPECT_TRUE(fake_drm_->plane_manager()->Commit(
+      &hdpl, /*should_modeset*/ false, nullptr, nullptr));
+  // The primary plane should still be in use since the commit was
+  // a pageflip test and did not change any KMS state.
+  EXPECT_TRUE(fake_drm_->plane_manager()->planes().front()->in_use());
+}
+
 TEST_P(HardwareDisplayPlaneManagerAtomicTest, MultipleFrames) {
   ui::DrmOverlayPlaneList assigns;
   assigns.push_back(ui::DrmOverlayPlane(fake_buffer_, nullptr));