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>
diff --git a/.ci/Makefile b/.ci/Makefile
index 0525fa2..ea9e85b 100644
--- a/.ci/Makefile
+++ b/.ci/Makefile
@@ -22,7 +22,6 @@
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 @@
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 @@
bool operator==(const drmModeModeInfo &m) const;
- uint32_t clock() const;
+ auto &GetRawMode() const {
+ return mode_;
+ }
- 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;
+ 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;
+ }
- 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 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 @@
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 @@
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 @@
}
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 @@
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 @@
/* 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 @@
}
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 @@
};
/* 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 @@
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 @@
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 @@
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;
}
};