drm_hwcomposer: Rework DrmMode class

Simplify code and raise-up clang-tidy level of DrmMode class
to 'normal'.

Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>

8 files changed, 52 insertions, 169 deletions

diff --git a/.ci/Makefile b/.ci/Makefile
index 0525fa2..ea9e85b 100644
--- a/.ci/Makefile
+++ b/.ci/Makefile
@@ -22,7 +22,6 @@ TIDY_FILES_OVERRIDE := \
     bufferinfo/legacy/BufferInfoMaliMeson.cpp:COARSE    \
     bufferinfo/legacy/BufferInfoMinigbm.cpp:COARSE      \
     drm/DrmFbImporter.h:FINE                            \
-    drm/DrmMode.h:COARSE                                \
     drm/DrmUnique.h:FINE                                \
     drm/VSyncWorker.cpp:COARSE                          \
     hwc2_device/DrmHwcTwo.cpp:COARSE                    \
diff --git a/drm/DrmConnector.h b/drm/DrmConnector.h
index 1e1a685..63150e7 100644
--- a/drm/DrmConnector.h
+++ b/drm/DrmConnector.h
@@ -123,7 +123,7 @@ class DrmConnector : public PipelineBindable<DrmConnector> {
 
   const uint32_t index_in_res_array_;
 
-  DrmMode active_mode_;
+  DrmMode active_mode_{};
   std::vector<DrmMode> modes_;
 
   DrmProperty dpms_property_;
diff --git a/drm/DrmMode.cpp b/drm/DrmMode.cpp
index 010ea1b..7cbea44 100644
--- a/drm/DrmMode.cpp
+++ b/drm/DrmMode.cpp
@@ -22,119 +22,22 @@
 
 namespace android {
 
-DrmMode::DrmMode(drmModeModeInfoPtr m)
-    : clock_(m->clock),
-      h_display_(m->hdisplay),
-      h_sync_start_(m->hsync_start),
-      h_sync_end_(m->hsync_end),
-      h_total_(m->htotal),
-      h_skew_(m->hskew),
-      v_display_(m->vdisplay),
-      v_sync_start_(m->vsync_start),
-      v_sync_end_(m->vsync_end),
-      v_total_(m->vtotal),
-      v_scan_(m->vscan),
-      v_refresh_(m->vrefresh),
-      flags_(m->flags),
-      type_(m->type),
-      name_(m->name) {
-}
+DrmMode::DrmMode(drmModeModeInfoPtr m) : mode_(*m){};
 
 bool DrmMode::operator==(const drmModeModeInfo &m) const {
-  return clock_ == m.clock && h_display_ == m.hdisplay &&
-         h_sync_start_ == m.hsync_start && h_sync_end_ == m.hsync_end &&
-         h_total_ == m.htotal && h_skew_ == m.hskew &&
-         v_display_ == m.vdisplay && v_sync_start_ == m.vsync_start &&
-         v_sync_end_ == m.vsync_end && v_total_ == m.vtotal &&
-         v_scan_ == m.vscan && flags_ == m.flags && type_ == m.type;
-}
-
-uint32_t DrmMode::clock() const {
-  return clock_;
-}
-
-uint16_t DrmMode::h_display() const {
-  return h_display_;
-}
-
-uint16_t DrmMode::h_sync_start() const {
-  return h_sync_start_;
-}
-
-uint16_t DrmMode::h_sync_end() const {
-  return h_sync_end_;
-}
-
-uint16_t DrmMode::h_total() const {
-  return h_total_;
-}
-
-uint16_t DrmMode::h_skew() const {
-  return h_skew_;
-}
-
-uint16_t DrmMode::v_display() const {
-  return v_display_;
-}
-
-uint16_t DrmMode::v_sync_start() const {
-  return v_sync_start_;
-}
-
-uint16_t DrmMode::v_sync_end() const {
-  return v_sync_end_;
-}
-
-uint16_t DrmMode::v_total() const {
-  return v_total_;
-}
-
-uint16_t DrmMode::v_scan() const {
-  return v_scan_;
-}
-
-float DrmMode::v_refresh() const {
-  if (clock_ == 0) {
-    return v_refresh_;
-  }
-  // Always recalculate refresh to report correct float rate
-  return static_cast<float>(clock_) / (float)(v_total_ * h_total_) * 1000.0F;
-}
-
-uint32_t DrmMode::flags() const {
-  return flags_;
-}
-
-uint32_t DrmMode::type() const {
-  return type_;
-}
-
-std::string DrmMode::name() const {
-  return name_ + "@" + std::to_string(v_refresh());
+  return memcmp(&m, &mode_, offsetof(drmModeModeInfo, name)) == 0;
 }
 
 auto DrmMode::CreateModeBlob(const DrmDevice &drm)
     -> DrmModeUserPropertyBlobUnique {
-  struct drm_mode_modeinfo drm_mode = {
-      .clock = clock_,
-      .hdisplay = h_display_,
-      .hsync_start = h_sync_start_,
-      .hsync_end = h_sync_end_,
-      .htotal = h_total_,
-      .hskew = h_skew_,
-      .vdisplay = v_display_,
-      .vsync_start = v_sync_start_,
-      .vsync_end = v_sync_end_,
-      .vtotal = v_total_,
-      .vscan = v_scan_,
-      .vrefresh = v_refresh_,
-      .flags = flags_,
-      .type = type_,
-  };
-  strncpy(drm_mode.name, name_.c_str(), DRM_DISPLAY_MODE_LEN);
+  struct drm_mode_modeinfo drm_mode = {};
+  /* drm_mode_modeinfo and drmModeModeInfo should be identical
+   * At least libdrm does the same memcpy in drmModeAttachMode();
+   */
+  memcpy(&drm_mode, &mode_, sizeof(struct drm_mode_modeinfo));
 
   return drm.RegisterUserPropertyBlob(&drm_mode,
-                                       sizeof(struct drm_mode_modeinfo));
+                                      sizeof(struct drm_mode_modeinfo));
 }
 
 }  // namespace android
diff --git a/drm/DrmMode.h b/drm/DrmMode.h
index c6ddc63..c5790a5 100644
--- a/drm/DrmMode.h
+++ b/drm/DrmMode.h
@@ -35,47 +35,26 @@ class DrmMode {
 
   bool operator==(const drmModeModeInfo &m) const;
 
-  uint32_t clock() const;
-
-  uint16_t h_display() const;
-  uint16_t h_sync_start() const;
-  uint16_t h_sync_end() const;
-  uint16_t h_total() const;
-  uint16_t h_skew() const;
-
-  uint16_t v_display() const;
-  uint16_t v_sync_start() const;
-  uint16_t v_sync_end() const;
-  uint16_t v_total() const;
-  uint16_t v_scan() const;
-  float v_refresh() const;
-
-  uint32_t flags() const;
-  uint32_t type() const;
-
-  std::string name() const;
+  auto &GetRawMode() const {
+    return mode_;
+  }
+
+  auto GetVRefresh() const {
+    if (mode_.clock == 0) {
+      return float(mode_.vrefresh);
+    }
+    // Always recalculate refresh to report correct float rate
+    return static_cast<float>(mode_.clock) /
+           (float)(mode_.vtotal * mode_.htotal) * 1000.0F;
+  }
+
+  auto GetName() const {
+    return std::string(mode_.name) + "@" + std::to_string(GetVRefresh());
+  }
 
   auto CreateModeBlob(const DrmDevice &drm) -> DrmModeUserPropertyBlobUnique;
 
  private:
-  uint32_t clock_ = 0;
-
-  uint16_t h_display_ = 0;
-  uint16_t h_sync_start_ = 0;
-  uint16_t h_sync_end_ = 0;
-  uint16_t h_total_ = 0;
-  uint16_t h_skew_ = 0;
-
-  uint16_t v_display_ = 0;
-  uint16_t v_sync_start_ = 0;
-  uint16_t v_sync_end_ = 0;
-  uint16_t v_total_ = 0;
-  uint16_t v_scan_ = 0;
-  uint16_t v_refresh_ = 0;
-
-  uint32_t flags_ = 0;
-  uint32_t type_ = 0;
-
-  std::string name_;
+  drmModeModeInfo mode_;
 };
 }  // namespace android
diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp
index 7b4fae4..a2cad28 100644
--- a/drm/VSyncWorker.cpp
+++ b/drm/VSyncWorker.cpp
@@ -80,8 +80,8 @@ int VSyncWorker::SyntheticWaitVBlank(int64_t *timestamp) {
 
   float refresh = 60.0F;  // Default to 60Hz refresh rate
   if (pipe_ != nullptr &&
-      pipe_->connector->Get()->GetActiveMode().v_refresh() != 0.0F) {
-    refresh = pipe_->connector->Get()->GetActiveMode().v_refresh();
+      pipe_->connector->Get()->GetActiveMode().GetVRefresh() != 0.0F) {
+    refresh = pipe_->connector->Get()->GetActiveMode().GetVRefresh();
   }
 
   auto phased_timestamp = GetPhasedVSync(kOneSecondNs /
diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp
index 333857e..cfef776 100644
--- a/hwc2_device/HwcDisplay.cpp
+++ b/hwc2_device/HwcDisplay.cpp
@@ -301,25 +301,25 @@ HWC2::Error HwcDisplay::GetDisplayAttribute(hwc2_config_t config,
   auto attribute = static_cast<HWC2::Attribute>(attribute_in);
   switch (attribute) {
     case HWC2::Attribute::Width:
-      *value = static_cast<int>(hwc_config.mode.h_display());
+      *value = static_cast<int>(hwc_config.mode.GetRawMode().hdisplay);
       break;
     case HWC2::Attribute::Height:
-      *value = static_cast<int>(hwc_config.mode.v_display());
+      *value = static_cast<int>(hwc_config.mode.GetRawMode().vdisplay);
       break;
     case HWC2::Attribute::VsyncPeriod:
       // in nanoseconds
-      *value = static_cast<int>(1E9 / hwc_config.mode.v_refresh());
+      *value = static_cast<int>(1E9 / hwc_config.mode.GetVRefresh());
       break;
     case HWC2::Attribute::DpiX:
       // Dots per 1000 inches
-      *value = mm_width ? static_cast<int>(hwc_config.mode.h_display() *
-                                           kUmPerInch / mm_width)
+      *value = mm_width ? int(hwc_config.mode.GetRawMode().hdisplay *
+                              kUmPerInch / mm_width)
                         : -1;
       break;
     case HWC2::Attribute::DpiY:
       // Dots per 1000 inches
-      *value = mm_height ? static_cast<int>(hwc_config.mode.v_display() *
-                                            kUmPerInch / mm_height)
+      *value = mm_height ? int(hwc_config.mode.GetRawMode().vdisplay *
+                               kUmPerInch / mm_height)
                          : -1;
       break;
 #if PLATFORM_SDK_VERSION > 29
@@ -451,7 +451,7 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) {
   }
 
   auto PrevModeVsyncPeriodNs = static_cast<int>(
-      1E9 / GetPipe().connector->Get()->GetActiveMode().v_refresh());
+      1E9 / GetPipe().connector->Get()->GetActiveMode().GetVRefresh());
 
   auto mode_update_commited_ = false;
   if (staged_mode_ &&
@@ -459,8 +459,8 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) {
     client_layer_.SetLayerDisplayFrame(
         (hwc_rect_t){.left = 0,
                      .top = 0,
-                     .right = static_cast<int>(staged_mode_->h_display()),
-                     .bottom = static_cast<int>(staged_mode_->v_display())});
+                     .right = int(staged_mode_->GetRawMode().hdisplay),
+                     .bottom = int(staged_mode_->GetRawMode().vdisplay)});
 
     configs_.active_config_id = staged_mode_config_id_;
 
diff --git a/hwc2_device/HwcDisplayConfigs.cpp b/hwc2_device/HwcDisplayConfigs.cpp
index 346572f..9727989 100644
--- a/hwc2_device/HwcDisplayConfigs.cpp
+++ b/hwc2_device/HwcDisplayConfigs.cpp
@@ -87,8 +87,10 @@ HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) {
     /* Find group for the new mode or create new group */
     uint32_t group_found = 0;
     for (auto &hwc_config : hwc_configs) {
-      if (mode.h_display() == hwc_config.second.mode.h_display() &&
-          mode.v_display() == hwc_config.second.mode.v_display()) {
+      if (mode.GetRawMode().hdisplay ==
+              hwc_config.second.mode.GetRawMode().hdisplay &&
+          mode.GetRawMode().vdisplay ==
+              hwc_config.second.mode.GetRawMode().vdisplay) {
         group_found = hwc_config.second.group_id;
       }
     }
@@ -97,9 +99,9 @@ HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) {
     }
 
     bool disabled = false;
-    if ((mode.flags() & DRM_MODE_FLAG_3D_MASK) != 0) {
+    if ((mode.GetRawMode().flags & DRM_MODE_FLAG_3D_MASK) != 0) {
       ALOGI("Disabling display mode %s (Modes with 3D flag aren't supported)",
-            mode.name().c_str());
+            mode.GetName().c_str());
       disabled = true;
     }
 
@@ -112,7 +114,7 @@ HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) {
     };
 
     /* Chwck if the mode is preferred */
-    if ((mode.type() & DRM_MODE_TYPE_PREFERRED) != 0 &&
+    if ((mode.GetRawMode().type & DRM_MODE_TYPE_PREFERRED) != 0 &&
         preferred_config_id == 0) {
       preferred_config_id = last_config_id;
       preferred_config_group_id = group_found;
@@ -167,7 +169,7 @@ HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) {
         ALOGI(
             "Group %i: Disabling display mode %s (This group should consist "
             "of %s modes)",
-            group, hwc_config.second.mode.name().c_str(),
+            group, hwc_config.second.mode.GetName().c_str(),
             group_contains_preferred_interlaced ? "interlaced" : "progressive");
 
         hwc_config.second.disabled = true;
@@ -183,13 +185,13 @@ HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) {
     for (uint32_t m2 = first_config_id; m2 < last_config_id; m2++) {
       if (m1 != m2 && hwc_configs[m1].group_id == hwc_configs[m2].group_id &&
           !hwc_configs[m1].disabled && !hwc_configs[m2].disabled &&
-          fabsf(hwc_configs[m1].mode.v_refresh() -
-                hwc_configs[m2].mode.v_refresh()) < kMinFpsDelta) {
+          fabsf(hwc_configs[m1].mode.GetVRefresh() -
+                hwc_configs[m2].mode.GetVRefresh()) < kMinFpsDelta) {
         ALOGI(
             "Group %i: Disabling display mode %s (Refresh rate value is "
             "too close to existing mode %s)",
-            hwc_configs[m2].group_id, hwc_configs[m2].mode.name().c_str(),
-            hwc_configs[m1].mode.name().c_str());
+            hwc_configs[m2].group_id, hwc_configs[m2].mode.GetName().c_str(),
+            hwc_configs[m1].mode.GetName().c_str());
 
         hwc_configs[m2].disabled = true;
       }
diff --git a/hwc2_device/HwcDisplayConfigs.h b/hwc2_device/HwcDisplayConfigs.h
index 639b302..98067c1 100644
--- a/hwc2_device/HwcDisplayConfigs.h
+++ b/hwc2_device/HwcDisplayConfigs.h
@@ -29,11 +29,11 @@ class DrmConnector;
 struct HwcDisplayConfig {
   uint32_t id{};
   uint32_t group_id{};
-  DrmMode mode;
+  DrmMode mode{};
   bool disabled{};
 
   bool IsInterlaced() const {
-    return (mode.flags() & DRM_MODE_FLAG_INTERLACE) != 0;
+    return (mode.GetRawMode().flags & DRM_MODE_FLAG_INTERLACE) != 0;
   }
 };