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>
diff --git a/.ci/Makefile b/.ci/Makefile
index 88cde2e..0525fa2 100644
--- a/.ci/Makefile
+++ b/.ci/Makefile
@@ -23,9 +23,7 @@
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 @@
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 @@
}
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 @@
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 @@
}
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 @@
// 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 @@
}
}
- 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 @@
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 @@
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);
+ return {};
- switch (type_) {
- case DRM_PROPERTY_TYPE_INT:
- return std::make_tuple(0, value_);
+ if ((flags_ & DRM_MODE_PROP_RANGE) != 0)
+ return value_;
- case DRM_PROPERTY_TYPE_ENUM:
- if (value_ >= enums_.size())
- return std::make_tuple(-ENOENT, 0);
+ if ((flags_ & DRM_MODE_PROP_ENUM) != 0) {
+ if (value_ >= enums_.size())
+ return {};
- return std::make_tuple(0, enums_[value_].value_);
-
- case DRM_PROPERTY_TYPE_OBJECT:
- return std::make_tuple(0, value_);
-
- case DRM_PROPERTY_TYPE_BITMASK:
- default:
- return std::make_tuple(-EINVAL, 0);
+ return enums_[value_].value;
}
+
+ if ((flags_ & DRM_MODE_PROP_OBJECT) != 0)
+ return value_;
+
+ return {};
}
-bool DrmProperty::is_immutable() const {
- return id_ && (flags_ & DRM_MODE_PROP_IMMUTABLE);
-}
-
-bool DrmProperty::is_range() const {
- return id_ && (flags_ & DRM_MODE_PROP_RANGE);
-}
-
-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 @@
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<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 @@
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_;
+ }
- std::tuple<int, uint64_t> value() const;
- bool is_immutable() const;
+ auto GetName() const {
+ return name_;
+ }
- bool is_range() const;
- std::tuple<int, uint64_t> range_min() const;
- std::tuple<int, uint64_t> range_max() 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;
+ }
+
+ 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 @@
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;