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}

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

@@ -80,8 +80,11 @@ bool CrtcController::Disable() {
 }
 
 bool CrtcController::AssignOverlayPlanes(HardwareDisplayPlaneList* plane_list,
-                                         const DrmOverlayPlaneList& overlays) {
-  DCHECK(!is_disabled_);
+                                         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 DrmOverlayPlane* primary = DrmOverlayPlane::GetPrimaryPlane(overlays);
   if (primary && !drm_->plane_manager()->ValidatePrimarySize(*primary, mode_)) {

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

@@ -51,7 +51,8 @@ class CrtcController {
   bool Disable();
 
   bool AssignOverlayPlanes(HardwareDisplayPlaneList* plane_list,
-                           const DrmOverlayPlaneList& planes);
+                           const DrmOverlayPlaneList& planes,
+                           bool is_modesetting);
 
   // 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,10 +84,20 @@ bool HardwareDisplayController::ModesetCrtc(const DrmOverlayPlane& primary,
                                             const drmModeModeInfo& mode) {
   DCHECK(primary.buffer.get());
   bool status = true;
-  for (const auto& controller : crtc_controllers_)
+
+  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);
+
     status &= controller->Modeset(
         primary, use_current_crtc_mode ? controller->mode() : mode,
         owned_hardware_planes_);
+  }
 
   is_disabled_ = false;
   ResetCursor();
@@ -170,12 +180,13 @@ bool HardwareDisplayController::ScheduleOrTestPageFlip(
 
   bool status = true;
   for (const auto& controller : crtc_controllers_) {
-    status &= controller->AssignOverlayPlanes(&owned_hardware_planes_,
-                                              pending_planes);
+    status &= controller->AssignOverlayPlanes(
+        &owned_hardware_planes_, pending_planes, /*is_modesetting=*/false);
   }
 
   status &= GetDrmDevice()->plane_manager()->Commit(
-      &owned_hardware_planes_, page_flip_request, out_fence);
+      &owned_hardware_planes_, /*should_modeset=*/false, page_flip_request,
+      out_fence);
 
   return status;
 }
@@ -351,6 +362,7 @@ 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,11 +30,18 @@ namespace {
 const drmModeModeInfo kDefaultMode = {0, 6, 0, 0, 0, 0, 4,     0,
                                       0, 0, 0, 0, 0, 0, {'\0'}};
 
-constexpr uint32_t kCrtcIdBase = 1;
+constexpr uint32_t kCrtcIdBase = 100;
 constexpr uint32_t kPrimaryCrtc = kCrtcIdBase;
 constexpr uint32_t kSecondaryCrtc = kCrtcIdBase + 1;
-constexpr uint32_t kConnectorIdBase = 10;
-constexpr uint32_t kPlaneOffset = 1000;
+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;
 
 const gfx::Size kDefaultModeSize(kDefaultMode.hdisplay, kDefaultMode.vdisplay);
 const gfx::Size kOverlaySize(kDefaultMode.hdisplay / 2,
@@ -103,10 +110,6 @@ 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"},
@@ -115,7 +118,7 @@ void HardwareDisplayControllerTest::InitializeDrmDevice(bool use_atomic) {
 
   std::vector<ui::MockDrmDevice::ConnectorProperties> connector_properties(2);
   std::map<uint32_t, std::string> connector_property_names = {
-      {2000, "CRTC_ID"},
+      {kCrtcIdPropId, "CRTC_ID"},
   };
   for (size_t i = 0; i < connector_properties.size(); ++i) {
     connector_properties[i].id = kConnectorIdBase + i;
@@ -279,6 +282,39 @@ 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);
 
@@ -317,6 +353,7 @@ 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;
@@ -330,13 +367,14 @@ TEST_F(HardwareDisplayControllerTest, CheckStateAfterPageFlip) {
 
   EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);
   EXPECT_EQ(1, page_flips_);
-  EXPECT_EQ(1, drm_->get_commit_count());
+  EXPECT_EQ(2, 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);
@@ -344,20 +382,6 @@ 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(
@@ -365,6 +389,7 @@ 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());
@@ -374,7 +399,7 @@ TEST_F(HardwareDisplayControllerTest, CheckOverlayPresent) {
   drm_->RunCallbacks();
   EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);
   EXPECT_EQ(1, page_flips_);
-  EXPECT_EQ(1, drm_->get_commit_count());
+  EXPECT_EQ(2, 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"));
@@ -387,13 +412,14 @@ 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(1, drm_->get_commit_count());
+  EXPECT_EQ(2, 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"));
@@ -403,14 +429,14 @@ TEST_F(HardwareDisplayControllerTest, CheckOverlayTestMode) {
   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(3, 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(3, drm_->get_commit_count());
+  EXPECT_EQ(4, 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"));
@@ -441,7 +467,7 @@ TEST_F(HardwareDisplayControllerTest, PageflipMirroredControllers) {
 
   ui::DrmOverlayPlane plane1(CreateBuffer(), nullptr);
   EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
-  EXPECT_EQ(2, drm_->get_set_crtc_call_count());
+  EXPECT_EQ(2, drm_->get_commit_count());
 
   ui::DrmOverlayPlane plane2(CreateBuffer(), nullptr);
   std::vector<ui::DrmOverlayPlane> planes;
@@ -450,7 +476,7 @@ TEST_F(HardwareDisplayControllerTest, PageflipMirroredControllers) {
   drm_->RunCallbacks();
   EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);
   EXPECT_EQ(1, page_flips_);
-  EXPECT_EQ(1, drm_->get_commit_count());
+  EXPECT_EQ(3, 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"));
@@ -597,10 +623,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,6 +105,7 @@ 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,9 +65,33 @@ bool HardwareDisplayPlaneManagerAtomic::Modeset(
     uint32_t framebuffer_id,
     uint32_t connector_id,
     const drmModeModeInfo& mode,
-    const HardwareDisplayPlaneList&) {
-  return drm_->SetCrtc(crtc_id, framebuffer_id,
-                       std::vector<uint32_t>(1, connector_id), 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);
 }
 
 bool HardwareDisplayPlaneManagerAtomic::DisableModeset(uint32_t crtc_id,
@@ -96,18 +120,23 @@ 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 = !page_flip_request;
+  bool test_only = !should_modeset && !page_flip_request;
+
   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(),
-          gfx::OVERLAY_TRANSFORM_NONE, base::kInvalidPlatformFile);
+      if (plane->in_use()) {
+        // 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(),
+                                   gfx::OVERLAY_TRANSFORM_NONE,
+                                   base::kInvalidPlatformFile);
+      }
     }
   }
 
@@ -146,11 +175,10 @@ bool HardwareDisplayPlaneManagerAtomic::Commit(
   }
 
   uint32_t flags = 0;
-  if (test_only) {
-    flags = DRM_MODE_ATOMIC_TEST_ONLY;
-  } else {
-    flags = DRM_MODE_ATOMIC_NONBLOCK;
-  }
+  if (should_modeset)
+    flags = DRM_MODE_ATOMIC_ALLOW_MODESET;
+  else
+    flags = test_only ? DRM_MODE_ATOMIC_TEST_ONLY : 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,6 +26,7 @@ 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

@@ -62,8 +62,11 @@ 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,6 +26,7 @@ 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

@@ -35,6 +35,7 @@ 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;
@@ -43,6 +44,7 @@ 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;
@@ -118,12 +120,12 @@ void HardwareDisplayPlaneManagerTest::InitializeDrmState(
     size_t planes_per_crtc) {
   std::map<uint32_t, std::string> crtc_property_names = {
       {kActivePropId, "ACTIVE"},
-      {1001, "MODE_ID"},
+      {kModePropId, "MODE_ID"},
   };
 
   std::vector<ui::MockDrmDevice::ConnectorProperties> connector_properties(1);
   std::map<uint32_t, std::string> connector_property_names = {
-      {2000, "CRTC_ID"},
+      {kCrtcIdPropId, "CRTC_ID"},
   };
   for (size_t i = 0; i < connector_properties.size(); ++i) {
     connector_properties[i].id = kConnectorIdBase + i;
@@ -223,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, page_flip_request, nullptr));
+  ASSERT_TRUE(fake_drm_->plane_manager()->Commit(
+      state, /*should_modeset*/ false, page_flip_request, nullptr));
 }
 
 uint64_t HardwareDisplayPlaneManagerTest::GetObjectPropertyValue(
@@ -406,7 +408,7 @@ TEST_P(HardwareDisplayPlaneManagerAtomicTest, Modeset) {
       crtc_properties_[0].id, kFrameBuffer, connector_properties_[0].id,
       kDefaultMode, state));
 
-  EXPECT_EQ(0, fake_drm_->get_commit_count());
+  EXPECT_EQ(1, fake_drm_->get_commit_count());
 }
 
 TEST_P(HardwareDisplayPlaneManagerAtomicTest, DisableModeset) {
@@ -420,6 +422,40 @@ 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_,
@@ -521,8 +557,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, page_flip_request, nullptr));
+  EXPECT_TRUE(fake_drm_->plane_manager()->Commit(
+      &hdpl, /*should_modeset*/ false, page_flip_request, nullptr));
   assigns.clear();
   assigns.push_back(ui::DrmOverlayPlane(primary_buffer, nullptr));
   fake_drm_->plane_manager()->BeginFrame(&hdpl);
@@ -531,8 +567,11 @@ TEST_P(HardwareDisplayPlaneManagerAtomicTest, UnusedPlanesAreReleased) {
   EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset, "FB_ID"));
   EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset + 1, "FB_ID"));
 
-  EXPECT_TRUE(
-      fake_drm_->plane_manager()->Commit(&hdpl, page_flip_request, nullptr));
+  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_NE(0u, GetPlanePropertyValue(kPlaneOffset, "FB_ID"));
   EXPECT_EQ(0u, GetPlanePropertyValue(kPlaneOffset + 1, "FB_ID"));
 }
@@ -865,8 +904,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_, page_flip_request,
-                                                 &out_fence));
+  EXPECT_TRUE(fake_drm_->plane_manager()->Commit(
+      &state_, /*should_modeset*/ false, page_flip_request, &out_fence));
   EXPECT_EQ(nullptr, out_fence);
 }
 

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

@@ -31,10 +31,18 @@ namespace {
 const drmModeModeInfo kDefaultMode = {0, 6, 0, 0, 0, 0, 4,     0,
                                       0, 0, 0, 0, 0, 0, {'\0'}};
 
-const uint32_t kPrimaryCrtc = 1;
-const uint32_t kPrimaryConnector = 2;
-const uint32_t kSecondaryCrtc = 3;
-const uint32_t kSecondaryConnector = 4;
+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;
 
 drmModeModeInfo Mode(uint16_t hdisplay, uint16_t vdisplay) {
   return {0, hdisplay, 0, 0, 0, 0, vdisplay, 0, 0, 0, 0, 0, 0, 0, {'\0'}};
@@ -44,8 +52,16 @@ drmModeModeInfo Mode(uint16_t hdisplay, uint16_t vdisplay) {
 
 class ScreenManagerTest : public testing::Test {
  public:
-  ScreenManagerTest() {}
-  ~ScreenManagerTest() override {}
+  struct PlaneState {
+    std::vector<uint32_t> formats;
+  };
+
+  struct CrtcState {
+    std::vector<PlaneState> planes;
+  };
+
+  ScreenManagerTest() = default;
+  ~ScreenManagerTest() override = default;
 
   gfx::Rect GetPrimaryBounds() const {
     return gfx::Rect(0, 0, kDefaultMode.hdisplay, kDefaultMode.vdisplay);
@@ -57,6 +73,95 @@ 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));
@@ -202,6 +307,24 @@ 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(),
@@ -353,6 +476,24 @@ 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(),