Skip to content

Commit

Permalink
Keep track of modeset_sequence_id for pageflips
Browse files Browse the repository at this point in the history
After successfully committing a pageflip, keep track of the current
modeset_sequence_id when the pageflip was committed. If there is a
modeset before the pageflip is presented, check the current
modeset_sequence_id and avoid calling ResetModesetBufferOfCrtc. Doing so
with a stale pageflip could end up releasing the DrmFramebuffer that was
committed with the modeset, and end up releasing the framebuffer while
it's associated with the crtc.

(cherry picked from commit f7859e4)

Bug: b:183238601
Test: Toggle primary displays while in extended mode
Change-Id: I2c06942998fdadd82c645c4d5cbb200344cd4610
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2812918
Commit-Queue: Drew Davenport <ddavenport@chromium.org>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Reviewed-by: Michael Spang <spang@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#871374}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2823582
Auto-Submit: Drew Davenport <ddavenport@chromium.org>
Commit-Queue: Michael Spang <spang@chromium.org>
Cr-Commit-Position: refs/branch-heads/4472@{#40}
Cr-Branched-From: 3d60439-refs/heads/master@{#870763}
  • Loading branch information
drewdavenport authored and Chromium LUCI CQ committed Apr 13, 2021
1 parent ca09169 commit 3fdc682
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 4 deletions.
17 changes: 13 additions & 4 deletions ui/ozone/platform/drm/gpu/hardware_display_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,13 @@ namespace {

void CompletePageFlip(
base::WeakPtr<HardwareDisplayController> hardware_display_controller_,
int modeset_sequence,
PresentationOnceCallback callback,
DrmOverlayPlaneList plane_list,
const gfx::PresentationFeedback& presentation_feedback) {
if (hardware_display_controller_) {
hardware_display_controller_->OnPageFlipComplete(std::move(plane_list),
presentation_feedback);
hardware_display_controller_->OnPageFlipComplete(
modeset_sequence, std::move(plane_list), presentation_feedback);
}
std::move(callback).Run(presentation_feedback);
}
Expand Down Expand Up @@ -180,6 +181,7 @@ void HardwareDisplayController::SchedulePageFlip(
// Everything was submitted successfully, wait for asynchronous completion.
page_flip_request->TakeCallback(
base::BindOnce(&CompletePageFlip, weak_ptr_factory_.GetWeakPtr(),
GetDrmDevice()->modeset_sequence_id(),
std::move(presentation_callback), std::move(plane_list)));
page_flip_request_ = std::move(page_flip_request);
}
Expand Down Expand Up @@ -401,15 +403,22 @@ scoped_refptr<DrmDevice> HardwareDisplayController::GetDrmDevice() const {
}

void HardwareDisplayController::OnPageFlipComplete(
int modeset_sequence,
DrmOverlayPlaneList pending_planes,
const gfx::PresentationFeedback& presentation_feedback) {
if (!page_flip_request_)
return; // Modeset occured during this page flip.

time_of_last_flip_ = presentation_feedback.timestamp;
current_planes_ = std::move(pending_planes);

for (const auto& controller : crtc_controllers_) {
GetDrmDevice()->plane_manager()->ResetModesetBufferOfCrtc(
controller->crtc());
// Only reset the modeset buffer of the crtcs for pageflips that were
// committed after the modeset.
if (modeset_sequence == GetDrmDevice()->modeset_sequence_id()) {
GetDrmDevice()->plane_manager()->ResetModesetBufferOfCrtc(
controller->crtc());
}
}
page_flip_request_ = nullptr;
}
Expand Down
1 change: 1 addition & 0 deletions ui/ozone/platform/drm/gpu/hardware_display_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ class HardwareDisplayController {
scoped_refptr<DrmDevice> GetDrmDevice() const;

void OnPageFlipComplete(
int modeset_sequence,
DrmOverlayPlaneList pending_planes,
const gfx::PresentationFeedback& presentation_feedback);

Expand Down
48 changes: 48 additions & 0 deletions ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -945,3 +945,51 @@ TEST_F(HardwareDisplayControllerTest, Disable) {
// No plane should be in use.
ASSERT_EQ(0, planes_in_use);
}

TEST_F(HardwareDisplayControllerTest, PageflipAfterModeset) {
scoped_refptr<ui::DrmFramebuffer> buffer = CreateBuffer();
ui::DrmOverlayPlane plane1(buffer, nullptr);

ModesetWithPlane(plane1);

EXPECT_EQ(drm_->plane_manager()
->GetCrtcStateForCrtcId(kPrimaryCrtc)
.modeset_framebuffer,
buffer);

std::vector<ui::DrmOverlayPlane> planes;
planes.push_back(plane1.Clone());
SchedulePageFlip(std::move(planes));
drm_->RunCallbacks();

// modeset_framebuffer should be cleared after the pageflip is complete.
EXPECT_EQ(drm_->plane_manager()
->GetCrtcStateForCrtcId(kPrimaryCrtc)
.modeset_framebuffer,
nullptr);
}

TEST_F(HardwareDisplayControllerTest, PageflipBeforeModeset) {
scoped_refptr<ui::DrmFramebuffer> buffer = CreateBuffer();
ui::DrmOverlayPlane plane1(buffer, nullptr);

EXPECT_TRUE(ModesetWithPlane(ui::DrmOverlayPlane(CreateBuffer(), nullptr)));

std::vector<ui::DrmOverlayPlane> planes;
planes.push_back(plane1.Clone());
SchedulePageFlip(std::move(planes));

EXPECT_TRUE(ModesetWithPlane(plane1));
EXPECT_EQ(drm_->plane_manager()
->GetCrtcStateForCrtcId(kPrimaryCrtc)
.modeset_framebuffer,
buffer);

// modeset_framebuffer should not be cleared when a pageflip callback is run
// after a modeset
drm_->RunCallbacks();
EXPECT_EQ(drm_->plane_manager()
->GetCrtcStateForCrtcId(kPrimaryCrtc)
.modeset_framebuffer,
buffer);
}

0 comments on commit 3fdc682

Please sign in to comment.