[Ozone-DRM] Remove support for overlays on DRM legacy

DRM legacy is never used with overlays due to a lack of synchronization
during page flip. Remove the complexity behing trying to page flip
overlays on legacy. Also move tests that deal with overlays to use
atomic configuration.

BUG=822853
TEST=Ran unittests
TEST=Ran chrome on Samus to validate legacy path still works
TEST=Ran chrome on Eve to ensure atomic path hasn't regressed

Change-Id: Ibc82aafd367c8d12106d3a0e835a3b4b38e0161a
Reviewed-on: https://chromium-review.googlesource.com/1120692Reviewed-by: Michael Spang <spang@chromium.org>
Commit-Queue: Daniel Nicoara <dnicoara@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577610}

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

@@ -149,7 +149,20 @@ void DrmOverlayValidatorTest::InitializeDrmState(
       crtc_states.size());
   std::vector<ui::MockDrmDevice::PlaneProperties> plane_properties;
   std::map<uint32_t, std::string> property_names = {
-      {kTypePropId, "type"}, {kInFormatsPropId, "IN_FORMATS"},
+      // Add all required properties.
+      {1000, "CRTC_ID"},
+      {1001, "CRTC_X"},
+      {1002, "CRTC_Y"},
+      {1003, "CRTC_W"},
+      {1004, "CRTC_H"},
+      {1005, "FB_ID"},
+      {1006, "SRC_X"},
+      {1007, "SRC_Y"},
+      {1008, "SRC_W"},
+      {1009, "SRC_H"},
+      // Defines some optional properties we use for convenience.
+      {kTypePropId, "type"},
+      {kInFormatsPropId, "IN_FORMATS"},
   };
 
   uint32_t plane_id = kPlaneIdBase;
@@ -164,17 +177,22 @@ void DrmOverlayValidatorTest::InitializeDrmState(
          ++plane_idx) {
       crtc_plane_properties[plane_idx].id = plane_id++;
       crtc_plane_properties[plane_idx].crtc_mask = 1 << crtc_idx;
-      crtc_plane_properties[plane_idx].properties = {
-          {.id = kTypePropId,
-           .value = plane_idx == 0 ? DRM_PLANE_TYPE_PRIMARY
-                                   : DRM_PLANE_TYPE_OVERLAY},
-          {.id = kInFormatsPropId, .value = property_id++},
-      };
-
-      drm_->SetPropertyBlob(ui::MockDrmDevice::AllocateInFormatsBlob(
-          crtc_plane_properties[plane_idx].properties[1].value,
-          crtc_states[crtc_idx].planes[plane_idx].formats,
-          std::vector<drm_format_modifier>()));
+
+      for (const auto& pair : 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(),
@@ -183,7 +201,7 @@ void DrmOverlayValidatorTest::InitializeDrmState(
   }
 
   drm_->InitializeState(crtc_properties, plane_properties, property_names,
-                        /* use_atomic= */ false);
+                        /* use_atomic= */ true);
 }
 
 void DrmOverlayValidatorTest::AddPlane(const ui::OverlayCheck_Params& params) {

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

@@ -13,6 +13,7 @@
 #include "ui/gfx/native_pixmap.h"
 #include "ui/gfx/presentation_feedback.h"
 #include "ui/ozone/platform/drm/gpu/crtc_controller.h"
+#include "ui/ozone/platform/drm/gpu/drm_gpu_util.h"
 #include "ui/ozone/platform/drm/gpu/hardware_display_controller.h"
 #include "ui/ozone/platform/drm/gpu/hardware_display_plane.h"
 #include "ui/ozone/platform/drm/gpu/mock_drm_device.h"
@@ -29,6 +30,7 @@ constexpr uint32_t kPrimaryCrtc = kCrtcIdBase;
 constexpr uint32_t kSecondaryCrtc = kCrtcIdBase + 1;
 constexpr uint32_t kPrimaryConnector = 10;
 constexpr uint32_t kSecondaryConnector = 11;
+constexpr uint32_t kPlaneOffset = 1000;
 
 const gfx::Size kDefaultModeSize(kDefaultMode.hdisplay, kDefaultMode.vdisplay);
 const gfx::Size kOverlaySize(kDefaultMode.hdisplay / 2,
@@ -50,6 +52,8 @@ class HardwareDisplayControllerTest : public testing::Test {
   void OnSubmission(gfx::SwapResult swap_result,
                     std::unique_ptr<gfx::GpuFence> out_fence);
   void OnPresentation(const gfx::PresentationFeedback& feedback);
+  uint64_t GetPlanePropertyValue(uint32_t plane,
+                                 const std::string& property_name);
 
  protected:
   std::unique_ptr<ui::HardwareDisplayController> controller_;
@@ -68,7 +72,7 @@ void HardwareDisplayControllerTest::SetUp() {
   last_swap_result_ = gfx::SwapResult::SWAP_FAILED;
 
   drm_ = new ui::MockDrmDevice;
-  InitializeDrmDevice(/* use_atomic= */ false);
+  InitializeDrmDevice(/* use_atomic= */ true);
 
   controller_.reset(new ui::HardwareDisplayController(
       std::unique_ptr<ui::CrtcController>(
@@ -112,7 +116,7 @@ void HardwareDisplayControllerTest::InitializeDrmDevice(bool use_atomic) {
       const uint32_t offset = plane_properties.size();
 
       ui::MockDrmDevice::PlaneProperties plane;
-      plane.id = 100 + offset;
+      plane.id = kPlaneOffset + offset;
       plane.crtc_mask = 1 << i;
       for (const auto& pair : property_names) {
         uint32_t value = 0;
@@ -157,6 +161,17 @@ void HardwareDisplayControllerTest::OnPresentation(
   last_presentation_feedback_ = feedback;
 }
 
+uint64_t HardwareDisplayControllerTest::GetPlanePropertyValue(
+    uint32_t plane,
+    const std::string& property_name) {
+  ui::DrmDevice::Property p{};
+  ui::ScopedDrmObjectPropertyPtr properties(
+      drm_->GetObjectProperties(plane, DRM_MODE_OBJECT_PLANE));
+  EXPECT_TRUE(ui::GetDrmPropertyForName(drm_.get(), properties.get(),
+                                        property_name, &p));
+  return p.value;
+}
+
 TEST_F(HardwareDisplayControllerTest, CheckModesettingResult) {
   ui::DrmOverlayPlane plane(scoped_refptr<ui::ScanoutBuffer>(
                                 new ui::MockScanoutBuffer(kDefaultModeSize)),
@@ -187,8 +202,10 @@ TEST_F(HardwareDisplayControllerTest, CheckStateAfterPageFlip) {
 
   EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);
   EXPECT_EQ(1, page_flips_);
-  EXPECT_EQ(1, drm_->get_page_flip_call_count());
-  EXPECT_EQ(0, drm_->get_overlay_flip_call_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) {
@@ -202,7 +219,7 @@ TEST_F(HardwareDisplayControllerTest, CheckStateIfModesetFails) {
 }
 
 TEST_F(HardwareDisplayControllerTest, CheckStateIfPageFlipFails) {
-  drm_->set_page_flip_expectation(false);
+  drm_->set_commit_expectation(false);
 
   ui::DrmOverlayPlane plane1(scoped_refptr<ui::ScanoutBuffer>(
                                  new ui::MockScanoutBuffer(kDefaultModeSize)),
@@ -238,8 +255,10 @@ 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_page_flip_call_count());
-  EXPECT_EQ(1, drm_->get_overlay_flip_call_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"));
 }
 
 TEST_F(HardwareDisplayControllerTest, CheckOverlayTestMode) {
@@ -258,22 +277,27 @@ TEST_F(HardwareDisplayControllerTest, CheckOverlayTestMode) {
   planes.push_back(plane2.Clone());
 
   SchedulePageFlip(ui::DrmOverlayPlane::Clone(planes));
+  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"));
 
   // A test call shouldn't cause new flips, but should succeed.
   EXPECT_TRUE(controller_->TestPageFlip(planes));
   drm_->RunCallbacks();
   EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);
   EXPECT_EQ(1, page_flips_);
-  EXPECT_EQ(1, drm_->get_page_flip_call_count());
-  EXPECT_EQ(1, drm_->get_overlay_flip_call_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(2, drm_->get_page_flip_call_count());
-  EXPECT_EQ(2, drm_->get_overlay_flip_call_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"));
 }
 
 TEST_F(HardwareDisplayControllerTest, AcceptUnderlays) {
@@ -317,8 +341,14 @@ TEST_F(HardwareDisplayControllerTest, PageflipMirroredControllers) {
   drm_->RunCallbacks();
   EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);
   EXPECT_EQ(1, page_flips_);
-  EXPECT_EQ(2, drm_->get_page_flip_call_count());
-  EXPECT_EQ(1, page_flips_);
+  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"));
+  EXPECT_EQ(0u, GetPlanePropertyValue(kPlaneOffset + 1, "FB_ID"));
+  // Second display:
+  EXPECT_NE(0u, GetPlanePropertyValue(kPlaneOffset + 2, "FB_ID"));
+  EXPECT_EQ(0u, GetPlanePropertyValue(kPlaneOffset + 3, "FB_ID"));
 }
 
 TEST_F(HardwareDisplayControllerTest, PlaneStateAfterRemoveCrtc) {
@@ -464,7 +494,7 @@ TEST_F(HardwareDisplayControllerTest, ModesetWhilePageFlipping) {
 }
 
 TEST_F(HardwareDisplayControllerTest, FailPageFlipping) {
-  drm_->set_page_flip_expectation(false);
+  drm_->set_commit_expectation(false);
 
   ui::DrmOverlayPlane plane1(scoped_refptr<ui::ScanoutBuffer>(
                                  new ui::MockScanoutBuffer(kDefaultModeSize)),

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

@@ -15,7 +15,6 @@
 #include "ui/ozone/platform/drm/gpu/drm_device.h"
 #include "ui/ozone/platform/drm/gpu/drm_gpu_util.h"
 #include "ui/ozone/platform/drm/gpu/hardware_display_plane.h"
-#include "ui/ozone/platform/drm/gpu/hardware_display_plane_dummy.h"
 #include "ui/ozone/platform/drm/gpu/scanout_buffer.h"
 
 namespace ui {
@@ -44,19 +43,6 @@ HardwareDisplayPlaneList::PageFlipInfo::PageFlipInfo(
 HardwareDisplayPlaneList::PageFlipInfo::~PageFlipInfo() {
 }
 
-HardwareDisplayPlaneList::PageFlipInfo::Plane::Plane(int plane,
-                                                     int framebuffer,
-                                                     const gfx::Rect& bounds,
-                                                     const gfx::Rect& src_rect)
-    : plane(plane),
-      framebuffer(framebuffer),
-      bounds(bounds),
-      src_rect(src_rect) {
-}
-
-HardwareDisplayPlaneList::PageFlipInfo::Plane::~Plane() {
-}
-
 HardwareDisplayPlaneManager::HardwareDisplayPlaneManager() : drm_(nullptr) {
 }
 
@@ -66,48 +52,18 @@ HardwareDisplayPlaneManager::~HardwareDisplayPlaneManager() {
 bool HardwareDisplayPlaneManager::Initialize(DrmDevice* drm) {
   drm_ = drm;
 
-  bool has_universal_planes = false;
 // Try to get all of the planes if possible, so we don't have to try to
 // discover hidden primary planes.
 #if defined(DRM_CLIENT_CAP_UNIVERSAL_PLANES)
-  has_universal_planes =
+  has_universal_planes_ =
       drm_->SetCapability(DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
 #endif
 
   if (!InitializeCrtcProperties(drm))
     return false;
 
-  ScopedDrmPlaneResPtr plane_resources = drm->GetPlaneResources();
-  if (!plane_resources) {
-    PLOG(ERROR) << "Failed to get plane resources.";
+  if (!InitializePlanes(drm))
     return false;
-  }
-
-  std::set<uint32_t> plane_ids;
-  for (uint32_t i = 0; i < plane_resources->count_planes; ++i) {
-    plane_ids.insert(plane_resources->planes[i]);
-    std::unique_ptr<HardwareDisplayPlane> plane(
-        CreatePlane(plane_resources->planes[i]));
-
-    if (plane->Initialize(drm))
-      planes_.push_back(std::move(plane));
-  }
-
-  // crbug.com/464085: if driver reports no primary planes for a crtc, create a
-  // dummy plane for which we can assign exactly one overlay.
-  // TODO(dnicoara): refactor this to simplify AssignOverlayPlanes and move
-  // this workaround into HardwareDisplayPlaneLegacy.
-  if (!has_universal_planes) {
-    for (size_t i = 0; i < crtc_properties_.size(); ++i) {
-      if (plane_ids.find(crtc_properties_[i].id - 1) == plane_ids.end()) {
-        std::unique_ptr<HardwareDisplayPlane> dummy_plane(
-            new HardwareDisplayPlaneDummy(crtc_properties_[i].id - 1, 1 << i));
-        if (dummy_plane->Initialize(drm)) {
-          planes_.push_back(std::move(dummy_plane));
-        }
-      }
-    }
-  }
 
   std::sort(planes_.begin(), planes_.end(),
             [](const std::unique_ptr<HardwareDisplayPlane>& l,

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

@@ -46,19 +46,6 @@ struct HardwareDisplayPlaneList {
     uint32_t crtc_id;
     uint32_t framebuffer;
     CrtcController* crtc;
-
-    struct Plane {
-      Plane(int plane,
-            int framebuffer,
-            const gfx::Rect& bounds,
-            const gfx::Rect& src_rect);
-      ~Plane();
-      int plane;
-      int framebuffer;
-      gfx::Rect bounds;
-      gfx::Rect src_rect;
-    };
-    std::vector<Plane> planes;
   };
   // In the case of non-atomic operation, this info will be used for
   // pageflipping.
@@ -158,6 +145,8 @@ class HardwareDisplayPlaneManager {
 
   bool InitializeCrtcProperties(DrmDevice* drm);
 
+  virtual bool InitializePlanes(DrmDevice* drm) = 0;
+
   virtual bool SetPlaneData(HardwareDisplayPlaneList* plane_list,
                             HardwareDisplayPlane* hw_plane,
                             const DrmOverlayPlane& overlay,
@@ -196,6 +185,8 @@ class HardwareDisplayPlaneManager {
   // calls to control it. Not owned.
   DrmDevice* drm_;
 
+  bool has_universal_planes_ = false;
+
   std::vector<std::unique_ptr<HardwareDisplayPlane>> planes_;
   std::vector<CrtcProperties> crtc_properties_;
   std::vector<uint32_t> supported_formats_;

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

@@ -229,6 +229,24 @@ bool HardwareDisplayPlaneManagerAtomic::SetPlaneData(
   return true;
 }
 
+bool HardwareDisplayPlaneManagerAtomic::InitializePlanes(DrmDevice* drm) {
+  ScopedDrmPlaneResPtr plane_resources = drm->GetPlaneResources();
+  if (!plane_resources) {
+    PLOG(ERROR) << "Failed to get plane resources.";
+    return false;
+  }
+
+  for (uint32_t i = 0; i < plane_resources->count_planes; ++i) {
+    std::unique_ptr<HardwareDisplayPlane> plane(
+        CreatePlane(plane_resources->planes[i]));
+
+    if (plane->Initialize(drm))
+      planes_.push_back(std::move(plane));
+  }
+
+  return true;
+}
+
 std::unique_ptr<HardwareDisplayPlane>
 HardwareDisplayPlaneManagerAtomic::CreatePlane(uint32_t plane_id) {
   return std::make_unique<HardwareDisplayPlaneAtomic>(plane_id);

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

@@ -41,6 +41,7 @@ class HardwareDisplayPlaneManagerAtomic : public HardwareDisplayPlaneManager {
                     CrtcController* crtc) override;
 
  private:
+  bool InitializePlanes(DrmDevice* drm) override;
   std::unique_ptr<HardwareDisplayPlane> CreatePlane(uint32_t plane_id) override;
   bool CommitColorMatrix(const CrtcProperties& crtc_props) override;
   bool CommitGammaCorrection(const CrtcProperties& crtc_props) override;

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

@@ -15,6 +15,7 @@
 #include "ui/ozone/platform/drm/gpu/crtc_controller.h"
 #include "ui/ozone/platform/drm/gpu/drm_device.h"
 #include "ui/ozone/platform/drm/gpu/hardware_display_plane.h"
+#include "ui/ozone/platform/drm/gpu/hardware_display_plane_dummy.h"
 #include "ui/ozone/platform/drm/gpu/page_flip_request.h"
 #include "ui/ozone/platform/drm/gpu/scanout_buffer.h"
 
@@ -56,25 +57,9 @@ bool HardwareDisplayPlaneManagerLegacy::Commit(
   }
   if (plane_list->plane_list.empty())  // No assigned planes, nothing to do.
     return true;
+
   bool ret = true;
-  // The order of operations here (set new planes, pageflip, clear old planes)
-  // is designed to minimze the chance of a significant artifact occurring.
-  // The planes must be updated first because the main plane no longer contains
-  // their content. The old planes are removed last because the previous primary
-  // plane used them as overlays and thus didn't contain their content, so we
-  // must first flip to the new primary plane, which does. The error here will
-  // be the delta of (new contents, old contents), but it should be barely
-  // noticeable.
   for (const auto& flip : plane_list->legacy_page_flips) {
-    for (const auto& plane : flip.planes) {
-      if (!drm_->PageFlipOverlay(flip.crtc_id, plane.framebuffer, plane.bounds,
-                                 plane.src_rect, plane.plane)) {
-        PLOG(ERROR) << "Cannot display plane on overlay: crtc=" << flip.crtc
-                    << " plane=" << plane.plane;
-        ret = false;
-        break;
-      }
-    }
     if (!drm_->PageFlip(flip.crtc_id, flip.framebuffer, page_flip_request)) {
       // 1) Permission Denied is a legitimate error.
       // 2) Device or resource busy is possible if we're page flipping a
@@ -91,20 +76,6 @@ bool HardwareDisplayPlaneManagerLegacy::Commit(
       }
     }
   }
-  // For each element in |old_plane_list|, if it hasn't been reclaimed (by
-  // this or any other HDPL), clear the overlay contents.
-  for (HardwareDisplayPlane* plane : plane_list->old_plane_list) {
-    if (!plane->in_use() && (plane->type() != HardwareDisplayPlane::kDummy)) {
-      // This plane is being released, so we need to zero it.
-      if (!drm_->PageFlipOverlay(plane->owning_crtc(), 0, gfx::Rect(),
-                                 gfx::Rect(), plane->id())) {
-        PLOG(ERROR) << "Cannot free overlay: crtc=" << plane->owning_crtc()
-                    << " plane=" << plane->id();
-        ret = false;
-        break;
-      }
-    }
-  }
 
   if (ret) {
     plane_list->plane_list.swap(plane_list->old_plane_list);
@@ -154,6 +125,50 @@ void HardwareDisplayPlaneManagerLegacy::RequestPlanesReadyCallback(
       std::move(callback));
 }
 
+bool HardwareDisplayPlaneManagerLegacy::InitializePlanes(DrmDevice* drm) {
+  ScopedDrmPlaneResPtr plane_resources = drm->GetPlaneResources();
+  if (!plane_resources) {
+    PLOG(ERROR) << "Failed to get plane resources.";
+    return false;
+  }
+
+  for (uint32_t i = 0; i < plane_resources->count_planes; ++i) {
+    std::unique_ptr<HardwareDisplayPlane> plane(
+        CreatePlane(plane_resources->planes[i]));
+
+    if (!plane->Initialize(drm))
+      continue;
+
+    // Overlays are not supported on the legacy path, so ignore all overlay
+    // planes.
+    if (plane->type() == HardwareDisplayPlane::kOverlay)
+      continue;
+
+    planes_.push_back(std::move(plane));
+  }
+
+  // https://crbug.com/464085: if driver reports no primary planes for a crtc,
+  // create a dummy plane for which we can assign exactly one overlay.
+  if (!has_universal_planes_) {
+    for (size_t i = 0; i < crtc_properties_.size(); ++i) {
+      uint32_t id = crtc_properties_[i].id - 1;
+      if (std::find_if(
+              planes_.begin(), planes_.end(),
+              [id](const std::unique_ptr<HardwareDisplayPlane>& plane) {
+                return plane->id() == id;
+              }) == planes_.end()) {
+        std::unique_ptr<HardwareDisplayPlane> dummy_plane(
+            new HardwareDisplayPlaneDummy(id, 1 << i));
+        if (dummy_plane->Initialize(drm)) {
+          planes_.push_back(std::move(dummy_plane));
+        }
+      }
+    }
+  }
+
+  return true;
+}
+
 bool HardwareDisplayPlaneManagerLegacy::SetPlaneData(
     HardwareDisplayPlaneList* plane_list,
     HardwareDisplayPlane* hw_plane,
@@ -164,18 +179,16 @@ bool HardwareDisplayPlaneManagerLegacy::SetPlaneData(
   // Legacy modesetting rejects transforms.
   if (overlay.plane_transform != gfx::OVERLAY_TRANSFORM_NONE)
     return false;
-  if ((hw_plane->type() == HardwareDisplayPlane::kDummy) ||
-      plane_list->legacy_page_flips.empty() ||
+
+  if (plane_list->legacy_page_flips.empty() ||
       plane_list->legacy_page_flips.back().crtc_id != crtc_id) {
     plane_list->legacy_page_flips.push_back(
         HardwareDisplayPlaneList::PageFlipInfo(
             crtc_id, overlay.buffer->GetOpaqueFramebufferId(), crtc));
   } else {
-    plane_list->legacy_page_flips.back().planes.push_back(
-        HardwareDisplayPlaneList::PageFlipInfo::Plane(
-            hw_plane->id(), overlay.buffer->GetOpaqueFramebufferId(),
-            overlay.display_bounds, src_rect));
+    return false;
   }
+
   return true;
 }
 

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

@@ -35,6 +35,7 @@ class HardwareDisplayPlaneManagerLegacy : public HardwareDisplayPlaneManager {
       base::OnceCallback<void(DrmOverlayPlaneList)> callback) override;
 
  protected:
+  bool InitializePlanes(DrmDevice* drm) override;
   bool SetPlaneData(HardwareDisplayPlaneList* plane_list,
                     HardwareDisplayPlane* hw_plane,
                     const DrmOverlayPlane& overlay,

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

@@ -395,18 +395,26 @@ bool MockDrmDevice::CommitProperties(
     uint32_t flags,
     uint32_t crtc_count,
     scoped_refptr<PageFlipRequest> page_flip_request) {
+  commit_count_++;
+  if (!commit_expectation_)
+    return false;
+
   for (uint32_t i = 0; i < request->cursor; ++i) {
     EXPECT_TRUE(ValidatePropertyValue(request->items[i].property_id,
                                       request->items[i].value));
+  }
+
+  if (!page_flip_request)
+    return true;
+
+  callbacks_.push(page_flip_request->AddPageFlip());
+  // Only update values if not testing.
+  for (uint32_t i = 0; i < request->cursor; ++i) {
     EXPECT_TRUE(UpdateProperty(request->items[i].object_id,
                                request->items[i].property_id,
                                request->items[i].value));
   }
 
-  commit_count_++;
-  if (page_flip_request) {
-    callbacks_.push(page_flip_request->AddPageFlip());
-  }
   return true;
 }
 

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

@@ -78,6 +78,7 @@ class MockDrmDevice : public DrmDevice {
   void set_legacy_gamma_ramp_expectation(bool state) {
     legacy_gamma_ramp_expectation_ = state;
   }
+  void set_commit_expectation(bool state) { commit_expectation_ = state; }
 
   uint32_t current_framebuffer() const { return current_framebuffer_; }
 
@@ -199,6 +200,7 @@ class MockDrmDevice : public DrmDevice {
   bool page_flip_expectation_;
   bool create_dumb_buffer_expectation_;
   bool legacy_gamma_ramp_expectation_ = false;
+  bool commit_expectation_ = true;
 
   uint32_t current_framebuffer_;