drm_hwcomposer: Tidy-up DrmCrtc class
Implement DrmCrtc instantiation through CreateInstance() static method,
which helps to reduce complexity of DrmDevice::Init() function.
Move CRTC-to-Display binding information to the DrmDevice class.
Move drm/DrmCrtc.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 7f43a8a..e203673 100644
--- a/.ci/Makefile
+++ b/.ci/Makefile
@@ -29,7 +29,6 @@
drm/DrmDevice.h:COARSE \
drm/DrmProperty.h:COARSE \
drm/DrmConnector.h:COARSE \
- drm/DrmCrtc.h:COARSE \
drm/DrmUnique.h:FINE \
drm/DrmEncoder.h:COARSE \
drm/DrmConnector.cpp:COARSE \
diff --git a/compositor/DrmDisplayCompositor.cpp b/compositor/DrmDisplayCompositor.cpp
index c2e51ee..0cb1d39 100644
--- a/compositor/DrmDisplayCompositor.cpp
+++ b/compositor/DrmDisplayCompositor.cpp
@@ -99,15 +99,15 @@
}
int64_t out_fence = -1;
- if (crtc->out_fence_ptr_property() &&
- !crtc->out_fence_ptr_property().AtomicSet(*pset, (uint64_t)&out_fence)) {
+ if (crtc->GetOutFencePtrProperty() &&
+ !crtc->GetOutFencePtrProperty().AtomicSet(*pset, (uint64_t)&out_fence)) {
return -EINVAL;
}
if (args.active) {
new_frame_state.crtc_active_state = *args.active;
- if (!crtc->active_property().AtomicSet(*pset, *args.active) ||
- !connector->crtc_id_property().AtomicSet(*pset, crtc->id())) {
+ if (!crtc->GetActiveProperty().AtomicSet(*pset, *args.active) ||
+ !connector->crtc_id_property().AtomicSet(*pset, crtc->GetId())) {
return -EINVAL;
}
}
@@ -121,7 +121,7 @@
return -EINVAL;
}
- if (!crtc->mode_property().AtomicSet(*pset, *new_frame_state.mode_blob)) {
+ if (!crtc->GetModeProperty().AtomicSet(*pset, *new_frame_state.mode_blob)) {
return -EINVAL;
}
}
@@ -154,7 +154,8 @@
auto &v = unused_planes;
v.erase(std::remove(v.begin(), v.end(), plane), v.end());
- if (plane->AtomicSetState(*pset, layer, source_layer, crtc->id()) != 0) {
+ if (plane->AtomicSetState(*pset, layer, source_layer, crtc->GetId()) !=
+ 0) {
return -EINVAL;
}
}
@@ -193,7 +194,7 @@
active_frame_state_ = std::move(new_frame_state);
- if (crtc->out_fence_ptr_property()) {
+ if (crtc->GetOutFencePtrProperty()) {
args.out_fence = UniqueFd((int)out_fence);
}
}
diff --git a/compositor/Planner.cpp b/compositor/Planner.cpp
index 24b43ba..d875b4b 100644
--- a/compositor/Planner.cpp
+++ b/compositor/Planner.cpp
@@ -46,7 +46,7 @@
std::vector<DrmPlane *> planes = GetUsablePlanes(crtc, primary_planes,
overlay_planes);
if (planes.empty()) {
- ALOGE("Display %d has no usable planes", crtc->display());
+ ALOGE("Crtc %d has no usable planes", crtc->GetId());
return std::make_tuple(-ENODEV, std::vector<DrmCompositionPlane>());
}
diff --git a/drm/DrmCrtc.cpp b/drm/DrmCrtc.cpp
index 08a1922..bc19141 100644
--- a/drm/DrmCrtc.cpp
+++ b/drm/DrmCrtc.cpp
@@ -27,60 +27,41 @@
namespace android {
-DrmCrtc::DrmCrtc(DrmDevice *drm, drmModeCrtcPtr c, unsigned pipe)
- : drm_(drm), id_(c->crtc_id), pipe_(pipe), display_(-1), mode_(&c->mode) {
+static int GetCrtcProperty(const DrmDevice &dev, const DrmCrtc &crtc,
+ const char *prop_name, DrmProperty *property) {
+ return dev.GetProperty(crtc.GetId(), DRM_MODE_OBJECT_CRTC, prop_name,
+ property);
}
-int DrmCrtc::Init() {
- int ret = drm_->GetCrtcProperty(*this, "ACTIVE", &active_property_);
+auto DrmCrtc::CreateInstance(DrmDevice &dev, uint32_t crtc_id, uint32_t index)
+ -> std::unique_ptr<DrmCrtc> {
+ auto crtc = MakeDrmModeCrtcUnique(dev.fd(), crtc_id);
+ if (!crtc) {
+ ALOGE("Failed to get CRTC %d", crtc_id);
+ return {};
+ }
+
+ auto c = std::unique_ptr<DrmCrtc>(new DrmCrtc(std::move(crtc), index));
+
+ int ret = GetCrtcProperty(dev, *c, "ACTIVE", &c->active_property_);
if (ret != 0) {
ALOGE("Failed to get ACTIVE property");
- return ret;
+ return {};
}
- ret = drm_->GetCrtcProperty(*this, "MODE_ID", &mode_property_);
+ ret = GetCrtcProperty(dev, *c, "MODE_ID", &c->mode_property_);
if (ret != 0) {
ALOGE("Failed to get MODE_ID property");
- return ret;
+ return {};
}
- ret = drm_->GetCrtcProperty(*this, "OUT_FENCE_PTR", &out_fence_ptr_property_);
+ ret = GetCrtcProperty(dev, *c, "OUT_FENCE_PTR", &c->out_fence_ptr_property_);
if (ret != 0) {
ALOGE("Failed to get OUT_FENCE_PTR property");
- return ret;
+ return {};
}
- return 0;
+
+ return c;
}
-uint32_t DrmCrtc::id() const {
- return id_;
-}
-
-unsigned DrmCrtc::pipe() const {
- return pipe_;
-}
-
-int DrmCrtc::display() const {
- return display_;
-}
-
-void DrmCrtc::set_display(int display) {
- display_ = display;
-}
-
-bool DrmCrtc::can_bind(int display) const {
- return display_ == -1 || display_ == display;
-}
-
-const DrmProperty &DrmCrtc::active_property() const {
- return active_property_;
-}
-
-const DrmProperty &DrmCrtc::mode_property() const {
- return mode_property_;
-}
-
-const DrmProperty &DrmCrtc::out_fence_ptr_property() const {
- return out_fence_ptr_property_;
-}
} // namespace android
diff --git a/drm/DrmCrtc.h b/drm/DrmCrtc.h
index 85e067a..42fc9f9 100644
--- a/drm/DrmCrtc.h
+++ b/drm/DrmCrtc.h
@@ -23,6 +23,7 @@
#include "DrmMode.h"
#include "DrmProperty.h"
+#include "DrmUnique.h"
namespace android {
@@ -30,32 +31,40 @@
class DrmCrtc {
public:
- DrmCrtc(DrmDevice *drm, drmModeCrtcPtr c, unsigned pipe);
+ static auto CreateInstance(DrmDevice &dev, uint32_t crtc_id, uint32_t index)
+ -> std::unique_ptr<DrmCrtc>;
+
+ DrmCrtc() = delete;
DrmCrtc(const DrmCrtc &) = delete;
DrmCrtc &operator=(const DrmCrtc &) = delete;
- int Init();
+ auto GetId() const {
+ return crtc_->crtc_id;
+ }
- uint32_t id() const;
- unsigned pipe() const;
+ auto GetIndexInResArray() const {
+ return index_in_res_array_;
+ }
- int display() const;
- void set_display(int display);
+ auto &GetActiveProperty() const {
+ return active_property_;
+ }
- bool can_bind(int display) const;
+ auto &GetModeProperty() const {
+ return mode_property_;
+ }
- const DrmProperty &active_property() const;
- const DrmProperty &mode_property() const;
- const DrmProperty &out_fence_ptr_property() const;
+ auto &GetOutFencePtrProperty() const {
+ return out_fence_ptr_property_;
+ }
private:
- DrmDevice *drm_;
+ DrmCrtc(DrmModeCrtcUnique crtc, uint32_t index)
+ : crtc_(std::move(crtc)), index_in_res_array_(index){};
- uint32_t id_;
- unsigned pipe_;
- int display_;
+ DrmModeCrtcUnique crtc_;
- DrmMode mode_;
+ const uint32_t index_in_res_array_;
DrmProperty active_property_;
DrmProperty mode_property_;
diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp
index a833c67..8f44d69 100644
--- a/drm/DrmDevice.cpp
+++ b/drm/DrmDevice.cpp
@@ -95,22 +95,12 @@
max_resolution_ = std::pair<uint32_t, uint32_t>(res->max_width,
res->max_height);
- for (int i = 0; !ret && i < res->count_crtcs; ++i) {
- auto c = MakeDrmModeCrtcUnique(fd(), res->crtcs[i]);
- if (!c) {
- ALOGE("Failed to get crtc %d", res->crtcs[i]);
- ret = -ENODEV;
- break;
+ for (int i = 0; i < res->count_crtcs; ++i) {
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+ auto crtc = DrmCrtc::CreateInstance(*this, res->crtcs[i], i);
+ if (crtc) {
+ crtcs_.emplace_back(std::move(crtc));
}
-
- std::unique_ptr<DrmCrtc> crtc(new DrmCrtc(this, c.get(), i));
-
- ret = crtc->Init();
- if (ret) {
- ALOGE("Failed to initialize crtc %d", res->crtcs[i]);
- break;
- }
- crtcs_.emplace_back(std::move(crtc));
}
std::vector<uint32_t> possible_clones;
@@ -125,10 +115,10 @@
std::vector<DrmCrtc *> possible_crtcs;
DrmCrtc *current_crtc = nullptr;
for (auto &crtc : crtcs_) {
- if ((1 << crtc->pipe()) & e->possible_crtcs)
+ if ((1 << crtc->GetIndexInResArray()) & e->possible_crtcs)
possible_crtcs.push_back(crtc.get());
- if (crtc->id() == e->crtc_id)
+ if (crtc->GetId() == e->crtc_id)
current_crtc = crtc.get();
}
@@ -241,11 +231,7 @@
}
DrmCrtc *DrmDevice::GetCrtcForDisplay(int display) const {
- for (const auto &crtc : crtcs_) {
- if (crtc->display() == display)
- return crtc.get();
- }
- return nullptr;
+ return bound_crtcs_.at(display);
}
const std::vector<std::unique_ptr<DrmCrtc>> &DrmDevice::crtcs() const {
@@ -259,9 +245,9 @@
int DrmDevice::TryEncoderForDisplay(int display, DrmEncoder *enc) {
/* First try to use the currently-bound crtc */
DrmCrtc *crtc = enc->crtc();
- if (crtc && crtc->can_bind(display)) {
- crtc->set_display(display);
- enc->set_crtc(crtc);
+ if (crtc && bound_crtcs_.count(display) == 0) {
+ bound_crtcs_[display] = crtc;
+ enc->set_crtc(crtc, display);
return 0;
}
@@ -271,9 +257,9 @@
if (crtc == enc->crtc())
continue;
- if (crtc->can_bind(display)) {
- crtc->set_display(display);
- enc->set_crtc(crtc);
+ if (bound_crtcs_.count(display) == 0) {
+ bound_crtcs_[display] = crtc;
+ enc->set_crtc(crtc, display);
return 0;
}
}
@@ -364,11 +350,6 @@
return found ? 0 : -ENOENT;
}
-int DrmDevice::GetCrtcProperty(const DrmCrtc &crtc, const char *prop_name,
- DrmProperty *property) const {
- return GetProperty(crtc.id(), DRM_MODE_OBJECT_CRTC, prop_name, property);
-}
-
int DrmDevice::GetConnectorProperty(const DrmConnector &connector,
const char *prop_name,
DrmProperty *property) const {
diff --git a/drm/DrmDevice.h b/drm/DrmDevice.h
index 9983d61..e3f6355 100644
--- a/drm/DrmDevice.h
+++ b/drm/DrmDevice.h
@@ -62,8 +62,6 @@
DrmConnector *GetConnectorForDisplay(int display) const;
DrmCrtc *GetCrtcForDisplay(int display) const;
- int GetCrtcProperty(const DrmCrtc &crtc, const char *prop_name,
- DrmProperty *property) const;
int GetConnectorProperty(const DrmConnector &connector, const char *prop_name,
DrmProperty *property) const;
@@ -108,6 +106,8 @@
std::pair<uint32_t, uint32_t> max_resolution_;
std::map<int, int> displays_;
+ std::map<int /*display*/, DrmCrtc *> bound_crtcs_;
+
bool HasAddFb2ModifiersSupport_{};
std::shared_ptr<DrmDevice> self;
diff --git a/drm/DrmEncoder.cpp b/drm/DrmEncoder.cpp
index ad05f88..e1628b2 100644
--- a/drm/DrmEncoder.cpp
+++ b/drm/DrmEncoder.cpp
@@ -48,9 +48,9 @@
possible_clones_.insert(possible_clone);
}
-void DrmEncoder::set_crtc(DrmCrtc *crtc) {
+void DrmEncoder::set_crtc(DrmCrtc *crtc, int display) {
crtc_ = crtc;
- display_ = crtc->display();
+ display_ = display;
}
int DrmEncoder::display() const {
diff --git a/drm/DrmEncoder.h b/drm/DrmEncoder.h
index b130b7d..a1f96d3 100644
--- a/drm/DrmEncoder.h
+++ b/drm/DrmEncoder.h
@@ -37,7 +37,7 @@
uint32_t id() const;
DrmCrtc *crtc() const;
- void set_crtc(DrmCrtc *crtc);
+ void set_crtc(DrmCrtc *crtc, int display);
bool can_bind(int display) const;
int display() const;
diff --git a/drm/DrmPlane.cpp b/drm/DrmPlane.cpp
index 5001457..6b5cec1 100644
--- a/drm/DrmPlane.cpp
+++ b/drm/DrmPlane.cpp
@@ -148,7 +148,7 @@
}
bool DrmPlane::IsCrtcSupported(const DrmCrtc &crtc) const {
- return ((1 << crtc.pipe()) & plane_->possible_crtcs) != 0;
+ return ((1 << crtc.GetIndexInResArray()) & plane_->possible_crtcs) != 0;
}
bool DrmPlane::IsValidForLayer(DrmHwcLayer *layer) {
diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp
index 8d1cb99..0ac626e 100644
--- a/drm/VSyncWorker.cpp
+++ b/drm/VSyncWorker.cpp
@@ -129,7 +129,8 @@
ALOGE("Failed to get crtc for display");
return;
}
- uint32_t high_crtc = (crtc->pipe() << DRM_VBLANK_HIGH_CRTC_SHIFT);
+ uint32_t high_crtc = (crtc->GetIndexInResArray()
+ << DRM_VBLANK_HIGH_CRTC_SHIFT);
drmVBlank vblank;
memset(&vblank, 0, sizeof(vblank));