drm_hwcomposer: Rework DrmProperty class

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

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

6 files changed, 65 insertions, 95 deletions

diff --git a/.ci/Makefile b/.ci/Makefile
index 88cde2e..0525fa2 100644
--- a/.ci/Makefile
+++ b/.ci/Makefile
@@ -23,9 +23,7 @@ TIDY_FILES_OVERRIDE := \
     bufferinfo/legacy/BufferInfoMinigbm.cpp:COARSE      \
     drm/DrmFbImporter.h:FINE                            \
     drm/DrmMode.h:COARSE                                \
-    drm/DrmProperty.h:COARSE                            \
     drm/DrmUnique.h:FINE                                \
-    drm/DrmProperty.cpp:COARSE                          \
     drm/VSyncWorker.cpp:COARSE                          \
     hwc2_device/DrmHwcTwo.cpp:COARSE                    \
     hwc2_device/DrmHwcTwo.h:COARSE                      \
diff --git a/drm/DrmAtomicStateManager.cpp b/drm/DrmAtomicStateManager.cpp
index 3582b82..16652c1 100644
--- a/drm/DrmAtomicStateManager.cpp
+++ b/drm/DrmAtomicStateManager.cpp
@@ -287,7 +287,7 @@ auto DrmAtomicStateManager::ActivateDisplayUsingDPMS() -> int {
                                      pipe_->connector->Get()->GetId(),
                                      pipe_->connector->Get()
                                          ->GetDpmsProperty()
-                                         .id(),
+                                         .GetId(),
                                      DRM_MODE_DPMS_ON);
 }
 
diff --git a/drm/DrmConnector.cpp b/drm/DrmConnector.cpp
index 69d029d..6fde6fb 100644
--- a/drm/DrmConnector.cpp
+++ b/drm/DrmConnector.cpp
@@ -99,18 +99,17 @@ int DrmConnector::UpdateEdidProperty() {
 }
 
 auto DrmConnector::GetEdidBlob() -> DrmModePropertyBlobUnique {
-  uint64_t blob_id = 0;
-  int ret = UpdateEdidProperty();
+  auto ret = UpdateEdidProperty();
   if (ret != 0) {
     return {};
   }
 
-  std::tie(ret, blob_id) = GetEdidProperty().value();
-  if (ret != 0) {
+  auto blob_id = GetEdidProperty().GetValue();
+  if (!blob_id) {
     return {};
   }
 
-  return MakeDrmModePropertyBlobUnique(drm_->GetFd(), blob_id);
+  return MakeDrmModePropertyBlobUnique(drm_->GetFd(), *blob_id);
 }
 
 bool DrmConnector::IsInternal() const {
diff --git a/drm/DrmPlane.cpp b/drm/DrmPlane.cpp
index 76a2c6e..4d03840 100644
--- a/drm/DrmPlane.cpp
+++ b/drm/DrmPlane.cpp
@@ -57,21 +57,19 @@ int DrmPlane::Init() {
     return -ENOTSUP;
   }
 
-  int ret = 0;
-  uint64_t type = 0;
-  std::tie(ret, type) = p.value();
-  if (ret != 0) {
+  auto type = p.GetValue();
+  if (!type) {
     ALOGE("Failed to get plane type property value");
-    return ret;
+    return -EINVAL;
   }
-  switch (type) {
+  switch (*type) {
     case DRM_PLANE_TYPE_OVERLAY:
     case DRM_PLANE_TYPE_PRIMARY:
     case DRM_PLANE_TYPE_CURSOR:
-      type_ = (uint32_t)type;
+      type_ = (uint32_t)*type;
       break;
     default:
-      ALOGE("Invalid plane type %" PRIu64, type);
+      ALOGE("Invalid plane type %" PRIu64, *type);
       return -EINVAL;
   }
 
@@ -148,9 +146,10 @@ int DrmPlane::Init() {
 }
 
 bool DrmPlane::IsCrtcSupported(const DrmCrtc &crtc) const {
-  unsigned int crtc_property_value = 0;
-  std::tie(std::ignore, crtc_property_value) = crtc_property_.value();
-  if (crtc_property_value != 0 && crtc_property_value != crtc.GetId() &&
+  auto crtc_prop_optval = crtc_property_.GetValue();
+  auto crtc_prop_val = crtc_prop_optval ? *crtc_prop_optval : 0;
+
+  if (crtc_prop_val != 0 && crtc_prop_val != crtc.GetId() &&
       GetType() == DRM_PLANE_TYPE_PRIMARY) {
     // Some DRM driver such as omap_drm allows sharing primary plane between
     // CRTCs, but the primay plane could not be shared if it has been used by
@@ -158,10 +157,9 @@ bool DrmPlane::IsCrtcSupported(const DrmCrtc &crtc) const {
     // in the kernel drivers/gpu/drm/drm_atomic.c file.
     // The current drm_hwc design is not ready to support such scenario yet,
     // so adding the CRTC status check here to workaorund for now.
-    ALOGW(
-        "%s: This Plane(id=%d) is activated for Crtc(id=%d), could not be used "
-        "for Crtc (id=%d)",
-        __FUNCTION__, GetId(), crtc_property_value, crtc.GetId());
+    ALOGW("%s: This Plane(id=%d) is activated for Crtc(id=%" PRIu64
+          "), could not be used for Crtc (id=%d)",
+          __FUNCTION__, GetId(), crtc_prop_val, crtc.GetId());
     return false;
   }
 
@@ -186,7 +184,7 @@ bool DrmPlane::IsValidForLayer(LayerData *layer) {
     }
   }
 
-  if (alpha_property_.id() == 0 && layer->pi.alpha != UINT16_MAX) {
+  if (!alpha_property_ && layer->pi.alpha != UINT16_MAX) {
     ALOGV("Alpha is not supported on plane %d", GetId());
     return false;
   }
@@ -251,11 +249,11 @@ auto DrmPlane::AtomicSetState(drmModeAtomicReq &pset, LayerData &layer,
     return -EINVAL;
   }
 
-  if (zpos_property_ && !zpos_property_.is_immutable()) {
+  if (zpos_property_ && !zpos_property_.IsImmutable()) {
     uint64_t min_zpos = 0;
 
     // Ignore ret and use min_zpos as 0 by default
-    std::tie(std::ignore, min_zpos) = zpos_property_.range_min();
+    std::tie(std::ignore, min_zpos) = zpos_property_.RangeMin();
 
     if (!zpos_property_.AtomicSet(pset, zpos + min_zpos)) {
       return -EINVAL;
diff --git a/drm/DrmProperty.cpp b/drm/DrmProperty.cpp
index e0bf25e..938b3ad 100644
--- a/drm/DrmProperty.cpp
+++ b/drm/DrmProperty.cpp
@@ -31,7 +31,7 @@
 namespace android {
 
 DrmProperty::DrmPropertyEnum::DrmPropertyEnum(drm_mode_property_enum *e)
-    : value_(e->value), name_(e->name) {
+    : value(e->value), name(e->name) {
 }
 
 DrmProperty::DrmProperty(uint32_t obj_id, drmModePropertyPtr p,
@@ -47,70 +47,43 @@ void DrmProperty::Init(uint32_t obj_id, drmModePropertyPtr p, uint64_t value) {
   value_ = value;
 
   for (int i = 0; i < p->count_values; ++i)
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic):
     values_.emplace_back(p->values[i]);
 
   for (int i = 0; i < p->count_enums; ++i)
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic):
     enums_.emplace_back(&p->enums[i]);
 
   for (int i = 0; i < p->count_blobs; ++i)
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic):
     blob_ids_.emplace_back(p->blob_ids[i]);
-
-  if (flags_ & DRM_MODE_PROP_RANGE)
-    type_ = DRM_PROPERTY_TYPE_INT;
-  else if (flags_ & DRM_MODE_PROP_ENUM)
-    type_ = DRM_PROPERTY_TYPE_ENUM;
-  else if (flags_ & DRM_MODE_PROP_OBJECT)
-    type_ = DRM_PROPERTY_TYPE_OBJECT;
-  else if (flags_ & DRM_MODE_PROP_BLOB)
-    type_ = DRM_PROPERTY_TYPE_BLOB;
-  else if (flags_ & DRM_MODE_PROP_BITMASK)
-    type_ = DRM_PROPERTY_TYPE_BITMASK;
-}
-
-uint32_t DrmProperty::id() const {
-  return id_;
-}
-
-std::string DrmProperty::name() const {
-  return name_;
 }
 
-std::tuple<int, uint64_t> DrmProperty::value() const {
-  if (type_ == DRM_PROPERTY_TYPE_BLOB)
-    return std::make_tuple(0, value_);
+std::optional<uint64_t> DrmProperty::GetValue() const {
+  if ((flags_ & DRM_MODE_PROP_BLOB) != 0)
+    return value_;
 
   if (values_.empty())
-    return std::make_tuple(-ENOENT, 0);
-
-  switch (type_) {
-    case DRM_PROPERTY_TYPE_INT:
-      return std::make_tuple(0, value_);
-
-    case DRM_PROPERTY_TYPE_ENUM:
-      if (value_ >= enums_.size())
-        return std::make_tuple(-ENOENT, 0);
+    return {};
 
-      return std::make_tuple(0, enums_[value_].value_);
+  if ((flags_ & DRM_MODE_PROP_RANGE) != 0)
+    return value_;
 
-    case DRM_PROPERTY_TYPE_OBJECT:
-      return std::make_tuple(0, value_);
+  if ((flags_ & DRM_MODE_PROP_ENUM) != 0) {
+    if (value_ >= enums_.size())
+      return {};
 
-    case DRM_PROPERTY_TYPE_BITMASK:
-    default:
-      return std::make_tuple(-EINVAL, 0);
+    return enums_[value_].value;
   }
-}
 
-bool DrmProperty::is_immutable() const {
-  return id_ && (flags_ & DRM_MODE_PROP_IMMUTABLE);
-}
+  if ((flags_ & DRM_MODE_PROP_OBJECT) != 0)
+    return value_;
 
-bool DrmProperty::is_range() const {
-  return id_ && (flags_ & DRM_MODE_PROP_RANGE);
+  return {};
 }
 
-std::tuple<int, uint64_t> DrmProperty::range_min() const {
-  if (!is_range())
+std::tuple<int, uint64_t> DrmProperty::RangeMin() const {
+  if (!IsRange())
     return std::make_tuple(-EINVAL, 0);
   if (values_.empty())
     return std::make_tuple(-ENOENT, 0);
@@ -118,8 +91,8 @@ std::tuple<int, uint64_t> DrmProperty::range_min() const {
   return std::make_tuple(0, values_[0]);
 }
 
-std::tuple<int, uint64_t> DrmProperty::range_max() const {
-  if (!is_range())
+std::tuple<int, uint64_t> DrmProperty::RangeMax() const {
+  if (!IsRange())
     return std::make_tuple(-EINVAL, 0);
   if (values_.size() < 2)
     return std::make_tuple(-ENOENT, 0);
@@ -130,8 +103,8 @@ std::tuple<int, uint64_t> DrmProperty::range_max() const {
 std::tuple<uint64_t, int> DrmProperty::GetEnumValueWithName(
     const std::string &name) const {
   for (const auto &it : enums_) {
-    if (it.name_ == name) {
-      return std::make_tuple(it.value_, 0);
+    if (it.name == name) {
+      return std::make_tuple(it.value, 0);
     }
   }
 
diff --git a/drm/DrmProperty.h b/drm/DrmProperty.h
index 5472c4e..516518b 100644
--- a/drm/DrmProperty.h
+++ b/drm/DrmProperty.h
@@ -20,20 +20,12 @@
 
 #include <cstdint>
 #include <map>
+#include <optional>
 #include <string>
 #include <vector>
 
 namespace android {
 
-enum DrmPropertyType {
-  DRM_PROPERTY_TYPE_INT,
-  DRM_PROPERTY_TYPE_ENUM,
-  DRM_PROPERTY_TYPE_OBJECT,
-  DRM_PROPERTY_TYPE_BLOB,
-  DRM_PROPERTY_TYPE_BITMASK,
-  DRM_PROPERTY_TYPE_INVALID,
-};
-
 class DrmProperty {
  public:
   DrmProperty() = default;
@@ -44,15 +36,26 @@ class DrmProperty {
   auto Init(uint32_t obj_id, drmModePropertyPtr p, uint64_t value) -> void;
   std::tuple<uint64_t, int> GetEnumValueWithName(const std::string &name) const;
 
-  uint32_t id() const;
-  std::string name() const;
+  auto GetId() const {
+    return id_;
+  }
+
+  auto GetName() const {
+    return name_;
+  }
 
-  std::tuple<int, uint64_t> value() const;
-  bool is_immutable() const;
+  auto GetValue() const -> std::optional<uint64_t>;
+
+  bool IsImmutable() const {
+    return id_ != 0 && (flags_ & DRM_MODE_PROP_IMMUTABLE) != 0;
+  }
+
+  bool IsRange() const {
+    return id_ != 0 && (flags_ & DRM_MODE_PROP_RANGE) != 0;
+  }
 
-  bool is_range() const;
-  std::tuple<int, uint64_t> range_min() const;
-  std::tuple<int, uint64_t> range_max() const;
+  auto RangeMin() const -> std::tuple<int, uint64_t>;
+  auto RangeMax() const -> std::tuple<int, uint64_t>;
 
   [[nodiscard]] auto AtomicSet(drmModeAtomicReq &pset, uint64_t value) const
       -> bool;
@@ -71,14 +74,13 @@ class DrmProperty {
     explicit DrmPropertyEnum(drm_mode_property_enum *e);
     ~DrmPropertyEnum() = default;
 
-    uint64_t value_;
-    std::string name_;
+    uint64_t value;
+    std::string name;
   };
 
   uint32_t obj_id_ = 0;
   uint32_t id_ = 0;
 
-  DrmPropertyType type_ = DRM_PROPERTY_TYPE_INVALID;
   uint32_t flags_ = 0;
   std::string name_;
   uint64_t value_ = 0;