drm_hwcomposer: Simplify DrmAtomicStateManager thread usage
1. Remove PresentTracker class.
2. Use separate mutex for thread synchronization. Before, we have always
experienced rescheduling due to calling condvar::notify inside the
mutex, which consumes some number of CPU cycles.
Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>
diff --git a/drm/DrmAtomicStateManager.cpp b/drm/DrmAtomicStateManager.cpp
index cd2d69a..b5e4629 100644
--- a/drm/DrmAtomicStateManager.cpp
+++ b/drm/DrmAtomicStateManager.cpp
@@ -22,17 +22,10 @@
#include "DrmAtomicStateManager.h"
#include <drm/drm_mode.h>
-#include <pthread.h>
-#include <sched.h>
#include <sync/sync.h>
#include <utils/Trace.h>
-#include <array>
#include <cassert>
-#include <cstdlib>
-#include <ctime>
-#include <sstream>
-#include <vector>
#include "drm/DrmCrtc.h"
#include "drm/DrmDevice.h"
@@ -42,6 +35,17 @@
namespace android {
+auto DrmAtomicStateManager::CreateInstance(DrmDisplayPipeline *pipe)
+ -> std::shared_ptr<DrmAtomicStateManager> {
+ auto dasm = std::shared_ptr<DrmAtomicStateManager>(
+ new DrmAtomicStateManager());
+
+ dasm->pipe_ = pipe;
+ std::thread(&DrmAtomicStateManager::ThreadFn, dasm.get(), dasm).detach();
+
+ return dasm;
+}
+
// NOLINTNEXTLINE (readability-function-cognitive-complexity): Fixme
auto DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) -> int {
// NOLINTNEXTLINE(misc-const-correctness)
@@ -167,10 +171,13 @@
}
if (nonblock) {
- last_present_fence_ = UniqueFd::Dup(out_fence);
- staged_frame_state_ = std::move(new_frame_state);
- frames_staged_++;
- ptt_->Notify();
+ {
+ const std::unique_lock lock(mutex_);
+ last_present_fence_ = UniqueFd::Dup(out_fence);
+ staged_frame_state_ = std::move(new_frame_state);
+ frames_staged_++;
+ }
+ cv_.notify_all();
} else {
active_frame_state_ = std::move(new_frame_state);
}
@@ -180,42 +187,29 @@
return 0;
}
-PresentTrackerThread::PresentTrackerThread(DrmAtomicStateManager *st_man)
- : st_man_(st_man),
- mutex_(&st_man_->pipe_->device->GetResMan().GetMainLock()) {
- pt_ = std::thread(&PresentTrackerThread::PresentTrackerThreadFn, this);
-}
-
-PresentTrackerThread::~PresentTrackerThread() {
- ALOGI("PresentTrackerThread successfully destroyed");
-}
-
-void PresentTrackerThread::PresentTrackerThreadFn() {
- /* object should be destroyed on thread exit */
- auto self = std::unique_ptr<PresentTrackerThread>(this);
-
+void DrmAtomicStateManager::ThreadFn(
+ const std::shared_ptr<DrmAtomicStateManager> &dasm) {
int tracking_at_the_moment = -1;
+ auto &main_mutex = pipe_->device->GetResMan().GetMainLock();
for (;;) {
UniqueFd present_fence;
{
- std::unique_lock lk(*mutex_);
- cv_.wait(lk, [&] {
- return st_man_ == nullptr ||
- st_man_->frames_staged_ > tracking_at_the_moment;
- });
+ std::unique_lock lk(mutex_);
+ cv_.wait(lk);
- if (st_man_ == nullptr) {
+ if (exit_thread_ || dasm.use_count() == 1)
break;
- }
- tracking_at_the_moment = st_man_->frames_staged_;
-
- present_fence = UniqueFd::Dup(st_man_->last_present_fence_.Get());
- if (!present_fence) {
+ if (frames_staged_ <= tracking_at_the_moment)
continue;
- }
+
+ tracking_at_the_moment = frames_staged_;
+
+ present_fence = UniqueFd::Dup(last_present_fence_.Get());
+ if (!present_fence)
+ continue;
}
{
@@ -230,17 +224,18 @@
}
{
- const std::unique_lock lk(*mutex_);
- if (st_man_ == nullptr) {
+ const std::unique_lock mlk(main_mutex);
+ const std::unique_lock lk(mutex_);
+ if (exit_thread_)
break;
- }
/* If resources is already cleaned-up by main thread, skip */
- if (tracking_at_the_moment > st_man_->frames_tracked_) {
- st_man_->CleanupPriorFrameResources();
- }
+ if (tracking_at_the_moment > frames_tracked_)
+ CleanupPriorFrameResources();
}
}
+
+ ALOGI("DrmAtomicStateManager thread exit");
}
void DrmAtomicStateManager::CleanupPriorFrameResources() {
diff --git a/drm/DrmAtomicStateManager.h b/drm/DrmAtomicStateManager.h
index b0b85ac..5f19bcc 100644
--- a/drm/DrmAtomicStateManager.h
+++ b/drm/DrmAtomicStateManager.h
@@ -14,16 +14,12 @@
* limitations under the License.
*/
-#ifndef ANDROID_DRM_ATOMIC_STATE_MANAGER_H_
-#define ANDROID_DRM_ATOMIC_STATE_MANAGER_H_
+#pragma once
#include <pthread.h>
-#include <functional>
#include <memory>
#include <optional>
-#include <sstream>
-#include <tuple>
#include "compositor/DrmKmsPlan.h"
#include "compositor/LayerData.h"
@@ -49,50 +45,26 @@
}
};
-class PresentTrackerThread {
- public:
- explicit PresentTrackerThread(DrmAtomicStateManager *st_man);
-
- ~PresentTrackerThread();
-
- void Stop() {
- /* Exit thread by signalling that object is no longer valid */
- st_man_ = nullptr;
- Notify();
- pt_.detach();
- }
-
- void Notify() {
- cv_.notify_all();
- }
-
- private:
- DrmAtomicStateManager *st_man_{};
-
- void PresentTrackerThreadFn();
-
- std::condition_variable cv_;
- std::thread pt_;
- std::mutex *mutex_;
-};
-
class DrmAtomicStateManager {
- friend class PresentTrackerThread;
-
public:
- explicit DrmAtomicStateManager(DrmDisplayPipeline *pipe)
- : pipe_(pipe),
- ptt_(std::make_unique<PresentTrackerThread>(this).release()){};
+ static auto CreateInstance(DrmDisplayPipeline *pipe)
+ -> std::shared_ptr<DrmAtomicStateManager>;
- DrmAtomicStateManager(const DrmAtomicStateManager &) = delete;
- ~DrmAtomicStateManager() {
- ptt_->Stop();
- }
+ ~DrmAtomicStateManager() = default;
auto ExecuteAtomicCommit(AtomicCommitArgs &args) -> int;
auto ActivateDisplayUsingDPMS() -> int;
+ void StopThread() {
+ {
+ const std::unique_lock lock(mutex_);
+ exit_thread_ = true;
+ }
+ cv_.notify_all();
+ }
+
private:
+ DrmAtomicStateManager() = default;
auto CommitFrame(AtomicCommitArgs &args) -> int;
struct KmsState {
@@ -118,18 +90,19 @@
};
}
- DrmDisplayPipeline *const pipe_;
+ DrmDisplayPipeline *pipe_{};
void CleanupPriorFrameResources();
- /* Present (swap) tracking */
- PresentTrackerThread *ptt_;
KmsState staged_frame_state_;
UniqueFd last_present_fence_;
int frames_staged_{};
int frames_tracked_{};
+
+ void ThreadFn(const std::shared_ptr<DrmAtomicStateManager> &dasm);
+ std::condition_variable cv_;
+ std::mutex mutex_;
+ bool exit_thread_{};
};
} // namespace android
-
-#endif // ANDROID_DRM_DISPLAY_COMPOSITOR_H_
diff --git a/drm/DrmDisplayPipeline.cpp b/drm/DrmDisplayPipeline.cpp
index 7973529..1a8ad5b 100644
--- a/drm/DrmDisplayPipeline.cpp
+++ b/drm/DrmDisplayPipeline.cpp
@@ -98,7 +98,7 @@
return {};
}
- pipe->atomic_state_manager = std::make_unique<DrmAtomicStateManager>(
+ pipe->atomic_state_manager = DrmAtomicStateManager::CreateInstance(
pipe.get());
return pipe;
@@ -189,4 +189,9 @@
return planes;
}
+DrmDisplayPipeline::~DrmDisplayPipeline() {
+ if (atomic_state_manager)
+ atomic_state_manager->StopThread();
+}
+
} // namespace android
diff --git a/drm/DrmDisplayPipeline.h b/drm/DrmDisplayPipeline.h
index a3958e1..cf64a36 100644
--- a/drm/DrmDisplayPipeline.h
+++ b/drm/DrmDisplayPipeline.h
@@ -74,6 +74,8 @@
auto GetUsablePlanes()
-> std::vector<std::shared_ptr<BindingOwner<DrmPlane>>>;
+ ~DrmDisplayPipeline();
+
DrmDevice *device;
std::shared_ptr<BindingOwner<DrmConnector>> connector;
@@ -81,7 +83,7 @@
std::shared_ptr<BindingOwner<DrmCrtc>> crtc;
std::shared_ptr<BindingOwner<DrmPlane>> primary_plane;
- std::unique_ptr<DrmAtomicStateManager> atomic_state_manager;
+ std::shared_ptr<DrmAtomicStateManager> atomic_state_manager;
};
} // namespace android