drm_hwcomposer: Tidy-up DrmEncoder class

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

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

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

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

diff --git a/.ci/Makefile b/.ci/Makefile
index e203673..e8704da 100644 (file)
--- a/.ci/Makefile
+++ b/.ci/Makefile
@@ -30,7 +30,6 @@ TIDY_FILES_OVERRIDE := \
     drm/DrmProperty.h:COARSE                            \
     drm/DrmConnector.h:COARSE                           \
     drm/DrmUnique.h:FINE                                \
-    drm/DrmEncoder.h:COARSE                             \
     drm/DrmConnector.cpp:COARSE                         \
     drm/DrmDevice.cpp:COARSE                            \
     drm/DrmProperty.cpp:COARSE                          \

diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp
index 8f44d69..f5e3521 100644 (file)
--- a/drm/DrmDevice.cpp
+++ b/drm/DrmDevice.cpp
@@ -103,36 +103,12 @@ std::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
     }
   }
 
-  std::vector<uint32_t> possible_clones;
-  for (int i = 0; !ret && i < res->count_encoders; ++i) {
-    auto e = MakeDrmModeEncoderUnique(fd(), res->encoders[i]);
-    if (!e) {
-      ALOGE("Failed to get encoder %d", res->encoders[i]);
-      ret = -ENODEV;
-      break;
-    }
-
-    std::vector<DrmCrtc *> possible_crtcs;
-    DrmCrtc *current_crtc = nullptr;
-    for (auto &crtc : crtcs_) {
-      if ((1 << crtc->GetIndexInResArray()) & e->possible_crtcs)
-        possible_crtcs.push_back(crtc.get());
-
-      if (crtc->GetId() == e->crtc_id)
-        current_crtc = crtc.get();
+  for (int i = 0; i < res->count_encoders; ++i) {
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+    auto enc = DrmEncoder::CreateInstance(*this, res->encoders[i], i);
+    if (enc) {
+      encoders_.emplace_back(std::move(enc));
     }
-
-    std::unique_ptr<DrmEncoder> enc(
-        new DrmEncoder(e.get(), current_crtc, possible_crtcs));
-    possible_clones.push_back(e->possible_clones);
-
-    encoders_.emplace_back(std::move(enc));
-  }
-
-  for (unsigned int i = 0; i < encoders_.size(); i++) {
-    for (unsigned int j = 0; j < encoders_.size(); j++)
-      if (possible_clones[i] & (1 << j))
-        encoders_[i]->AddPossibleClone(encoders_[j].get());
   }
 
   for (int i = 0; !ret && i < res->count_connectors; ++i) {
@@ -147,9 +123,9 @@ std::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
     DrmEncoder *current_encoder = nullptr;
     for (int j = 0; j < c->count_encoders; ++j) {
       for (auto &encoder : encoders_) {
-        if (encoder->id() == c->encoders[j])
+        if (encoder->GetId() == c->encoders[j])
           possible_encoders.push_back(encoder.get());
-        if (encoder->id() == c->encoder_id)
+        if (encoder->GetId() == c->encoder_id)
           current_encoder = encoder.get();
       }
     }
@@ -244,22 +220,23 @@ 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();
+  auto *crtc = FindCrtcById(enc->GetCurrentCrtcId());
   if (crtc && bound_crtcs_.count(display) == 0) {
     bound_crtcs_[display] = crtc;
-    enc->set_crtc(crtc, display);
+    bound_encoders_[crtc] = enc;
     return 0;
   }
 
   /* Try to find a possible crtc which will work */
-  for (DrmCrtc *crtc : enc->possible_crtcs()) {
-    /* We've already tried this earlier */
-    if (crtc == enc->crtc())
+  for (auto &crtc : crtcs_) {
+    /* Crtc not supported or we've already tried this earlier */
+    if (!enc->SupportsCrtc(*crtc) || crtc->GetId() == enc->GetCurrentCrtcId()) {
       continue;
+    }
 
     if (bound_crtcs_.count(display) == 0) {
-      bound_crtcs_[display] = crtc;
-      enc->set_crtc(crtc, display);
+      bound_crtcs_[display] = crtc.get();
+      bound_encoders_[crtc.get()] = enc;
       return 0;
     }
   }

diff --git a/drm/DrmDevice.h b/drm/DrmDevice.h
index e3f6355..82b0f6a 100644 (file)
--- a/drm/DrmDevice.h
+++ b/drm/DrmDevice.h
@@ -85,6 +85,16 @@ class DrmDevice {
 
   static auto IsKMSDev(const char *path) -> bool;
 
+  auto FindCrtcById(uint32_t id) const -> DrmCrtc * {
+    for (const auto &crtc : crtcs_) {
+      if (crtc->GetId() == id) {
+        return crtc.get();
+      }
+    };
+
+    return nullptr;
+  }
+
   int GetProperty(uint32_t obj_id, uint32_t obj_type, const char *prop_name,
                   DrmProperty *property) const;
 
@@ -107,6 +117,7 @@ class DrmDevice {
   std::map<int, int> displays_;
 
   std::map<int /*display*/, DrmCrtc *> bound_crtcs_;
+  std::map<DrmCrtc *, DrmEncoder *> bound_encoders_;
 
   bool HasAddFb2ModifiersSupport_{};
 

diff --git a/drm/DrmEncoder.cpp b/drm/DrmEncoder.cpp
index e1628b2..8049a5c 100644 (file)
--- a/drm/DrmEncoder.cpp
+++ b/drm/DrmEncoder.cpp
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#define LOG_TAG "hwc-drm-encoder"
+
 #include "DrmEncoder.h"
 
 #include <xf86drmMode.h>
@@ -21,43 +23,19 @@
 #include <cstdint>
 
 #include "DrmDevice.h"
+#include "utils/log.h"
 
 namespace android {
 
-DrmEncoder::DrmEncoder(drmModeEncoderPtr e, DrmCrtc *current_crtc,
-                       std::vector<DrmCrtc *> possible_crtcs)
-    : id_(e->encoder_id),
-      crtc_(current_crtc),
-      display_(-1),
-      possible_crtcs_(std::move(possible_crtcs)) {
-}
-
-uint32_t DrmEncoder::id() const {
-  return id_;
-}
+auto DrmEncoder::CreateInstance(DrmDevice &dev, uint32_t encoder_id,
+                                uint32_t index) -> std::unique_ptr<DrmEncoder> {
+  auto e = MakeDrmModeEncoderUnique(dev.fd(), encoder_id);
+  if (!e) {
+    ALOGE("Failed to get encoder %d", encoder_id);
+    return {};
+  }
 
-DrmCrtc *DrmEncoder::crtc() const {
-  return crtc_;
+  return std::unique_ptr<DrmEncoder>(new DrmEncoder(std::move(e), index));
 }
 
-bool DrmEncoder::CanClone(DrmEncoder *encoder) {
-  return possible_clones_.find(encoder) != possible_clones_.end();
-}
-
-void DrmEncoder::AddPossibleClone(DrmEncoder *possible_clone) {
-  possible_clones_.insert(possible_clone);
-}
-
-void DrmEncoder::set_crtc(DrmCrtc *crtc, int display) {
-  crtc_ = crtc;
-  display_ = display;
-}
-
-int DrmEncoder::display() const {
-  return display_;
-}
-
-bool DrmEncoder::can_bind(int display) const {
-  return display_ == -1 || display_ == display;
-}
 }  // namespace android

diff --git a/drm/DrmEncoder.h b/drm/DrmEncoder.h
index a1f96d3..e66d6f1 100644 (file)
--- a/drm/DrmEncoder.h
+++ b/drm/DrmEncoder.h
@@ -29,31 +29,39 @@ namespace android {
 
 class DrmEncoder {
  public:
-  DrmEncoder(drmModeEncoderPtr e, DrmCrtc *current_crtc,
-             std::vector<DrmCrtc *> possible_crtcs);
+  static auto CreateInstance(DrmDevice &dev, uint32_t encoder_id,
+                             uint32_t index) -> std::unique_ptr<DrmEncoder>;
+
   DrmEncoder(const DrmEncoder &) = delete;
   DrmEncoder &operator=(const DrmEncoder &) = delete;
 
-  uint32_t id() const;
+  auto GetId() const {
+    return enc_->encoder_id;
+  };
+
+  auto GetIndexInResArray() const {
+    return index_in_res_array_;
+  }
 
-  DrmCrtc *crtc() const;
-  void set_crtc(DrmCrtc *crtc, int display);
-  bool can_bind(int display) const;
-  int display() const;
+  auto CanClone(DrmEncoder &encoder) {
+    return (enc_->possible_clones & (1 << encoder.GetIndexInResArray())) != 0;
+  }
 
-  const std::vector<DrmCrtc *> &possible_crtcs() const {
-    return possible_crtcs_;
+  auto SupportsCrtc(DrmCrtc &crtc) {
+    return (enc_->possible_crtcs & (1 << crtc.GetIndexInResArray())) != 0;
+  }
+
+  auto GetCurrentCrtcId() {
+    return enc_->crtc_id;
   }
-  bool CanClone(DrmEncoder *encoder);
-  void AddPossibleClone(DrmEncoder *possible_clone);
 
  private:
-  uint32_t id_;
-  DrmCrtc *crtc_;
-  int display_;
+  DrmEncoder(DrmModeEncoderUnique enc, uint32_t index)
+      : enc_(std::move(enc)), index_in_res_array_(index){};
+
+  DrmModeEncoderUnique enc_;
 
-  std::vector<DrmCrtc *> possible_crtcs_;
-  std::set<DrmEncoder *> possible_clones_;
+  const uint32_t index_in_res_array_;
 };
 }  // namespace android