Make DisableOverlayPlanes atomic Commit Blocking

Problem with old non blocking commit:
In some cases with high resolution and high frame rate (such as playing graphics or media content on a 4K 60Hz monitor), switching modes (extended->mirrored) will cause DRM to get 2 nearly back to back nonblocking commits from Chrome: One for disabling planes on old controller, and another actual content flip. Due to the close time proximity of the 2 nonblocking commits, while prepping the 2nd one, DRM will throw -EBUSY error because at this time, the flip_done for the 1st commit was not yet complete as userspace is not allowed to get ahead of the previous commit with nonblocking ones (a kernel check).
Behavior after the EBUSY depends on the application. On Chrome, the GPU crashes, and resets, trying to recover to a good state, but content would be reset as well.

Fix with blocking commit:
Making DisablingOverlayPlanes a blocking commit will guarantee that the flip_done for this commit is complete before OS sends any subsequent non-blocking page flips. On blocking commits, DRM does not hand control back to OS until well after the flip_done is complete.

Rationale of the new design:
DisablingOverlayPlanes happens on the same thread and during the same time as the full modeset, which is a blocking commit. So makes sense that all modeset-related commits to be blocking until everything has been set/reset properly.


BUG: b/171409360,b/172306392
TEST: with 4K display attached to a kohaku, switch from extended to mirror mode.
Change-Id: Icff491be107267eb49cf3a2846b6d219a0f82c4d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2559170
Commit-Queue: Mark Yacoub <markyacoub@google.com>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831976}

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

@@ -298,20 +298,24 @@ bool HardwareDisplayPlaneManagerAtomic::Commit(
 
 bool HardwareDisplayPlaneManagerAtomic::DisableOverlayPlanes(
     HardwareDisplayPlaneList* plane_list) {
-  for (HardwareDisplayPlane* plane : plane_list->old_plane_list) {
-    plane->set_in_use(false);
-    plane->set_owning_crtc(0);
+  bool ret = true;
 
-    HardwareDisplayPlaneAtomic* atomic_plane =
-        static_cast<HardwareDisplayPlaneAtomic*>(plane);
-    atomic_plane->AssignPlaneProps(0, 0, gfx::Rect(), gfx::Rect(),
-                                   gfx::OVERLAY_TRANSFORM_NONE,
-                                   base::kInvalidPlatformFile);
-    atomic_plane->SetPlaneProps(plane_list->atomic_property_set.get());
+  if (!plane_list->old_plane_list.empty()) {
+    for (HardwareDisplayPlane* plane : plane_list->old_plane_list) {
+      plane->set_in_use(false);
+      plane->set_owning_crtc(0);
+
+      HardwareDisplayPlaneAtomic* atomic_plane =
+          static_cast<HardwareDisplayPlaneAtomic*>(plane);
+      atomic_plane->AssignPlaneProps(0, 0, gfx::Rect(), gfx::Rect(),
+                                     gfx::OVERLAY_TRANSFORM_NONE,
+                                     base::kInvalidPlatformFile);
+      atomic_plane->SetPlaneProps(plane_list->atomic_property_set.get());
+    }
+    ret = drm_->CommitProperties(plane_list->atomic_property_set.get(),
+                                 /*flags=*/0, 0 /*unused*/, nullptr);
+    PLOG_IF(ERROR, !ret) << "Failed to commit properties for page flip.";
   }
-  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_unittest.cc

@@ -516,7 +516,7 @@ TEST_P(HardwareDisplayPlaneManagerAtomicTest, DisableModeset) {
   EXPECT_TRUE(fake_drm_->plane_manager()->Commit(
       std::move(commit_request), DRM_MODE_ATOMIC_ALLOW_MODESET));
 
-  EXPECT_EQ(2, fake_drm_->get_commit_count());
+  EXPECT_EQ(1, fake_drm_->get_commit_count());
 }
 
 TEST_P(HardwareDisplayPlaneManagerAtomicTest, CheckPropsAfterModeset) {

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

@@ -442,6 +442,9 @@ void ScreenManager::HandleMirrorIfExists(
   // CrtcController should be merged and picking the mode should be done
   // properly within HardwareDisplayController.
   if (mirror != controllers_.end() && controller != mirror) {
+    // TODO(markyacoub): RemoveCrtc makes a blocking commit to
+    // DisableOverlayPlanes. This should be redesigned and included as part of
+    // the Modeset commit.
     (*mirror)->AddCrtc((*controller)->RemoveCrtc(config.drm, config.crtc));
     controllers_.erase(controller);
   }