Ozone: HDC should only keep track of planes owned by it’s crtc controllers

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

Index: ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc
diff --git a/ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc b/ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc
index 761438ef80b04ab81fe117e4ee9623ddb52c4153..1008352497e3f5f93204945bc98cde79993f965a 100644
--- a/ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc
+++ b/ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc
@@ -273,10 +273,89 @@ TEST_F(HardwareDisplayControllerTest, PlaneStateAfterRemoveCrtc) {
       owned_plane = plane;
   ASSERT_TRUE(owned_plane != nullptr);
   EXPECT_EQ(kPrimaryCrtc, owned_plane->owning_crtc());
-  // Removing the crtc should free the plane.
+  // Removing the crtc should not free the plane or change ownership.
   scoped_ptr<ui::CrtcController> crtc =
       controller_->RemoveCrtc(drm_, kPrimaryCrtc);
+  EXPECT_TRUE(owned_plane->in_use());
+  EXPECT_EQ(kPrimaryCrtc, owned_plane->owning_crtc());
+  // Check that controller doesn't effect the state of removed plane in
+  // subsequent page flip.
+  EXPECT_TRUE(controller_->SchedulePageFlip(
+      planes, false, false,
+      base::Bind(&HardwareDisplayControllerTest::PageFlipCallback,
+                 base::Unretained(this))));
+  drm_->RunCallbacks();
+  EXPECT_TRUE(owned_plane->in_use());
+  EXPECT_EQ(kPrimaryCrtc, owned_plane->owning_crtc());
+}
+
+TEST_F(HardwareDisplayControllerTest, PlaneStateAfterDestroyingCrtc) {
+  ui::OverlayPlane plane1(scoped_refptr<ui::ScanoutBuffer>(
+      new MockScanoutBuffer(kDefaultModeSize)));
+  EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
+  std::vector<ui::OverlayPlane> planes =
+      std::vector<ui::OverlayPlane>(1, plane1);
+  EXPECT_TRUE(controller_->SchedulePageFlip(
+      planes, false, false,
+      base::Bind(&HardwareDisplayControllerTest::PageFlipCallback,
+                 base::Unretained(this))));
+  drm_->RunCallbacks();
+
+  const ui::HardwareDisplayPlane* owned_plane = nullptr;
+  for (const auto& plane : drm_->plane_manager()->planes())
+    if (plane->in_use())
+      owned_plane = plane;
+  ASSERT_TRUE(owned_plane != nullptr);
+  EXPECT_EQ(kPrimaryCrtc, owned_plane->owning_crtc());
+  scoped_ptr<ui::CrtcController> crtc =
+      controller_->RemoveCrtc(drm_, kPrimaryCrtc);
+  // Destroying crtc should free the plane.
+  crtc.reset();
+  uint32_t crtc_nullid = 0;
   EXPECT_FALSE(owned_plane->in_use());
+  EXPECT_EQ(crtc_nullid, owned_plane->owning_crtc());
+}
+
+TEST_F(HardwareDisplayControllerTest, PlaneStateAfterAddCrtc) {
+  ui::OverlayPlane plane1(scoped_refptr<ui::ScanoutBuffer>(
+      new MockScanoutBuffer(kDefaultModeSize)));
+  EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
+  std::vector<ui::OverlayPlane> planes =
+      std::vector<ui::OverlayPlane>(1, plane1);
+  EXPECT_TRUE(controller_->SchedulePageFlip(
+      planes, false, false,
+      base::Bind(&HardwareDisplayControllerTest::PageFlipCallback,
+                 base::Unretained(this))));
+  drm_->RunCallbacks();
+
+  ui::HardwareDisplayPlane* primary_crtc_plane = nullptr;
+  for (const auto& plane : drm_->plane_manager()->planes()) {
+    if (plane->in_use() && kPrimaryCrtc == plane->owning_crtc())
+      primary_crtc_plane = plane;
+  }
+
+  ASSERT_TRUE(primary_crtc_plane != nullptr);
+
+  scoped_ptr<ui::HardwareDisplayController> hdc_controller;
+  hdc_controller.reset(new ui::HardwareDisplayController(
+      controller_->RemoveCrtc(drm_, kPrimaryCrtc), controller_->origin()));
+  EXPECT_TRUE(controller_->SchedulePageFlip(
+      planes, false, false,
+      base::Bind(&HardwareDisplayControllerTest::PageFlipCallback,
+                 base::Unretained(this))));
+  drm_->RunCallbacks();
+  EXPECT_TRUE(primary_crtc_plane->in_use());
+  EXPECT_EQ(kPrimaryCrtc, primary_crtc_plane->owning_crtc());
+
+  primary_crtc_plane->set_in_use(false);
+  primary_crtc_plane->set_owning_crtc(0);

[dnicoara, 2015/08/19 16:08:46]

Do you need these 2 calls? set_in_use() is called in
HDPlaneManager::BeginFrame() and set_owning_crtc(0) shouldn't matter.

[kalyank, 2015/08/19 16:15:29]

This was to test that the plane was actually added to hdc_controller. If addcrtc
call fails to add the planes for some reason, the checks after page flip call
here should fail.

+  EXPECT_TRUE(hdc_controller->SchedulePageFlip(
+      planes, false, false,
+      base::Bind(&HardwareDisplayControllerTest::PageFlipCallback,
+                 base::Unretained(this))));
+  drm_->RunCallbacks();
+  EXPECT_TRUE(primary_crtc_plane->in_use());
+  EXPECT_EQ(kPrimaryCrtc, primary_crtc_plane->owning_crtc());
 }
 
 TEST_F(HardwareDisplayControllerTest, ModesetWhilePageFlipping) {