Skip to content

Commit 59bb481

Browse files
author
Roman Stratiienko
committed
drm_hwcomposer: Add non-blocking commit support
This change fixes FPS drop on multidisplay devices. Also in some cases it gives graphics pipeline more free time to draw in advance. Signed-off-by: Roman Stratiienko <[email protected]>
1 parent dd21494 commit 59bb481

File tree

2 files changed

+167
-22
lines changed

2 files changed

+167
-22
lines changed

drm/DrmAtomicStateManager.cpp

Lines changed: 116 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
* limitations under the License.
1515
*/
1616

17+
#undef NDEBUG /* Required for assert to work */
18+
1719
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
1820
#define LOG_TAG "hwc-drm-atomic-state-manager"
1921

@@ -26,6 +28,7 @@
2628
#include <utils/Trace.h>
2729

2830
#include <array>
31+
#include <cassert>
2932
#include <cstdlib>
3033
#include <ctime>
3134
#include <sstream>
@@ -70,13 +73,15 @@ auto DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) -> int {
7073
return -ENOMEM;
7174
}
7275

73-
int64_t out_fence = -1;
74-
if (crtc->GetOutFencePtrProperty() &&
75-
!crtc->GetOutFencePtrProperty().AtomicSet(*pset, uint64_t(&out_fence))) {
76+
int out_fence = -1;
77+
if (!crtc->GetOutFencePtrProperty().AtomicSet(*pset, uint64_t(&out_fence))) {
7678
return -EINVAL;
7779
}
7880

81+
bool nonblock = true;
82+
7983
if (args.active) {
84+
nonblock = false;
8085
new_frame_state.crtc_active_state = *args.active;
8186
if (!crtc->GetActiveProperty().AtomicSet(*pset, *args.active ? 1 : 0) ||
8287
!connector->GetCrtcIdProperty().AtomicSet(*pset, crtc->GetId())) {
@@ -100,7 +105,6 @@ auto DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) -> int {
100105
auto unused_planes = new_frame_state.used_planes;
101106

102107
if (args.composition) {
103-
new_frame_state.used_framebuffers.clear();
104108
new_frame_state.used_planes.clear();
105109

106110
for (auto &joining : args.composition->plan) {
@@ -130,31 +134,126 @@ auto DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) -> int {
130134
}
131135

132136
uint32_t flags = DRM_MODE_ATOMIC_ALLOW_MODESET;
133-
if (args.test_only)
134-
flags |= DRM_MODE_ATOMIC_TEST_ONLY;
137+
138+
if (args.test_only) {
139+
return drmModeAtomicCommit(drm->GetFd(), pset.get(),
140+
flags | DRM_MODE_ATOMIC_TEST_ONLY, drm);
141+
}
142+
143+
if (last_present_fence_) {
144+
ATRACE_NAME("WaitPriorFramePresented");
145+
146+
constexpr int kTimeoutMs = 500;
147+
int err = sync_wait(last_present_fence_.Get(), kTimeoutMs);
148+
if (err != 0) {
149+
ALOGE("sync_wait(fd=%i) returned: %i (errno: %i)",
150+
last_present_fence_.Get(), err, errno);
151+
}
152+
153+
CleanupPriorFrameResources();
154+
}
155+
156+
if (nonblock) {
157+
flags |= DRM_MODE_ATOMIC_NONBLOCK;
158+
}
135159

136160
int err = drmModeAtomicCommit(drm->GetFd(), pset.get(), flags, drm);
161+
137162
if (err != 0) {
138-
if (!args.test_only)
139-
ALOGE("Failed to commit pset ret=%d\n", err);
163+
ALOGE("Failed to commit pset ret=%d\n", err);
140164
return err;
141165
}
142166

143-
if (!args.test_only) {
144-
if (args.display_mode) {
145-
/* TODO(nobody): we still need this for synthetic vsync, remove after
146-
* vsync reworked */
147-
connector->SetActiveMode(*args.display_mode);
167+
if (nonblock) {
168+
last_present_fence_ = UniqueFd::Dup(out_fence);
169+
staged_frame_state_ = std::move(new_frame_state);
170+
frames_staged_++;
171+
ptt_->Notify();
172+
} else {
173+
active_frame_state_ = std::move(new_frame_state);
174+
}
175+
176+
if (args.display_mode) {
177+
/* TODO(nobody): we still need this for synthetic vsync, remove after
178+
* vsync reworked */
179+
connector->SetActiveMode(*args.display_mode);
180+
}
181+
182+
args.out_fence = UniqueFd(out_fence);
183+
184+
return 0;
185+
}
186+
187+
PresentTrackerThread::PresentTrackerThread(DrmAtomicStateManager *st_man)
188+
: st_man_(st_man),
189+
mutex_(&st_man_->pipe_->device->GetResMan().GetMainLock()) {
190+
pt_ = std::thread(&PresentTrackerThread::PresentTrackerThreadFn, this);
191+
}
192+
193+
PresentTrackerThread::~PresentTrackerThread() {
194+
ALOGI("PresentTrackerThread successfully destroyed");
195+
}
196+
197+
void PresentTrackerThread::PresentTrackerThreadFn() {
198+
/* object should be destroyed on thread exit */
199+
auto self = std::unique_ptr<PresentTrackerThread>(this);
200+
201+
int tracking_at_the_moment = -1;
202+
203+
for (;;) {
204+
UniqueFd present_fence;
205+
206+
{
207+
std::unique_lock lk(*mutex_);
208+
cv_.wait(lk, [&] {
209+
return st_man_ == nullptr ||
210+
st_man_->frames_staged_ > tracking_at_the_moment;
211+
});
212+
213+
if (st_man_ == nullptr) {
214+
break;
215+
}
216+
217+
tracking_at_the_moment = st_man_->frames_staged_;
218+
219+
present_fence = UniqueFd::Dup(st_man_->last_present_fence_.Get());
220+
if (!present_fence) {
221+
continue;
222+
}
148223
}
149224

150-
active_frame_state_ = std::move(new_frame_state);
225+
{
226+
ATRACE_NAME("AsyncWaitForBuffersSwap");
227+
constexpr int kTimeoutMs = 500;
228+
int err = sync_wait(present_fence.Get(), kTimeoutMs);
229+
if (err != 0) {
230+
ALOGE("sync_wait(fd=%i) returned: %i (errno: %i)", present_fence.Get(),
231+
err, errno);
232+
}
233+
}
151234

152-
if (crtc->GetOutFencePtrProperty()) {
153-
args.out_fence = UniqueFd((int)out_fence);
235+
{
236+
std::unique_lock lk(*mutex_);
237+
if (st_man_ == nullptr) {
238+
break;
239+
}
240+
241+
/* If resources is already cleaned-up by main thread, skip */
242+
if (tracking_at_the_moment > st_man_->frames_tracked_) {
243+
st_man_->CleanupPriorFrameResources();
244+
}
154245
}
155246
}
247+
}
156248

157-
return 0;
249+
void DrmAtomicStateManager::CleanupPriorFrameResources() {
250+
assert(frames_staged_ - frames_tracked_ == 1);
251+
assert(last_present_fence_);
252+
253+
ATRACE_NAME("CleanupPriorFrameResources");
254+
frames_tracked_++;
255+
active_frame_state_ = std::move(staged_frame_state_);
256+
last_present_fence_ = {};
158257
}
159258

160259
auto DrmAtomicStateManager::ExecuteAtomicCommit(AtomicCommitArgs &args) -> int {

drm/DrmAtomicStateManager.h

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,45 @@ struct AtomicCommitArgs {
4949
}
5050
};
5151

52+
class PresentTrackerThread {
53+
public:
54+
explicit PresentTrackerThread(DrmAtomicStateManager *st_man);
55+
56+
~PresentTrackerThread();
57+
58+
void Stop() {
59+
/* Exit thread by signalling that object is no longer valid */
60+
st_man_ = nullptr;
61+
Notify();
62+
pt_.detach();
63+
}
64+
65+
void Notify() {
66+
cv_.notify_all();
67+
}
68+
69+
private:
70+
DrmAtomicStateManager *st_man_{};
71+
72+
void PresentTrackerThreadFn();
73+
74+
std::condition_variable cv_;
75+
std::thread pt_;
76+
std::mutex *mutex_;
77+
};
78+
5279
class DrmAtomicStateManager {
80+
friend class PresentTrackerThread;
81+
5382
public:
54-
explicit DrmAtomicStateManager(DrmDisplayPipeline *pipe) : pipe_(pipe){};
83+
explicit DrmAtomicStateManager(DrmDisplayPipeline *pipe)
84+
: pipe_(pipe),
85+
ptt_(std::make_unique<PresentTrackerThread>(this).release()){};
86+
5587
DrmAtomicStateManager(const DrmAtomicStateManager &) = delete;
56-
~DrmAtomicStateManager() = default;
88+
~DrmAtomicStateManager() {
89+
ptt_->Stop();
90+
}
5791

5892
auto ExecuteAtomicCommit(AtomicCommitArgs &args) -> int;
5993
auto ActivateDisplayUsingDPMS() -> int;
@@ -70,20 +104,32 @@ class DrmAtomicStateManager {
70104

71105
DrmModeUserPropertyBlobUnique mode_blob;
72106

107+
int release_fence_pt_index{};
108+
73109
/* To avoid setting the inactive state twice, which will fail the commit */
74110
bool crtc_active_state{};
75111
} active_frame_state_;
76112

77113
auto NewFrameState() -> KmsState {
114+
auto *prev_frame_state = &active_frame_state_;
78115
return (KmsState){
79-
.used_planes = active_frame_state_.used_planes,
80-
.used_framebuffers = active_frame_state_.used_framebuffers,
81-
.crtc_active_state = active_frame_state_.crtc_active_state,
116+
.used_planes = prev_frame_state->used_planes,
117+
.crtc_active_state = prev_frame_state->crtc_active_state,
82118
};
83119
}
84120

85121
DrmDisplayPipeline *const pipe_;
122+
123+
void CleanupPriorFrameResources();
124+
125+
/* Present (swap) tracking */
126+
PresentTrackerThread *ptt_;
127+
KmsState staged_frame_state_;
128+
UniqueFd last_present_fence_;
129+
int frames_staged_{};
130+
int frames_tracked_{};
86131
};
132+
87133
} // namespace android
88134

89135
#endif // ANDROID_DRM_DISPLAY_COMPOSITOR_H_

0 commit comments

Comments
 (0)