drm_hwcomposer: Tidy-up DrmCrtc class

Implement DrmCrtc instantiation through CreateInstance() static method,
which helps to reduce complexity of DrmDevice::Init() function.

Move CRTC-to-Display binding information to the DrmDevice class.

Move drm/DrmCrtc.h to Normal clang-tidy checks list by fixing
clang-tidy findings.

Signed-off-by: Roman Stratiienko <roman.o.stratiienko@globallogic.com>

11 files changed, 76 insertions, 104 deletions

diff --git a/.ci/Makefile b/.ci/Makefile
index 7f43a8a..e203673 100644
--- a/.ci/Makefile
+++ b/.ci/Makefile
@@ -29,7 +29,6 @@ TIDY_FILES_OVERRIDE := \
     drm/DrmDevice.h:COARSE                              \
     drm/DrmProperty.h:COARSE                            \
     drm/DrmConnector.h:COARSE                           \
-    drm/DrmCrtc.h:COARSE                                \
     drm/DrmUnique.h:FINE                                \
     drm/DrmEncoder.h:COARSE                             \
     drm/DrmConnector.cpp:COARSE                         \
diff --git a/compositor/DrmDisplayCompositor.cpp b/compositor/DrmDisplayCompositor.cpp
index c2e51ee..0cb1d39 100644
--- a/compositor/DrmDisplayCompositor.cpp
+++ b/compositor/DrmDisplayCompositor.cpp
@@ -99,15 +99,15 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int {
   }
 
   int64_t out_fence = -1;
-  if (crtc->out_fence_ptr_property() &&
-      !crtc->out_fence_ptr_property().AtomicSet(*pset, (uint64_t)&out_fence)) {
+  if (crtc->GetOutFencePtrProperty() &&
+      !crtc->GetOutFencePtrProperty().AtomicSet(*pset, (uint64_t)&out_fence)) {
     return -EINVAL;
   }
 
   if (args.active) {
     new_frame_state.crtc_active_state = *args.active;
-    if (!crtc->active_property().AtomicSet(*pset, *args.active) ||
-        !connector->crtc_id_property().AtomicSet(*pset, crtc->id())) {
+    if (!crtc->GetActiveProperty().AtomicSet(*pset, *args.active) ||
+        !connector->crtc_id_property().AtomicSet(*pset, crtc->GetId())) {
       return -EINVAL;
     }
   }
@@ -121,7 +121,7 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int {
       return -EINVAL;
     }
 
-    if (!crtc->mode_property().AtomicSet(*pset, *new_frame_state.mode_blob)) {
+    if (!crtc->GetModeProperty().AtomicSet(*pset, *new_frame_state.mode_blob)) {
       return -EINVAL;
     }
   }
@@ -154,7 +154,8 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int {
       auto &v = unused_planes;
       v.erase(std::remove(v.begin(), v.end(), plane), v.end());
 
-      if (plane->AtomicSetState(*pset, layer, source_layer, crtc->id()) != 0) {
+      if (plane->AtomicSetState(*pset, layer, source_layer, crtc->GetId()) !=
+          0) {
         return -EINVAL;
       }
     }
@@ -193,7 +194,7 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int {
 
     active_frame_state_ = std::move(new_frame_state);
 
-    if (crtc->out_fence_ptr_property()) {
+    if (crtc->GetOutFencePtrProperty()) {
       args.out_fence = UniqueFd((int)out_fence);
     }
   }
diff --git a/compositor/Planner.cpp b/compositor/Planner.cpp
index 24b43ba..d875b4b 100644
--- a/compositor/Planner.cpp
+++ b/compositor/Planner.cpp
@@ -46,7 +46,7 @@ std::tuple<int, std::vector<DrmCompositionPlane>> Planner::ProvisionPlanes(
   std::vector<DrmPlane *> planes = GetUsablePlanes(crtc, primary_planes,
                                                    overlay_planes);
   if (planes.empty()) {
-    ALOGE("Display %d has no usable planes", crtc->display());
+    ALOGE("Crtc %d has no usable planes", crtc->GetId());
     return std::make_tuple(-ENODEV, std::vector<DrmCompositionPlane>());
   }
 
diff --git a/drm/DrmCrtc.cpp b/drm/DrmCrtc.cpp
index 08a1922..bc19141 100644
--- a/drm/DrmCrtc.cpp
+++ b/drm/DrmCrtc.cpp
@@ -27,60 +27,41 @@
 
 namespace android {
 
-DrmCrtc::DrmCrtc(DrmDevice *drm, drmModeCrtcPtr c, unsigned pipe)
-    : drm_(drm), id_(c->crtc_id), pipe_(pipe), display_(-1), mode_(&c->mode) {
+static int GetCrtcProperty(const DrmDevice &dev, const DrmCrtc &crtc,
+                           const char *prop_name, DrmProperty *property) {
+  return dev.GetProperty(crtc.GetId(), DRM_MODE_OBJECT_CRTC, prop_name,
+                         property);
 }
 
-int DrmCrtc::Init() {
-  int ret = drm_->GetCrtcProperty(*this, "ACTIVE", &active_property_);
+auto DrmCrtc::CreateInstance(DrmDevice &dev, uint32_t crtc_id, uint32_t index)
+    -> std::unique_ptr<DrmCrtc> {
+  auto crtc = MakeDrmModeCrtcUnique(dev.fd(), crtc_id);
+  if (!crtc) {
+    ALOGE("Failed to get CRTC %d", crtc_id);
+    return {};
+  }
+
+  auto c = std::unique_ptr<DrmCrtc>(new DrmCrtc(std::move(crtc), index));
+
+  int ret = GetCrtcProperty(dev, *c, "ACTIVE", &c->active_property_);
   if (ret != 0) {
     ALOGE("Failed to get ACTIVE property");
-    return ret;
+    return {};
   }
 
-  ret = drm_->GetCrtcProperty(*this, "MODE_ID", &mode_property_);
+  ret = GetCrtcProperty(dev, *c, "MODE_ID", &c->mode_property_);
   if (ret != 0) {
     ALOGE("Failed to get MODE_ID property");
-    return ret;
+    return {};
   }
 
-  ret = drm_->GetCrtcProperty(*this, "OUT_FENCE_PTR", &out_fence_ptr_property_);
+  ret = GetCrtcProperty(dev, *c, "OUT_FENCE_PTR", &c->out_fence_ptr_property_);
   if (ret != 0) {
     ALOGE("Failed to get OUT_FENCE_PTR property");
-    return ret;
+    return {};
   }
-  return 0;
-}
 
-uint32_t DrmCrtc::id() const {
-  return id_;
+  return c;
 }
 
-unsigned DrmCrtc::pipe() const {
-  return pipe_;
-}
-
-int DrmCrtc::display() const {
-  return display_;
-}
-
-void DrmCrtc::set_display(int display) {
-  display_ = display;
-}
-
-bool DrmCrtc::can_bind(int display) const {
-  return display_ == -1 || display_ == display;
-}
-
-const DrmProperty &DrmCrtc::active_property() const {
-  return active_property_;
-}
-
-const DrmProperty &DrmCrtc::mode_property() const {
-  return mode_property_;
-}
-
-const DrmProperty &DrmCrtc::out_fence_ptr_property() const {
-  return out_fence_ptr_property_;
-}
 }  // namespace android
diff --git a/drm/DrmCrtc.h b/drm/DrmCrtc.h
index 85e067a..42fc9f9 100644
--- a/drm/DrmCrtc.h
+++ b/drm/DrmCrtc.h
@@ -23,6 +23,7 @@
 
 #include "DrmMode.h"
 #include "DrmProperty.h"
+#include "DrmUnique.h"
 
 namespace android {
 
@@ -30,32 +31,40 @@ class DrmDevice;
 
 class DrmCrtc {
  public:
-  DrmCrtc(DrmDevice *drm, drmModeCrtcPtr c, unsigned pipe);
+  static auto CreateInstance(DrmDevice &dev, uint32_t crtc_id, uint32_t index)
+      -> std::unique_ptr<DrmCrtc>;
+
+  DrmCrtc() = delete;
   DrmCrtc(const DrmCrtc &) = delete;
   DrmCrtc &operator=(const DrmCrtc &) = delete;
 
-  int Init();
+  auto GetId() const {
+    return crtc_->crtc_id;
+  }
 
-  uint32_t id() const;
-  unsigned pipe() const;
+  auto GetIndexInResArray() const {
+    return index_in_res_array_;
+  }
 
-  int display() const;
-  void set_display(int display);
+  auto &GetActiveProperty() const {
+    return active_property_;
+  }
 
-  bool can_bind(int display) const;
+  auto &GetModeProperty() const {
+    return mode_property_;
+  }
 
-  const DrmProperty &active_property() const;
-  const DrmProperty &mode_property() const;
-  const DrmProperty &out_fence_ptr_property() const;
+  auto &GetOutFencePtrProperty() const {
+    return out_fence_ptr_property_;
+  }
 
  private:
-  DrmDevice *drm_;
+  DrmCrtc(DrmModeCrtcUnique crtc, uint32_t index)
+      : crtc_(std::move(crtc)), index_in_res_array_(index){};
 
-  uint32_t id_;
-  unsigned pipe_;
-  int display_;
+  DrmModeCrtcUnique crtc_;
 
-  DrmMode mode_;
+  const uint32_t index_in_res_array_;
 
   DrmProperty active_property_;
   DrmProperty mode_property_;
diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp
index a833c67..8f44d69 100644
--- a/drm/DrmDevice.cpp
+++ b/drm/DrmDevice.cpp
@@ -95,22 +95,12 @@ std::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
   max_resolution_ = std::pair<uint32_t, uint32_t>(res->max_width,
                                                   res->max_height);
 
-  for (int i = 0; !ret && i < res->count_crtcs; ++i) {
-    auto c = MakeDrmModeCrtcUnique(fd(), res->crtcs[i]);
-    if (!c) {
-      ALOGE("Failed to get crtc %d", res->crtcs[i]);
-      ret = -ENODEV;
-      break;
-    }
-
-    std::unique_ptr<DrmCrtc> crtc(new DrmCrtc(this, c.get(), i));
-
-    ret = crtc->Init();
-    if (ret) {
-      ALOGE("Failed to initialize crtc %d", res->crtcs[i]);
-      break;
+  for (int i = 0; i < res->count_crtcs; ++i) {
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+    auto crtc = DrmCrtc::CreateInstance(*this, res->crtcs[i], i);
+    if (crtc) {
+      crtcs_.emplace_back(std::move(crtc));
     }
-    crtcs_.emplace_back(std::move(crtc));
   }
 
   std::vector<uint32_t> possible_clones;
@@ -125,10 +115,10 @@ std::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
     std::vector<DrmCrtc *> possible_crtcs;
     DrmCrtc *current_crtc = nullptr;
     for (auto &crtc : crtcs_) {
-      if ((1 << crtc->pipe()) & e->possible_crtcs)
+      if ((1 << crtc->GetIndexInResArray()) & e->possible_crtcs)
         possible_crtcs.push_back(crtc.get());
 
-      if (crtc->id() == e->crtc_id)
+      if (crtc->GetId() == e->crtc_id)
         current_crtc = crtc.get();
     }
 
@@ -241,11 +231,7 @@ DrmConnector *DrmDevice::GetConnectorForDisplay(int display) const {
 }
 
 DrmCrtc *DrmDevice::GetCrtcForDisplay(int display) const {
-  for (const auto &crtc : crtcs_) {
-    if (crtc->display() == display)
-      return crtc.get();
-  }
-  return nullptr;
+  return bound_crtcs_.at(display);
 }
 
 const std::vector<std::unique_ptr<DrmCrtc>> &DrmDevice::crtcs() const {
@@ -259,9 +245,9 @@ uint32_t DrmDevice::next_mode_id() {
 int DrmDevice::TryEncoderForDisplay(int display, DrmEncoder *enc) {
   /* First try to use the currently-bound crtc */
   DrmCrtc *crtc = enc->crtc();
-  if (crtc && crtc->can_bind(display)) {
-    crtc->set_display(display);
-    enc->set_crtc(crtc);
+  if (crtc && bound_crtcs_.count(display) == 0) {
+    bound_crtcs_[display] = crtc;
+    enc->set_crtc(crtc, display);
     return 0;
   }
 
@@ -271,9 +257,9 @@ int DrmDevice::TryEncoderForDisplay(int display, DrmEncoder *enc) {
     if (crtc == enc->crtc())
       continue;
 
-    if (crtc->can_bind(display)) {
-      crtc->set_display(display);
-      enc->set_crtc(crtc);
+    if (bound_crtcs_.count(display) == 0) {
+      bound_crtcs_[display] = crtc;
+      enc->set_crtc(crtc, display);
       return 0;
     }
   }
@@ -364,11 +350,6 @@ int DrmDevice::GetProperty(uint32_t obj_id, uint32_t obj_type,
   return found ? 0 : -ENOENT;
 }
 
-int DrmDevice::GetCrtcProperty(const DrmCrtc &crtc, const char *prop_name,
-                               DrmProperty *property) const {
-  return GetProperty(crtc.id(), DRM_MODE_OBJECT_CRTC, prop_name, property);
-}
-
 int DrmDevice::GetConnectorProperty(const DrmConnector &connector,
                                     const char *prop_name,
                                     DrmProperty *property) const {
diff --git a/drm/DrmDevice.h b/drm/DrmDevice.h
index 9983d61..e3f6355 100644
--- a/drm/DrmDevice.h
+++ b/drm/DrmDevice.h
@@ -62,8 +62,6 @@ class DrmDevice {
   DrmConnector *GetConnectorForDisplay(int display) const;
   DrmCrtc *GetCrtcForDisplay(int display) const;
 
-  int GetCrtcProperty(const DrmCrtc &crtc, const char *prop_name,
-                      DrmProperty *property) const;
   int GetConnectorProperty(const DrmConnector &connector, const char *prop_name,
                            DrmProperty *property) const;
 
@@ -108,6 +106,8 @@ class DrmDevice {
   std::pair<uint32_t, uint32_t> max_resolution_;
   std::map<int, int> displays_;
 
+  std::map<int /*display*/, DrmCrtc *> bound_crtcs_;
+
   bool HasAddFb2ModifiersSupport_{};
 
   std::shared_ptr<DrmDevice> self;
diff --git a/drm/DrmEncoder.cpp b/drm/DrmEncoder.cpp
index ad05f88..e1628b2 100644
--- a/drm/DrmEncoder.cpp
+++ b/drm/DrmEncoder.cpp
@@ -48,9 +48,9 @@ void DrmEncoder::AddPossibleClone(DrmEncoder *possible_clone) {
   possible_clones_.insert(possible_clone);
 }
 
-void DrmEncoder::set_crtc(DrmCrtc *crtc) {
+void DrmEncoder::set_crtc(DrmCrtc *crtc, int display) {
   crtc_ = crtc;
-  display_ = crtc->display();
+  display_ = display;
 }
 
 int DrmEncoder::display() const {
diff --git a/drm/DrmEncoder.h b/drm/DrmEncoder.h
index b130b7d..a1f96d3 100644
--- a/drm/DrmEncoder.h
+++ b/drm/DrmEncoder.h
@@ -37,7 +37,7 @@ class DrmEncoder {
   uint32_t id() const;
 
   DrmCrtc *crtc() const;
-  void set_crtc(DrmCrtc *crtc);
+  void set_crtc(DrmCrtc *crtc, int display);
   bool can_bind(int display) const;
   int display() const;
 
diff --git a/drm/DrmPlane.cpp b/drm/DrmPlane.cpp
index 5001457..6b5cec1 100644
--- a/drm/DrmPlane.cpp
+++ b/drm/DrmPlane.cpp
@@ -148,7 +148,7 @@ int DrmPlane::Init() {
 }
 
 bool DrmPlane::IsCrtcSupported(const DrmCrtc &crtc) const {
-  return ((1 << crtc.pipe()) & plane_->possible_crtcs) != 0;
+  return ((1 << crtc.GetIndexInResArray()) & plane_->possible_crtcs) != 0;
 }
 
 bool DrmPlane::IsValidForLayer(DrmHwcLayer *layer) {
diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp
index 8d1cb99..0ac626e 100644
--- a/drm/VSyncWorker.cpp
+++ b/drm/VSyncWorker.cpp
@@ -129,7 +129,8 @@ void VSyncWorker::Routine() {
     ALOGE("Failed to get crtc for display");
     return;
   }
-  uint32_t high_crtc = (crtc->pipe() << DRM_VBLANK_HIGH_CRTC_SHIFT);
+  uint32_t high_crtc = (crtc->GetIndexInResArray()
+                        << DRM_VBLANK_HIGH_CRTC_SHIFT);
 
   drmVBlank vblank;
   memset(&vblank, 0, sizeof(vblank));