diff options
author | Roman Stratiienko <roman.o.stratiienko@globallogic.com> | 2022-01-30 20:28:46 +0200 |
---|---|---|
committer | Roman Stratiienko <roman.o.stratiienko@globallogic.com> | 2022-01-31 21:31:32 +0200 |
commit | 10be875de071672e61a52ab0679bbc6803df8c51 (patch) | |
tree | 0bbe92cbdb37e83745e2bc03a3a1048d8fa989c5 | |
parent | edb97ed8b58952641aae12f77225df305acead53 (diff) | |
download | drm_hwcomposer-10be875de071672e61a52ab0679bbc6803df8c51.tar.gz |
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>
-rw-r--r-- | .ci/Makefile | 1 | ||||
-rw-r--r-- | compositor/DrmDisplayCompositor.cpp | 15 | ||||
-rw-r--r-- | compositor/Planner.cpp | 2 | ||||
-rw-r--r-- | drm/DrmCrtc.cpp | 61 | ||||
-rw-r--r-- | drm/DrmCrtc.h | 39 | ||||
-rw-r--r-- | drm/DrmDevice.cpp | 47 | ||||
-rw-r--r-- | drm/DrmDevice.h | 4 | ||||
-rw-r--r-- | drm/DrmEncoder.cpp | 4 | ||||
-rw-r--r-- | drm/DrmEncoder.h | 2 | ||||
-rw-r--r-- | drm/DrmPlane.cpp | 2 | ||||
-rw-r--r-- | drm/VSyncWorker.cpp | 3 |
11 files changed, 76 insertions, 104 deletions
diff --git a/.ci/Makefile b/.ci/Makefile index 7f43a8a..e203673 100644 --- a/.ci/Makefile +++ b/.ci/Makefile @@ -29,7 +29,6 @@ TIDY_FILES_OVERRIDE := \ 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 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { } 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 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { 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 DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { 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 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { 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::tuple<int, std::vector<DrmCompositionPlane>> Planner::ProvisionPlanes( 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; -} -uint32_t DrmCrtc::id() const { - return id_; + return c; } -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 DrmDevice; 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 @@ std::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) { 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; - } - - 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; + 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)); } - crtcs_.emplace_back(std::move(crtc)); } std::vector<uint32_t> possible_clones; @@ -125,10 +115,10 @@ std::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) { 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 @@ DrmConnector *DrmDevice::GetConnectorForDisplay(int display) const { } 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 @@ uint32_t DrmDevice::next_mode_id() { 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 @@ int DrmDevice::TryEncoderForDisplay(int display, DrmEncoder *enc) { 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 @@ int DrmDevice::GetProperty(uint32_t obj_id, uint32_t obj_type, 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 @@ class DrmDevice { 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 @@ class DrmDevice { 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 @@ void DrmEncoder::AddPossibleClone(DrmEncoder *possible_clone) { 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 @@ class DrmEncoder { 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 @@ int DrmPlane::Init() { } 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 @@ void VSyncWorker::Routine() { 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)); |