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>

5 files changed, 66 insertions, 90 deletions

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 @@ auto DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) -> int {
   }
 
   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 @@ auto DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) -> int {
   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_;
+      if (frames_staged_ <= tracking_at_the_moment)
+        continue;
+
+      tracking_at_the_moment = frames_staged_;
 
-      present_fence = UniqueFd::Dup(st_man_->last_present_fence_.Get());
-      if (!present_fence) {
+      present_fence = UniqueFd::Dup(last_present_fence_.Get());
+      if (!present_fence)
         continue;
-      }
     }
 
     {
@@ -230,17 +224,18 @@ void PresentTrackerThread::PresentTrackerThreadFn() {
     }
 
     {
-      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 @@ struct AtomicCommitArgs {
   }
 };
 
-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 @@ class DrmAtomicStateManager {
     };
   }
 
-  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 @@ static auto TryCreatePipeline(DrmDevice &dev, DrmConnector &connector,
     return {};
   }
 
-  pipe->atomic_state_manager = std::make_unique<DrmAtomicStateManager>(
+  pipe->atomic_state_manager = DrmAtomicStateManager::CreateInstance(
       pipe.get());
 
   return pipe;
@@ -189,4 +189,9 @@ auto DrmDisplayPipeline::GetUsablePlanes()
   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 @@ struct DrmDisplayPipeline {
   auto GetUsablePlanes()
       -> std::vector<std::shared_ptr<BindingOwner<DrmPlane>>>;
 
+  ~DrmDisplayPipeline();
+
   DrmDevice *device;
 
   std::shared_ptr<BindingOwner<DrmConnector>> connector;
@@ -81,7 +83,7 @@ struct DrmDisplayPipeline {
   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
diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h
index 68353e9..4ad57b7 100644
--- a/hwc2_device/HwcDisplay.h
+++ b/hwc2_device/HwcDisplay.h
@@ -20,6 +20,7 @@
 
 #include <atomic>
 #include <optional>
+#include <sstream>
 
 #include "HwcDisplayConfigs.h"
 #include "compositor/LayerData.h"