drm_hwcomposer: Tidy-up DrmConnector class
Implement DrmConnector instantiation through CreateInstance() static method,
which helps to reduce complexity of DrmDevice::Init() function.
Move Connector-to-CRTC binding information to the DrmDevice class.
Move drm/DrmConnector.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/drm/DrmConnector.cpp b/drm/DrmConnector.cpp
index 7cbec95..5e00dd6 100644
--- a/drm/DrmConnector.cpp
+++ b/drm/DrmConnector.cpp
@@ -29,10 +29,12 @@
#include "utils/log.h"
#ifndef DRM_MODE_CONNECTOR_SPI
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
#define DRM_MODE_CONNECTOR_SPI 19
#endif
#ifndef DRM_MODE_CONNECTOR_USB
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
#define DRM_MODE_CONNECTOR_USB 20
#endif
@@ -40,62 +42,60 @@
constexpr size_t kTypesCount = 21;
-DrmConnector::DrmConnector(DrmDevice *drm, drmModeConnectorPtr c,
- DrmEncoder *current_encoder,
- std::vector<DrmEncoder *> &possible_encoders)
- : drm_(drm),
- id_(c->connector_id),
- encoder_(current_encoder),
- display_(-1),
- type_(c->connector_type),
- type_id_(c->connector_type_id),
- state_(c->connection),
- mm_width_(c->mmWidth),
- mm_height_(c->mmHeight),
- possible_encoders_(possible_encoders) {
+static bool GetOptionalConnectorProperty(const DrmDevice &dev,
+ const DrmConnector &connector,
+ const char *prop_name,
+ DrmProperty *property) {
+ return dev.GetProperty(connector.GetId(), DRM_MODE_OBJECT_CONNECTOR,
+ prop_name, property) == 0;
}
-int DrmConnector::Init() {
- int ret = drm_->GetConnectorProperty(*this, "DPMS", &dpms_property_);
- if (ret) {
- ALOGE("Could not get DPMS property\n");
- return ret;
+static bool GetConnectorProperty(const DrmDevice &dev,
+ const DrmConnector &connector,
+ const char *prop_name, DrmProperty *property) {
+ if (!GetOptionalConnectorProperty(dev, connector, prop_name, property)) {
+ ALOGE("Could not get %s property\n", prop_name);
+ return false;
}
- ret = drm_->GetConnectorProperty(*this, "CRTC_ID", &crtc_id_property_);
- if (ret) {
- ALOGE("Could not get CRTC_ID property\n");
- return ret;
+ return true;
+}
+
+auto DrmConnector::CreateInstance(DrmDevice &dev, uint32_t connector_id,
+ uint32_t index)
+ -> std::unique_ptr<DrmConnector> {
+ auto conn = MakeDrmModeConnectorUnique(dev.fd(), connector_id);
+ if (!conn) {
+ ALOGE("Failed to get connector %d", connector_id);
+ return {};
}
- UpdateEdidProperty();
- if (writeback()) {
- ret = drm_->GetConnectorProperty(*this, "WRITEBACK_PIXEL_FORMATS",
- &writeback_pixel_formats_);
- if (ret) {
- ALOGE("Could not get WRITEBACK_PIXEL_FORMATS connector_id = %d\n", id_);
- return ret;
- }
- ret = drm_->GetConnectorProperty(*this, "WRITEBACK_FB_ID",
- &writeback_fb_id_);
- if (ret) {
- ALOGE("Could not get WRITEBACK_FB_ID connector_id = %d\n", id_);
- return ret;
- }
- ret = drm_->GetConnectorProperty(*this, "WRITEBACK_OUT_FENCE_PTR",
- &writeback_out_fence_);
- if (ret) {
- ALOGE("Could not get WRITEBACK_OUT_FENCE_PTR connector_id = %d\n", id_);
- return ret;
- }
+
+ auto c = std::unique_ptr<DrmConnector>(
+ new DrmConnector(std::move(conn), &dev, index));
+
+ if (!GetConnectorProperty(dev, *c, "DPMS", &c->dpms_property_) ||
+ !GetConnectorProperty(dev, *c, "CRTC_ID", &c->crtc_id_property_)) {
+ return {};
}
- return 0;
+
+ c->UpdateEdidProperty();
+
+ if (c->IsWriteback() &&
+ (!GetConnectorProperty(dev, *c, "WRITEBACK_PIXEL_FORMATS",
+ &c->writeback_pixel_formats_) ||
+ !GetConnectorProperty(dev, *c, "WRITEBACK_FB_ID",
+ &c->writeback_fb_id_) ||
+ !GetConnectorProperty(dev, *c, "WRITEBACK_OUT_FENCE_PTR",
+ &c->writeback_out_fence_))) {
+ return {};
+ }
+
+ return c;
}
int DrmConnector::UpdateEdidProperty() {
- int ret = drm_->GetConnectorProperty(*this, "EDID", &edid_property_);
- if (ret) {
- ALOGW("Could not get EDID property\n");
- }
- return ret;
+ return GetOptionalConnectorProperty(*drm_, *this, "EDID", &edid_property_)
+ ? 0
+ : -EINVAL;
}
auto DrmConnector::GetEdidBlob() -> DrmModePropertyBlobUnique {
@@ -105,7 +105,7 @@
return {};
}
- std::tie(ret, blob_id) = edid_property().value();
+ std::tie(ret, blob_id) = GetEdidProperty().value();
if (ret != 0) {
return {};
}
@@ -113,137 +113,79 @@
return MakeDrmModePropertyBlobUnique(drm_->fd(), blob_id);
}
-uint32_t DrmConnector::id() const {
- return id_;
+bool DrmConnector::IsInternal() const {
+ auto type = connector_->connector_type;
+ return type == DRM_MODE_CONNECTOR_LVDS || type == DRM_MODE_CONNECTOR_eDP ||
+ type == DRM_MODE_CONNECTOR_DSI || type == DRM_MODE_CONNECTOR_VIRTUAL ||
+ type == DRM_MODE_CONNECTOR_DPI || type == DRM_MODE_CONNECTOR_SPI;
}
-int DrmConnector::display() const {
- return display_;
+bool DrmConnector::IsExternal() const {
+ auto type = connector_->connector_type;
+ return type == DRM_MODE_CONNECTOR_HDMIA ||
+ type == DRM_MODE_CONNECTOR_DisplayPort ||
+ type == DRM_MODE_CONNECTOR_DVID || type == DRM_MODE_CONNECTOR_DVII ||
+ type == DRM_MODE_CONNECTOR_VGA || type == DRM_MODE_CONNECTOR_USB;
}
-void DrmConnector::set_display(int display) {
- display_ = display;
-}
-
-bool DrmConnector::internal() const {
- return type_ == DRM_MODE_CONNECTOR_LVDS || type_ == DRM_MODE_CONNECTOR_eDP ||
- type_ == DRM_MODE_CONNECTOR_DSI ||
- type_ == DRM_MODE_CONNECTOR_VIRTUAL ||
- type_ == DRM_MODE_CONNECTOR_DPI || type_ == DRM_MODE_CONNECTOR_SPI;
-}
-
-bool DrmConnector::external() const {
- return type_ == DRM_MODE_CONNECTOR_HDMIA ||
- type_ == DRM_MODE_CONNECTOR_DisplayPort ||
- type_ == DRM_MODE_CONNECTOR_DVID || type_ == DRM_MODE_CONNECTOR_DVII ||
- type_ == DRM_MODE_CONNECTOR_VGA || type_ == DRM_MODE_CONNECTOR_USB;
-}
-
-bool DrmConnector::writeback() const {
+bool DrmConnector::IsWriteback() const {
#ifdef DRM_MODE_CONNECTOR_WRITEBACK
- return type_ == DRM_MODE_CONNECTOR_WRITEBACK;
+ return connector_->connector_type == DRM_MODE_CONNECTOR_WRITEBACK;
#else
return false;
#endif
}
-bool DrmConnector::valid_type() const {
- return internal() || external() || writeback();
+bool DrmConnector::IsValid() const {
+ return IsInternal() || IsExternal() || IsWriteback();
}
-std::string DrmConnector::name() const {
+std::string DrmConnector::GetName() const {
constexpr std::array<const char *, kTypesCount> kNames =
{"None", "VGA", "DVI-I", "DVI-D", "DVI-A", "Composite",
"SVIDEO", "LVDS", "Component", "DIN", "DP", "HDMI-A",
"HDMI-B", "TV", "eDP", "Virtual", "DSI", "DPI",
"Writeback", "SPI", "USB"};
- if (type_ < kTypesCount) {
+ if (connector_->connector_type < kTypesCount) {
std::ostringstream name_buf;
- name_buf << kNames[type_] << "-" << type_id_;
+ name_buf << kNames[connector_->connector_type] << "-"
+ << connector_->connector_type_id;
return name_buf.str();
}
- ALOGE("Unknown type in connector %d, could not make his name", id_);
+ ALOGE("Unknown type in connector %d, could not make his name", GetId());
return "None";
}
int DrmConnector::UpdateModes() {
- drmModeConnectorPtr c = drmModeGetConnector(drm_->fd(), id_);
- if (!c) {
- ALOGE("Failed to get connector %d", id_);
+ auto conn = MakeDrmModeConnectorUnique(drm_->fd(), GetId());
+ if (!conn) {
+ ALOGE("Failed to get connector %d", GetId());
return -ENODEV;
}
-
- state_ = c->connection;
+ connector_ = std::move(conn);
modes_.clear();
- for (int i = 0; i < c->count_modes; ++i) {
+ for (int i = 0; i < connector_->count_modes; ++i) {
bool exists = false;
for (const DrmMode &mode : modes_) {
- if (mode == c->modes[i]) {
+ if (mode == connector_->modes[i]) {
exists = true;
break;
}
}
if (!exists) {
- modes_.emplace_back(DrmMode(&c->modes[i]));
+ modes_.emplace_back(DrmMode(&connector_->modes[i]));
}
}
return 0;
}
-const DrmMode &DrmConnector::active_mode() const {
- return active_mode_;
-}
-
-void DrmConnector::set_active_mode(const DrmMode &mode) {
+void DrmConnector::SetActiveMode(DrmMode &mode) {
active_mode_ = mode;
}
-const DrmProperty &DrmConnector::dpms_property() const {
- return dpms_property_;
-}
-
-const DrmProperty &DrmConnector::crtc_id_property() const {
- return crtc_id_property_;
-}
-
-const DrmProperty &DrmConnector::edid_property() const {
- return edid_property_;
-}
-
-const DrmProperty &DrmConnector::writeback_pixel_formats() const {
- return writeback_pixel_formats_;
-}
-
-const DrmProperty &DrmConnector::writeback_fb_id() const {
- return writeback_fb_id_;
-}
-
-const DrmProperty &DrmConnector::writeback_out_fence() const {
- return writeback_out_fence_;
-}
-
-DrmEncoder *DrmConnector::encoder() const {
- return encoder_;
-}
-
-void DrmConnector::set_encoder(DrmEncoder *encoder) {
- encoder_ = encoder;
-}
-
-drmModeConnection DrmConnector::state() const {
- return state_;
-}
-
-uint32_t DrmConnector::mm_width() const {
- return mm_width_;
-}
-
-uint32_t DrmConnector::mm_height() const {
- return mm_height_;
-}
} // namespace android
diff --git a/drm/DrmConnector.h b/drm/DrmConnector.h
index 2bcb543..5f8ba88 100644
--- a/drm/DrmConnector.h
+++ b/drm/DrmConnector.h
@@ -34,67 +34,95 @@
class DrmConnector {
public:
- DrmConnector(DrmDevice *drm, drmModeConnectorPtr c,
- DrmEncoder *current_encoder,
- std::vector<DrmEncoder *> &possible_encoders);
+ static auto CreateInstance(DrmDevice &dev, uint32_t connector_id,
+ uint32_t index) -> std::unique_ptr<DrmConnector>;
+
DrmConnector(const DrmProperty &) = delete;
DrmConnector &operator=(const DrmProperty &) = delete;
- int Init();
int UpdateEdidProperty();
auto GetEdidBlob() -> DrmModePropertyBlobUnique;
- uint32_t id() const;
+ auto GetDev() const -> DrmDevice & {
+ return *drm_;
+ }
- int display() const;
- void set_display(int display);
+ auto GetId() const {
+ return connector_->connector_id;
+ }
- bool internal() const;
- bool external() const;
- bool writeback() const;
- bool valid_type() const;
+ auto GetIndexInResArray() const {
+ return index_in_res_array_;
+ }
- std::string name() const;
+ auto GetCurrentEncoderId() const {
+ return connector_->encoder_id;
+ }
+
+ auto SupportsEncoder(DrmEncoder &enc) const {
+ for (int i = 0; i < connector_->count_encoders; i++) {
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+ if (connector_->encoders[i] == enc.GetId()) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ bool IsInternal() const;
+ bool IsExternal() const;
+ bool IsWriteback() const;
+ bool IsValid() const;
+
+ std::string GetName() const;
int UpdateModes();
- const std::vector<DrmMode> &modes() const {
+ auto &GetModes() const {
return modes_;
}
- const DrmMode &active_mode() const;
- void set_active_mode(const DrmMode &mode);
- const DrmProperty &dpms_property() const;
- const DrmProperty &crtc_id_property() const;
- const DrmProperty &edid_property() const;
- const DrmProperty &writeback_pixel_formats() const;
- const DrmProperty &writeback_fb_id() const;
- const DrmProperty &writeback_out_fence() const;
-
- const std::vector<DrmEncoder *> &possible_encoders() const {
- return possible_encoders_;
+ auto &GetActiveMode() const {
+ return active_mode_;
}
- DrmEncoder *encoder() const;
- void set_encoder(DrmEncoder *encoder);
- drmModeConnection state() const;
+ void SetActiveMode(DrmMode &mode);
- uint32_t mm_width() const;
- uint32_t mm_height() const;
+ auto &GetDpmsProperty() const {
+ return dpms_property_;
+ }
+
+ auto &GetCrtcIdProperty() const {
+ return crtc_id_property_;
+ }
+
+ auto &GetEdidProperty() const {
+ return edid_property_;
+ }
+
+ auto IsConnected() const {
+ return connector_->connection == DRM_MODE_CONNECTED;
+ }
+
+ auto GetMmWidth() const {
+ return connector_->mmWidth;
+ }
+
+ auto GetMmHeight() const {
+ return connector_->mmHeight;
+ };
private:
- DrmDevice *drm_;
+ DrmConnector(DrmModeConnectorUnique connector, DrmDevice *drm, uint32_t index)
+ : connector_(std::move(connector)),
+ drm_(drm),
+ index_in_res_array_(index){};
- uint32_t id_;
- DrmEncoder *encoder_;
- int display_;
+ DrmModeConnectorUnique connector_;
+ DrmDevice *const drm_;
- uint32_t type_;
- uint32_t type_id_;
- drmModeConnection state_;
-
- uint32_t mm_width_;
- uint32_t mm_height_;
+ const uint32_t index_in_res_array_;
DrmMode active_mode_;
std::vector<DrmMode> modes_;
@@ -105,8 +133,6 @@
DrmProperty writeback_pixel_formats_;
DrmProperty writeback_fb_id_;
DrmProperty writeback_out_fence_;
-
- std::vector<DrmEncoder *> possible_encoders_;
};
} // namespace android
diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp
index f5e3521..a6d2289 100644
--- a/drm/DrmDevice.cpp
+++ b/drm/DrmDevice.cpp
@@ -111,47 +111,28 @@
}
}
- for (int i = 0; !ret && i < res->count_connectors; ++i) {
- auto c = MakeDrmModeConnectorUnique(fd(), res->connectors[i]);
- if (!c) {
- ALOGE("Failed to get connector %d", res->connectors[i]);
- ret = -ENODEV;
- break;
+ for (int i = 0; i < res->count_connectors; ++i) {
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+ auto conn = DrmConnector::CreateInstance(*this, res->connectors[i], i);
+
+ if (!conn) {
+ continue;
}
- std::vector<DrmEncoder *> possible_encoders;
- DrmEncoder *current_encoder = nullptr;
- for (int j = 0; j < c->count_encoders; ++j) {
- for (auto &encoder : encoders_) {
- if (encoder->GetId() == c->encoders[j])
- possible_encoders.push_back(encoder.get());
- if (encoder->GetId() == c->encoder_id)
- current_encoder = encoder.get();
- }
- }
-
- std::unique_ptr<DrmConnector> conn(
- new DrmConnector(this, c.get(), current_encoder, possible_encoders));
-
- ret = conn->Init();
- if (ret) {
- ALOGE("Init connector %d failed", res->connectors[i]);
- break;
- }
-
- if (conn->writeback())
+ if (conn->IsWriteback()) {
writeback_connectors_.emplace_back(std::move(conn));
- else
+ } else {
connectors_.emplace_back(std::move(conn));
+ }
}
auto add_displays = [this, &num_displays](bool internal, bool connected) {
for (auto &conn : connectors_) {
- bool is_connected = conn->state() == DRM_MODE_CONNECTED;
- if ((internal ? conn->internal() : conn->external()) &&
+ bool is_connected = conn->IsConnected();
+ if ((internal ? conn->IsInternal() : conn->IsExternal()) &&
(connected ? is_connected : !is_connected)) {
- conn->set_display(num_displays);
- displays_[num_displays] = num_displays;
+ bound_connectors_[num_displays] = conn.get();
+ connectors_to_display_id_[conn.get()] = num_displays;
++num_displays;
}
}
@@ -187,23 +168,19 @@
for (auto &conn : connectors_) {
ret = CreateDisplayPipe(conn.get());
if (ret) {
- ALOGE("Failed CreateDisplayPipe %d with %d", conn->id(), ret);
+ ALOGE("Failed CreateDisplayPipe %d with %d", conn->GetId(), ret);
return std::make_tuple(ret, 0);
}
}
- return std::make_tuple(ret, displays_.size());
+ return std::make_tuple(ret, bound_connectors_.size());
}
bool DrmDevice::HandlesDisplay(int display) const {
- return displays_.find(display) != displays_.end();
+ return bound_connectors_.count(display) != 0;
}
DrmConnector *DrmDevice::GetConnectorForDisplay(int display) const {
- for (const auto &conn : connectors_) {
- if (conn->display() == display)
- return conn.get();
- }
- return nullptr;
+ return bound_connectors_.at(display);
}
DrmCrtc *DrmDevice::GetCrtcForDisplay(int display) const {
@@ -246,11 +223,13 @@
}
int DrmDevice::CreateDisplayPipe(DrmConnector *connector) {
- int display = connector->display();
+ int display = connectors_to_display_id_.at(connector);
/* Try to use current setup first */
- if (connector->encoder()) {
- int ret = TryEncoderForDisplay(display, connector->encoder());
+ auto *enc0 = FindEncoderById(connector->GetCurrentEncoderId());
+ if (enc0 != nullptr && encoders_to_display_id_.count(enc0) == 0) {
+ int ret = TryEncoderForDisplay(display, enc0);
if (!ret) {
+ encoders_to_display_id_[enc0] = display;
return 0;
}
@@ -260,10 +239,15 @@
}
}
- for (DrmEncoder *enc : connector->possible_encoders()) {
- int ret = TryEncoderForDisplay(display, enc);
+ for (auto &enc : encoders_) {
+ if (!connector->SupportsEncoder(*enc) ||
+ encoders_to_display_id_.count(enc.get()) != 0) {
+ continue;
+ }
+
+ int ret = TryEncoderForDisplay(display, enc.get());
if (!ret) {
- connector->set_encoder(enc);
+ encoders_to_display_id_[enc.get()] = display;
return 0;
}
@@ -272,8 +256,7 @@
return ret;
}
}
- ALOGE("Could not find a suitable encoder/crtc for display %d",
- connector->display());
+ ALOGE("Could not find a suitable encoder/crtc for display %d", display);
return -ENODEV;
}
@@ -327,13 +310,6 @@
return found ? 0 : -ENOENT;
}
-int DrmDevice::GetConnectorProperty(const DrmConnector &connector,
- const char *prop_name,
- DrmProperty *property) const {
- return GetProperty(connector.id(), DRM_MODE_OBJECT_CONNECTOR, prop_name,
- property);
-}
-
std::string DrmDevice::GetName() const {
auto *ver = drmGetVersion(fd());
if (!ver) {
diff --git a/drm/DrmDevice.h b/drm/DrmDevice.h
index 82b0f6a..cd87127 100644
--- a/drm/DrmDevice.h
+++ b/drm/DrmDevice.h
@@ -62,9 +62,6 @@
DrmConnector *GetConnectorForDisplay(int display) const;
DrmCrtc *GetCrtcForDisplay(int display) const;
- int GetConnectorProperty(const DrmConnector &connector, const char *prop_name,
- DrmProperty *property) const;
-
std::string GetName() const;
const std::vector<std::unique_ptr<DrmCrtc>> &crtcs() const;
@@ -95,6 +92,20 @@
return nullptr;
}
+ auto FindEncoderById(uint32_t id) const -> DrmEncoder * {
+ for (const auto &enc : encoders_) {
+ if (enc->GetId() == id) {
+ return enc.get();
+ }
+ };
+
+ return nullptr;
+ }
+
+ auto GetDisplayId(DrmConnector *conn) {
+ return connectors_to_display_id_.at(conn);
+ }
+
int GetProperty(uint32_t obj_id, uint32_t obj_type, const char *prop_name,
DrmProperty *property) const;
@@ -114,9 +125,11 @@
std::pair<uint32_t, uint32_t> min_resolution_;
std::pair<uint32_t, uint32_t> max_resolution_;
- std::map<int, int> displays_;
std::map<int /*display*/, DrmCrtc *> bound_crtcs_;
+ std::map<int /*display*/, DrmConnector *> bound_connectors_;
+ std::map<DrmConnector *, int /*display*/> connectors_to_display_id_;
+ std::map<DrmEncoder *, int /*display*/> encoders_to_display_id_;
std::map<DrmCrtc *, DrmEncoder *> bound_encoders_;
bool HasAddFb2ModifiersSupport_{};
diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp
index 0ac626e..3bcb4d0 100644
--- a/drm/VSyncWorker.cpp
+++ b/drm/VSyncWorker.cpp
@@ -87,11 +87,11 @@
float refresh = 60.0F; // Default to 60Hz refresh rate
DrmConnector *conn = drm_->GetConnectorForDisplay(display_);
- if (conn && conn->active_mode().v_refresh() != 0.0F)
- refresh = conn->active_mode().v_refresh();
+ if (conn && conn->GetActiveMode().v_refresh() != 0.0F)
+ refresh = conn->GetActiveMode().v_refresh();
else
ALOGW("Vsync worker active with conn=%p refresh=%f\n", conn,
- conn ? conn->active_mode().v_refresh() : 0.0F);
+ conn ? conn->GetActiveMode().v_refresh() : 0.0F);
int64_t phased_timestamp = GetPhasedVSync(kOneSecondNs /
static_cast<int>(refresh),