Commit a681c5fd authored by Daniele Castagna's avatar Daniele Castagna Committed by Commit Bot

ozone: Disable primary plane when HDC gets disabled

When HardwareDisplayController get disabled we should disable all
planes, including the primary, by setting the fb to 0.
Not doing so can have privacy implications, where we might end up
reading back the last fb when modesetting again and show to the user
contents that should not be seen.

In the past, when legacy modesetting was used, the fb associated with
the crtc was set to 0 when calling drmModeSetCrtc. After that we'd
disable the other planes but the primary.

Since we moved to the atomic API for modesetting, we now need to
explicitly set to 0 the fb of the primary plane along the other planes.


Bug: 1102859
Test: Close/open the lid on krane, no old contents are visible.
Change-Id: Ib93347b59cc847898c0713f81ad0903773699b07
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412974
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: default avatarMitsuru Oshima <oshima@chromium.org>
Reviewed-by: default avatarGil Dekel <gildekel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807476}
parent 74eda5a3
...@@ -400,11 +400,10 @@ TEST_F(HardwareDisplayControllerTest, PlaneStateAfterRemoveCrtc) { ...@@ -400,11 +400,10 @@ TEST_F(HardwareDisplayControllerTest, PlaneStateAfterRemoveCrtc) {
EXPECT_EQ(kPrimaryCrtc, primary_crtc_plane->owning_crtc()); EXPECT_EQ(kPrimaryCrtc, primary_crtc_plane->owning_crtc());
EXPECT_EQ(kSecondaryCrtc, secondary_crtc_plane->owning_crtc()); EXPECT_EQ(kSecondaryCrtc, secondary_crtc_plane->owning_crtc());
// Removing the crtc should not free the plane or change ownership. // Removing the crtc should free the plane.
std::unique_ptr<ui::CrtcController> crtc = std::unique_ptr<ui::CrtcController> crtc =
controller_->RemoveCrtc(drm_, kPrimaryCrtc); controller_->RemoveCrtc(drm_, kPrimaryCrtc);
EXPECT_TRUE(primary_crtc_plane->in_use()); EXPECT_FALSE(primary_crtc_plane->in_use());
EXPECT_EQ(kPrimaryCrtc, primary_crtc_plane->owning_crtc());
EXPECT_TRUE(secondary_crtc_plane->in_use()); EXPECT_TRUE(secondary_crtc_plane->in_use());
EXPECT_EQ(kSecondaryCrtc, secondary_crtc_plane->owning_crtc()); EXPECT_EQ(kSecondaryCrtc, secondary_crtc_plane->owning_crtc());
...@@ -414,8 +413,7 @@ TEST_F(HardwareDisplayControllerTest, PlaneStateAfterRemoveCrtc) { ...@@ -414,8 +413,7 @@ TEST_F(HardwareDisplayControllerTest, PlaneStateAfterRemoveCrtc) {
drm_->RunCallbacks(); drm_->RunCallbacks();
EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_); EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);
EXPECT_EQ(2, page_flips_); EXPECT_EQ(2, page_flips_);
EXPECT_TRUE(primary_crtc_plane->in_use()); EXPECT_FALSE(primary_crtc_plane->in_use());
EXPECT_EQ(kPrimaryCrtc, primary_crtc_plane->owning_crtc());
EXPECT_TRUE(secondary_crtc_plane->in_use()); EXPECT_TRUE(secondary_crtc_plane->in_use());
EXPECT_EQ(kSecondaryCrtc, secondary_crtc_plane->owning_crtc()); EXPECT_EQ(kSecondaryCrtc, secondary_crtc_plane->owning_crtc());
} }
...@@ -474,8 +472,7 @@ TEST_F(HardwareDisplayControllerTest, PlaneStateAfterAddCrtc) { ...@@ -474,8 +472,7 @@ TEST_F(HardwareDisplayControllerTest, PlaneStateAfterAddCrtc) {
drm_->RunCallbacks(); drm_->RunCallbacks();
EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_); EXPECT_EQ(gfx::SwapResult::SWAP_ACK, last_swap_result_);
EXPECT_EQ(2, page_flips_); EXPECT_EQ(2, page_flips_);
EXPECT_TRUE(primary_crtc_plane->in_use()); EXPECT_FALSE(primary_crtc_plane->in_use());
EXPECT_EQ(kPrimaryCrtc, primary_crtc_plane->owning_crtc());
// We reset state of plane here to test that the plane was actually added to // We reset state of plane here to test that the plane was actually added to
// hdc_controller. In which case, the right state should be set to plane // hdc_controller. In which case, the right state should be set to plane
...@@ -588,6 +585,6 @@ TEST_F(HardwareDisplayControllerTest, Disable) { ...@@ -588,6 +585,6 @@ TEST_F(HardwareDisplayControllerTest, Disable) {
if (plane->in_use()) if (plane->in_use())
planes_in_use++; planes_in_use++;
} }
// Only the primary plane is in use. // No plane should be in use.
ASSERT_EQ(1, planes_in_use); ASSERT_EQ(0, planes_in_use);
} }
...@@ -196,8 +196,6 @@ bool HardwareDisplayPlaneManagerAtomic::Commit( ...@@ -196,8 +196,6 @@ bool HardwareDisplayPlaneManagerAtomic::Commit(
bool HardwareDisplayPlaneManagerAtomic::DisableOverlayPlanes( bool HardwareDisplayPlaneManagerAtomic::DisableOverlayPlanes(
HardwareDisplayPlaneList* plane_list) { HardwareDisplayPlaneList* plane_list) {
for (HardwareDisplayPlane* plane : plane_list->old_plane_list) { for (HardwareDisplayPlane* plane : plane_list->old_plane_list) {
if (plane->type() != HardwareDisplayPlane::kOverlay)
continue;
plane->set_in_use(false); plane->set_in_use(false);
plane->set_owning_crtc(0); plane->set_owning_crtc(0);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment