Revert "Ozone: Implement Atomic Modeset"

This reverts commit b0806021.

Reason for revert: Causes "this monitor is not getting along" notification using CableMatters DP2 dock with 1 display

Original change's description:
> Ozone: Implement Atomic Modeset
>
> If the device supports KMS Atomic APIs, Enable CRTC using Atomic Modeset
> Commit instead of legacy SetCRTC.
>
> BUG=987274
> TEST=HardwareDisplayControllerTest.CheckModesettingSetsProps
> HardwareDisplayPlaneManagerAtomicTest.CheckPropsAfterModeset
>
> Change-Id: I116a2caf8602aa6470c9cf17a65fa12c045f8d6b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2039752
> Commit-Queue: Mark Yacoub <markyacoub@google.com>
> Auto-Submit: Mark Yacoub <markyacoub@google.com>
> Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#740811}

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

Bug: 1056900
Change-Id: Ia9641a5089afac272e7bb1299072b5dbac4ec304
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2080775
Auto-Submit: Mark Yacoub <markyacoub@google.com>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Commit-Queue: Mark Yacoub <markyacoub@google.com>
Cr-Commit-Position: refs/heads/master@{#745630}

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

@@ -80,11 +80,8 @@ bool CrtcController::Disable() {
 }
 
 bool CrtcController::AssignOverlayPlanes(HardwareDisplayPlaneList* plane_list,
-                                         const DrmOverlayPlaneList& overlays,
-                                         bool is_modesetting) {
-  // If we're in the process of modesetting, the CRTC is still disabled.
-  // Once the modeset is done, we expect it to be enabled.
-  DCHECK(is_modesetting || !is_disabled_);
+                                         const DrmOverlayPlaneList& overlays) {
+  DCHECK(!is_disabled_);
 
   const DrmOverlayPlane* primary = DrmOverlayPlane::GetPrimaryPlane(overlays);
   if (primary && !drm_->plane_manager()->ValidatePrimarySize(*primary, mode_)) {

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

@@ -51,8 +51,7 @@ class CrtcController {
   bool Disable();
 
   bool AssignOverlayPlanes(HardwareDisplayPlaneList* plane_list,
-                           const DrmOverlayPlaneList& planes,
-                           bool is_modesetting);
+                           const DrmOverlayPlaneList& planes);
 
   // Returns a vector of format modifiers for the given fourcc format
   // on this CRTCs primary plane. A format modifier describes the

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

@@ -84,20 +84,10 @@ bool HardwareDisplayController::ModesetCrtc(const DrmOverlayPlane& primary,
                                             const drmModeModeInfo& mode) {
   DCHECK(primary.buffer.get());
   bool status = true;
-
-  GetDrmDevice()->plane_manager()->BeginFrame(&owned_hardware_planes_);
-  DrmOverlayPlaneList plane_list;
-  plane_list.push_back(primary.Clone());
-
-  for (const auto& controller : crtc_controllers_) {
-    status &=
-        controller->AssignOverlayPlanes(&owned_hardware_planes_, plane_list,
-                                        /*is_modesetting=*/true);
-
+  for (const auto& controller : crtc_controllers_)
     status &= controller->Modeset(
         primary, use_current_crtc_mode ? controller->mode() : mode,
         owned_hardware_planes_);
-  }
 
   is_disabled_ = false;
   ResetCursor();
@@ -180,13 +170,12 @@ bool HardwareDisplayController::ScheduleOrTestPageFlip(
 
   bool status = true;
   for (const auto& controller : crtc_controllers_) {
-    status &= controller->AssignOverlayPlanes(
-        &owned_hardware_planes_, pending_planes, /*is_modesetting=*/false);
+    status &= controller->AssignOverlayPlanes(&owned_hardware_planes_,
+                                              pending_planes);
   }
 
   status &= GetDrmDevice()->plane_manager()->Commit(
-      &owned_hardware_planes_, /*should_modeset=*/false, page_flip_request,
-      out_fence);
+      &owned_hardware_planes_, page_flip_request, out_fence);
 
   return status;
 }
@@ -362,7 +351,6 @@ void HardwareDisplayController::OnModesetComplete(
   // pending planes to the same values so that the callback keeps the correct
   // state.
   page_flip_request_ = nullptr;
-  owned_hardware_planes_.legacy_page_flips.clear();
   current_planes_.clear();
   current_planes_.push_back(primary.Clone());
   time_of_last_flip_ = base::TimeTicks::Now();

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

@@ -30,18 +30,11 @@ namespace {
 const drmModeModeInfo kDefaultMode = {0, 6, 0, 0, 0, 0, 4,     0,
                                       0, 0, 0, 0, 0, 0, {'\0'}};
 
-constexpr uint32_t kCrtcIdBase = 100;
+constexpr uint32_t kCrtcIdBase = 1;
 constexpr uint32_t kPrimaryCrtc = kCrtcIdBase;
 constexpr uint32_t kSecondaryCrtc = kCrtcIdBase + 1;
-constexpr uint32_t kConnectorIdBase = 200;
-constexpr uint32_t kPlaneOffset = 300;
-constexpr uint32_t kInFormatsBlobPropId = 400;
-
-constexpr uint32_t kActivePropId = 1000;
-constexpr uint32_t kModePropId = 1001;
-constexpr uint32_t kCrtcIdPropId = 2000;
-constexpr uint32_t kTypePropId = 3010;
-constexpr uint32_t kInFormatsPropId = 3011;
+constexpr uint32_t kConnectorIdBase = 10;
+constexpr uint32_t kPlaneOffset = 1000;
 
 const gfx::Size kDefaultModeSize(kDefaultMode.hdisplay, kDefaultMode.vdisplay);
 const gfx::Size kOverlaySize(kDefaultMode.hdisplay / 2,
@@ -111,6 +104,10 @@ void HardwareDisplayControllerTest::TearDown() {
 }
 
 void HardwareDisplayControllerTest::InitializeDrmDevice(bool use_atomic) {
+  constexpr uint32_t kTypePropId = 3010;
+  constexpr uint32_t kInFormatsPropId = 3011;
+  constexpr uint32_t kInFormatsBlobPropId = 400;
+
   std::vector<ui::MockDrmDevice::CrtcProperties> crtc_properties(2);
   std::map<uint32_t, std::string> crtc_property_names = {
       {1000, "ACTIVE"},
@@ -119,7 +116,7 @@ void HardwareDisplayControllerTest::InitializeDrmDevice(bool use_atomic) {
 
   std::vector<ui::MockDrmDevice::ConnectorProperties> connector_properties(2);
   std::map<uint32_t, std::string> connector_property_names = {
-      {kCrtcIdPropId, "CRTC_ID"},
+      {2000, "CRTC_ID"},
   };
   for (size_t i = 0; i < connector_properties.size(); ++i) {
     connector_properties[i].id = kConnectorIdBase + i;
@@ -283,39 +280,6 @@ TEST_F(HardwareDisplayControllerTest, ModifiersWithConnectorType) {
             internal_modifiers.end());
 }
 
-TEST_F(HardwareDisplayControllerTest, CheckModesettingSetsProps) {
-  ui::DrmOverlayPlane plane1(CreateBuffer(), nullptr);
-
-  EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
-
-  ui::DrmOverlayPlane plane2(CreateBuffer(), nullptr);
-  std::vector<ui::DrmOverlayPlane> planes = {};
-  planes.push_back(plane2.Clone());
-
-  SchedulePageFlip(std::move(planes));
-
-  // Test props values after modesetting.
-  ui::DrmDevice::Property connector_prop_crtc_id = {};
-  ui::ScopedDrmObjectPropertyPtr connector_props =
-      drm_->GetObjectProperties(kConnectorIdBase, DRM_MODE_OBJECT_CONNECTOR);
-  ui::GetDrmPropertyForName(drm_.get(), connector_props.get(), "CRTC_ID",
-                            &connector_prop_crtc_id);
-  EXPECT_EQ(kCrtcIdPropId, connector_prop_crtc_id.id);
-  EXPECT_EQ(kCrtcIdBase, connector_prop_crtc_id.value);
-
-  ui::DrmDevice::Property crtc_prop_for_name = {};
-  ui::ScopedDrmObjectPropertyPtr crtc_props =
-      drm_->GetObjectProperties(kPrimaryCrtc, DRM_MODE_OBJECT_CRTC);
-  GetDrmPropertyForName(drm_.get(), crtc_props.get(), "ACTIVE",
-                        &crtc_prop_for_name);
-  EXPECT_EQ(kActivePropId, crtc_prop_for_name.id);
-  EXPECT_EQ(1U, crtc_prop_for_name.value);
-
-  GetDrmPropertyForName(drm_.get(), crtc_props.get(), "MODE_ID",
-                        &crtc_prop_for_name);
-  EXPECT_EQ(kModePropId, crtc_prop_for_name.id);
-}
-
 TEST_F(HardwareDisplayControllerTest, CheckDisableResetsProps) {
   ui::DrmOverlayPlane plane1(CreateBuffer(), nullptr);
 
@@ -354,7 +318,6 @@ TEST_F(HardwareDisplayControllerTest, CheckStateAfterPageFlip) {
   ui::DrmOverlayPlane plane1(CreateBuffer(), nullptr);
 
   EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
-  EXPECT_EQ(1, drm_->get_commit_count());
 
   ui::DrmOverlayPlane plane2(CreateBuffer(), nullptr);
   std::vector<ui::DrmOverlayPlane> planes;
@@ -368,14 +331,13 @@ TEST_F(HardwareDisplayControllerTest, CheckStateAfterPageFlip) {
 
   EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);
   EXPECT_EQ(1, page_flips_);
-  EXPECT_EQ(2, drm_->get_commit_count());
+  EXPECT_EQ(1, drm_->get_commit_count());
   // Verify only the primary display have a valid framebuffer.
   EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset, "FB_ID"));
   EXPECT_EQ(0u, GetPlanePropertyValue(kPlaneOffset + 1, "FB_ID"));
 }
 
 TEST_F(HardwareDisplayControllerTest, CheckStateIfModesetFails) {
-  InitializeDrmDevice(/* use_atomic */ false);
   drm_->set_set_crtc_expectation(false);
 
   ui::DrmOverlayPlane plane(CreateBuffer(), nullptr);
@@ -383,6 +345,20 @@ TEST_F(HardwareDisplayControllerTest, CheckStateIfModesetFails) {
   EXPECT_FALSE(controller_->Modeset(plane, kDefaultMode));
 }
 
+TEST_F(HardwareDisplayControllerTest, CheckStateIfPageFlipFails) {
+  drm_->set_commit_expectation(false);
+
+  ui::DrmOverlayPlane plane1(CreateBuffer(), nullptr);
+
+  EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
+
+  ui::DrmOverlayPlane plane2(CreateBuffer(), nullptr);
+  std::vector<ui::DrmOverlayPlane> planes;
+  planes.push_back(plane2.Clone());
+  EXPECT_DEATH_IF_SUPPORTED(SchedulePageFlip(std::move(planes)),
+                            "SchedulePageFlip failed");
+}
+
 TEST_F(HardwareDisplayControllerTest, CheckOverlayPresent) {
   ui::DrmOverlayPlane plane1(CreateBuffer(), nullptr);
   ui::DrmOverlayPlane plane2(
@@ -390,7 +366,6 @@ TEST_F(HardwareDisplayControllerTest, CheckOverlayPresent) {
       gfx::Rect(kOverlaySize), gfx::RectF(kDefaultModeSizeF), true, nullptr);
 
   EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
-  EXPECT_EQ(1, drm_->get_commit_count());
 
   std::vector<ui::DrmOverlayPlane> planes;
   planes.push_back(plane1.Clone());
@@ -400,7 +375,7 @@ TEST_F(HardwareDisplayControllerTest, CheckOverlayPresent) {
   drm_->RunCallbacks();
   EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);
   EXPECT_EQ(1, page_flips_);
-  EXPECT_EQ(2, drm_->get_commit_count());
+  EXPECT_EQ(1, drm_->get_commit_count());
   // Verify both planes on the primary display have a valid framebuffer.
   EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset, "FB_ID"));
   EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset + 1, "FB_ID"));
@@ -413,14 +388,13 @@ TEST_F(HardwareDisplayControllerTest, CheckOverlayTestMode) {
       gfx::Rect(kOverlaySize), gfx::RectF(kDefaultModeSizeF), true, nullptr);
 
   EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
-  EXPECT_EQ(1, drm_->get_commit_count());
 
   std::vector<ui::DrmOverlayPlane> planes;
   planes.push_back(plane1.Clone());
   planes.push_back(plane2.Clone());
 
   SchedulePageFlip(ui::DrmOverlayPlane::Clone(planes));
-  EXPECT_EQ(2, drm_->get_commit_count());
+  EXPECT_EQ(1, drm_->get_commit_count());
   // Verify both planes on the primary display have a valid framebuffer.
   EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset, "FB_ID"));
   EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset + 1, "FB_ID"));
@@ -430,14 +404,14 @@ TEST_F(HardwareDisplayControllerTest, CheckOverlayTestMode) {
   drm_->RunCallbacks();
   EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);
   EXPECT_EQ(1, page_flips_);
-  EXPECT_EQ(3, drm_->get_commit_count());
+  EXPECT_EQ(2, drm_->get_commit_count());
 
   // Regular flips should continue on normally.
   SchedulePageFlip(ui::DrmOverlayPlane::Clone(planes));
   drm_->RunCallbacks();
   EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);
   EXPECT_EQ(2, page_flips_);
-  EXPECT_EQ(4, drm_->get_commit_count());
+  EXPECT_EQ(3, drm_->get_commit_count());
   // Verify both planes on the primary display have a valid framebuffer.
   EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset, "FB_ID"));
   EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset + 1, "FB_ID"));
@@ -468,7 +442,7 @@ TEST_F(HardwareDisplayControllerTest, PageflipMirroredControllers) {
 
   ui::DrmOverlayPlane plane1(CreateBuffer(), nullptr);
   EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
-  EXPECT_EQ(2, drm_->get_commit_count());
+  EXPECT_EQ(2, drm_->get_set_crtc_call_count());
 
   ui::DrmOverlayPlane plane2(CreateBuffer(), nullptr);
   std::vector<ui::DrmOverlayPlane> planes;
@@ -477,7 +451,7 @@ TEST_F(HardwareDisplayControllerTest, PageflipMirroredControllers) {
   drm_->RunCallbacks();
   EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);
   EXPECT_EQ(1, page_flips_);
-  EXPECT_EQ(3, drm_->get_commit_count());
+  EXPECT_EQ(1, drm_->get_commit_count());
   // Verify only the displays have a valid framebuffer on the primary plane.
   // First display:
   EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset, "FB_ID"));
@@ -624,10 +598,10 @@ TEST_F(HardwareDisplayControllerTest, ModesetWhilePageFlipping) {
 }
 
 TEST_F(HardwareDisplayControllerTest, FailPageFlipping) {
+  drm_->set_commit_expectation(false);
+
   ui::DrmOverlayPlane plane1(CreateBuffer(), nullptr);
   EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
-
-  drm_->set_commit_expectation(false);
   std::vector<ui::DrmOverlayPlane> planes;
   planes.push_back(plane1.Clone());
   EXPECT_DEATH_IF_SUPPORTED(SchedulePageFlip(std::move(planes)),

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

@@ -97,7 +97,7 @@ class HardwareDisplayPlaneManager {
                                    uint32_t crtc_id);
 
   // Commit the plane states in |plane_list|.
-  // if |should_modeset| is set, it only modesets without page flipping.
+  //
   // If |page_flip_request| is null, this tests the plane configuration without
   // submitting it.
   // The fence returned in |out_fence| will signal when the currently scanned
@@ -105,7 +105,6 @@ class HardwareDisplayPlaneManager {
   // |page_flip_request|. Note that the returned fence may be a nullptr
   // if the system doesn't support out fences.
   virtual bool Commit(HardwareDisplayPlaneList* plane_list,
-                      bool should_modeset,
                       scoped_refptr<PageFlipRequest> page_flip_request,
                       std::unique_ptr<gfx::GpuFence>* out_fence) = 0;
 

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

@@ -65,33 +65,9 @@ bool HardwareDisplayPlaneManagerAtomic::Modeset(
     uint32_t framebuffer_id,
     uint32_t connector_id,
     const drmModeModeInfo& mode,
-    const HardwareDisplayPlaneList& plane_list) {
-  const int connector_idx = LookupConnectorIndex(connector_id);
-  DCHECK_GE(connector_idx, 0);
-  connectors_props_[connector_idx].crtc_id.value = crtc_id;
-  bool res =
-      AddPropertyIfValid(plane_list.atomic_property_set.get(), connector_id,
-                         connectors_props_[connector_idx].crtc_id);
-
-  const int crtc_idx = LookupCrtcIndex(crtc_id);
-  DCHECK_GE(crtc_idx, 0);
-
-  crtc_state_[crtc_idx].properties.active.value = 1UL;
-  ScopedDrmPropertyBlob mode_blob =
-      drm_->CreatePropertyBlob(&mode, sizeof(mode));
-  crtc_state_[crtc_idx].properties.mode_id.value =
-      mode_blob ? mode_blob->id() : 0;
-
-  res &= AddPropertyIfValid(plane_list.atomic_property_set.get(), crtc_id,
-                            crtc_state_[crtc_idx].properties.active);
-  res &= AddPropertyIfValid(plane_list.atomic_property_set.get(), crtc_id,
-                            crtc_state_[crtc_idx].properties.mode_id);
-
-  DCHECK(res);
-  return Commit(const_cast<HardwareDisplayPlaneList*>(&plane_list),
-                /*should_modeset=*/true,
-                /*page_flip_request=*/nullptr,
-                /*out_fence=*/nullptr);
+    const HardwareDisplayPlaneList&) {
+  return drm_->SetCrtc(crtc_id, framebuffer_id,
+                       std::vector<uint32_t>(1, connector_id), mode);
 }
 
 bool HardwareDisplayPlaneManagerAtomic::DisableModeset(uint32_t crtc_id,
@@ -120,21 +96,12 @@ bool HardwareDisplayPlaneManagerAtomic::DisableModeset(uint32_t crtc_id,
 
 bool HardwareDisplayPlaneManagerAtomic::Commit(
     HardwareDisplayPlaneList* plane_list,
-    bool should_modeset,
     scoped_refptr<PageFlipRequest> page_flip_request,
     std::unique_ptr<gfx::GpuFence>* out_fence) {
-  bool test_only = !should_modeset && !page_flip_request;
-
+  bool test_only = !page_flip_request;
   for (HardwareDisplayPlane* plane : plane_list->old_plane_list) {
     if (!base::Contains(plane_list->plane_list, plane)) {
-      // |plane| is shared state between |old_plane_list| and |plane_list|.
-      // When we call BeginFrame(), we reset in_use since we need to be able to
-      // allocate the planes as needed. The current frame might not need to use
-      // |plane|, thus |plane->in_use()| would be false even though the previous
-      // frame used it. It's existence in |old_plane_list| is sufficient to
-      // signal that |plane| was in use previously.
-      // TODO(markyacoub): Add a unittest that planes should be reset whether
-      // they're originally in_use or not.
+      // This plane is being released, so we need to zero it.
       plane->set_in_use(false);
       HardwareDisplayPlaneAtomic* atomic_plane =
           static_cast<HardwareDisplayPlaneAtomic*>(plane);
@@ -179,10 +146,11 @@ bool HardwareDisplayPlaneManagerAtomic::Commit(
   }
 
   uint32_t flags = 0;
-  if (should_modeset)
-    flags = DRM_MODE_ATOMIC_ALLOW_MODESET;
-  else
-    flags = test_only ? DRM_MODE_ATOMIC_TEST_ONLY : DRM_MODE_ATOMIC_NONBLOCK;
+  if (test_only) {
+    flags = DRM_MODE_ATOMIC_TEST_ONLY;
+  } else {
+    flags = DRM_MODE_ATOMIC_NONBLOCK;
+  }
 
   // After we perform the atomic commit, and if the caller has requested an
   // out-fence, the out_fence_fds vector will contain any provided out-fence

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

@@ -26,7 +26,6 @@ class HardwareDisplayPlaneManagerAtomic : public HardwareDisplayPlaneManager {
                const HardwareDisplayPlaneList& plane_list) override;
   bool DisableModeset(uint32_t crtc_id, uint32_t connector) override;
   bool Commit(HardwareDisplayPlaneList* plane_list,
-              bool should_modeset,
               scoped_refptr<PageFlipRequest> page_flip_request,
               std::unique_ptr<gfx::GpuFence>* out_fence) override;
   bool DisableOverlayPlanes(HardwareDisplayPlaneList* plane_list) override;

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

@@ -63,11 +63,8 @@ bool HardwareDisplayPlaneManagerLegacy::DisableModeset(uint32_t crtc_id,
 
 bool HardwareDisplayPlaneManagerLegacy::Commit(
     HardwareDisplayPlaneList* plane_list,
-    bool should_modeset,
     scoped_refptr<PageFlipRequest> page_flip_request,
     std::unique_ptr<gfx::GpuFence>* out_fence) {
-  DCHECK(!should_modeset);
-
   bool test_only = !page_flip_request;
   if (test_only) {
     for (HardwareDisplayPlane* plane : plane_list->plane_list) {

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

@@ -26,7 +26,6 @@ class HardwareDisplayPlaneManagerLegacy : public HardwareDisplayPlaneManager {
                const HardwareDisplayPlaneList& plane_list) override;
   bool DisableModeset(uint32_t crtc_id, uint32_t connector) override;
   bool Commit(HardwareDisplayPlaneList* plane_list,
-              bool should_modeset,
               scoped_refptr<PageFlipRequest> page_flip_request,
               std::unique_ptr<gfx::GpuFence>* out_fence) override;
   bool DisableOverlayPlanes(HardwareDisplayPlaneList* plane_list) override;

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

@@ -37,7 +37,6 @@ constexpr uint32_t kCrtcIdBase = 500;
 constexpr uint32_t kConnectorIdBase = 700;
 
 constexpr uint32_t kActivePropId = 1000;
-constexpr uint32_t kModePropId = 1001;
 constexpr uint32_t kBackgroundColorPropId = 1002;
 constexpr uint32_t kCtmPropId = 1003;
 constexpr uint32_t kGammaLutPropId = 1004;
@@ -46,7 +45,6 @@ constexpr uint32_t kDegammaLutPropId = 1006;
 constexpr uint32_t kDegammaLutSizePropId = 1007;
 constexpr uint32_t kOutFencePtrPropId = 1008;
 
-constexpr uint32_t kCrtcIdPropId = 2000;
 constexpr uint32_t kTypePropId = 3010;
 constexpr uint32_t kInFormatsPropId = 3011;
 constexpr uint32_t kPlaneCtmId = 3012;
@@ -122,12 +120,12 @@ void HardwareDisplayPlaneManagerTest::InitializeDrmState(
     size_t planes_per_crtc) {
   std::map<uint32_t, std::string> crtc_property_names = {
       {kActivePropId, "ACTIVE"},
-      {kModePropId, "MODE_ID"},
+      {1001, "MODE_ID"},
   };
 
   std::vector<ui::MockDrmDevice::ConnectorProperties> connector_properties(1);
   std::map<uint32_t, std::string> connector_property_names = {
-      {kCrtcIdPropId, "CRTC_ID"},
+      {2000, "CRTC_ID"},
   };
   for (size_t i = 0; i < connector_properties.size(); ++i) {
     connector_properties[i].id = kConnectorIdBase + i;
@@ -227,8 +225,8 @@ void HardwareDisplayPlaneManagerTest::PerformPageFlip(
       state, assigns, crtc_properties_[crtc_idx].id));
   scoped_refptr<ui::PageFlipRequest> page_flip_request =
       base::MakeRefCounted<ui::PageFlipRequest>(base::TimeDelta());
-  ASSERT_TRUE(fake_drm_->plane_manager()->Commit(
-      state, /*should_modeset*/ false, page_flip_request, nullptr));
+  ASSERT_TRUE(
+      fake_drm_->plane_manager()->Commit(state, page_flip_request, nullptr));
 }
 
 uint64_t HardwareDisplayPlaneManagerTest::GetObjectPropertyValue(
@@ -410,7 +408,7 @@ TEST_P(HardwareDisplayPlaneManagerAtomicTest, Modeset) {
       crtc_properties_[0].id, kFrameBuffer, connector_properties_[0].id,
       kDefaultMode, state));
 
-  EXPECT_EQ(1, fake_drm_->get_commit_count());
+  EXPECT_EQ(0, fake_drm_->get_commit_count());
 }
 
 TEST_P(HardwareDisplayPlaneManagerAtomicTest, DisableModeset) {
@@ -424,40 +422,6 @@ TEST_P(HardwareDisplayPlaneManagerAtomicTest, DisableModeset) {
   EXPECT_EQ(1, fake_drm_->get_commit_count());
 }
 
-TEST_P(HardwareDisplayPlaneManagerAtomicTest, CheckPropsAfterModeset) {
-  InitializeDrmState(/*crtc_count=*/1, /*planes_per_crtc=*/1);
-  fake_drm_->InitializeState(crtc_properties_, connector_properties_,
-                             plane_properties_, property_names_,
-                             /*use_atomic=*/true);
-
-  constexpr uint32_t kFrameBuffer = 2;
-  ui::HardwareDisplayPlaneList state;
-  EXPECT_TRUE(fake_drm_->plane_manager()->Modeset(
-      crtc_properties_[0].id, kFrameBuffer, connector_properties_[0].id,
-      kDefaultMode, state));
-
-  // Test props values after modesetting.
-  ui::DrmDevice::Property connector_prop_crtc_id;
-  ui::ScopedDrmObjectPropertyPtr connector_props =
-      fake_drm_->GetObjectProperties(kConnectorIdBase,
-                                     DRM_MODE_OBJECT_CONNECTOR);
-  ui::GetDrmPropertyForName(fake_drm_.get(), connector_props.get(), "CRTC_ID",
-                            &connector_prop_crtc_id);
-  EXPECT_EQ(kCrtcIdPropId, connector_prop_crtc_id.id);
-
-  ui::DrmDevice::Property crtc_prop_for_name;
-  ui::ScopedDrmObjectPropertyPtr crtc_props =
-      fake_drm_->GetObjectProperties(kCrtcIdBase, DRM_MODE_OBJECT_CRTC);
-  ui::GetDrmPropertyForName(fake_drm_.get(), crtc_props.get(), "ACTIVE",
-                            &crtc_prop_for_name);
-  EXPECT_EQ(kActivePropId, crtc_prop_for_name.id);
-  EXPECT_EQ(1U, crtc_prop_for_name.value);
-
-  ui::GetDrmPropertyForName(fake_drm_.get(), crtc_props.get(), "MODE_ID",
-                            &crtc_prop_for_name);
-  EXPECT_EQ(kModePropId, crtc_prop_for_name.id);
-}
-
 TEST_P(HardwareDisplayPlaneManagerAtomicTest, CheckPropsAfterDisable) {
   InitializeDrmState(/*crtc_count=*/1, /*planes_per_crtc=*/1);
   fake_drm_->InitializeState(crtc_properties_, connector_properties_,
@@ -559,8 +523,8 @@ TEST_P(HardwareDisplayPlaneManagerAtomicTest, UnusedPlanesAreReleased) {
   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()->Commit(&hdpl, page_flip_request, nullptr));
   assigns.clear();
   assigns.push_back(ui::DrmOverlayPlane(primary_buffer, nullptr));
   fake_drm_->plane_manager()->BeginFrame(&hdpl);
@@ -569,11 +533,8 @@ TEST_P(HardwareDisplayPlaneManagerAtomicTest, UnusedPlanesAreReleased) {
   EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset, "FB_ID"));
   EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset + 1, "FB_ID"));
 
-  for (auto* plane : hdpl.old_plane_list)
-    plane->set_in_use(true);
-
-  EXPECT_TRUE(fake_drm_->plane_manager()->Commit(
-      &hdpl, /*should_modeset*/ false, page_flip_request, nullptr));
+  EXPECT_TRUE(
+      fake_drm_->plane_manager()->Commit(&hdpl, page_flip_request, nullptr));
   EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset, "FB_ID"));
   EXPECT_EQ(0u, GetPlanePropertyValue(kPlaneOffset + 1, "FB_ID"));
 }
@@ -906,8 +867,8 @@ TEST_P(HardwareDisplayPlaneManagerAtomicTest,
       base::MakeRefCounted<ui::PageFlipRequest>(base::TimeDelta());
 
   std::unique_ptr<gfx::GpuFence> out_fence;
-  EXPECT_TRUE(fake_drm_->plane_manager()->Commit(
-      &state_, /*should_modeset*/ false, page_flip_request, &out_fence));
+  EXPECT_TRUE(fake_drm_->plane_manager()->Commit(&state_, page_flip_request,
+                                                 &out_fence));
   EXPECT_EQ(nullptr, out_fence);
 }
 

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

@@ -31,18 +31,10 @@ namespace {
 const drmModeModeInfo kDefaultMode = {0, 6, 0, 0, 0, 0, 4,     0,
                                       0, 0, 0, 0, 0, 0, {'\0'}};
 
-constexpr uint32_t kCrtcIdBase = 100;
-constexpr uint32_t kPrimaryCrtc = kCrtcIdBase;
-constexpr uint32_t kSecondaryCrtc = kCrtcIdBase + 1;
-
-constexpr uint32_t kConnectorIdBase = 200;
-constexpr uint32_t kPrimaryConnector = kConnectorIdBase;
-constexpr uint32_t kSecondaryConnector = kConnectorIdBase + 1;
-constexpr uint32_t kPlaneIdBase = 300;
-constexpr uint32_t kInFormatsBlobPropIdBase = 400;
-
-constexpr uint32_t kTypePropId = 3010;
-constexpr uint32_t kInFormatsPropId = 3011;
+const uint32_t kPrimaryCrtc = 1;
+const uint32_t kPrimaryConnector = 2;
+const uint32_t kSecondaryCrtc = 3;
+const uint32_t kSecondaryConnector = 4;
 
 drmModeModeInfo Mode(uint16_t hdisplay, uint16_t vdisplay) {
   return {0, hdisplay, 0, 0, 0, 0, vdisplay, 0, 0, 0, 0, 0, 0, 0, {'\0'}};
@@ -52,16 +44,8 @@ drmModeModeInfo Mode(uint16_t hdisplay, uint16_t vdisplay) {
 
 class ScreenManagerTest : public testing::Test {
  public:
-  struct PlaneState {
-    std::vector<uint32_t> formats;
-  };
-
-  struct CrtcState {
-    std::vector<PlaneState> planes;
-  };
-
-  ScreenManagerTest() = default;
-  ~ScreenManagerTest() override = default;
+  ScreenManagerTest() {}
+  ~ScreenManagerTest() override {}
 
   gfx::Rect GetPrimaryBounds() const {
     return gfx::Rect(0, 0, kDefaultMode.hdisplay, kDefaultMode.vdisplay);
@@ -73,95 +57,6 @@ class ScreenManagerTest : public testing::Test {
                      kDefaultMode.vdisplay);
   }
 
-  void InitializeDrmState(const std::vector<CrtcState>& crtc_states) {
-    std::vector<ui::MockDrmDevice::CrtcProperties> crtc_properties(
-        crtc_states.size());
-    std::map<uint32_t, std::string> crtc_property_names = {
-        {1000, "ACTIVE"},
-        {1001, "MODE_ID"},
-    };
-
-    std::vector<ui::MockDrmDevice::ConnectorProperties> connector_properties(2);
-    std::map<uint32_t, std::string> connector_property_names = {
-        {2000, "CRTC_ID"},
-    };
-    for (size_t i = 0; i < connector_properties.size(); ++i) {
-      connector_properties[i].id = kPrimaryConnector + i;
-      for (const auto& pair : connector_property_names) {
-        connector_properties[i].properties.push_back(
-            {/* .id = */ pair.first, /* .value = */ 0});
-      }
-    }
-
-    std::vector<ui::MockDrmDevice::PlaneProperties> plane_properties;
-    std::map<uint32_t, std::string> plane_property_names = {
-        // Add all required properties.
-        {3000, "CRTC_ID"},
-        {3001, "CRTC_X"},
-        {3002, "CRTC_Y"},
-        {3003, "CRTC_W"},
-        {3004, "CRTC_H"},
-        {3005, "FB_ID"},
-        {3006, "SRC_X"},
-        {3007, "SRC_Y"},
-        {3008, "SRC_W"},
-        {3009, "SRC_H"},
-        // Defines some optional properties we use for convenience.
-        {kTypePropId, "type"},
-        {kInFormatsPropId, "IN_FORMATS"},
-    };
-
-    uint32_t plane_id = kPlaneIdBase;
-    uint32_t property_id = kInFormatsBlobPropIdBase;
-
-    for (size_t crtc_idx = 0; crtc_idx < crtc_states.size(); ++crtc_idx) {
-      crtc_properties[crtc_idx].id = kPrimaryCrtc + crtc_idx;
-      for (const auto& pair : crtc_property_names) {
-        crtc_properties[crtc_idx].properties.push_back(
-            {/* .id = */ pair.first, /* .value = */ 0});
-      }
-
-      std::vector<ui::MockDrmDevice::PlaneProperties> crtc_plane_properties(
-          crtc_states[crtc_idx].planes.size());
-      for (size_t plane_idx = 0;
-           plane_idx < crtc_states[crtc_idx].planes.size(); ++plane_idx) {
-        crtc_plane_properties[plane_idx].id = plane_id++;
-        crtc_plane_properties[plane_idx].crtc_mask = 1 << crtc_idx;
-
-        for (const auto& pair : plane_property_names) {
-          uint64_t value = 0;
-          if (pair.first == kTypePropId) {
-            value = plane_idx == 0 ? DRM_PLANE_TYPE_PRIMARY
-                                   : DRM_PLANE_TYPE_OVERLAY;
-          } else if (pair.first == kInFormatsPropId) {
-            value = property_id++;
-            drm_->SetPropertyBlob(ui::MockDrmDevice::AllocateInFormatsBlob(
-                value, crtc_states[crtc_idx].planes[plane_idx].formats,
-                std::vector<drm_format_modifier>()));
-          }
-
-          crtc_plane_properties[plane_idx].properties.push_back(
-              {/* .id = */ pair.first, /* .value = */ value});
-        }
-      }
-
-      plane_properties.insert(plane_properties.end(),
-                              crtc_plane_properties.begin(),
-                              crtc_plane_properties.end());
-    }
-
-    std::map<uint32_t, std::string> property_names;
-    property_names.insert(crtc_property_names.begin(),
-                          crtc_property_names.end());
-    property_names.insert(connector_property_names.begin(),
-                          connector_property_names.end());
-    property_names.insert(plane_property_names.begin(),
-                          plane_property_names.end());
-    drm_->InitializeState(crtc_properties, connector_properties,
-                          plane_properties, property_names,
-                          /* use_atomic= */ true);
-  }
-
   void SetUp() override {
     auto gbm = std::make_unique<ui::MockGbmDevice>();
     drm_ = new ui::MockDrmDevice(std::move(gbm));
@@ -307,24 +202,6 @@ TEST_F(ScreenManagerTest, CheckForControllersInMirroredMode) {
 }
 
 TEST_F(ScreenManagerTest, CheckMirrorModeTransitions) {
-  std::vector<CrtcState> crtc_states = {
-      {
-          /* .planes = */
-          {
-              {/* .formats = */ {DRM_FORMAT_XRGB8888}},
-              {/* .formats = */ {DRM_FORMAT_XRGB8888, DRM_FORMAT_NV12}},
-          },
-      },
-      {
-          /* .planes = */
-          {
-              {/* .formats = */ {DRM_FORMAT_XRGB8888}},
-              {/* .formats = */ {DRM_FORMAT_XRGB8888, DRM_FORMAT_NV12}},
-          },
-      },
-  };
-  InitializeDrmState(crtc_states);
-
   screen_manager_->AddDisplayController(drm_, kPrimaryCrtc, kPrimaryConnector);
   screen_manager_->ConfigureDisplayController(
       drm_, kPrimaryCrtc, kPrimaryConnector, GetPrimaryBounds().origin(),
@@ -476,24 +353,6 @@ TEST_F(ScreenManagerTest, ReuseFramebufferIfDisabledThenReEnabled) {
 }
 
 TEST_F(ScreenManagerTest, CheckMirrorModeAfterBeginReEnabled) {
-  std::vector<CrtcState> crtc_states = {
-      {
-          /* .planes = */
-          {
-              {/* .formats = */ {DRM_FORMAT_XRGB8888}},
-              {/* .formats = */ {DRM_FORMAT_XRGB8888, DRM_FORMAT_NV12}},
-          },
-      },
-      {
-          /* .planes = */
-          {
-              {/* .formats = */ {DRM_FORMAT_XRGB8888}},
-              {/* .formats = */ {DRM_FORMAT_XRGB8888, DRM_FORMAT_NV12}},
-          },
-      },
-  };
-  InitializeDrmState(crtc_states);
-
   screen_manager_->AddDisplayController(drm_, kPrimaryCrtc, kPrimaryConnector);
   screen_manager_->ConfigureDisplayController(
       drm_, kPrimaryCrtc, kPrimaryConnector, GetPrimaryBounds().origin(),