Disable all removed displays together.

When multiples displays are removed at the same time, commit a disable
modeset of all them together instead of each separately.
This should prevent unplugged MST connectors from being in zombie mode.

BUG=b/172069469, b/169386252
TEST=unplug and replug 2 connectors on MST hub, ScreenManagerTest.CheckMultipleControllersAfterBeingRemoved

Change-Id: If24f9016462b9c69633148202641f7ba6bae9114
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2512550
Commit-Queue: Mark Yacoub <markyacoub@google.com>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823623}

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

@@ -36,13 +36,6 @@ CrtcController::~CrtcController() {
         plane->set_in_use(false);
       }
     }
-
-    // Disable CRTC controller.
-    CommitRequest commit_request;
-    commit_request.push_back(
-        CrtcCommitRequest::DisableCrtcRequest(crtc_, connector_));
-    drm_->plane_manager()->Commit(std::move(commit_request),
-                                  DRM_MODE_ATOMIC_ALLOW_MODESET);
   }
 }
 

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

@@ -336,15 +336,18 @@ DrmDisplay* DrmGpuDisplayManager::FindDisplay(int64_t display_id) {
 void DrmGpuDisplayManager::NotifyScreenManager(
     const std::vector<std::unique_ptr<DrmDisplay>>& new_displays,
     const std::vector<std::unique_ptr<DrmDisplay>>& old_displays) const {
+  ScreenManager::CrtcsWithDrmList controllers_to_remove;
   for (const auto& old_display : old_displays) {
     auto it = std::find_if(new_displays.begin(), new_displays.end(),
                            DisplayComparator(old_display.get()));
 
     if (it == new_displays.end()) {
-      screen_manager_->RemoveDisplayController(old_display->drm(),
-                                               old_display->crtc());
+      controllers_to_remove.emplace_back(old_display->crtc(),
+                                         old_display->drm());
     }
   }
+  if (!controllers_to_remove.empty())
+    screen_manager_->RemoveDisplayControllers(controllers_to_remove);
 
   for (const auto& new_display : new_displays) {
     auto it = std::find_if(old_displays.begin(), old_displays.end(),

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

@@ -173,17 +173,54 @@ void ScreenManager::AddDisplayController(const scoped_refptr<DrmDevice>& drm,
       std::make_unique<CrtcController>(drm, crtc, connector), gfx::Point()));
 }
 
-void ScreenManager::RemoveDisplayController(const scoped_refptr<DrmDevice>& drm,
-                                            uint32_t crtc) {
-  HardwareDisplayControllers::iterator it = FindDisplayController(drm, crtc);
-  if (it != controllers_.end()) {
-    bool is_mirrored = (*it)->IsMirrored();
-    (*it)->RemoveCrtc(drm, crtc);
-    if (!is_mirrored) {
-      controllers_.erase(it);
-      UpdateControllerToWindowMapping();
+void ScreenManager::RemoveDisplayControllers(
+    const CrtcsWithDrmList& controllers_to_remove) {
+  // Split them to different lists unique to each DRM Device.
+  base::flat_map<scoped_refptr<DrmDevice>, CrtcsWithDrmList>
+      controllers_for_drm_devices;
+  for (const auto& controller : controllers_to_remove) {
+    auto drm = controller.second;
+    auto it = controllers_for_drm_devices.find(drm);
+    if (it == controllers_for_drm_devices.end()) {
+      controllers_for_drm_devices.insert(
+          std::make_pair(drm, CrtcsWithDrmList()));
+    }
+    controllers_for_drm_devices[drm].emplace_back(controller);
+  }
+
+  bool should_update_controllers_to_window_mapping = false;
+  for (const auto& controllers_on_drm : controllers_for_drm_devices) {
+    CrtcsWithDrmList controllers_to_remove = controllers_on_drm.second;
+
+    CommitRequest commit_request;
+    auto drm = controllers_on_drm.first;
+    for (const auto& controller : controllers_to_remove) {
+      uint32_t crtc_id = controller.first;
+      auto it = FindDisplayController(drm, crtc_id);
+      if (it == controllers_.end())
+        continue;
+
+      bool is_mirrored = (*it)->IsMirrored();
+
+      std::unique_ptr<CrtcController> crtc = (*it)->RemoveCrtc(drm, crtc_id);
+      if (crtc->is_enabled()) {
+        commit_request.push_back(CrtcCommitRequest::DisableCrtcRequest(
+            crtc->crtc(), crtc->connector()));
+      }
+
+      if (!is_mirrored) {
+        controllers_.erase(it);
+        should_update_controllers_to_window_mapping = true;
+      }
+    }
+    if (!commit_request.empty()) {
+      drm->plane_manager()->Commit(std::move(commit_request),
+                                   DRM_MODE_ATOMIC_ALLOW_MODESET);
     }
   }
+
+  if (should_update_controllers_to_window_mapping)
+    UpdateControllerToWindowMapping();
 }
 
 base::flat_map<int64_t, bool> ScreenManager::ConfigureDisplayControllers(

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

@@ -31,6 +31,9 @@ class DrmWindow;
 // Responsible for keeping track of active displays and configuring them.
 class ScreenManager {
  public:
+  using CrtcsWithDrmList =
+      std::vector<std::pair<uint32_t, const scoped_refptr<DrmDevice>>>;
+
   struct ControllerConfigParams {
     ControllerConfigParams(int64_t display_id,
                            scoped_refptr<DrmDevice> drm,
@@ -60,10 +63,9 @@ class ScreenManager {
                             uint32_t crtc,
                             uint32_t connector);
 
-  // Remove a display controller from the list of active controllers. The
-  // controller is removed since it was disconnected.
-  void RemoveDisplayController(const scoped_refptr<DrmDevice>& drm,
-                               uint32_t crtc);
+  // Remove display controllers from the list of active controllers. The
+  // controllers are removed since they were disconnected.
+  void RemoveDisplayControllers(const CrtcsWithDrmList& controllers_to_remove);
 
   // Enables/Disables the display controller based on if a mode exists.
   base::flat_map<int64_t, bool> ConfigureDisplayControllers(

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

@@ -299,10 +299,40 @@ TEST_F(ScreenManagerTest, CheckControllerAfterItIsRemoved) {
 
   EXPECT_TRUE(screen_manager_->GetDisplayController(GetPrimaryBounds()));
 
-  screen_manager_->RemoveDisplayController(drm_, kPrimaryCrtc);
+  ScreenManager::CrtcsWithDrmList controllers_to_remove;
+
+  controllers_to_remove.emplace_back(kPrimaryCrtc, drm_);
+  screen_manager_->RemoveDisplayControllers(controllers_to_remove);
   EXPECT_FALSE(screen_manager_->GetDisplayController(GetPrimaryBounds()));
 }
 
+TEST_F(ScreenManagerTest, CheckMultipleControllersAfterBeingRemoved) {
+  InitializeDrmStateWithDefault(drm_.get());
+
+  screen_manager_->AddDisplayController(drm_, kPrimaryCrtc, kPrimaryConnector);
+  screen_manager_->AddDisplayController(drm_, kSecondaryCrtc,
+                                        kSecondaryConnector);
+
+  ScreenManager::ControllerConfigsList controllers_to_enable;
+  controllers_to_enable.emplace_back(
+      kPrimaryDisplayId, drm_, kPrimaryCrtc, kPrimaryConnector,
+      GetPrimaryBounds().origin(),
+      std::make_unique<drmModeModeInfo>(kDefaultMode));
+  controllers_to_enable.emplace_back(
+      kSecondaryDisplayId, drm_, kSecondaryCrtc, kSecondaryConnector,
+      GetSecondaryBounds().origin(),
+      std::make_unique<drmModeModeInfo>(kDefaultMode));
+  screen_manager_->ConfigureDisplayControllers(controllers_to_enable);
+
+  ScreenManager::CrtcsWithDrmList controllers_to_remove;
+  controllers_to_remove.emplace_back(kPrimaryCrtc, drm_);
+  controllers_to_remove.emplace_back(kSecondaryCrtc, drm_);
+  screen_manager_->RemoveDisplayControllers(controllers_to_remove);
+
+  EXPECT_FALSE(screen_manager_->GetDisplayController(GetPrimaryBounds()));
+  EXPECT_FALSE(screen_manager_->GetDisplayController(GetSecondaryBounds()));
+}
+
 TEST_F(ScreenManagerTest, CheckDuplicateConfiguration) {
   InitializeDrmStateWithDefault(drm_.get(), /*is_atomic*/ false);
 
@@ -517,7 +547,9 @@ TEST_F(ScreenManagerTest, MonitorGoneInMirrorMode) {
       std::make_unique<drmModeModeInfo>(secondary_mode));
   screen_manager_->ConfigureDisplayControllers(controllers_to_enable);
 
-  screen_manager_->RemoveDisplayController(drm_, kSecondaryCrtc);
+  ScreenManager::CrtcsWithDrmList controllers_to_remove;
+  controllers_to_remove.emplace_back(kSecondaryCrtc, drm_);
+  screen_manager_->RemoveDisplayControllers(controllers_to_remove);
 
   ui::HardwareDisplayController* controller =
       screen_manager_->GetDisplayController(GetPrimaryBounds());
@@ -855,7 +887,9 @@ TEST_F(ScreenManagerTest, ShouldDissociateWindowOnControllerRemoval) {
 
   EXPECT_TRUE(screen_manager_->GetWindow(window_id)->GetController());
 
-  screen_manager_->RemoveDisplayController(drm_, kPrimaryCrtc);
+  ScreenManager::CrtcsWithDrmList controllers_to_remove;
+  controllers_to_remove.emplace_back(kPrimaryCrtc, drm_);
+  screen_manager_->RemoveDisplayControllers(controllers_to_remove);
 
   EXPECT_FALSE(screen_manager_->GetWindow(window_id)->GetController());
 
@@ -1063,7 +1097,9 @@ TEST_F(ScreenManagerTest, ShouldNotHardwareMirrorDifferentDrmDevices) {
 
   // Disconnect first display. Second display moves to origin.
   {
-    screen_manager.RemoveDisplayController(drm_device1, kCrtc1);
+    ScreenManager::CrtcsWithDrmList controllers_to_remove;
+    controllers_to_remove.emplace_back(kCrtc1, drm_device1);
+    screen_manager.RemoveDisplayControllers(controllers_to_remove);
 
     ScreenManager::ControllerConfigsList controllers_to_enable;
     std::unique_ptr<drmModeModeInfo> secondary_mode =