From dfac456143276fd2e148e76a29165a9ed6c73b8c Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Thu, 20 Jan 2022 17:12:40 +0200 Subject: drm_hwcomposer: Fix build_deploy.sh script Composer service has to be started again. Signed-off-by: Roman Stratiienko --- build_deploy.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/build_deploy.sh b/build_deploy.sh index ba9732b..ef25e5c 100755 --- a/build_deploy.sh +++ b/build_deploy.sh @@ -14,10 +14,10 @@ mm adb root && adb remount && adb sync vendor adb shell stop -adb shell stop vendor.hwcomposer-2-1 || true -adb shell stop vendor.hwcomposer-2-2 || true -adb shell stop vendor.hwcomposer-2-3 || true -adb shell stop vendor.hwcomposer-2-4 || true +adb shell stop vendor.hwcomposer-2-1 && adb shell start vendor.hwcomposer-2-1 || true +adb shell stop vendor.hwcomposer-2-2 && adb shell start vendor.hwcomposer-2-2 || true +adb shell stop vendor.hwcomposer-2-3 && adb shell start vendor.hwcomposer-2-3 || true +adb shell stop vendor.hwcomposer-2-4 && adb shell start vendor.hwcomposer-2-4 || true [ $HWCLOG -eq "1" ] && adb logcat -c -- cgit v1.2.3 From 938a74244d7afbff431a0430e37f462968ed55a3 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Sat, 29 Jan 2022 00:10:07 +0200 Subject: drm_hwcomposer: Simplify DrmHwcTwo::GetDisplay() Cosmetic change: make GetDisplay() non-static class method + use deduced return type. Signed-off-by: Roman Stratiienko --- hwc2_device/DrmHwcTwo.h | 13 +++++-------- hwc2_device/hwc2_device.cpp | 8 ++++---- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/hwc2_device/DrmHwcTwo.h b/hwc2_device/DrmHwcTwo.h index f38ba05..fa13170 100644 --- a/hwc2_device/DrmHwcTwo.h +++ b/hwc2_device/DrmHwcTwo.h @@ -37,14 +37,6 @@ class DrmHwcTwo { #endif std::pair refresh_callback_{}; - static HwcDisplay *GetDisplay(DrmHwcTwo *hwc, hwc2_display_t display_handle) { - auto it = hwc->displays_.find(display_handle); - if (it == hwc->displays_.end()) - return nullptr; - - return &it->second; - } - // Device functions HWC2::Error CreateVirtualDisplay(uint32_t width, uint32_t height, int32_t *format, hwc2_display_t *display); @@ -55,6 +47,11 @@ class DrmHwcTwo { hwc2_function_pointer_t function); HWC2::Error CreateDisplay(hwc2_display_t displ, HWC2::DisplayType type); + auto GetDisplay(hwc2_display_t display_handle) { + return displays_.count(display_handle) != 0 ? &displays_.at(display_handle) + : nullptr; + } + auto &GetResMan() { return resource_manager_; } diff --git a/hwc2_device/hwc2_device.cpp b/hwc2_device/hwc2_device.cpp index 6d258e8..57bc205 100644 --- a/hwc2_device/hwc2_device.cpp +++ b/hwc2_device/hwc2_device.cpp @@ -74,8 +74,8 @@ static int32_t DisplayHook(hwc2_device_t *dev, hwc2_display_t display_handle, GetFuncName(__PRETTY_FUNCTION__).c_str()); DrmHwcTwo *hwc = ToDrmHwcTwo(dev); const std::lock_guard lock(hwc->GetResMan().GetMainLock()); - HwcDisplay *display = DrmHwcTwo::GetDisplay(hwc, display_handle); - if (!display) + auto *display = hwc->GetDisplay(display_handle); + if (display == nullptr) return static_cast(HWC2::Error::BadDisplay); return static_cast((display->*func)(std::forward(args)...)); @@ -88,8 +88,8 @@ static int32_t LayerHook(hwc2_device_t *dev, hwc2_display_t display_handle, layer_handle, GetFuncName(__PRETTY_FUNCTION__).c_str()); DrmHwcTwo *hwc = ToDrmHwcTwo(dev); const std::lock_guard lock(hwc->GetResMan().GetMainLock()); - HwcDisplay *display = DrmHwcTwo::GetDisplay(hwc, display_handle); - if (!display) + auto *display = hwc->GetDisplay(display_handle); + if (display == nullptr) return static_cast(HWC2::Error::BadDisplay); HwcLayer *layer = display->get_layer(layer_handle); -- cgit v1.2.3 From b671fab16a08830703f5fe73037563ff8001a1a3 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Sat, 29 Jan 2022 00:50:22 +0200 Subject: drm_hwcomposer: Tidy-up DrmPlane class This allow to throw away few lines from DrmDevice::Init() making it less complicated. Signed-off-by: Roman Stratiienko --- compositor/Planner.cpp | 4 ++-- drm/DrmDevice.cpp | 20 ++++-------------- drm/DrmPlane.cpp | 54 ++++++++++++++++++++++++++--------------------- drm/DrmPlane.h | 27 ++++++++++++++++-------- hwc2_device/DrmHwcTwo.cpp | 2 +- 5 files changed, 55 insertions(+), 52 deletions(-) diff --git a/compositor/Planner.cpp b/compositor/Planner.cpp index f43e314..24b43ba 100644 --- a/compositor/Planner.cpp +++ b/compositor/Planner.cpp @@ -31,10 +31,10 @@ std::vector Planner::GetUsablePlanes( std::vector usable_planes; std::copy_if(primary_planes->begin(), primary_planes->end(), std::back_inserter(usable_planes), - [=](DrmPlane *plane) { return plane->GetCrtcSupported(*crtc); }); + [=](DrmPlane *plane) { return plane->IsCrtcSupported(*crtc); }); std::copy_if(overlay_planes->begin(), overlay_planes->end(), std::back_inserter(usable_planes), - [=](DrmPlane *plane) { return plane->GetCrtcSupported(*crtc); }); + [=](DrmPlane *plane) { return plane->IsCrtcSupported(*crtc); }); return usable_planes; } diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp index 8245b78..1c8427c 100644 --- a/drm/DrmDevice.cpp +++ b/drm/DrmDevice.cpp @@ -305,25 +305,13 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { } for (uint32_t i = 0; i < plane_res->count_planes; ++i) { - auto p = MakeDrmModePlaneUnique(fd(), plane_res->planes[i]); - if (!p) { - ALOGE("Failed to get plane %d", plane_res->planes[i]); - ret = -ENODEV; - break; - } + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + auto plane = DrmPlane::CreateInstance(*this, plane_res->planes[i]); - std::unique_ptr plane(new DrmPlane(this, p.get())); - - ret = plane->Init(); - if (ret) { - ALOGE("Init plane %d failed", plane_res->planes[i]); - break; + if (plane) { + planes_.emplace_back(std::move(plane)); } - - planes_.emplace_back(std::move(plane)); } - if (ret) - return std::make_tuple(ret, 0); for (auto &conn : connectors_) { ret = CreateDisplayPipe(conn.get()); diff --git a/drm/DrmPlane.cpp b/drm/DrmPlane.cpp index 25a4902..5001457 100644 --- a/drm/DrmPlane.cpp +++ b/drm/DrmPlane.cpp @@ -29,15 +29,28 @@ namespace android { -DrmPlane::DrmPlane(DrmDevice *drm, drmModePlanePtr p) - : drm_(drm), - id_(p->plane_id), - possible_crtc_mask_(p->possible_crtcs), - // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) - formats_(p->formats, p->formats + p->count_formats) { +auto DrmPlane::CreateInstance(DrmDevice &dev, uint32_t plane_id) + -> std::unique_ptr { + auto p = MakeDrmModePlaneUnique(dev.fd(), plane_id); + if (!p) { + ALOGE("Failed to get plane %d", plane_id); + return {}; + } + + auto plane = std::unique_ptr(new DrmPlane(dev, std::move(p))); + + if (plane->Init() != 0) { + ALOGE("Failed to init plane %d", plane_id); + return {}; + } + + return plane; } int DrmPlane::Init() { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + formats_ = {plane_->formats, plane_->formats + plane_->count_formats}; + DrmProperty p; if (!GetPlaneProperty("type", p)) { @@ -134,38 +147,38 @@ int DrmPlane::Init() { return 0; } -bool DrmPlane::GetCrtcSupported(const DrmCrtc &crtc) const { - return ((1 << crtc.pipe()) & possible_crtc_mask_) != 0; +bool DrmPlane::IsCrtcSupported(const DrmCrtc &crtc) const { + return ((1 << crtc.pipe()) & plane_->possible_crtcs) != 0; } bool DrmPlane::IsValidForLayer(DrmHwcLayer *layer) { if (!rotation_property_) { if (layer->transform != DrmHwcTransform::kIdentity) { - ALOGV("No rotation property on plane %d", id_); + ALOGV("No rotation property on plane %d", GetId()); return false; } } else { if (transform_enum_map_.count(layer->transform) == 0) { - ALOGV("Transform is not supported on plane %d", id_); + ALOGV("Transform is not supported on plane %d", GetId()); return false; } } if (alpha_property_.id() == 0 && layer->alpha != UINT16_MAX) { - ALOGV("Alpha is not supported on plane %d", id_); + ALOGV("Alpha is not supported on plane %d", GetId()); return false; } if (blending_enum_map_.count(layer->blending) == 0 && layer->blending != DrmHwcBlending::kNone && layer->blending != DrmHwcBlending::kPreMult) { - ALOGV("Blending is not supported on plane %d", id_); + ALOGV("Blending is not supported on plane %d", GetId()); return false; } uint32_t format = layer->buffer_info.format; if (!IsFormatSupported(format)) { - ALOGV("Plane %d does not supports %c%c%c%c format", id_, format, + ALOGV("Plane %d does not supports %c%c%c%c format", GetId(), format, format >> 8, format >> 16, format >> 24); return false; } @@ -173,10 +186,6 @@ bool DrmPlane::IsValidForLayer(DrmHwcLayer *layer) { return true; } -uint32_t DrmPlane::GetType() const { - return type_; -} - bool DrmPlane::IsFormatSupported(uint32_t format) const { return std::find(std::begin(formats_), std::end(formats_), format) != std::end(formats_); @@ -290,20 +299,17 @@ auto DrmPlane::AtomicDisablePlane(drmModeAtomicReq &pset) -> int { return 0; } -const DrmProperty &DrmPlane::GetZPosProperty() const { - return zpos_property_; -} - auto DrmPlane::GetPlaneProperty(const char *prop_name, DrmProperty &property, Presence presence) -> bool { - int err = drm_->GetProperty(id_, DRM_MODE_OBJECT_PLANE, prop_name, &property); + int err = drm_->GetProperty(GetId(), DRM_MODE_OBJECT_PLANE, prop_name, + &property); if (err != 0) { if (presence == Presence::kMandatory) { ALOGE("Could not get mandatory property \"%s\" from plane %d", prop_name, - id_); + GetId()); } else { ALOGV("Could not get optional property \"%s\" from plane %d", prop_name, - id_); + GetId()); } return false; } diff --git a/drm/DrmPlane.h b/drm/DrmPlane.h index e1ee920..9826f67 100644 --- a/drm/DrmPlane.h +++ b/drm/DrmPlane.h @@ -33,16 +33,18 @@ struct DrmHwcLayer; class DrmPlane { public: - DrmPlane(DrmDevice *drm, drmModePlanePtr p); DrmPlane(const DrmPlane &) = delete; DrmPlane &operator=(const DrmPlane &) = delete; - int Init(); + static auto CreateInstance(DrmDevice &dev, uint32_t plane_id) + -> std::unique_ptr; - bool GetCrtcSupported(const DrmCrtc &crtc) const; + bool IsCrtcSupported(const DrmCrtc &crtc) const; bool IsValidForLayer(DrmHwcLayer *layer); - uint32_t GetType() const; + auto GetType() const { + return type_; + } bool IsFormatSupported(uint32_t format) const; bool HasNonRgbFormat() const; @@ -50,19 +52,26 @@ class DrmPlane { auto AtomicSetState(drmModeAtomicReq &pset, DrmHwcLayer &layer, uint32_t zpos, uint32_t crtc_id) -> int; auto AtomicDisablePlane(drmModeAtomicReq &pset) -> int; - const DrmProperty &GetZPosProperty() const; + auto &GetZPosProperty() const { + return zpos_property_; + } + + auto GetId() const { + return plane_->plane_id; + } private: - DrmDevice *drm_; - uint32_t id_; + DrmPlane(DrmDevice &dev, DrmModePlaneUnique plane) + : drm_(&dev), plane_(std::move(plane)){}; + DrmDevice *const drm_; + DrmModePlaneUnique plane_; enum class Presence { kOptional, kMandatory }; + auto Init() -> int; auto GetPlaneProperty(const char *prop_name, DrmProperty &property, Presence presence = Presence::kMandatory) -> bool; - uint32_t possible_crtc_mask_; - uint32_t type_{}; std::vector formats_; diff --git a/hwc2_device/DrmHwcTwo.cpp b/hwc2_device/DrmHwcTwo.cpp index a2a093f..87cc238 100644 --- a/hwc2_device/DrmHwcTwo.cpp +++ b/hwc2_device/DrmHwcTwo.cpp @@ -43,7 +43,7 @@ HWC2::Error DrmHwcTwo::CreateDisplay(hwc2_display_t displ, } auto display_planes = std::vector(); for (const auto &plane : drm->planes()) { - if (plane->GetCrtcSupported(*crtc)) + if (plane->IsCrtcSupported(*crtc)) display_planes.push_back(plane.get()); } displays_.at(displ).Init(&display_planes); -- cgit v1.2.3 From 456e2d6b0d3a98e62e942c083cde02e853c4af24 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Sat, 29 Jan 2022 01:17:39 +0200 Subject: drm_hwcomposer: Handle primary display in GetDisplayConnectionType() Primary display should always be internal. Closes: https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/-/issues/58 Signed-off-by: Roman Stratiienko --- hwc2_device/HwcDisplay.cpp | 9 ++++++++- hwc2_device/HwcDisplay.h | 5 ++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp index 8936136..f18e00d 100644 --- a/hwc2_device/HwcDisplay.cpp +++ b/hwc2_device/HwcDisplay.cpp @@ -708,7 +708,14 @@ std::vector HwcDisplay::GetOrderLayersByZPos() { #if PLATFORM_SDK_VERSION > 29 HWC2::Error HwcDisplay::GetDisplayConnectionType(uint32_t *outType) { - if (connector_->internal()) + if (IsInHeadlessMode()) { + *outType = static_cast(HWC2::DisplayConnectionType::Internal); + return HWC2::Error::None; + } + /* Primary display should be always internal, + * otherwise SF will be unhappy and will crash + */ + if (connector_->internal() || handle_ == kPrimaryDisplay) *outType = static_cast(HWC2::DisplayConnectionType::Internal); else if (connector_->external()) *outType = static_cast(HWC2::DisplayConnectionType::External); diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h index c3e0f6e..76456b7 100644 --- a/hwc2_device/HwcDisplay.h +++ b/hwc2_device/HwcDisplay.h @@ -33,6 +33,8 @@ namespace android { class Backend; class DrmHwcTwo; +inline constexpr uint32_t kPrimaryDisplay = 0; + class HwcDisplay { public: HwcDisplay(ResourceManager *resource_manager, DrmDevice *drm, @@ -207,7 +209,8 @@ class HwcDisplay { * https://source.android.com/devices/graphics/hotplug#handling-common-scenarios */ bool IsInHeadlessMode() { - return handle_ == 0 && connector_->state() != DRM_MODE_CONNECTED; + return handle_ == kPrimaryDisplay && + connector_->state() != DRM_MODE_CONNECTED; } private: -- cgit v1.2.3 From edb97ed8b58952641aae12f77225df305acead53 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Sat, 29 Jan 2022 19:53:14 +0200 Subject: drm_hwcomposer: Remove ability to prioritize primary display This feature isn't correctly fits hwc2 and SF requirements. Primary display prioritization shall be done by introducing ability to override internal/external connector type for any connector. 'vendor.hwc.drm.primary_display_order' property is no longer relevant. Signed-off-by: Roman Stratiienko --- drm/DrmDevice.cpp | 123 +++++++----------------------------------------------- 1 file changed, 14 insertions(+), 109 deletions(-) diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp index 1c8427c..a833c67 100644 --- a/drm/DrmDevice.cpp +++ b/drm/DrmDevice.cpp @@ -34,84 +34,8 @@ #include "utils/log.h" #include "utils/properties.h" -static void trim_left(std::string *str) { - str->erase(std::begin(*str), - std::find_if(std::begin(*str), std::end(*str), - [](int ch) { return std::isspace(ch) == 0; })); -} - -static void trim_right(std::string *str) { - str->erase(std::find_if(std::rbegin(*str), std::rend(*str), - [](int ch) { return std::isspace(ch) == 0; }) - .base(), - std::end(*str)); -} - -static void trim(std::string *str) { - trim_left(str); - trim_right(str); -} - namespace android { -static std::vector read_primary_display_order_prop() { - std::array display_order_buf{}; - property_get("vendor.hwc.drm.primary_display_order", display_order_buf.data(), - "..."); - - std::vector display_order; - std::istringstream str(display_order_buf.data()); - for (std::string conn_name; std::getline(str, conn_name, ',');) { - trim(&conn_name); - display_order.push_back(std::move(conn_name)); - } - return display_order; -} - -static std::vector make_primary_display_candidates( - const std::vector> &connectors) { - std::vector primary_candidates; - std::transform(std::begin(connectors), std::end(connectors), - std::back_inserter(primary_candidates), - [](const std::unique_ptr &conn) { - return conn.get(); - }); - primary_candidates.erase(std::remove_if(std::begin(primary_candidates), - std::end(primary_candidates), - [](const DrmConnector *conn) { - return conn->state() != - DRM_MODE_CONNECTED; - }), - std::end(primary_candidates)); - - std::vector display_order = read_primary_display_order_prop(); - bool use_other = display_order.back() == "..."; - - // putting connectors from primary_display_order first - auto curr_connector = std::begin(primary_candidates); - for (const std::string &display_name : display_order) { - auto it = std::find_if(std::begin(primary_candidates), - std::end(primary_candidates), - [&display_name](const DrmConnector *conn) { - return conn->name() == display_name; - }); - if (it != std::end(primary_candidates)) { - std::iter_swap(it, curr_connector); - ++curr_connector; - } - } - - if (use_other) { - // then putting internal connectors second, everything else afterwards - std::partition(curr_connector, std::end(primary_candidates), - [](const DrmConnector *conn) { return conn->internal(); }); - } else { - primary_candidates.erase(curr_connector, std::end(primary_candidates)); - } - - return primary_candidates; -} - DrmDevice::DrmDevice() { self.reset(this); mDrmFbImporter = std::make_unique(self); @@ -171,10 +95,6 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { max_resolution_ = std::pair(res->max_width, res->max_height); - // Assumes that the primary display will always be in the first - // drm_device opened. - bool found_primary = num_displays != 0; - for (int i = 0; !ret && i < res->count_crtcs; ++i) { auto c = MakeDrmModeCrtcUnique(fd(), res->crtcs[i]); if (!c) { @@ -259,40 +179,25 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { connectors_.emplace_back(std::move(conn)); } - // Primary display priority: - // 1) vendor.hwc.drm.primary_display_order property - // 2) internal connectors - // 3) anything else - std::vector - primary_candidates = make_primary_display_candidates(connectors_); - if (!primary_candidates.empty() && !found_primary) { - DrmConnector &conn = **std::begin(primary_candidates); - conn.set_display(num_displays); - displays_[num_displays] = num_displays; - ++num_displays; - found_primary = true; - } else { - ALOGE( - "Failed to find primary display from " - "\"vendor.hwc.drm.primary_display_order\" property"); - } - - // If no priority display were found then pick first available as primary and - // for the others assign consecutive display_numbers. - for (auto &conn : connectors_) { - if (conn->external() || conn->internal()) { - if (!found_primary) { - conn->set_display(num_displays); - displays_[num_displays] = num_displays; - found_primary = true; - ++num_displays; - } else if (conn->display() < 0) { + 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()) && + (connected ? is_connected : !is_connected)) { conn->set_display(num_displays); displays_[num_displays] = num_displays; ++num_displays; } } - } + }; + + /* Put internal first to ensure Primary display will be internal + * in case at least 1 internal is available + */ + add_displays(/*internal = */ true, /*connected = */ true); + add_displays(/*internal = */ false, /*connected = */ true); + add_displays(/*internal = */ true, /*connected = */ false); + add_displays(/*internal = */ false, /*connected = */ false); // Catch-all for the above loops if (ret) -- cgit v1.2.3 From 10be875de071672e61a52ab0679bbc6803df8c51 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Sun, 30 Jan 2022 20:28:46 +0200 Subject: 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 --- .ci/Makefile | 1 - compositor/DrmDisplayCompositor.cpp | 15 ++++----- compositor/Planner.cpp | 2 +- drm/DrmCrtc.cpp | 61 +++++++++++++------------------------ drm/DrmCrtc.h | 39 +++++++++++++++--------- drm/DrmDevice.cpp | 47 +++++++++------------------- drm/DrmDevice.h | 4 +-- drm/DrmEncoder.cpp | 4 +-- drm/DrmEncoder.h | 2 +- drm/DrmPlane.cpp | 2 +- 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> Planner::ProvisionPlanes( std::vector 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()); } 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 { + auto crtc = MakeDrmModeCrtcUnique(dev.fd(), crtc_id); + if (!crtc) { + ALOGE("Failed to get CRTC %d", crtc_id); + return {}; + } + + auto c = std::unique_ptr(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() = 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 DrmDevice::Init(const char *path, int num_displays) { max_resolution_ = std::pair(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 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 possible_clones; @@ -125,10 +115,10 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { std::vector 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> &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 max_resolution_; std::map displays_; + std::map bound_crtcs_; + bool HasAddFb2ModifiersSupport_{}; std::shared_ptr 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)); -- cgit v1.2.3 From 027987b1fe91a496e1397d165ced6b3c7026aa06 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Sun, 30 Jan 2022 21:06:35 +0200 Subject: drm_hwcomposer: Tidy-up DrmEncoder class Implement DrmEncoder instantiation through CreateInstance() static method, which helps to reduce complexity of DrmDevice::Init() function. Move Encoder-to-CRTC binding information to the DrmDevice class. Move drm/DrmEncoder.h to Normal clang-tidy checks list by fixing clang-tidy findings. Signed-off-by: Roman Stratiienko --- .ci/Makefile | 1 - drm/DrmDevice.cpp | 53 +++++++++++++++-------------------------------------- drm/DrmDevice.h | 11 +++++++++++ drm/DrmEncoder.cpp | 44 +++++++++++--------------------------------- drm/DrmEncoder.h | 40 ++++++++++++++++++++++++---------------- 5 files changed, 61 insertions(+), 88 deletions(-) diff --git a/.ci/Makefile b/.ci/Makefile index e203673..e8704da 100644 --- a/.ci/Makefile +++ b/.ci/Makefile @@ -30,7 +30,6 @@ TIDY_FILES_OVERRIDE := \ drm/DrmProperty.h:COARSE \ drm/DrmConnector.h:COARSE \ drm/DrmUnique.h:FINE \ - drm/DrmEncoder.h:COARSE \ drm/DrmConnector.cpp:COARSE \ drm/DrmDevice.cpp:COARSE \ drm/DrmProperty.cpp:COARSE \ diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp index 8f44d69..f5e3521 100644 --- a/drm/DrmDevice.cpp +++ b/drm/DrmDevice.cpp @@ -103,36 +103,12 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { } } - std::vector possible_clones; - for (int i = 0; !ret && i < res->count_encoders; ++i) { - auto e = MakeDrmModeEncoderUnique(fd(), res->encoders[i]); - if (!e) { - ALOGE("Failed to get encoder %d", res->encoders[i]); - ret = -ENODEV; - break; - } - - std::vector possible_crtcs; - DrmCrtc *current_crtc = nullptr; - for (auto &crtc : crtcs_) { - if ((1 << crtc->GetIndexInResArray()) & e->possible_crtcs) - possible_crtcs.push_back(crtc.get()); - - if (crtc->GetId() == e->crtc_id) - current_crtc = crtc.get(); + for (int i = 0; i < res->count_encoders; ++i) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + auto enc = DrmEncoder::CreateInstance(*this, res->encoders[i], i); + if (enc) { + encoders_.emplace_back(std::move(enc)); } - - std::unique_ptr enc( - new DrmEncoder(e.get(), current_crtc, possible_crtcs)); - possible_clones.push_back(e->possible_clones); - - encoders_.emplace_back(std::move(enc)); - } - - for (unsigned int i = 0; i < encoders_.size(); i++) { - for (unsigned int j = 0; j < encoders_.size(); j++) - if (possible_clones[i] & (1 << j)) - encoders_[i]->AddPossibleClone(encoders_[j].get()); } for (int i = 0; !ret && i < res->count_connectors; ++i) { @@ -147,9 +123,9 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { DrmEncoder *current_encoder = nullptr; for (int j = 0; j < c->count_encoders; ++j) { for (auto &encoder : encoders_) { - if (encoder->id() == c->encoders[j]) + if (encoder->GetId() == c->encoders[j]) possible_encoders.push_back(encoder.get()); - if (encoder->id() == c->encoder_id) + if (encoder->GetId() == c->encoder_id) current_encoder = encoder.get(); } } @@ -244,22 +220,23 @@ 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(); + auto *crtc = FindCrtcById(enc->GetCurrentCrtcId()); if (crtc && bound_crtcs_.count(display) == 0) { bound_crtcs_[display] = crtc; - enc->set_crtc(crtc, display); + bound_encoders_[crtc] = enc; return 0; } /* Try to find a possible crtc which will work */ - for (DrmCrtc *crtc : enc->possible_crtcs()) { - /* We've already tried this earlier */ - if (crtc == enc->crtc()) + for (auto &crtc : crtcs_) { + /* Crtc not supported or we've already tried this earlier */ + if (!enc->SupportsCrtc(*crtc) || crtc->GetId() == enc->GetCurrentCrtcId()) { continue; + } if (bound_crtcs_.count(display) == 0) { - bound_crtcs_[display] = crtc; - enc->set_crtc(crtc, display); + bound_crtcs_[display] = crtc.get(); + bound_encoders_[crtc.get()] = enc; return 0; } } diff --git a/drm/DrmDevice.h b/drm/DrmDevice.h index e3f6355..82b0f6a 100644 --- a/drm/DrmDevice.h +++ b/drm/DrmDevice.h @@ -85,6 +85,16 @@ class DrmDevice { static auto IsKMSDev(const char *path) -> bool; + auto FindCrtcById(uint32_t id) const -> DrmCrtc * { + for (const auto &crtc : crtcs_) { + if (crtc->GetId() == id) { + return crtc.get(); + } + }; + + return nullptr; + } + int GetProperty(uint32_t obj_id, uint32_t obj_type, const char *prop_name, DrmProperty *property) const; @@ -107,6 +117,7 @@ class DrmDevice { std::map displays_; std::map bound_crtcs_; + std::map bound_encoders_; bool HasAddFb2ModifiersSupport_{}; diff --git a/drm/DrmEncoder.cpp b/drm/DrmEncoder.cpp index e1628b2..8049a5c 100644 --- a/drm/DrmEncoder.cpp +++ b/drm/DrmEncoder.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +#define LOG_TAG "hwc-drm-encoder" + #include "DrmEncoder.h" #include @@ -21,43 +23,19 @@ #include #include "DrmDevice.h" +#include "utils/log.h" namespace android { -DrmEncoder::DrmEncoder(drmModeEncoderPtr e, DrmCrtc *current_crtc, - std::vector possible_crtcs) - : id_(e->encoder_id), - crtc_(current_crtc), - display_(-1), - possible_crtcs_(std::move(possible_crtcs)) { -} - -uint32_t DrmEncoder::id() const { - return id_; -} +auto DrmEncoder::CreateInstance(DrmDevice &dev, uint32_t encoder_id, + uint32_t index) -> std::unique_ptr { + auto e = MakeDrmModeEncoderUnique(dev.fd(), encoder_id); + if (!e) { + ALOGE("Failed to get encoder %d", encoder_id); + return {}; + } -DrmCrtc *DrmEncoder::crtc() const { - return crtc_; + return std::unique_ptr(new DrmEncoder(std::move(e), index)); } -bool DrmEncoder::CanClone(DrmEncoder *encoder) { - return possible_clones_.find(encoder) != possible_clones_.end(); -} - -void DrmEncoder::AddPossibleClone(DrmEncoder *possible_clone) { - possible_clones_.insert(possible_clone); -} - -void DrmEncoder::set_crtc(DrmCrtc *crtc, int display) { - crtc_ = crtc; - display_ = display; -} - -int DrmEncoder::display() const { - return display_; -} - -bool DrmEncoder::can_bind(int display) const { - return display_ == -1 || display_ == display; -} } // namespace android diff --git a/drm/DrmEncoder.h b/drm/DrmEncoder.h index a1f96d3..e66d6f1 100644 --- a/drm/DrmEncoder.h +++ b/drm/DrmEncoder.h @@ -29,31 +29,39 @@ namespace android { class DrmEncoder { public: - DrmEncoder(drmModeEncoderPtr e, DrmCrtc *current_crtc, - std::vector possible_crtcs); + static auto CreateInstance(DrmDevice &dev, uint32_t encoder_id, + uint32_t index) -> std::unique_ptr; + DrmEncoder(const DrmEncoder &) = delete; DrmEncoder &operator=(const DrmEncoder &) = delete; - uint32_t id() const; + auto GetId() const { + return enc_->encoder_id; + }; + + auto GetIndexInResArray() const { + return index_in_res_array_; + } - DrmCrtc *crtc() const; - void set_crtc(DrmCrtc *crtc, int display); - bool can_bind(int display) const; - int display() const; + auto CanClone(DrmEncoder &encoder) { + return (enc_->possible_clones & (1 << encoder.GetIndexInResArray())) != 0; + } - const std::vector &possible_crtcs() const { - return possible_crtcs_; + auto SupportsCrtc(DrmCrtc &crtc) { + return (enc_->possible_crtcs & (1 << crtc.GetIndexInResArray())) != 0; + } + + auto GetCurrentCrtcId() { + return enc_->crtc_id; } - bool CanClone(DrmEncoder *encoder); - void AddPossibleClone(DrmEncoder *possible_clone); private: - uint32_t id_; - DrmCrtc *crtc_; - int display_; + DrmEncoder(DrmModeEncoderUnique enc, uint32_t index) + : enc_(std::move(enc)), index_in_res_array_(index){}; + + DrmModeEncoderUnique enc_; - std::vector possible_crtcs_; - std::set possible_clones_; + const uint32_t index_in_res_array_; }; } // namespace android -- cgit v1.2.3 From 650299a235ba82cafceef114d552ff4067ffbf1e Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Sun, 30 Jan 2022 23:46:10 +0200 Subject: 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 --- .ci/Makefile | 2 - backend/BackendManager.cpp | 4 +- compositor/DrmDisplayCompositor.cpp | 10 +- drm/DrmConnector.cpp | 212 +++++++++++++----------------------- drm/DrmConnector.h | 110 ++++++++++++------- drm/DrmDevice.cpp | 84 +++++--------- drm/DrmDevice.h | 21 +++- drm/VSyncWorker.cpp | 6 +- hwc2_device/DrmHwcTwo.cpp | 26 ++--- hwc2_device/HwcDisplay.cpp | 10 +- hwc2_device/HwcDisplay.h | 3 +- hwc2_device/HwcDisplayConfigs.cpp | 8 +- 12 files changed, 224 insertions(+), 272 deletions(-) diff --git a/.ci/Makefile b/.ci/Makefile index e8704da..d138968 100644 --- a/.ci/Makefile +++ b/.ci/Makefile @@ -28,9 +28,7 @@ TIDY_FILES_OVERRIDE := \ drm/DrmMode.h:COARSE \ drm/DrmDevice.h:COARSE \ drm/DrmProperty.h:COARSE \ - drm/DrmConnector.h:COARSE \ drm/DrmUnique.h:FINE \ - drm/DrmConnector.cpp:COARSE \ drm/DrmDevice.cpp:COARSE \ drm/DrmProperty.cpp:COARSE \ drm/UEventListener.cpp:COARSE \ diff --git a/backend/BackendManager.cpp b/backend/BackendManager.cpp index aadef36..4a735eb 100644 --- a/backend/BackendManager.cpp +++ b/backend/BackendManager.cpp @@ -51,13 +51,13 @@ int BackendManager::SetBackendForDisplay(HwcDisplay *display) { display->set_backend(GetBackendByName(backend_name)); if (display->backend() == nullptr) { ALOGE("Failed to set backend '%s' for '%s' and driver '%s'", - backend_name.c_str(), display->connector()->name().c_str(), + backend_name.c_str(), display->connector()->GetName().c_str(), driver_name.c_str()); return -EINVAL; } ALOGI("Backend '%s' for '%s' and driver '%s' was successfully set", - backend_name.c_str(), display->connector()->name().c_str(), + backend_name.c_str(), display->connector()->GetName().c_str(), driver_name.c_str()); return 0; diff --git a/compositor/DrmDisplayCompositor.cpp b/compositor/DrmDisplayCompositor.cpp index 0cb1d39..c2db7fc 100644 --- a/compositor/DrmDisplayCompositor.cpp +++ b/compositor/DrmDisplayCompositor.cpp @@ -107,7 +107,7 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { if (args.active) { new_frame_state.crtc_active_state = *args.active; if (!crtc->GetActiveProperty().AtomicSet(*pset, *args.active) || - !connector->crtc_id_property().AtomicSet(*pset, crtc->GetId())) { + !connector->GetCrtcIdProperty().AtomicSet(*pset, crtc->GetId())) { return -EINVAL; } } @@ -189,7 +189,7 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { if (args.display_mode) { /* TODO(nobody): we still need this for synthetic vsync, remove after * vsync reworked */ - connector->set_active_mode(*args.display_mode); + connector->SetActiveMode(*args.display_mode); } active_frame_state_ = std::move(new_frame_state); @@ -229,9 +229,9 @@ auto DrmDisplayCompositor::ActivateDisplayUsingDPMS() -> int { return -ENODEV; } - if (connector->dpms_property()) { - drmModeConnectorSetProperty(drm->fd(), connector->id(), - connector->dpms_property().id(), + if (connector->GetDpmsProperty()) { + drmModeConnectorSetProperty(drm->fd(), connector->GetId(), + connector->GetDpmsProperty().id(), DRM_MODE_DPMS_ON); } return 0; 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 @@ namespace android { constexpr size_t kTypesCount = 21; -DrmConnector::DrmConnector(DrmDevice *drm, drmModeConnectorPtr c, - DrmEncoder *current_encoder, - std::vector &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 { + 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( + 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 @@ auto DrmConnector::GetEdidBlob() -> DrmModePropertyBlobUnique { return {}; } - std::tie(ret, blob_id) = edid_property().value(); + std::tie(ret, blob_id) = GetEdidProperty().value(); if (ret != 0) { return {}; } @@ -113,137 +113,79 @@ auto DrmConnector::GetEdidBlob() -> DrmModePropertyBlobUnique { return MakeDrmModePropertyBlobUnique(drm_->fd(), blob_id); } -uint32_t DrmConnector::id() const { - return id_; -} - -int DrmConnector::display() const { - return display_; +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; } -void DrmConnector::set_display(int display) { - display_ = 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; } -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 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 DrmDevice; class DrmConnector { public: - DrmConnector(DrmDevice *drm, drmModeConnectorPtr c, - DrmEncoder *current_encoder, - std::vector &possible_encoders); + static auto CreateInstance(DrmDevice &dev, uint32_t connector_id, + uint32_t index) -> std::unique_ptr; + 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_; + } + + auto GetId() const { + return connector_->connector_id; + } + + auto GetIndexInResArray() const { + return index_in_res_array_; + } + + 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; + } + } - int display() const; - void set_display(int display); + return false; + } - bool internal() const; - bool external() const; - bool writeback() const; - bool valid_type() const; + bool IsInternal() const; + bool IsExternal() const; + bool IsWriteback() const; + bool IsValid() const; - std::string name() const; + std::string GetName() const; int UpdateModes(); - const std::vector &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 &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_; + } - private: - DrmDevice *drm_; + auto &GetCrtcIdProperty() const { + return crtc_id_property_; + } + + auto &GetEdidProperty() const { + return edid_property_; + } - uint32_t id_; - DrmEncoder *encoder_; - int display_; + auto IsConnected() const { + return connector_->connection == DRM_MODE_CONNECTED; + } - uint32_t type_; - uint32_t type_id_; - drmModeConnection state_; + auto GetMmWidth() const { + return connector_->mmWidth; + } - uint32_t mm_width_; - uint32_t mm_height_; + auto GetMmHeight() const { + return connector_->mmHeight; + }; + + private: + DrmConnector(DrmModeConnectorUnique connector, DrmDevice *drm, uint32_t index) + : connector_(std::move(connector)), + drm_(drm), + index_in_res_array_(index){}; + + DrmModeConnectorUnique connector_; + DrmDevice *const drm_; + + const uint32_t index_in_res_array_; DrmMode active_mode_; std::vector modes_; @@ -105,8 +133,6 @@ class DrmConnector { DrmProperty writeback_pixel_formats_; DrmProperty writeback_fb_id_; DrmProperty writeback_out_fence_; - - std::vector 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 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { } } - 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; - } - - std::vector 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 conn( - new DrmConnector(this, c.get(), current_encoder, possible_encoders)); + for (int i = 0; i < res->count_connectors; ++i) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + auto conn = DrmConnector::CreateInstance(*this, res->connectors[i], i); - ret = conn->Init(); - if (ret) { - ALOGE("Init connector %d failed", res->connectors[i]); - break; + if (!conn) { + continue; } - 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 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { 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::TryEncoderForDisplay(int display, DrmEncoder *enc) { } 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 @@ int DrmDevice::CreateDisplayPipe(DrmConnector *connector) { } } - 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 @@ int DrmDevice::CreateDisplayPipe(DrmConnector *connector) { 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 @@ int DrmDevice::GetProperty(uint32_t obj_id, uint32_t obj_type, 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 @@ class DrmDevice { 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> &crtcs() const; @@ -95,6 +92,20 @@ class DrmDevice { 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 @@ class DrmDevice { std::pair min_resolution_; std::pair max_resolution_; - std::map displays_; std::map bound_crtcs_; + std::map bound_connectors_; + std::map connectors_to_display_id_; + std::map encoders_to_display_id_; std::map 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 @@ int VSyncWorker::SyntheticWaitVBlank(int64_t *timestamp) { 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(refresh), diff --git a/hwc2_device/DrmHwcTwo.cpp b/hwc2_device/DrmHwcTwo.cpp index 87cc238..698c1e5 100644 --- a/hwc2_device/DrmHwcTwo.cpp +++ b/hwc2_device/DrmHwcTwo.cpp @@ -165,42 +165,40 @@ void DrmHwcTwo::HandleDisplayHotplug(hwc2_display_t displayid, int state) { void DrmHwcTwo::HandleInitialHotplugState(DrmDevice *drmDevice) { for (const auto &conn : drmDevice->connectors()) { - int display_id = conn->display(); + int display_id = drmDevice->GetDisplayId(conn.get()); auto &display = displays_.at(display_id); - if (conn->state() != DRM_MODE_CONNECTED && !display.IsInHeadlessMode()) + if (!conn->IsConnected() && !display.IsInHeadlessMode()) continue; - HandleDisplayHotplug(conn->display(), display.IsInHeadlessMode() - ? DRM_MODE_CONNECTED - : conn->state()); + HandleDisplayHotplug(display_id, display.IsInHeadlessMode() + ? 1 + : (conn->IsConnected() ? 1 : 0)); } } void DrmHwcTwo::HandleHotplugUEvent() { for (const auto &drm : resource_manager_.GetDrmDevices()) { for (const auto &conn : drm->connectors()) { - drmModeConnection old_state = conn->state(); - drmModeConnection cur_state = conn->UpdateModes() - ? DRM_MODE_UNKNOWNCONNECTION - : conn->state(); + int display_id = drm->GetDisplayId(conn.get()); + bool old_state = conn->IsConnected(); + bool cur_state = conn->UpdateModes() ? false : conn->IsConnected(); if (cur_state == old_state) continue; ALOGI("%s event for connector %u on display %d", - cur_state == DRM_MODE_CONNECTED ? "Plug" : "Unplug", conn->id(), - conn->display()); + cur_state == DRM_MODE_CONNECTED ? "Plug" : "Unplug", conn->GetId(), + display_id); - int display_id = conn->display(); auto &display = displays_.at(display_id); display.ChosePreferredConfig(); - if (cur_state != DRM_MODE_CONNECTED) { + if (cur_state) { display.ClearDisplay(); } HandleDisplayHotplug(display_id, display.IsInHeadlessMode() ? DRM_MODE_CONNECTED - : cur_state); + : (cur_state ? 1 : 0)); } } } diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp index f18e00d..54cf867 100644 --- a/hwc2_device/HwcDisplay.cpp +++ b/hwc2_device/HwcDisplay.cpp @@ -69,7 +69,7 @@ std::string HwcDisplay::Dump() { } std::stringstream ss; - ss << "- Display on: " << connector_->name() << "\n" + ss << "- Display on: " << connector_->GetName() << "\n" << " Flattening state: " << flattening_state_str << "\n" << "Statistics since system boot:\n" << DumpDelta(total_stats_) << "\n\n" @@ -363,7 +363,7 @@ HWC2::Error HwcDisplay::GetDisplayConfigs(uint32_t *num_configs, HWC2::Error HwcDisplay::GetDisplayName(uint32_t *size, char *name) { std::ostringstream stream; - stream << "display-" << connector_->id(); + stream << "display-" << connector_->GetId(); std::string string = stream.str(); size_t length = string.length(); if (!name) { @@ -715,9 +715,9 @@ HWC2::Error HwcDisplay::GetDisplayConnectionType(uint32_t *outType) { /* Primary display should be always internal, * otherwise SF will be unhappy and will crash */ - if (connector_->internal() || handle_ == kPrimaryDisplay) + if (connector_->IsInternal() || handle_ == kPrimaryDisplay) *outType = static_cast(HWC2::DisplayConnectionType::Internal); - else if (connector_->external()) + else if (connector_->IsExternal()) *outType = static_cast(HWC2::DisplayConnectionType::External); else return HWC2::Error::BadConfig; @@ -785,7 +785,7 @@ HWC2::Error HwcDisplay::GetDisplayIdentificationData(uint8_t *outPort, } else { *outDataSize = blob->length; } - *outPort = connector_->id(); + *outPort = connector_->GetId(); return HWC2::Error::None; } diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h index 76456b7..3e8a548 100644 --- a/hwc2_device/HwcDisplay.h +++ b/hwc2_device/HwcDisplay.h @@ -209,8 +209,7 @@ class HwcDisplay { * https://source.android.com/devices/graphics/hotplug#handling-common-scenarios */ bool IsInHeadlessMode() { - return handle_ == kPrimaryDisplay && - connector_->state() != DRM_MODE_CONNECTED; + return handle_ == kPrimaryDisplay && !connector_->IsConnected(); } private: diff --git a/hwc2_device/HwcDisplayConfigs.cpp b/hwc2_device/HwcDisplayConfigs.cpp index 16f1ed0..f6ccdb7 100644 --- a/hwc2_device/HwcDisplayConfigs.cpp +++ b/hwc2_device/HwcDisplayConfigs.cpp @@ -61,14 +61,14 @@ HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) { return HWC2::Error::BadDisplay; } - if (connector.modes().empty()) { + if (connector.GetModes().empty()) { ALOGE("No modes reported by KMS"); return HWC2::Error::BadDisplay; } hwc_configs.clear(); - mm_width = connector.mm_width(); - mm_height = connector.mm_height(); + mm_width = connector.GetMmWidth(); + mm_height = connector.GetMmHeight(); preferred_config_id = 0; int preferred_config_group_id = 0; @@ -77,7 +77,7 @@ HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) { int last_group_id = 1; /* Group modes */ - for (const auto &mode : connector.modes()) { + for (const auto &mode : connector.GetModes()) { /* Find group for the new mode or create new group */ int group_found = 0; for (auto &hwc_config : hwc_configs) { -- cgit v1.2.3 From 7d89911c32c5ccc74e69349212713f02bfc95aef Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Mon, 31 Jan 2022 11:30:27 +0200 Subject: drm_hwcomposer: Tidy-up DrmDevice class 1. Move drm/DrmConnector.h to Normal clang-tidy checks list by fixing clang-tidy findings. 2. Remove DrmDevice self-reference. 3. Replace shared_ptr reference to DrmDevice in DrmFbImporter with a pointer, making ResourceManager only owner of DrmDevice and its chilren. Signed-off-by: Roman Stratiienko --- .ci/Makefile | 2 - compositor/DrmDisplayCompositor.cpp | 4 +- drm/DrmConnector.cpp | 6 +-- drm/DrmCrtc.cpp | 2 +- drm/DrmDevice.cpp | 87 +++++++++++++++++++++---------------- drm/DrmDevice.h | 29 +++++-------- drm/DrmEncoder.cpp | 2 +- drm/DrmFbImporter.cpp | 19 ++++---- drm/DrmFbImporter.h | 13 +++--- drm/DrmPlane.cpp | 2 +- drm/VSyncWorker.cpp | 2 +- hwc2_device/DrmHwcTwo.cpp | 6 +-- hwc2_device/HwcDisplay.cpp | 4 +- 13 files changed, 89 insertions(+), 89 deletions(-) diff --git a/.ci/Makefile b/.ci/Makefile index d138968..bca785c 100644 --- a/.ci/Makefile +++ b/.ci/Makefile @@ -26,10 +26,8 @@ TIDY_FILES_OVERRIDE := \ compositor/DrmDisplayCompositor.cpp:COARSE \ drm/DrmFbImporter.h:FINE \ drm/DrmMode.h:COARSE \ - drm/DrmDevice.h:COARSE \ drm/DrmProperty.h:COARSE \ drm/DrmUnique.h:FINE \ - drm/DrmDevice.cpp:COARSE \ drm/DrmProperty.cpp:COARSE \ drm/UEventListener.cpp:COARSE \ drm/VSyncWorker.cpp:COARSE \ diff --git a/compositor/DrmDisplayCompositor.cpp b/compositor/DrmDisplayCompositor.cpp index c2db7fc..d25fb98 100644 --- a/compositor/DrmDisplayCompositor.cpp +++ b/compositor/DrmDisplayCompositor.cpp @@ -178,7 +178,7 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { if (args.test_only) flags |= DRM_MODE_ATOMIC_TEST_ONLY; - int err = drmModeAtomicCommit(drm->fd(), pset.get(), flags, drm); + int err = drmModeAtomicCommit(drm->GetFd(), pset.get(), flags, drm); if (err) { if (!args.test_only) ALOGE("Failed to commit pset ret=%d\n", err); @@ -230,7 +230,7 @@ auto DrmDisplayCompositor::ActivateDisplayUsingDPMS() -> int { } if (connector->GetDpmsProperty()) { - drmModeConnectorSetProperty(drm->fd(), connector->GetId(), + drmModeConnectorSetProperty(drm->GetFd(), connector->GetId(), connector->GetDpmsProperty().id(), DRM_MODE_DPMS_ON); } diff --git a/drm/DrmConnector.cpp b/drm/DrmConnector.cpp index 5e00dd6..4737316 100644 --- a/drm/DrmConnector.cpp +++ b/drm/DrmConnector.cpp @@ -63,7 +63,7 @@ static bool GetConnectorProperty(const DrmDevice &dev, auto DrmConnector::CreateInstance(DrmDevice &dev, uint32_t connector_id, uint32_t index) -> std::unique_ptr { - auto conn = MakeDrmModeConnectorUnique(dev.fd(), connector_id); + auto conn = MakeDrmModeConnectorUnique(dev.GetFd(), connector_id); if (!conn) { ALOGE("Failed to get connector %d", connector_id); return {}; @@ -110,7 +110,7 @@ auto DrmConnector::GetEdidBlob() -> DrmModePropertyBlobUnique { return {}; } - return MakeDrmModePropertyBlobUnique(drm_->fd(), blob_id); + return MakeDrmModePropertyBlobUnique(drm_->GetFd(), blob_id); } bool DrmConnector::IsInternal() const { @@ -159,7 +159,7 @@ std::string DrmConnector::GetName() const { } int DrmConnector::UpdateModes() { - auto conn = MakeDrmModeConnectorUnique(drm_->fd(), GetId()); + auto conn = MakeDrmModeConnectorUnique(drm_->GetFd(), GetId()); if (!conn) { ALOGE("Failed to get connector %d", GetId()); return -ENODEV; diff --git a/drm/DrmCrtc.cpp b/drm/DrmCrtc.cpp index bc19141..b54f14b 100644 --- a/drm/DrmCrtc.cpp +++ b/drm/DrmCrtc.cpp @@ -35,7 +35,7 @@ static int GetCrtcProperty(const DrmDevice &dev, const DrmCrtc &crtc, auto DrmCrtc::CreateInstance(DrmDevice &dev, uint32_t crtc_id, uint32_t index) -> std::unique_ptr { - auto crtc = MakeDrmModeCrtcUnique(dev.fd(), crtc_id); + auto crtc = MakeDrmModeCrtcUnique(dev.GetFd(), crtc_id); if (!crtc) { ALOGE("Failed to get CRTC %d", crtc_id); return {}; diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp index a6d2289..29dd95f 100644 --- a/drm/DrmDevice.cpp +++ b/drm/DrmDevice.cpp @@ -37,54 +37,53 @@ namespace android { DrmDevice::DrmDevice() { - self.reset(this); - mDrmFbImporter = std::make_unique(self); + drm_fb_importer_ = std::make_unique(*this); } // NOLINTNEXTLINE (readability-function-cognitive-complexity): Fixme std::tuple DrmDevice::Init(const char *path, int num_displays) { /* TODO: Use drmOpenControl here instead */ fd_ = UniqueFd(open(path, O_RDWR | O_CLOEXEC)); - if (fd() < 0) { + if (!fd_) { // NOLINTNEXTLINE(concurrency-mt-unsafe): Fixme ALOGE("Failed to open dri %s: %s", path, strerror(errno)); return std::make_tuple(-ENODEV, 0); } - int ret = drmSetClientCap(fd(), DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1); - if (ret) { + int ret = drmSetClientCap(GetFd(), DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1); + if (ret != 0) { ALOGE("Failed to set universal plane cap %d", ret); return std::make_tuple(ret, 0); } - ret = drmSetClientCap(fd(), DRM_CLIENT_CAP_ATOMIC, 1); - if (ret) { + ret = drmSetClientCap(GetFd(), DRM_CLIENT_CAP_ATOMIC, 1); + if (ret != 0) { ALOGE("Failed to set atomic cap %d", ret); return std::make_tuple(ret, 0); } #ifdef DRM_CLIENT_CAP_WRITEBACK_CONNECTORS - ret = drmSetClientCap(fd(), DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1); - if (ret) { + ret = drmSetClientCap(GetFd(), DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1); + if (ret != 0) { ALOGI("Failed to set writeback cap %d", ret); ret = 0; } #endif uint64_t cap_value = 0; - if (drmGetCap(fd(), DRM_CAP_ADDFB2_MODIFIERS, &cap_value)) { + if (drmGetCap(GetFd(), DRM_CAP_ADDFB2_MODIFIERS, &cap_value) != 0) { ALOGW("drmGetCap failed. Fallback to no modifier support."); cap_value = 0; } HasAddFb2ModifiersSupport_ = cap_value != 0; - drmSetMaster(fd()); - if (!drmIsMaster(fd())) { + drmSetMaster(GetFd()); + if (drmIsMaster(GetFd()) == 0) { ALOGE("DRM/KMS master access required"); return std::make_tuple(-EACCES, 0); } - auto res = MakeDrmModeResUnique(fd()); + auto res = MakeDrmModeResUnique(GetFd()); if (!res) { ALOGE("Failed to get DrmDevice resources"); return std::make_tuple(-ENODEV, 0); @@ -147,10 +146,10 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { add_displays(/*internal = */ false, /*connected = */ false); // Catch-all for the above loops - if (ret) + if (ret != 0) return std::make_tuple(ret, 0); - auto plane_res = MakeDrmModePlaneResUnique(fd()); + auto plane_res = MakeDrmModePlaneResUnique(GetFd()); if (!plane_res) { ALOGE("Failed to get plane resources"); return std::make_tuple(-ENOENT, 0); @@ -167,7 +166,7 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { for (auto &conn : connectors_) { ret = CreateDisplayPipe(conn.get()); - if (ret) { + if (ret != 0) { ALOGE("Failed CreateDisplayPipe %d with %d", conn->GetId(), ret); return std::make_tuple(ret, 0); } @@ -187,18 +186,10 @@ DrmCrtc *DrmDevice::GetCrtcForDisplay(int display) const { return bound_crtcs_.at(display); } -const std::vector> &DrmDevice::crtcs() const { - return crtcs_; -} - -uint32_t DrmDevice::next_mode_id() { - return ++mode_id_; -} - int DrmDevice::TryEncoderForDisplay(int display, DrmEncoder *enc) { /* First try to use the currently-bound crtc */ auto *crtc = FindCrtcById(enc->GetCurrentCrtcId()); - if (crtc && bound_crtcs_.count(display) == 0) { + if (crtc != nullptr && bound_crtcs_.count(display) == 0) { bound_crtcs_[display] = crtc; bound_encoders_[crtc] = enc; return 0; @@ -228,7 +219,7 @@ int DrmDevice::CreateDisplayPipe(DrmConnector *connector) { auto *enc0 = FindEncoderById(connector->GetCurrentEncoderId()); if (enc0 != nullptr && encoders_to_display_id_.count(enc0) == 0) { int ret = TryEncoderForDisplay(display, enc0); - if (!ret) { + if (ret == 0) { encoders_to_display_id_[enc0] = display; return 0; } @@ -246,7 +237,7 @@ int DrmDevice::CreateDisplayPipe(DrmConnector *connector) { } int ret = TryEncoderForDisplay(display, enc.get()); - if (!ret) { + if (ret == 0) { encoders_to_display_id_[enc.get()] = display; return 0; } @@ -264,10 +255,11 @@ auto DrmDevice::RegisterUserPropertyBlob(void *data, size_t length) const -> DrmModeUserPropertyBlobUnique { struct drm_mode_create_blob create_blob {}; create_blob.length = length; + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) create_blob.data = (__u64)data; - int ret = drmIoctl(fd(), DRM_IOCTL_MODE_CREATEPROPBLOB, &create_blob); - if (ret) { + int ret = drmIoctl(GetFd(), DRM_IOCTL_MODE_CREATEPROPBLOB, &create_blob); + if (ret != 0) { ALOGE("Failed to create mode property blob %d", ret); return {}; } @@ -276,7 +268,8 @@ auto DrmDevice::RegisterUserPropertyBlob(void *data, size_t length) const new uint32_t(create_blob.blob_id), [this](const uint32_t *it) { struct drm_mode_destroy_blob destroy_blob {}; destroy_blob.blob_id = (__u32)*it; - int err = drmIoctl(fd(), DRM_IOCTL_MODE_DESTROYPROPBLOB, &destroy_blob); + int err = drmIoctl(GetFd(), DRM_IOCTL_MODE_DESTROYPROPBLOB, + &destroy_blob); if (err != 0) { ALOGE("Failed to destroy mode property blob %" PRIu32 "/%d", *it, err); @@ -290,16 +283,18 @@ int DrmDevice::GetProperty(uint32_t obj_id, uint32_t obj_type, const char *prop_name, DrmProperty *property) const { drmModeObjectPropertiesPtr props = nullptr; - props = drmModeObjectGetProperties(fd(), obj_id, obj_type); - if (!props) { + props = drmModeObjectGetProperties(GetFd(), obj_id, obj_type); + if (props == nullptr) { ALOGE("Failed to get properties for %d/%x", obj_id, obj_type); return -ENODEV; } bool found = false; for (int i = 0; !found && (size_t)i < props->count_props; ++i) { - drmModePropertyPtr p = drmModeGetProperty(fd(), props->props[i]); - if (!strcmp(p->name, prop_name)) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + drmModePropertyPtr p = drmModeGetProperty(GetFd(), props->props[i]); + if (strcmp(p->name, prop_name) == 0) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) property->Init(obj_id, p, props->prop_values[i]); found = true; } @@ -311,9 +306,9 @@ int DrmDevice::GetProperty(uint32_t obj_id, uint32_t obj_type, } std::string DrmDevice::GetName() const { - auto *ver = drmGetVersion(fd()); - if (!ver) { - ALOGW("Failed to get drm version for fd=%d", fd()); + auto *ver = drmGetVersion(GetFd()); + if (ver == nullptr) { + ALOGW("Failed to get drm version for fd=%d", GetFd()); return "generic"; } @@ -339,4 +334,22 @@ auto DrmDevice::IsKMSDev(const char *path) -> bool { return is_kms; } +auto DrmDevice::GetConnectors() + -> const std::vector> & { + return connectors_; +} + +auto DrmDevice::GetPlanes() -> const std::vector> & { + return planes_; +} + +auto DrmDevice::GetCrtcs() -> const std::vector> & { + return crtcs_; +} + +auto DrmDevice::GetEncoders() + -> const std::vector> & { + return encoders_; +} + } // namespace android diff --git a/drm/DrmDevice.h b/drm/DrmDevice.h index cd87127..6d792c2 100644 --- a/drm/DrmDevice.h +++ b/drm/DrmDevice.h @@ -39,23 +39,20 @@ class DrmDevice { std::tuple Init(const char *path, int num_displays); - int fd() const { + auto GetFd() const { return fd_.Get(); } - const std::vector> &connectors() const { - return connectors_; - } - - const std::vector> &planes() const { - return planes_; - } + auto GetConnectors() -> const std::vector> &; + auto GetPlanes() -> const std::vector> &; + auto GetCrtcs() -> const std::vector> &; + auto GetEncoders() -> const std::vector> &; - std::pair min_resolution() const { + auto GetMinResolution() const { return min_resolution_; } - std::pair max_resolution() const { + auto GetMaxResolution() const { return max_resolution_; } @@ -64,9 +61,6 @@ class DrmDevice { std::string GetName() const; - const std::vector> &crtcs() const; - uint32_t next_mode_id(); - auto RegisterUserPropertyBlob(void *data, size_t length) const -> DrmModeUserPropertyBlobUnique; @@ -76,8 +70,8 @@ class DrmDevice { return HasAddFb2ModifiersSupport_; } - DrmFbImporter &GetDrmFbImporter() { - return *mDrmFbImporter; + auto &GetDrmFbImporter() { + return *drm_fb_importer_; } static auto IsKMSDev(const char *path) -> bool; @@ -115,7 +109,6 @@ class DrmDevice { int CreateDisplayPipe(DrmConnector *connector); UniqueFd fd_; - uint32_t mode_id_ = 0; std::vector> connectors_; std::vector> writeback_connectors_; @@ -134,9 +127,7 @@ class DrmDevice { bool HasAddFb2ModifiersSupport_{}; - std::shared_ptr self; - - std::unique_ptr mDrmFbImporter; + std::unique_ptr drm_fb_importer_; }; } // namespace android diff --git a/drm/DrmEncoder.cpp b/drm/DrmEncoder.cpp index 8049a5c..eed5b5f 100644 --- a/drm/DrmEncoder.cpp +++ b/drm/DrmEncoder.cpp @@ -29,7 +29,7 @@ namespace android { auto DrmEncoder::CreateInstance(DrmDevice &dev, uint32_t encoder_id, uint32_t index) -> std::unique_ptr { - auto e = MakeDrmModeEncoderUnique(dev.fd(), encoder_id); + auto e = MakeDrmModeEncoderUnique(dev.GetFd(), encoder_id); if (!e) { ALOGE("Failed to get encoder %d", encoder_id); return {}; diff --git a/drm/DrmFbImporter.cpp b/drm/DrmFbImporter.cpp index 6f2abe8..eeab076 100644 --- a/drm/DrmFbImporter.cpp +++ b/drm/DrmFbImporter.cpp @@ -32,7 +32,7 @@ namespace android { auto DrmFbIdHandle::CreateInstance(hwc_drm_bo_t *bo, GemHandle first_gem_handle, - const std::shared_ptr &drm) + DrmDevice &drm) -> std::shared_ptr { // NOLINTNEXTLINE(cppcoreguidelines-owning-memory): priv. constructor usage std::shared_ptr local(new DrmFbIdHandle(drm)); @@ -44,7 +44,7 @@ auto DrmFbIdHandle::CreateInstance(hwc_drm_bo_t *bo, GemHandle first_gem_handle, for (size_t i = 1; i < local->gem_handles_.size(); i++) { if (bo->prime_fds[i] > 0) { if (bo->prime_fds[i] != bo->prime_fds[0]) { - err = drmPrimeFDToHandle(drm->fd(), bo->prime_fds[i], + err = drmPrimeFDToHandle(drm.GetFd(), bo->prime_fds[i], &local->gem_handles_.at(i)); if (err != 0) { ALOGE("failed to import prime fd %d errno=%d", bo->prime_fds[i], @@ -59,7 +59,7 @@ auto DrmFbIdHandle::CreateInstance(hwc_drm_bo_t *bo, GemHandle first_gem_handle, bool has_modifiers = bo->modifiers[0] != DRM_FORMAT_MOD_NONE && bo->modifiers[0] != DRM_FORMAT_MOD_INVALID; - if (!drm->HasAddFb2ModifiersSupport() && has_modifiers) { + if (!drm.HasAddFb2ModifiersSupport() && has_modifiers) { ALOGE("No ADDFB2 with modifier support. Can't import modifier %" PRIu64, bo->modifiers[0]); local.reset(); @@ -68,11 +68,11 @@ auto DrmFbIdHandle::CreateInstance(hwc_drm_bo_t *bo, GemHandle first_gem_handle, /* Create framebuffer object */ if (!has_modifiers) { - err = drmModeAddFB2(drm->fd(), bo->width, bo->height, bo->format, + err = drmModeAddFB2(drm.GetFd(), bo->width, bo->height, bo->format, &local->gem_handles_[0], &bo->pitches[0], &bo->offsets[0], &local->fb_id_, 0); } else { - err = drmModeAddFB2WithModifiers(drm->fd(), bo->width, bo->height, + err = drmModeAddFB2WithModifiers(drm.GetFd(), bo->width, bo->height, bo->format, &local->gem_handles_[0], &bo->pitches[0], &bo->offsets[0], &bo->modifiers[0], &local->fb_id_, @@ -88,7 +88,7 @@ auto DrmFbIdHandle::CreateInstance(hwc_drm_bo_t *bo, GemHandle first_gem_handle, DrmFbIdHandle::~DrmFbIdHandle() { /* Destroy framebuffer object */ - if (drmModeRmFB(drm_->fd(), fb_id_) != 0) { + if (drmModeRmFB(drm_->GetFd(), fb_id_) != 0) { ALOGE("Failed to rm fb"); } @@ -109,7 +109,7 @@ DrmFbIdHandle::~DrmFbIdHandle() { continue; } gem_close.handle = gem_handles_[i]; - int32_t err = drmIoctl(drm_->fd(), DRM_IOCTL_GEM_CLOSE, &gem_close); + int32_t err = drmIoctl(drm_->GetFd(), DRM_IOCTL_GEM_CLOSE, &gem_close); if (err != 0) { ALOGE("Failed to close gem handle %d, errno: %d", gem_handles_[i], errno); } @@ -120,7 +120,8 @@ auto DrmFbImporter::GetOrCreateFbId(hwc_drm_bo_t *bo) -> std::shared_ptr { /* Lookup DrmFbIdHandle in cache first. First handle serves as a cache key. */ GemHandle first_handle = 0; - int32_t err = drmPrimeFDToHandle(drm_->fd(), bo->prime_fds[0], &first_handle); + int32_t err = drmPrimeFDToHandle(drm_->GetFd(), bo->prime_fds[0], + &first_handle); if (err != 0) { ALOGE("Failed to import prime fd %d ret=%d", bo->prime_fds[0], err); @@ -143,7 +144,7 @@ auto DrmFbImporter::GetOrCreateFbId(hwc_drm_bo_t *bo) } /* No DrmFbIdHandle found in cache, create framebuffer object */ - auto fb_id_handle = DrmFbIdHandle::CreateInstance(bo, first_handle, drm_); + auto fb_id_handle = DrmFbIdHandle::CreateInstance(bo, first_handle, *drm_); if (fb_id_handle) { drm_fb_id_handle_cache_[first_handle] = fb_id_handle; } diff --git a/drm/DrmFbImporter.h b/drm/DrmFbImporter.h index efeb457..7f17bbe 100644 --- a/drm/DrmFbImporter.h +++ b/drm/DrmFbImporter.h @@ -37,8 +37,7 @@ namespace android { class DrmFbIdHandle { public: static auto CreateInstance(hwc_drm_bo_t *bo, GemHandle first_gem_handle, - const std::shared_ptr &drm) - -> std::shared_ptr; + DrmDevice &drm) -> std::shared_ptr; ~DrmFbIdHandle(); DrmFbIdHandle(DrmFbIdHandle &&) = delete; @@ -51,10 +50,9 @@ class DrmFbIdHandle { } private: - explicit DrmFbIdHandle(std::shared_ptr drm) - : drm_(std::move(drm)){}; + explicit DrmFbIdHandle(DrmDevice &drm) : drm_(&drm){}; - const std::shared_ptr drm_; + DrmDevice *const drm_; uint32_t fb_id_{}; std::array gem_handles_{}; @@ -62,8 +60,7 @@ class DrmFbIdHandle { class DrmFbImporter { public: - explicit DrmFbImporter(std::shared_ptr drm) - : drm_(std::move(drm)){}; + explicit DrmFbImporter(DrmDevice &drm) : drm_(&drm){}; ~DrmFbImporter() = default; DrmFbImporter(const DrmFbImporter &) = delete; DrmFbImporter(DrmFbImporter &&) = delete; @@ -84,7 +81,7 @@ class DrmFbImporter { } } - const std::shared_ptr drm_; + DrmDevice *const drm_; std::map> drm_fb_id_handle_cache_; }; diff --git a/drm/DrmPlane.cpp b/drm/DrmPlane.cpp index 6b5cec1..28f48f3 100644 --- a/drm/DrmPlane.cpp +++ b/drm/DrmPlane.cpp @@ -31,7 +31,7 @@ namespace android { auto DrmPlane::CreateInstance(DrmDevice &dev, uint32_t plane_id) -> std::unique_ptr { - auto p = MakeDrmModePlaneUnique(dev.fd(), plane_id); + auto p = MakeDrmModePlaneUnique(dev.GetFd(), plane_id); if (!p) { ALOGE("Failed to get plane %d", plane_id); return {}; diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp index 3bcb4d0..64af85b 100644 --- a/drm/VSyncWorker.cpp +++ b/drm/VSyncWorker.cpp @@ -140,7 +140,7 @@ void VSyncWorker::Routine() { vblank.request.sequence = 1; int64_t timestamp = 0; - ret = drmWaitVBlank(drm_->fd(), &vblank); + ret = drmWaitVBlank(drm_->GetFd(), &vblank); if (ret == -EINTR) return; diff --git a/hwc2_device/DrmHwcTwo.cpp b/hwc2_device/DrmHwcTwo.cpp index 698c1e5..31c1440 100644 --- a/hwc2_device/DrmHwcTwo.cpp +++ b/hwc2_device/DrmHwcTwo.cpp @@ -42,7 +42,7 @@ HWC2::Error DrmHwcTwo::CreateDisplay(hwc2_display_t displ, return HWC2::Error::BadDisplay; } auto display_planes = std::vector(); - for (const auto &plane : drm->planes()) { + for (const auto &plane : drm->GetPlanes()) { if (plane->IsCrtcSupported(*crtc)) display_planes.push_back(plane.get()); } @@ -164,7 +164,7 @@ void DrmHwcTwo::HandleDisplayHotplug(hwc2_display_t displayid, int state) { } void DrmHwcTwo::HandleInitialHotplugState(DrmDevice *drmDevice) { - for (const auto &conn : drmDevice->connectors()) { + for (const auto &conn : drmDevice->GetConnectors()) { int display_id = drmDevice->GetDisplayId(conn.get()); auto &display = displays_.at(display_id); @@ -178,7 +178,7 @@ void DrmHwcTwo::HandleInitialHotplugState(DrmDevice *drmDevice) { void DrmHwcTwo::HandleHotplugUEvent() { for (const auto &drm : resource_manager_.GetDrmDevices()) { - for (const auto &conn : drm->connectors()) { + for (const auto &conn : drm->GetConnectors()) { int display_id = drm->GetDisplayId(conn.get()); bool old_state = conn->IsConnected(); diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp index 54cf867..ea3957c 100644 --- a/hwc2_device/HwcDisplay.cpp +++ b/hwc2_device/HwcDisplay.cpp @@ -258,8 +258,8 @@ HWC2::Error HwcDisplay::GetChangedCompositionTypes(uint32_t *num_elements, HWC2::Error HwcDisplay::GetClientTargetSupport(uint32_t width, uint32_t height, int32_t /*format*/, int32_t dataspace) { - std::pair min = drm_->min_resolution(); - std::pair max = drm_->max_resolution(); + std::pair min = drm_->GetMinResolution(); + std::pair max = drm_->GetMaxResolution(); if (IsInHeadlessMode()) { return HWC2::Error::None; } -- cgit v1.2.3 From cad8e0ca57c268d179730c8ef68edd31350a11d9 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Mon, 31 Jan 2022 16:40:16 +0200 Subject: drm_hwcomposer: Introduce DrmDisplayPipeline class Create systematic way of binding DRM objects (Crtc,Encoder,Planes...) to the pipeline using RAII. Use it to create the pipeline. + Allow pipeline creation to fail. Closes: https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/-/issues/14 Signed-off-by: Roman Stratiienko --- Android.bp | 1 + drm/DrmConnector.h | 2 +- drm/DrmCrtc.h | 3 +- drm/DrmDevice.cpp | 124 ++++++++------------------------- drm/DrmDevice.h | 10 +-- drm/DrmDisplayPipeline.cpp | 168 +++++++++++++++++++++++++++++++++++++++++++++ drm/DrmDisplayPipeline.h | 86 +++++++++++++++++++++++ drm/DrmEncoder.h | 3 +- drm/DrmPlane.h | 2 +- 9 files changed, 292 insertions(+), 107 deletions(-) create mode 100644 drm/DrmDisplayPipeline.cpp create mode 100644 drm/DrmDisplayPipeline.h diff --git a/Android.bp b/Android.bp index b3bceaa..9236b3e 100644 --- a/Android.bp +++ b/Android.bp @@ -91,6 +91,7 @@ filegroup { "drm/DrmConnector.cpp", "drm/DrmCrtc.cpp", "drm/DrmDevice.cpp", + "drm/DrmDisplayPipeline.cpp", "drm/DrmEncoder.cpp", "drm/DrmFbImporter.cpp", "drm/DrmMode.cpp", diff --git a/drm/DrmConnector.h b/drm/DrmConnector.h index 5f8ba88..629b3cc 100644 --- a/drm/DrmConnector.h +++ b/drm/DrmConnector.h @@ -32,7 +32,7 @@ namespace android { class DrmDevice; -class DrmConnector { +class DrmConnector : public PipelineBindable { public: static auto CreateInstance(DrmDevice &dev, uint32_t connector_id, uint32_t index) -> std::unique_ptr; diff --git a/drm/DrmCrtc.h b/drm/DrmCrtc.h index 42fc9f9..ebf0a97 100644 --- a/drm/DrmCrtc.h +++ b/drm/DrmCrtc.h @@ -21,6 +21,7 @@ #include +#include "DrmDisplayPipeline.h" #include "DrmMode.h" #include "DrmProperty.h" #include "DrmUnique.h" @@ -29,7 +30,7 @@ namespace android { class DrmDevice; -class DrmCrtc { +class DrmCrtc : public PipelineBindable { public: static auto CreateInstance(DrmDevice &dev, uint32_t crtc_id, uint32_t index) -> std::unique_ptr; diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp index 29dd95f..ece9437 100644 --- a/drm/DrmDevice.cpp +++ b/drm/DrmDevice.cpp @@ -66,7 +66,6 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { ret = drmSetClientCap(GetFd(), DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1); if (ret != 0) { ALOGI("Failed to set writeback cap %d", ret); - ret = 0; } #endif @@ -125,14 +124,31 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { } } + auto plane_res = MakeDrmModePlaneResUnique(GetFd()); + if (!plane_res) { + ALOGE("Failed to get plane resources"); + return std::make_tuple(-ENOENT, 0); + } + + for (uint32_t i = 0; i < plane_res->count_planes; ++i) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + auto plane = DrmPlane::CreateInstance(*this, plane_res->planes[i]); + + if (plane) { + planes_.emplace_back(std::move(plane)); + } + } + auto add_displays = [this, &num_displays](bool internal, bool connected) { for (auto &conn : connectors_) { bool is_connected = conn->IsConnected(); if ((internal ? conn->IsInternal() : conn->IsExternal()) && (connected ? is_connected : !is_connected)) { - bound_connectors_[num_displays] = conn.get(); - connectors_to_display_id_[conn.get()] = num_displays; - ++num_displays; + auto pipe = DrmDisplayPipeline::CreatePipeline(*conn); + if (pipe) { + pipelines_[num_displays] = std::move(pipe); + ++num_displays; + } } } }; @@ -145,110 +161,28 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { add_displays(/*internal = */ true, /*connected = */ false); add_displays(/*internal = */ false, /*connected = */ false); - // Catch-all for the above loops - if (ret != 0) - return std::make_tuple(ret, 0); - - auto plane_res = MakeDrmModePlaneResUnique(GetFd()); - if (!plane_res) { - ALOGE("Failed to get plane resources"); - return std::make_tuple(-ENOENT, 0); - } - - for (uint32_t i = 0; i < plane_res->count_planes; ++i) { - // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) - auto plane = DrmPlane::CreateInstance(*this, plane_res->planes[i]); - - if (plane) { - planes_.emplace_back(std::move(plane)); - } - } - - for (auto &conn : connectors_) { - ret = CreateDisplayPipe(conn.get()); - if (ret != 0) { - ALOGE("Failed CreateDisplayPipe %d with %d", conn->GetId(), ret); - return std::make_tuple(ret, 0); - } - } - return std::make_tuple(ret, bound_connectors_.size()); + return std::make_tuple(0, pipelines_.size()); } bool DrmDevice::HandlesDisplay(int display) const { - return bound_connectors_.count(display) != 0; + return pipelines_.count(display) != 0; } DrmConnector *DrmDevice::GetConnectorForDisplay(int display) const { - return bound_connectors_.at(display); + return pipelines_.at(display)->connector->Get(); } DrmCrtc *DrmDevice::GetCrtcForDisplay(int display) const { - return bound_crtcs_.at(display); + return pipelines_.at(display)->crtc->Get(); } -int DrmDevice::TryEncoderForDisplay(int display, DrmEncoder *enc) { - /* First try to use the currently-bound crtc */ - auto *crtc = FindCrtcById(enc->GetCurrentCrtcId()); - if (crtc != nullptr && bound_crtcs_.count(display) == 0) { - bound_crtcs_[display] = crtc; - bound_encoders_[crtc] = enc; - return 0; - } - - /* Try to find a possible crtc which will work */ - for (auto &crtc : crtcs_) { - /* Crtc not supported or we've already tried this earlier */ - if (!enc->SupportsCrtc(*crtc) || crtc->GetId() == enc->GetCurrentCrtcId()) { - continue; - } - - if (bound_crtcs_.count(display) == 0) { - bound_crtcs_[display] = crtc.get(); - bound_encoders_[crtc.get()] = enc; - return 0; - } - } - - /* We can't use the encoder, but nothing went wrong, try another one */ - return -EAGAIN; -} - -int DrmDevice::CreateDisplayPipe(DrmConnector *connector) { - int display = connectors_to_display_id_.at(connector); - /* Try to use current setup first */ - auto *enc0 = FindEncoderById(connector->GetCurrentEncoderId()); - if (enc0 != nullptr && encoders_to_display_id_.count(enc0) == 0) { - int ret = TryEncoderForDisplay(display, enc0); - if (ret == 0) { - encoders_to_display_id_[enc0] = display; - return 0; - } - - if (ret != -EAGAIN) { - ALOGE("Could not set mode %d/%d", display, ret); - return ret; - } - } - - 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 == 0) { - encoders_to_display_id_[enc.get()] = display; - return 0; - } - - if (ret != -EAGAIN) { - ALOGE("Could not set mode %d/%d", display, ret); - return ret; +auto DrmDevice::GetDisplayId(DrmConnector *conn) -> int { + for (auto &dpipe : pipelines_) { + if (dpipe.second->connector->Get() == conn) { + return dpipe.first; } } - ALOGE("Could not find a suitable encoder/crtc for display %d", display); - return -ENODEV; + return -1; } auto DrmDevice::RegisterUserPropertyBlob(void *data, size_t length) const diff --git a/drm/DrmDevice.h b/drm/DrmDevice.h index 6d792c2..8d9a34c 100644 --- a/drm/DrmDevice.h +++ b/drm/DrmDevice.h @@ -96,9 +96,7 @@ class DrmDevice { return nullptr; } - auto GetDisplayId(DrmConnector *conn) { - return connectors_to_display_id_.at(conn); - } + auto GetDisplayId(DrmConnector *conn) -> int; int GetProperty(uint32_t obj_id, uint32_t obj_type, const char *prop_name, DrmProperty *property) const; @@ -119,11 +117,7 @@ class DrmDevice { std::pair min_resolution_; std::pair max_resolution_; - std::map bound_crtcs_; - std::map bound_connectors_; - std::map connectors_to_display_id_; - std::map encoders_to_display_id_; - std::map bound_encoders_; + std::map> pipelines_; bool HasAddFb2ModifiersSupport_{}; diff --git a/drm/DrmDisplayPipeline.cpp b/drm/DrmDisplayPipeline.cpp new file mode 100644 index 0000000..8a490f8 --- /dev/null +++ b/drm/DrmDisplayPipeline.cpp @@ -0,0 +1,168 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "hwc-drm-display-pipeline" + +#include "DrmDisplayPipeline.h" + +#include "DrmConnector.h" +#include "DrmCrtc.h" +#include "DrmDevice.h" +#include "DrmEncoder.h" +#include "DrmPlane.h" +#include "compositor/DrmDisplayCompositor.h" +#include "utils/log.h" + +namespace android { + +template +auto PipelineBindable::BindPipeline(DrmDisplayPipeline *pipeline, + bool return_object_if_bound) + -> std::shared_ptr> { + auto owner_object = owner_object_.lock(); + if (owner_object) { + if (bound_pipeline_ == pipeline && return_object_if_bound) { + return owner_object; + } + + return {}; + } + owner_object = std::make_shared>(static_cast(this)); + + bound_pipeline_ = pipeline; + return owner_object; +} + +static auto TryCreatePipeline(DrmDevice &dev, DrmConnector &connector, + DrmEncoder &enc, DrmCrtc &crtc) + -> std::unique_ptr { + /* Check if resources are available */ + + auto pipe = std::make_unique(); + pipe->device = &dev; + + pipe->connector = connector.BindPipeline(pipe.get()); + pipe->encoder = enc.BindPipeline(pipe.get()); + pipe->crtc = crtc.BindPipeline(pipe.get()); + + if (!pipe->connector || !pipe->encoder || !pipe->crtc) { + return {}; + } + + std::vector primary_planes; + std::vector overlay_planes; + + /* Attach necessary resources */ + auto display_planes = std::vector(); + for (const auto &plane : dev.GetPlanes()) { + if (plane->IsCrtcSupported(crtc)) { + if (plane->GetType() == DRM_PLANE_TYPE_PRIMARY) { + primary_planes.emplace_back(plane.get()); + } else if (plane->GetType() == DRM_PLANE_TYPE_OVERLAY) { + overlay_planes.emplace_back(plane.get()); + } else { + ALOGI("Ignoring cursor plane %d", plane->GetId()); + } + } + } + + if (primary_planes.empty()) { + ALOGE("Primary plane for CRTC %d not found", crtc.GetId()); + return {}; + } + + if (primary_planes.size() > 1) { + ALOGE("Found more than 1 primary plane for CRTC %d", crtc.GetId()); + return {}; + } + + pipe->primary_plane = primary_planes[0]->BindPipeline(pipe.get()); + if (!pipe->primary_plane) { + ALOGE("Primary plane %d is already owned. Internal error.", + primary_planes[0]->GetId()); + return {}; + } + + bool use_overlay_planes = true; // TODO(rsglobal): restore + // strtol(use_overlay_planes_prop, nullptr, + // 10); + if (use_overlay_planes) { + for (auto *plane : overlay_planes) { + auto op = plane->BindPipeline(pipe.get()); + if (op) { + pipe->overlay_planes.emplace_back(op); + } + } + } + + return pipe; +} + +static auto TryCreatePipelineUsingEncoder(DrmDevice &dev, DrmConnector &conn, + DrmEncoder &enc) + -> std::unique_ptr { + /* First try to use the currently-bound crtc */ + auto *crtc = dev.FindCrtcById(enc.GetCurrentCrtcId()); + if (crtc != nullptr) { + auto pipeline = TryCreatePipeline(dev, conn, enc, *crtc); + if (pipeline) { + return pipeline; + } + } + + /* Try to find a possible crtc which will work */ + for (const auto &crtc : dev.GetCrtcs()) { + if (enc.SupportsCrtc(*crtc)) { + auto pipeline = TryCreatePipeline(dev, conn, enc, *crtc); + if (pipeline) { + return pipeline; + } + } + } + + /* We can't use this encoder, but nothing went wrong, try another one */ + return {}; +} + +auto DrmDisplayPipeline::CreatePipeline(DrmConnector &connector) + -> std::unique_ptr { + auto &dev = connector.GetDev(); + /* Try to use current setup first */ + auto *encoder = dev.FindEncoderById(connector.GetCurrentEncoderId()); + + if (encoder != nullptr) { + auto pipeline = TryCreatePipelineUsingEncoder(dev, connector, *encoder); + if (pipeline) { + return pipeline; + } + } + + for (const auto &enc : dev.GetEncoders()) { + if (connector.SupportsEncoder(*enc)) { + auto pipeline = TryCreatePipelineUsingEncoder(dev, connector, *enc); + if (pipeline) { + return pipeline; + } + } + } + + ALOGE("Could not find a suitable encoder/crtc for connector %s", + connector.GetName().c_str()); + + return {}; +} + +} // namespace android diff --git a/drm/DrmDisplayPipeline.h b/drm/DrmDisplayPipeline.h new file mode 100644 index 0000000..cb5b9e2 --- /dev/null +++ b/drm/DrmDisplayPipeline.h @@ -0,0 +1,86 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_DRMDISPLAYPIPELINE_H_ +#define ANDROID_DRMDISPLAYPIPELINE_H_ + +#include +#include + +namespace android { + +class DrmConnector; +class DrmDevice; +class DrmPlane; +class DrmCrtc; +class DrmEncoder; +class DrmDisplayCompositor; + +struct DrmDisplayPipeline; + +template +class BindingOwner; + +template +class PipelineBindable { + friend class BindingOwner; + + public: + auto *GetPipeline() { + return bound_pipeline_; + } + + auto BindPipeline(DrmDisplayPipeline *pipeline, + bool return_object_if_bound = false) + -> std::shared_ptr>; + + private: + DrmDisplayPipeline *bound_pipeline_; + std::weak_ptr> owner_object_; +}; + +template +class BindingOwner { + public: + explicit BindingOwner(B *pb) : bindable_(pb){}; + ~BindingOwner() { + bindable_->bound_pipeline_ = nullptr; + } + + B *Get() { + return bindable_; + } + + private: + B *const bindable_; +}; + +struct DrmDisplayPipeline { + static auto CreatePipeline(DrmConnector &connector) + -> std::unique_ptr; + + DrmDevice *device; + + std::shared_ptr> connector; + std::shared_ptr> encoder; + std::shared_ptr> crtc; + std::shared_ptr> primary_plane; + std::vector>> overlay_planes; +}; + +} // namespace android + +#endif diff --git a/drm/DrmEncoder.h b/drm/DrmEncoder.h index e66d6f1..39a695c 100644 --- a/drm/DrmEncoder.h +++ b/drm/DrmEncoder.h @@ -24,10 +24,11 @@ #include #include "DrmCrtc.h" +#include "DrmDisplayPipeline.h" namespace android { -class DrmEncoder { +class DrmEncoder : public PipelineBindable { public: static auto CreateInstance(DrmDevice &dev, uint32_t encoder_id, uint32_t index) -> std::unique_ptr; diff --git a/drm/DrmPlane.h b/drm/DrmPlane.h index 9826f67..65f458f 100644 --- a/drm/DrmPlane.h +++ b/drm/DrmPlane.h @@ -31,7 +31,7 @@ namespace android { class DrmDevice; struct DrmHwcLayer; -class DrmPlane { +class DrmPlane : public PipelineBindable { public: DrmPlane(const DrmPlane &) = delete; DrmPlane &operator=(const DrmPlane &) = delete; -- cgit v1.2.3 From 33a71fab806cfac406d4f26c870f9bd975435975 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Wed, 2 Feb 2022 12:53:14 +0200 Subject: drm_hwcomposer: Fix PipelineBindable::BindPipeline We should assign weak pointer object to really take ownership. Fixes: cad8e0ca57c2 ("drm_hwcomposer: Introduce DrmDisplayPipeline class") Signed-off-by: Roman Stratiienko --- drm/DrmDisplayPipeline.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/drm/DrmDisplayPipeline.cpp b/drm/DrmDisplayPipeline.cpp index 8a490f8..69d28b3 100644 --- a/drm/DrmDisplayPipeline.cpp +++ b/drm/DrmDisplayPipeline.cpp @@ -42,6 +42,7 @@ auto PipelineBindable::BindPipeline(DrmDisplayPipeline *pipeline, } owner_object = std::make_shared>(static_cast(this)); + owner_object_ = owner_object; bound_pipeline_ = pipeline; return owner_object; } -- cgit v1.2.3 From 19c162fe790ce9da373686512f451c6ed276bca8 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Tue, 1 Feb 2022 09:35:08 +0200 Subject: drm_hwcomposer: Initialize HwcDisplay using DrmDisplayPIpeline HwcDisplay can now take all necessary objects from DrmDisplayPipeline. Signed-off-by: Roman Stratiienko --- backend/Backend.cpp | 4 +- backend/BackendManager.cpp | 8 ++- compositor/DrmDisplayCompositor.cpp | 57 +++++------------ compositor/DrmDisplayCompositor.h | 7 +-- drm/DrmDevice.cpp | 9 +-- drm/DrmDevice.h | 10 ++- drm/DrmDisplayPipeline.cpp | 13 +++- drm/DrmDisplayPipeline.h | 2 + drm/ResourceManager.cpp | 10 ++- drm/ResourceManager.h | 4 +- drm/VSyncWorker.cpp | 57 +++++++---------- drm/VSyncWorker.h | 10 ++- hwc2_device/DrmHwcTwo.cpp | 18 ++---- hwc2_device/HwcDisplay.cpp | 121 ++++++++++++++---------------------- hwc2_device/HwcDisplay.h | 34 +++------- 15 files changed, 134 insertions(+), 230 deletions(-) diff --git a/backend/Backend.cpp b/backend/Backend.cpp index 98862ba..7d82eef 100644 --- a/backend/Backend.cpp +++ b/backend/Backend.cpp @@ -119,8 +119,8 @@ void Backend::MarkValidated(std::vector &layers, std::tuple Backend::GetExtraClientRange( HwcDisplay *display, const std::vector &layers, int client_start, size_t client_size) { - size_t avail_planes = display->primary_planes().size() + - display->overlay_planes().size(); + size_t avail_planes = 1 /* primary planes count*/ + + display->GetPipe().overlay_planes.size(); /* * If more layers then planes, save one plane diff --git a/backend/BackendManager.cpp b/backend/BackendManager.cpp index 4a735eb..9bf6324 100644 --- a/backend/BackendManager.cpp +++ b/backend/BackendManager.cpp @@ -42,7 +42,7 @@ int BackendManager::RegisterBackend(const std::string &name, } int BackendManager::SetBackendForDisplay(HwcDisplay *display) { - std::string driver_name(display->drm()->GetName()); + std::string driver_name(display->GetPipe().device->GetName()); char backend_override[PROPERTY_VALUE_MAX]; property_get("vendor.hwc.backend_override", backend_override, driver_name.c_str()); @@ -51,13 +51,15 @@ int BackendManager::SetBackendForDisplay(HwcDisplay *display) { display->set_backend(GetBackendByName(backend_name)); if (display->backend() == nullptr) { ALOGE("Failed to set backend '%s' for '%s' and driver '%s'", - backend_name.c_str(), display->connector()->GetName().c_str(), + backend_name.c_str(), + display->GetPipe().connector->Get()->GetName().c_str(), driver_name.c_str()); return -EINVAL; } ALOGI("Backend '%s' for '%s' and driver '%s' was successfully set", - backend_name.c_str(), display->connector()->GetName().c_str(), + backend_name.c_str(), + display->GetPipe().connector->Get()->GetName().c_str(), driver_name.c_str()); return 0; diff --git a/compositor/DrmDisplayCompositor.cpp b/compositor/DrmDisplayCompositor.cpp index d25fb98..bd0247d 100644 --- a/compositor/DrmDisplayCompositor.cpp +++ b/compositor/DrmDisplayCompositor.cpp @@ -39,20 +39,6 @@ namespace android { -auto DrmDisplayCompositor::Init(ResourceManager *resource_manager, int display) - -> int { - resource_manager_ = resource_manager; - display_ = display; - DrmDevice *drm = resource_manager_->GetDrmDevice(display); - if (!drm) { - ALOGE("Could not find drmdevice for display"); - return -EINVAL; - } - - initialized_ = true; - return 0; -} - // NOLINTNEXTLINE (readability-function-cognitive-complexity): Fixme auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { ATRACE_CALL(); @@ -79,18 +65,9 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { auto new_frame_state = NewFrameState(); - DrmDevice *drm = resource_manager_->GetDrmDevice(display_); - - DrmConnector *connector = drm->GetConnectorForDisplay(display_); - if (!connector) { - ALOGE("Could not locate connector for display %d", display_); - return -ENODEV; - } - DrmCrtc *crtc = drm->GetCrtcForDisplay(display_); - if (!crtc) { - ALOGE("Could not locate crtc for display %d", display_); - return -ENODEV; - } + auto *drm = pipe_->device; + auto *connector = pipe_->connector->Get(); + auto *crtc = pipe_->crtc->Get(); auto pset = MakeDrmModeAtomicReqUnique(); if (!pset) { @@ -113,8 +90,7 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { } if (args.display_mode) { - new_frame_state.mode_blob = args.display_mode.value().CreateModeBlob( - *resource_manager_->GetDrmDevice(display_)); + new_frame_state.mode_blob = args.display_mode.value().CreateModeBlob(*drm); if (!new_frame_state.mode_blob) { ALOGE("Failed to create mode_blob"); @@ -207,12 +183,14 @@ auto DrmDisplayCompositor::ExecuteAtomicCommit(AtomicCommitArgs &args) -> int { if (!args.test_only) { if (err) { - ALOGE("Composite failed for display %d", display_); + ALOGE("Composite failed for pipeline %s", + pipe_->connector->Get()->GetName().c_str()); // Disable the hw used by the last active composition. This allows us to // signal the release fences from that composition to avoid hanging. AtomicCommitArgs cl_args = {.clear_active_composition = true}; if (CommitFrame(cl_args)) { - ALOGE("Failed to clean-up active composition for display %d", display_); + ALOGE("Failed to clean-up active composition for pipeline %s", + pipe_->connector->Get()->GetName().c_str()); } return err; } @@ -222,19 +200,12 @@ auto DrmDisplayCompositor::ExecuteAtomicCommit(AtomicCommitArgs &args) -> int { } // namespace android auto DrmDisplayCompositor::ActivateDisplayUsingDPMS() -> int { - auto *drm = resource_manager_->GetDrmDevice(display_); - auto *connector = drm->GetConnectorForDisplay(display_); - if (connector == nullptr) { - ALOGE("Could not locate connector for display %d", display_); - return -ENODEV; - } - - if (connector->GetDpmsProperty()) { - drmModeConnectorSetProperty(drm->GetFd(), connector->GetId(), - connector->GetDpmsProperty().id(), - DRM_MODE_DPMS_ON); - } - return 0; + return drmModeConnectorSetProperty(pipe_->device->GetFd(), + pipe_->connector->Get()->GetId(), + pipe_->connector->Get() + ->GetDpmsProperty() + .id(), + DRM_MODE_DPMS_ON); } } // namespace android diff --git a/compositor/DrmDisplayCompositor.h b/compositor/DrmDisplayCompositor.h index 9679520..bff3de7 100644 --- a/compositor/DrmDisplayCompositor.h +++ b/compositor/DrmDisplayCompositor.h @@ -54,9 +54,8 @@ struct AtomicCommitArgs { class DrmDisplayCompositor { public: - DrmDisplayCompositor() = default; + explicit DrmDisplayCompositor(DrmDisplayPipeline *pipe) : pipe_(pipe){}; ~DrmDisplayCompositor() = default; - auto Init(ResourceManager *resource_manager, int display) -> int; auto ExecuteAtomicCommit(AtomicCommitArgs &args) -> int; @@ -88,9 +87,7 @@ class DrmDisplayCompositor { }; } - ResourceManager *resource_manager_ = nullptr; - bool initialized_{}; - int display_ = -1; + DrmDisplayPipeline *const pipe_; }; } // namespace android diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp index ece9437..d1ae7c9 100644 --- a/drm/DrmDevice.cpp +++ b/drm/DrmDevice.cpp @@ -30,6 +30,7 @@ #include #include +#include "compositor/DrmDisplayCompositor.h" #include "drm/DrmPlane.h" #include "utils/log.h" #include "utils/properties.h" @@ -168,14 +169,6 @@ bool DrmDevice::HandlesDisplay(int display) const { return pipelines_.count(display) != 0; } -DrmConnector *DrmDevice::GetConnectorForDisplay(int display) const { - return pipelines_.at(display)->connector->Get(); -} - -DrmCrtc *DrmDevice::GetCrtcForDisplay(int display) const { - return pipelines_.at(display)->crtc->Get(); -} - auto DrmDevice::GetDisplayId(DrmConnector *conn) -> int { for (auto &dpipe : pipelines_) { if (dpipe.second->connector->Get() == conn) { diff --git a/drm/DrmDevice.h b/drm/DrmDevice.h index 8d9a34c..5220760 100644 --- a/drm/DrmDevice.h +++ b/drm/DrmDevice.h @@ -56,8 +56,10 @@ class DrmDevice { return max_resolution_; } - DrmConnector *GetConnectorForDisplay(int display) const; - DrmCrtc *GetCrtcForDisplay(int display) const; + auto *GetPipelineForDisplay(int display) { + return pipelines_.count(display) != 0 ? pipelines_.at(display).get() + : nullptr; + } std::string GetName() const; @@ -102,10 +104,6 @@ class DrmDevice { DrmProperty *property) const; private: - int TryEncoderForDisplay(int display, DrmEncoder *enc); - - int CreateDisplayPipe(DrmConnector *connector); - UniqueFd fd_; std::vector> connectors_; diff --git a/drm/DrmDisplayPipeline.cpp b/drm/DrmDisplayPipeline.cpp index 69d28b3..0ce1afc 100644 --- a/drm/DrmDisplayPipeline.cpp +++ b/drm/DrmDisplayPipeline.cpp @@ -25,6 +25,7 @@ #include "DrmPlane.h" #include "compositor/DrmDisplayCompositor.h" #include "utils/log.h" +#include "utils/properties.h" namespace android { @@ -97,9 +98,13 @@ static auto TryCreatePipeline(DrmDevice &dev, DrmConnector &connector, return {}; } - bool use_overlay_planes = true; // TODO(rsglobal): restore - // strtol(use_overlay_planes_prop, nullptr, - // 10); + char use_overlay_planes_prop[PROPERTY_VALUE_MAX]; + property_get("vendor.hwc.drm.use_overlay_planes", use_overlay_planes_prop, + "1"); + constexpr int kStrtolBase = 10; + bool use_overlay_planes = strtol(use_overlay_planes_prop, nullptr, + kStrtolBase) != 0; + if (use_overlay_planes) { for (auto *plane : overlay_planes) { auto op = plane->BindPipeline(pipe.get()); @@ -109,6 +114,8 @@ static auto TryCreatePipeline(DrmDevice &dev, DrmConnector &connector, } } + pipe->compositor = std::make_unique(pipe.get()); + return pipe; } diff --git a/drm/DrmDisplayPipeline.h b/drm/DrmDisplayPipeline.h index cb5b9e2..a3e2dd9 100644 --- a/drm/DrmDisplayPipeline.h +++ b/drm/DrmDisplayPipeline.h @@ -79,6 +79,8 @@ struct DrmDisplayPipeline { std::shared_ptr> crtc; std::shared_ptr> primary_plane; std::vector>> overlay_planes; + + std::unique_ptr compositor; }; } // namespace android diff --git a/drm/ResourceManager.cpp b/drm/ResourceManager.cpp index 46f77e4..a7d99ee 100644 --- a/drm/ResourceManager.cpp +++ b/drm/ResourceManager.cpp @@ -24,7 +24,9 @@ #include #include "bufferinfo/BufferInfoGetter.h" +#include "compositor/DrmDisplayCompositor.h" #include "drm/DrmDevice.h" +#include "drm/DrmDisplayPipeline.h" #include "drm/DrmPlane.h" #include "utils/log.h" #include "utils/properties.h" @@ -95,10 +97,12 @@ int ResourceManager::AddDrmDevice(const std::string &path) { return ret; } -DrmDevice *ResourceManager::GetDrmDevice(int display) { +DrmDisplayPipeline *ResourceManager::GetPipeline(int display) { for (auto &drm : drms_) { - if (drm->HandlesDisplay(display)) - return drm.get(); + auto *pipe = drm->GetPipelineForDisplay(display); + if (pipe != nullptr) { + return pipe; + } } return nullptr; } diff --git a/drm/ResourceManager.h b/drm/ResourceManager.h index f54d682..caeb098 100644 --- a/drm/ResourceManager.h +++ b/drm/ResourceManager.h @@ -33,8 +33,8 @@ class ResourceManager { ~ResourceManager(); int Init(); - DrmDevice *GetDrmDevice(int display); - const std::vector> &GetDrmDevices() const { + auto GetPipeline(int display) -> DrmDisplayPipeline *; + auto &GetDrmDevices() const { return drms_; } int GetDisplayCount() const { diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp index 64af85b..8cb4d08 100644 --- a/drm/VSyncWorker.cpp +++ b/drm/VSyncWorker.cpp @@ -29,19 +29,12 @@ namespace android { -VSyncWorker::VSyncWorker() - : Worker("vsync", HAL_PRIORITY_URGENT_DISPLAY), - drm_(nullptr), - display_(-1), - enabled_(false), - last_timestamp_(-1) { -} +VSyncWorker::VSyncWorker() : Worker("vsync", HAL_PRIORITY_URGENT_DISPLAY){}; -auto VSyncWorker::Init(DrmDevice *drm, int display, +auto VSyncWorker::Init(DrmDisplayPipeline *pipe, std::function callback) -> int { - drm_ = drm; - display_ = display; + pipe_ = pipe; callback_ = std::move(callback); return InitWorker(); @@ -86,12 +79,10 @@ int VSyncWorker::SyntheticWaitVBlank(int64_t *timestamp) { return ret; float refresh = 60.0F; // Default to 60Hz refresh rate - DrmConnector *conn = drm_->GetConnectorForDisplay(display_); - 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->GetActiveMode().v_refresh() : 0.0F); + if (pipe_ != nullptr && + pipe_->connector->Get()->GetActiveMode().v_refresh() != 0.0F) { + refresh = pipe_->connector->Get()->GetActiveMode().v_refresh(); + } int64_t phased_timestamp = GetPhasedVSync(kOneSecondNs / static_cast(refresh), @@ -121,28 +112,26 @@ void VSyncWorker::Routine() { } } - int display = display_; + auto *pipe = pipe_; Unlock(); - DrmCrtc *crtc = drm_->GetCrtcForDisplay(display); - if (!crtc) { - ALOGE("Failed to get crtc for display"); - return; - } - uint32_t high_crtc = (crtc->GetIndexInResArray() - << DRM_VBLANK_HIGH_CRTC_SHIFT); + ret = -EAGAIN; + int64_t timestamp = 0; + drmVBlank vblank{}; - drmVBlank vblank; - memset(&vblank, 0, sizeof(vblank)); - vblank.request.type = (drmVBlankSeqType)(DRM_VBLANK_RELATIVE | - (high_crtc & - DRM_VBLANK_HIGH_CRTC_MASK)); - vblank.request.sequence = 1; + if (pipe != nullptr) { + uint32_t high_crtc = (pipe->crtc->Get()->GetIndexInResArray() + << DRM_VBLANK_HIGH_CRTC_SHIFT); - int64_t timestamp = 0; - ret = drmWaitVBlank(drm_->GetFd(), &vblank); - if (ret == -EINTR) - return; + vblank.request.type = (drmVBlankSeqType)(DRM_VBLANK_RELATIVE | + (high_crtc & + DRM_VBLANK_HIGH_CRTC_MASK)); + vblank.request.sequence = 1; + + ret = drmWaitVBlank(pipe->device->GetFd(), &vblank); + if (ret == -EINTR) + return; + } if (ret) { ret = SyntheticWaitVBlank(×tamp); diff --git a/drm/VSyncWorker.h b/drm/VSyncWorker.h index 67aaa16..1e6d39f 100644 --- a/drm/VSyncWorker.h +++ b/drm/VSyncWorker.h @@ -36,7 +36,7 @@ class VSyncWorker : public Worker { VSyncWorker(); ~VSyncWorker() override = default; - auto Init(DrmDevice *drm, int display, + auto Init(DrmDisplayPipeline *pipe, std::function callback) -> int; void VSyncControl(bool enabled); @@ -48,13 +48,11 @@ class VSyncWorker : public Worker { int64_t GetPhasedVSync(int64_t frame_ns, int64_t current) const; int SyntheticWaitVBlank(int64_t *timestamp); - DrmDevice *drm_; - std::function callback_; - int display_; - std::atomic_bool enabled_; - int64_t last_timestamp_; + DrmDisplayPipeline *pipe_ = nullptr; + std::atomic_bool enabled_ = false; + int64_t last_timestamp_ = -1; }; } // namespace android diff --git a/hwc2_device/DrmHwcTwo.cpp b/hwc2_device/DrmHwcTwo.cpp index 31c1440..4cb0fc3 100644 --- a/hwc2_device/DrmHwcTwo.cpp +++ b/hwc2_device/DrmHwcTwo.cpp @@ -27,26 +27,16 @@ DrmHwcTwo::DrmHwcTwo() = default; HWC2::Error DrmHwcTwo::CreateDisplay(hwc2_display_t displ, HWC2::DisplayType type) { - DrmDevice *drm = resource_manager_.GetDrmDevice(static_cast(displ)); - if (!drm) { + auto *pipe = resource_manager_.GetPipeline(static_cast(displ)); + if (!pipe) { ALOGE("Failed to get a valid drmresource"); return HWC2::Error::NoResources; } displays_.emplace(std::piecewise_construct, std::forward_as_tuple(displ), - std::forward_as_tuple(&resource_manager_, drm, displ, type, + std::forward_as_tuple(&resource_manager_, pipe, displ, type, this)); - DrmCrtc *crtc = drm->GetCrtcForDisplay(static_cast(displ)); - if (!crtc) { - ALOGE("Failed to get crtc for display %d", static_cast(displ)); - return HWC2::Error::BadDisplay; - } - auto display_planes = std::vector(); - for (const auto &plane : drm->GetPlanes()) { - if (plane->IsCrtcSupported(*crtc)) - display_planes.push_back(plane.get()); - } - displays_.at(displ).Init(&display_planes); + displays_.at(displ).Init(); return HWC2::Error::None; } diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp index ea3957c..ac5d196 100644 --- a/hwc2_device/HwcDisplay.cpp +++ b/hwc2_device/HwcDisplay.cpp @@ -68,8 +68,12 @@ std::string HwcDisplay::Dump() { " VSync remains"; } + std::string connector_name = IsInHeadlessMode() + ? "NULL-DISPLAY" + : GetPipe().connector->Get()->GetName(); + std::stringstream ss; - ss << "- Display on: " << connector_->GetName() << "\n" + ss << "- Display on: " << connector_name << "\n" << " Flattening state: " << flattening_state_str << "\n" << "Statistics since system boot:\n" << DumpDelta(total_stats_) << "\n\n" @@ -80,12 +84,12 @@ std::string HwcDisplay::Dump() { return ss.str(); } -HwcDisplay::HwcDisplay(ResourceManager *resource_manager, DrmDevice *drm, - hwc2_display_t handle, HWC2::DisplayType type, - DrmHwcTwo *hwc2) +HwcDisplay::HwcDisplay(ResourceManager *resource_manager, + DrmDisplayPipeline *pipeline, hwc2_display_t handle, + HWC2::DisplayType type, DrmHwcTwo *hwc2) : hwc2_(hwc2), resource_manager_(resource_manager), - drm_(drm), + pipeline_(pipeline), handle_(handle), type_(type), color_transform_hint_(HAL_COLOR_TRANSFORM_IDENTITY) { @@ -104,43 +108,11 @@ void HwcDisplay::ClearDisplay() { } AtomicCommitArgs a_args = {.clear_active_composition = true}; - compositor_.ExecuteAtomicCommit(a_args); + GetPipe().compositor->ExecuteAtomicCommit(a_args); } -HWC2::Error HwcDisplay::Init(std::vector *planes) { - int display = static_cast(handle_); - int ret = compositor_.Init(resource_manager_, display); - if (ret) { - ALOGE("Failed display compositor init for display %d (%d)", display, ret); - return HWC2::Error::NoResources; - } - - // Split up the given display planes into primary and overlay to properly - // interface with the composition - char use_overlay_planes_prop[PROPERTY_VALUE_MAX]; - property_get("vendor.hwc.drm.use_overlay_planes", use_overlay_planes_prop, - "1"); - bool use_overlay_planes = strtol(use_overlay_planes_prop, nullptr, 10); - for (auto &plane : *planes) { - if (plane->GetType() == DRM_PLANE_TYPE_PRIMARY) - primary_planes_.push_back(plane); - else if (use_overlay_planes && (plane)->GetType() == DRM_PLANE_TYPE_OVERLAY) - overlay_planes_.push_back(plane); - } - - crtc_ = drm_->GetCrtcForDisplay(display); - if (!crtc_) { - ALOGE("Failed to get crtc for display %d", display); - return HWC2::Error::BadDisplay; - } - - connector_ = drm_->GetConnectorForDisplay(display); - if (!connector_) { - ALOGE("Failed to get connector for display %d", display); - return HWC2::Error::BadDisplay; - } - - ret = vsync_worker_.Init(drm_, display, [this](int64_t timestamp) { +HWC2::Error HwcDisplay::Init() { + int ret = vsync_worker_.Init(pipeline_, [this](int64_t timestamp) { const std::lock_guard lock(hwc2_->GetResMan().GetMainLock()); /* vsync callback */ #if PLATFORM_SDK_VERSION > 29 @@ -159,34 +131,30 @@ HWC2::Error HwcDisplay::Init(std::vector *planes) { } }); if (ret) { - ALOGE("Failed to create event worker for d=%d %d\n", display, ret); + ALOGE("Failed to create event worker for d=%d %d\n", int(handle_), ret); return HWC2::Error::BadDisplay; } - ret = flattening_vsync_worker_ - .Init(drm_, display, [this](int64_t /*timestamp*/) { - const std::lock_guard lock( - hwc2_->GetResMan().GetMainLock()); - /* Frontend flattening */ - if (flattenning_state_ > - ClientFlattenningState::ClientRefreshRequested && - --flattenning_state_ == - ClientFlattenningState::ClientRefreshRequested && - hwc2_->refresh_callback_.first != nullptr && - hwc2_->refresh_callback_.second != nullptr) { - hwc2_->refresh_callback_.first(hwc2_->refresh_callback_.second, - handle_); - flattening_vsync_worker_.VSyncControl(false); - } - }); + ret = flattening_vsync_worker_.Init(pipeline_, [this](int64_t /*timestamp*/) { + const std::lock_guard lock(hwc2_->GetResMan().GetMainLock()); + /* Frontend flattening */ + if (flattenning_state_ > ClientFlattenningState::ClientRefreshRequested && + --flattenning_state_ == + ClientFlattenningState::ClientRefreshRequested && + hwc2_->refresh_callback_.first != nullptr && + hwc2_->refresh_callback_.second != nullptr) { + hwc2_->refresh_callback_.first(hwc2_->refresh_callback_.second, handle_); + flattening_vsync_worker_.VSyncControl(false); + } + }); if (ret) { - ALOGE("Failed to create event worker for d=%d %d\n", display, ret); + ALOGE("Failed to create event worker for d=%d %d\n", int(handle_), ret); return HWC2::Error::BadDisplay; } ret = BackendManager::GetInstance().SetBackendForDisplay(this); if (ret) { - ALOGE("Failed to set backend for d=%d %d\n", display, ret); + ALOGE("Failed to set backend for d=%d %d\n", int(handle_), ret); return HWC2::Error::BadDisplay; } @@ -196,7 +164,7 @@ HWC2::Error HwcDisplay::Init(std::vector *planes) { } HWC2::Error HwcDisplay::ChosePreferredConfig() { - HWC2::Error err = configs_.Update(*connector_); + HWC2::Error err = configs_.Update(*GetPipe().connector->Get()); if (!IsInHeadlessMode() && err != HWC2::Error::None) return HWC2::Error::BadDisplay; @@ -258,8 +226,8 @@ HWC2::Error HwcDisplay::GetChangedCompositionTypes(uint32_t *num_elements, HWC2::Error HwcDisplay::GetClientTargetSupport(uint32_t width, uint32_t height, int32_t /*format*/, int32_t dataspace) { - std::pair min = drm_->GetMinResolution(); - std::pair max = drm_->GetMaxResolution(); + std::pair min = GetPipe().device->GetMinResolution(); + std::pair max = GetPipe().device->GetMaxResolution(); if (IsInHeadlessMode()) { return HWC2::Error::None; } @@ -363,7 +331,7 @@ HWC2::Error HwcDisplay::GetDisplayConfigs(uint32_t *num_configs, HWC2::Error HwcDisplay::GetDisplayName(uint32_t *size, char *name) { std::ostringstream stream; - stream << "display-" << connector_->GetId(); + stream << "display-" << GetPipe().connector->Get()->GetId(); std::string string = stream.str(); size_t length = string.length(); if (!name) { @@ -471,7 +439,7 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) { for (std::pair &l : z_map) { DrmHwcLayer layer; l.second->PopulateDrmLayer(&layer); - int ret = layer.ImportBuffer(drm_); + int ret = layer.ImportBuffer(GetPipe().device); if (ret) { ALOGE("Failed to import layer, ret=%d", ret); return HWC2::Error::NoResources; @@ -479,7 +447,8 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) { composition_layers.emplace_back(std::move(layer)); } - auto composition = std::make_shared(crtc_); + auto composition = std::make_shared( + GetPipe().crtc->Get()); // TODO(nobody): Don't always assume geometry changed int ret = composition->SetLayers(composition_layers.data(), @@ -489,8 +458,12 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) { return HWC2::Error::BadLayer; } - std::vector primary_planes(primary_planes_); - std::vector overlay_planes(overlay_planes_); + std::vector primary_planes; + primary_planes.emplace_back(pipeline_->primary_plane->Get()); + std::vector overlay_planes; + for (const auto &owned_plane : pipeline_->overlay_planes) { + overlay_planes.emplace_back(owned_plane->Get()); + } ret = composition->Plan(&primary_planes, &overlay_planes); if (ret) { ALOGV("Failed to plan the composition ret=%d", ret); @@ -501,7 +474,7 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) { if (staged_mode) { a_args.display_mode = *staged_mode; } - ret = compositor_.ExecuteAtomicCommit(a_args); + ret = GetPipe().compositor->ExecuteAtomicCommit(a_args); if (ret) { if (!a_args.test_only) @@ -656,7 +629,7 @@ HWC2::Error HwcDisplay::SetPowerMode(int32_t mode_in) { * true, as the next composition frame will implicitly activate * the display */ - return compositor_.ActivateDisplayUsingDPMS() == 0 + return GetPipe().compositor->ActivateDisplayUsingDPMS() == 0 ? HWC2::Error::None : HWC2::Error::BadParameter; break; @@ -668,7 +641,7 @@ HWC2::Error HwcDisplay::SetPowerMode(int32_t mode_in) { return HWC2::Error::BadParameter; }; - int err = compositor_.ExecuteAtomicCommit(a_args); + int err = GetPipe().compositor->ExecuteAtomicCommit(a_args); if (err) { ALOGE("Failed to apply the dpms composition err=%d", err); return HWC2::Error::BadParameter; @@ -715,9 +688,9 @@ HWC2::Error HwcDisplay::GetDisplayConnectionType(uint32_t *outType) { /* Primary display should be always internal, * otherwise SF will be unhappy and will crash */ - if (connector_->IsInternal() || handle_ == kPrimaryDisplay) + if (GetPipe().connector->Get()->IsInternal() || handle_ == kPrimaryDisplay) *outType = static_cast(HWC2::DisplayConnectionType::Internal); - else if (connector_->IsExternal()) + else if (GetPipe().connector->Get()->IsExternal()) *outType = static_cast(HWC2::DisplayConnectionType::External); else return HWC2::Error::BadConfig; @@ -772,7 +745,7 @@ HWC2::Error HwcDisplay::SetContentType(int32_t contentType) { HWC2::Error HwcDisplay::GetDisplayIdentificationData(uint8_t *outPort, uint32_t *outDataSize, uint8_t *outData) { - auto blob = connector_->GetEdidBlob(); + auto blob = GetPipe().connector->Get()->GetEdidBlob(); if (!blob) { ALOGE("Failed to get edid property value."); @@ -785,7 +758,7 @@ HWC2::Error HwcDisplay::GetDisplayIdentificationData(uint8_t *outPort, } else { *outDataSize = blob->length; } - *outPort = connector_->GetId(); + *outPort = GetPipe().connector->Get()->GetId(); return HWC2::Error::None; } diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h index 3e8a548..b73404a 100644 --- a/hwc2_device/HwcDisplay.h +++ b/hwc2_device/HwcDisplay.h @@ -37,10 +37,10 @@ inline constexpr uint32_t kPrimaryDisplay = 0; class HwcDisplay { public: - HwcDisplay(ResourceManager *resource_manager, DrmDevice *drm, + HwcDisplay(ResourceManager *resource_manager, DrmDisplayPipeline *pipeline, hwc2_display_t handle, HWC2::DisplayType type, DrmHwcTwo *hwc2); HwcDisplay(const HwcDisplay &) = delete; - HWC2::Error Init(std::vector *planes); + HWC2::Error Init(); HWC2::Error CreateComposition(AtomicCommitArgs &a_args); std::vector GetOrderLayersByZPos(); @@ -144,28 +144,12 @@ class HwcDisplay { const Backend *backend() const; void set_backend(std::unique_ptr backend); - const std::vector &primary_planes() const { - return primary_planes_; - } - - const std::vector &overlay_planes() const { - return overlay_planes_; - } - std::map &layers() { return layers_; } - const DrmDisplayCompositor &compositor() const { - return compositor_; - } - - const DrmDevice *drm() const { - return drm_; - } - - const DrmConnector *connector() const { - return connector_; + auto &GetPipe() { + return *pipeline_; } ResourceManager *resource_manager() const { @@ -209,7 +193,8 @@ class HwcDisplay { * https://source.android.com/devices/graphics/hotplug#handling-common-scenarios */ bool IsInHeadlessMode() { - return handle_ == kPrimaryDisplay && !connector_->IsConnected(); + return handle_ == kPrimaryDisplay && + !GetPipe().connector->Get()->IsConnected(); } private: @@ -233,17 +218,12 @@ class HwcDisplay { std::optional staged_mode; ResourceManager *resource_manager_; - DrmDevice *drm_; - DrmDisplayCompositor compositor_; - std::vector primary_planes_; - std::vector overlay_planes_; + DrmDisplayPipeline *const pipeline_; std::unique_ptr backend_; VSyncWorker vsync_worker_; - DrmConnector *connector_ = nullptr; - DrmCrtc *crtc_ = nullptr; hwc2_display_t handle_; HWC2::DisplayType type_; uint32_t layer_idx_ = 0; -- cgit v1.2.3 From 3dacd47d13e3b739cf6909a23b9a5f40152e5eea Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Tue, 11 Jan 2022 19:18:34 +0200 Subject: drm_hwcomposer: Dynamic DrmDisplayPipeline to HwcDisplay bindings The following use scenarios are now possible: 1. When no display connected, primary HwcDisplay is created in headless mode. 2. When user connects first display, it binds to primary slot, replacing headless HwcDisplay. 3. When user connects another display it binds to the new HwcDisplay slot, creating new display for the framework. 4. When user disconnects first (Primary) display, drm_hwc detaches second display and attaches it to the Primary slot. In this case framework is notified as Primary display resolution updated (Plugged->Plugged transition). And second display is disconnected (Plugged->Unplugged transition). DrmDisplayPipeline is now created on demand (after hotplug event). HwcDisplay class is now destructed on connector unplug, which will give us ability to destroy any resource caches (will be required for FB caching logic). Signed-off-by: Roman Stratiienko --- backend/Backend.cpp | 2 +- drm/DrmDevice.cpp | 56 ++----------- drm/DrmDevice.h | 15 +--- drm/ResourceManager.cpp | 121 +++++++++++++++++++++------- drm/ResourceManager.h | 43 ++++++---- hwc2_device/DrmHwcTwo.cpp | 164 ++++++++++++++++++++------------------ hwc2_device/DrmHwcTwo.h | 32 +++++--- hwc2_device/HwcDisplay.cpp | 94 +++++++++++++++++----- hwc2_device/HwcDisplay.h | 31 +++---- hwc2_device/HwcDisplayConfigs.cpp | 16 ++-- hwc2_device/HwcDisplayConfigs.h | 4 +- hwc2_device/hwc2_device.cpp | 6 -- 12 files changed, 346 insertions(+), 238 deletions(-) diff --git a/backend/Backend.cpp b/backend/Backend.cpp index 7d82eef..d707192 100644 --- a/backend/Backend.cpp +++ b/backend/Backend.cpp @@ -86,7 +86,7 @@ bool Backend::IsClientLayer(HwcDisplay *display, HwcLayer *layer) { !BufferInfoGetter::GetInstance()->IsHandleUsable(layer->GetBuffer()) || display->color_transform_hint() != HAL_COLOR_TRANSFORM_IDENTITY || (layer->RequireScalingOrPhasing() && - display->resource_manager()->ForcedScalingWithGpu()); + display->GetHwc2()->GetResMan().ForcedScalingWithGpu()); } bool Backend::HardwareSupportsLayerType(HWC2::Composition comp_type) { diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp index d1ae7c9..e5f41e8 100644 --- a/drm/DrmDevice.cpp +++ b/drm/DrmDevice.cpp @@ -22,12 +22,8 @@ #include #include -#include -#include -#include #include #include -#include #include #include "compositor/DrmDisplayCompositor.h" @@ -41,26 +37,25 @@ DrmDevice::DrmDevice() { drm_fb_importer_ = std::make_unique(*this); } -// NOLINTNEXTLINE (readability-function-cognitive-complexity): Fixme -std::tuple DrmDevice::Init(const char *path, int num_displays) { +auto DrmDevice::Init(const char *path) -> int { /* TODO: Use drmOpenControl here instead */ fd_ = UniqueFd(open(path, O_RDWR | O_CLOEXEC)); if (!fd_) { // NOLINTNEXTLINE(concurrency-mt-unsafe): Fixme ALOGE("Failed to open dri %s: %s", path, strerror(errno)); - return std::make_tuple(-ENODEV, 0); + return -ENODEV; } int ret = drmSetClientCap(GetFd(), DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1); if (ret != 0) { ALOGE("Failed to set universal plane cap %d", ret); - return std::make_tuple(ret, 0); + return ret; } ret = drmSetClientCap(GetFd(), DRM_CLIENT_CAP_ATOMIC, 1); if (ret != 0) { ALOGE("Failed to set atomic cap %d", ret); - return std::make_tuple(ret, 0); + return ret; } #ifdef DRM_CLIENT_CAP_WRITEBACK_CONNECTORS @@ -80,13 +75,13 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { drmSetMaster(GetFd()); if (drmIsMaster(GetFd()) == 0) { ALOGE("DRM/KMS master access required"); - return std::make_tuple(-EACCES, 0); + return -EACCES; } auto res = MakeDrmModeResUnique(GetFd()); if (!res) { ALOGE("Failed to get DrmDevice resources"); - return std::make_tuple(-ENODEV, 0); + return -ENODEV; } min_resolution_ = std::pair(res->min_width, @@ -128,7 +123,7 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { auto plane_res = MakeDrmModePlaneResUnique(GetFd()); if (!plane_res) { ALOGE("Failed to get plane resources"); - return std::make_tuple(-ENOENT, 0); + return -ENOENT; } for (uint32_t i = 0; i < plane_res->count_planes; ++i) { @@ -140,42 +135,7 @@ std::tuple DrmDevice::Init(const char *path, int num_displays) { } } - auto add_displays = [this, &num_displays](bool internal, bool connected) { - for (auto &conn : connectors_) { - bool is_connected = conn->IsConnected(); - if ((internal ? conn->IsInternal() : conn->IsExternal()) && - (connected ? is_connected : !is_connected)) { - auto pipe = DrmDisplayPipeline::CreatePipeline(*conn); - if (pipe) { - pipelines_[num_displays] = std::move(pipe); - ++num_displays; - } - } - } - }; - - /* Put internal first to ensure Primary display will be internal - * in case at least 1 internal is available - */ - add_displays(/*internal = */ true, /*connected = */ true); - add_displays(/*internal = */ false, /*connected = */ true); - add_displays(/*internal = */ true, /*connected = */ false); - add_displays(/*internal = */ false, /*connected = */ false); - - return std::make_tuple(0, pipelines_.size()); -} - -bool DrmDevice::HandlesDisplay(int display) const { - return pipelines_.count(display) != 0; -} - -auto DrmDevice::GetDisplayId(DrmConnector *conn) -> int { - for (auto &dpipe : pipelines_) { - if (dpipe.second->connector->Get() == conn) { - return dpipe.first; - } - } - return -1; + return 0; } auto DrmDevice::RegisterUserPropertyBlob(void *data, size_t length) const diff --git a/drm/DrmDevice.h b/drm/DrmDevice.h index 5220760..f2530ee 100644 --- a/drm/DrmDevice.h +++ b/drm/DrmDevice.h @@ -37,7 +37,7 @@ class DrmDevice { DrmDevice(); ~DrmDevice() = default; - std::tuple Init(const char *path, int num_displays); + auto Init(const char *path) -> int; auto GetFd() const { return fd_.Get(); @@ -56,19 +56,12 @@ class DrmDevice { return max_resolution_; } - auto *GetPipelineForDisplay(int display) { - return pipelines_.count(display) != 0 ? pipelines_.at(display).get() - : nullptr; - } - std::string GetName() const; auto RegisterUserPropertyBlob(void *data, size_t length) const -> DrmModeUserPropertyBlobUnique; - bool HandlesDisplay(int display) const; - - bool HasAddFb2ModifiersSupport() const { + auto HasAddFb2ModifiersSupport() const { return HasAddFb2ModifiersSupport_; } @@ -98,8 +91,6 @@ class DrmDevice { return nullptr; } - auto GetDisplayId(DrmConnector *conn) -> int; - int GetProperty(uint32_t obj_id, uint32_t obj_type, const char *prop_name, DrmProperty *property) const; @@ -115,8 +106,6 @@ class DrmDevice { std::pair min_resolution_; std::pair max_resolution_; - std::map> pipelines_; - bool HasAddFb2ModifiersSupport_{}; std::unique_ptr drm_fb_importer_; diff --git a/drm/ResourceManager.cpp b/drm/ResourceManager.cpp index a7d99ee..789eca3 100644 --- a/drm/ResourceManager.cpp +++ b/drm/ResourceManager.cpp @@ -33,25 +33,34 @@ namespace android { -ResourceManager::ResourceManager() : num_displays_(0) { +ResourceManager::ResourceManager( + PipelineToFrontendBindingInterface *p2f_bind_interface) + : frontend_interface_(p2f_bind_interface) { + if (uevent_listener_.Init() != 0) { + ALOGE("Can't initialize event listener"); + } } ResourceManager::~ResourceManager() { uevent_listener_.Exit(); } -int ResourceManager::Init() { +void ResourceManager::Init() { + if (initialized_) { + ALOGE("Already initialized"); + return; + } + char path_pattern[PROPERTY_VALUE_MAX]; // Could be a valid path or it can have at the end of it the wildcard % // which means that it will try open all devices until an error is met. int path_len = property_get("vendor.hwc.drm.device", path_pattern, "/dev/dri/card%"); - int ret = 0; if (path_pattern[path_len - 1] != '%') { - ret = AddDrmDevice(std::string(path_pattern)); + AddDrmDevice(std::string(path_pattern)); } else { path_pattern[path_len - 1] = '\0'; - for (int idx = 0; ret == 0; ++idx) { + for (int idx = 0;; ++idx) { std::ostringstream path; path << path_pattern << idx; @@ -59,51 +68,109 @@ int ResourceManager::Init() { if (stat(path.str().c_str(), &buf) != 0) break; - if (DrmDevice::IsKMSDev(path.str().c_str())) - ret = AddDrmDevice(path.str()); + if (DrmDevice::IsKMSDev(path.str().c_str())) { + AddDrmDevice(path.str()); + } } } - if (num_displays_ == 0) { - ALOGE("Failed to initialize any displays"); - return ret != 0 ? -EINVAL : ret; - } - char scale_with_gpu[PROPERTY_VALUE_MAX]; property_get("vendor.hwc.drm.scale_with_gpu", scale_with_gpu, "0"); scale_with_gpu_ = bool(strncmp(scale_with_gpu, "0", 1)); if (BufferInfoGetter::GetInstance() == nullptr) { ALOGE("Failed to initialize BufferInfoGetter"); - return -EINVAL; + return; } - ret = uevent_listener_.Init(); - if (ret != 0) { - ALOGE("Can't initialize event listener %d", ret); - return ret; + uevent_listener_.RegisterHotplugHandler([this] { + const std::lock_guard lock(GetMainLock()); + UpdateFrontendDisplays(); + }); + + UpdateFrontendDisplays(); + + initialized_ = true; +} + +void ResourceManager::DeInit() { + if (!initialized_) { + ALOGE("Not initialized"); + return; } - return 0; + uevent_listener_.RegisterHotplugHandler([] {}); + + DetachAllFrontendDisplays(); + drms_.clear(); + + initialized_ = false; } int ResourceManager::AddDrmDevice(const std::string &path) { auto drm = std::make_unique(); - int displays_added = 0; - int ret = 0; - std::tie(ret, displays_added) = drm->Init(path.c_str(), num_displays_); + int ret = drm->Init(path.c_str()); drms_.push_back(std::move(drm)); - num_displays_ += displays_added; return ret; } -DrmDisplayPipeline *ResourceManager::GetPipeline(int display) { +void ResourceManager::UpdateFrontendDisplays() { + auto ordered_connectors = GetOrderedConnectors(); + + for (auto *conn : ordered_connectors) { + conn->UpdateModes(); + bool connected = conn->IsConnected(); + bool attached = attached_pipelines_.count(conn) != 0; + + if (connected != attached) { + ALOGI("%s connector %s", connected ? "Attaching" : "Detaching", + conn->GetName().c_str()); + + if (connected) { + auto pipeline = DrmDisplayPipeline::CreatePipeline(*conn); + frontend_interface_->BindDisplay(pipeline.get()); + attached_pipelines_[conn] = std::move(pipeline); + } else { + auto &pipeline = attached_pipelines_[conn]; + frontend_interface_->UnbindDisplay(pipeline.get()); + attached_pipelines_.erase(conn); + } + } + } + frontend_interface_->FinalizeDisplayBinding(); +} + +void ResourceManager::DetachAllFrontendDisplays() { + for (auto &p : attached_pipelines_) { + frontend_interface_->UnbindDisplay(p.second.get()); + } + attached_pipelines_.clear(); + frontend_interface_->FinalizeDisplayBinding(); +} + +auto ResourceManager::GetOrderedConnectors() -> std::vector { + /* Put internal displays first then external to + * ensure Internal will take Primary slot + */ + + std::vector ordered_connectors; + + for (auto &drm : drms_) { + for (const auto &conn : drm->GetConnectors()) { + if (conn->IsInternal()) { + ordered_connectors.emplace_back(conn.get()); + } + } + } + for (auto &drm : drms_) { - auto *pipe = drm->GetPipelineForDisplay(display); - if (pipe != nullptr) { - return pipe; + for (const auto &conn : drm->GetConnectors()) { + if (conn->IsExternal()) { + ordered_connectors.emplace_back(conn.get()); + } } } - return nullptr; + + return ordered_connectors; } } // namespace android diff --git a/drm/ResourceManager.h b/drm/ResourceManager.h index caeb098..c4c3edd 100644 --- a/drm/ResourceManager.h +++ b/drm/ResourceManager.h @@ -20,42 +20,48 @@ #include #include "DrmDevice.h" +#include "DrmDisplayPipeline.h" #include "DrmFbImporter.h" #include "UEventListener.h" namespace android { +class PipelineToFrontendBindingInterface { + public: + virtual ~PipelineToFrontendBindingInterface() = default; + virtual bool BindDisplay(DrmDisplayPipeline *); + virtual bool UnbindDisplay(DrmDisplayPipeline *); + virtual void FinalizeDisplayBinding(); +}; + class ResourceManager { public: - ResourceManager(); + explicit ResourceManager( + PipelineToFrontendBindingInterface *p2f_bind_interface); ResourceManager(const ResourceManager &) = delete; ResourceManager &operator=(const ResourceManager &) = delete; + ResourceManager(const ResourceManager &&) = delete; + ResourceManager &&operator=(const ResourceManager &&) = delete; ~ResourceManager(); - int Init(); - auto GetPipeline(int display) -> DrmDisplayPipeline *; - auto &GetDrmDevices() const { - return drms_; - } - int GetDisplayCount() const { - return num_displays_; - } + void Init(); + + void DeInit(); + bool ForcedScalingWithGpu() const { return scale_with_gpu_; } - UEventListener *GetUEventListener() { - return &uevent_listener_; - } - auto &GetMainLock() { return main_lock_; } private: - int AddDrmDevice(std::string const &path); + auto AddDrmDevice(std::string const &path) -> int; + auto GetOrderedConnectors() -> std::vector; + void UpdateFrontendDisplays(); + void DetachAllFrontendDisplays(); - int num_displays_; std::vector> drms_; bool scale_with_gpu_{}; @@ -63,6 +69,13 @@ class ResourceManager { UEventListener uevent_listener_; std::mutex main_lock_; + + std::map> + attached_pipelines_; + + PipelineToFrontendBindingInterface *const frontend_interface_; + + bool initialized_{}; }; } // namespace android diff --git a/hwc2_device/DrmHwcTwo.cpp b/hwc2_device/DrmHwcTwo.cpp index 4cb0fc3..2002b85 100644 --- a/hwc2_device/DrmHwcTwo.cpp +++ b/hwc2_device/DrmHwcTwo.cpp @@ -18,51 +18,96 @@ #include "DrmHwcTwo.h" +#include + #include "backend/Backend.h" #include "utils/log.h" namespace android { -DrmHwcTwo::DrmHwcTwo() = default; +DrmHwcTwo::DrmHwcTwo() : resource_manager_(this){}; + +/* Must be called after every display attach/detach cycle */ +void DrmHwcTwo::FinalizeDisplayBinding() { + if (displays_.count(kPrimaryDisplay) == 0) { + /* Create/update new headless display if no other displays exists + * or reattach different display to make it primary + */ -HWC2::Error DrmHwcTwo::CreateDisplay(hwc2_display_t displ, - HWC2::DisplayType type) { - auto *pipe = resource_manager_.GetPipeline(static_cast(displ)); - if (!pipe) { - ALOGE("Failed to get a valid drmresource"); - return HWC2::Error::NoResources; + if (display_handles_.empty()) { + /* Enable headless mode */ + ALOGI("No pipelines available. Creating null-display for headless mode"); + displays_[kPrimaryDisplay] = std::make_unique< + HwcDisplay>(nullptr, kPrimaryDisplay, HWC2::DisplayType::Physical, + this); + } else { + auto *pipe = display_handles_.begin()->first; + ALOGI("Primary display was disconnected, reattaching '%s' as new primary", + pipe->connector->Get()->GetName().c_str()); + UnbindDisplay(pipe); + BindDisplay(pipe); + if (displays_.count(kPrimaryDisplay) == 0) { + ALOGE("FIXME!!! Still no primary display after reattaching..."); + } + } } - displays_.emplace(std::piecewise_construct, std::forward_as_tuple(displ), - std::forward_as_tuple(&resource_manager_, pipe, displ, type, - this)); - displays_.at(displ).Init(); - return HWC2::Error::None; + // Finally, send hotplug events to the client + for (auto &dhe : deferred_hotplug_events_) { + SendHotplugEventToClient(dhe.first, dhe.second); + } + deferred_hotplug_events_.clear(); } -HWC2::Error DrmHwcTwo::Init() { - int rv = resource_manager_.Init(); - if (rv) { - ALOGE("Can't initialize the resource manager %d", rv); - return HWC2::Error::NoResources; +bool DrmHwcTwo::BindDisplay(DrmDisplayPipeline *pipeline) { + if (display_handles_.count(pipeline) != 0) { + ALOGE("%s, pipeline is already used by another display, FIXME!!!: %p", + __func__, pipeline); + return false; } - HWC2::Error ret = HWC2::Error::None; - for (int i = 0; i < resource_manager_.GetDisplayCount(); i++) { - ret = CreateDisplay(i, HWC2::DisplayType::Physical); - if (ret != HWC2::Error::None) { - ALOGE("Failed to create display %d with error %d", i, ret); - return ret; - } + uint32_t disp_handle = kPrimaryDisplay; + + if (displays_.count(kPrimaryDisplay) != 0 && + !displays_[kPrimaryDisplay]->IsInHeadlessMode()) { + disp_handle = ++last_display_handle_; + } + + auto disp = std::make_unique(pipeline, disp_handle, + HWC2::DisplayType::Physical, this); + + if (disp_handle == kPrimaryDisplay) { + displays_.erase(disp_handle); } - resource_manager_.GetUEventListener()->RegisterHotplugHandler([this] { - const std::lock_guard lock(GetResMan().GetMainLock()); + ALOGI("Attaching pipeline '%s' to the display #%d%s", + pipeline->connector->Get()->GetName().c_str(), (int)disp_handle, + disp_handle == kPrimaryDisplay ? " (Primary)" : ""); + + displays_[disp_handle] = std::move(disp); + display_handles_[pipeline] = disp_handle; + + return true; +} + +bool DrmHwcTwo::UnbindDisplay(DrmDisplayPipeline *pipeline) { + if (display_handles_.count(pipeline) == 0) { + ALOGE("%s, can't find the display, pipeline: %p", __func__, pipeline); + return false; + } + auto handle = display_handles_[pipeline]; - HandleHotplugUEvent(); - }); + ALOGI("Detaching the pipeline '%s' from the display #%i%s", + pipeline->connector->Get()->GetName().c_str(), (int)handle, + handle == kPrimaryDisplay ? " (Primary)" : ""); - return ret; + display_handles_.erase(pipeline); + if (displays_.count(handle) == 0) { + ALOGE("%s, can't find the display, handle: %" PRIu64, __func__, handle); + return false; + } + displays_.erase(handle); + return true; } HWC2::Error DrmHwcTwo::CreateVirtualDisplay(uint32_t /*width*/, @@ -89,8 +134,8 @@ void DrmHwcTwo::Dump(uint32_t *outSize, char *outBuffer) { output << "-- drm_hwcomposer --\n\n"; - for (std::pair &dp : displays_) - output << dp.second.Dump(); + for (auto &disp : displays_) + output << disp.second->Dump(); mDumpString = output.str(); *outSize = static_cast(mDumpString.size()); @@ -107,9 +152,13 @@ HWC2::Error DrmHwcTwo::RegisterCallback(int32_t descriptor, switch (static_cast(descriptor)) { case HWC2::Callback::Hotplug: { hotplug_callback_ = std::make_pair(HWC2_PFN_HOTPLUG(function), data); - const auto &drm_devices = resource_manager_.GetDrmDevices(); - for (const auto &device : drm_devices) - HandleInitialHotplugState(device.get()); + if (function != nullptr) { + resource_manager_.Init(); + } else { + resource_manager_.DeInit(); + /* Headless display may still be here, remove it */ + displays_.erase(kPrimaryDisplay); + } break; } case HWC2::Callback::Refresh: { @@ -132,7 +181,8 @@ HWC2::Error DrmHwcTwo::RegisterCallback(int32_t descriptor, return HWC2::Error::None; } -void DrmHwcTwo::HandleDisplayHotplug(hwc2_display_t displayid, int state) { +void DrmHwcTwo::SendHotplugEventToClient(hwc2_display_t displayid, + bool connected) { auto &mutex = GetResMan().GetMainLock(); if (mutex.try_lock()) { ALOGE("FIXME!!!: Main mutex must be locked in %s", __func__); @@ -147,50 +197,10 @@ void DrmHwcTwo::HandleDisplayHotplug(hwc2_display_t displayid, int state) { */ mutex.unlock(); hc.first(hc.second, displayid, - state == DRM_MODE_CONNECTED ? HWC2_CONNECTION_CONNECTED - : HWC2_CONNECTION_DISCONNECTED); + connected == DRM_MODE_CONNECTED ? HWC2_CONNECTION_CONNECTED + : HWC2_CONNECTION_DISCONNECTED); mutex.lock(); } } -void DrmHwcTwo::HandleInitialHotplugState(DrmDevice *drmDevice) { - for (const auto &conn : drmDevice->GetConnectors()) { - int display_id = drmDevice->GetDisplayId(conn.get()); - auto &display = displays_.at(display_id); - - if (!conn->IsConnected() && !display.IsInHeadlessMode()) - continue; - HandleDisplayHotplug(display_id, display.IsInHeadlessMode() - ? 1 - : (conn->IsConnected() ? 1 : 0)); - } -} - -void DrmHwcTwo::HandleHotplugUEvent() { - for (const auto &drm : resource_manager_.GetDrmDevices()) { - for (const auto &conn : drm->GetConnectors()) { - int display_id = drm->GetDisplayId(conn.get()); - - bool old_state = conn->IsConnected(); - bool cur_state = conn->UpdateModes() ? false : conn->IsConnected(); - if (cur_state == old_state) - continue; - - ALOGI("%s event for connector %u on display %d", - cur_state == DRM_MODE_CONNECTED ? "Plug" : "Unplug", conn->GetId(), - display_id); - - auto &display = displays_.at(display_id); - display.ChosePreferredConfig(); - if (cur_state) { - display.ClearDisplay(); - } - - HandleDisplayHotplug(display_id, display.IsInHeadlessMode() - ? DRM_MODE_CONNECTED - : (cur_state ? 1 : 0)); - } - } -} - } // namespace android diff --git a/hwc2_device/DrmHwcTwo.h b/hwc2_device/DrmHwcTwo.h index fa13170..83fbd79 100644 --- a/hwc2_device/DrmHwcTwo.h +++ b/hwc2_device/DrmHwcTwo.h @@ -24,11 +24,10 @@ namespace android { -class DrmHwcTwo { +class DrmHwcTwo : public PipelineToFrontendBindingInterface { public: DrmHwcTwo(); - - HWC2::Error Init(); + ~DrmHwcTwo() override = default; std::pair hotplug_callback_{}; std::pair vsync_callback_{}; @@ -45,27 +44,38 @@ class DrmHwcTwo { uint32_t GetMaxVirtualDisplayCount(); HWC2::Error RegisterCallback(int32_t descriptor, hwc2_callback_data_t data, hwc2_function_pointer_t function); - HWC2::Error CreateDisplay(hwc2_display_t displ, HWC2::DisplayType type); auto GetDisplay(hwc2_display_t display_handle) { - return displays_.count(display_handle) != 0 ? &displays_.at(display_handle) - : nullptr; + return displays_.count(display_handle) != 0 + ? displays_[display_handle].get() + : nullptr; } auto &GetResMan() { return resource_manager_; } - private: - void HandleDisplayHotplug(hwc2_display_t displayid, int state); - void HandleInitialHotplugState(DrmDevice *drmDevice); + void ScheduleHotplugEvent(hwc2_display_t displayid, bool connected) { + deferred_hotplug_events_[displayid] = connected; + } - void HandleHotplugUEvent(); + // PipelineToFrontendBindingInterface + bool BindDisplay(DrmDisplayPipeline *pipeline) override; + bool UnbindDisplay(DrmDisplayPipeline *pipeline) override; + void FinalizeDisplayBinding() override; + + private: + void SendHotplugEventToClient(hwc2_display_t displayid, bool connected); ResourceManager resource_manager_; - std::map displays_; + std::map> displays_; + std::map display_handles_; std::string mDumpString; + + std::map deferred_hotplug_events_; + + uint32_t last_display_handle_ = kPrimaryDisplay; }; } // namespace android diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp index ac5d196..7ca4549 100644 --- a/hwc2_device/HwcDisplay.cpp +++ b/hwc2_device/HwcDisplay.cpp @@ -27,6 +27,9 @@ namespace android { +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +uint32_t HwcDisplay::layer_idx_ = 2; /* Start from 2. See destroyLayer() */ + std::string HwcDisplay::DumpDelta(HwcDisplay::Stats delta) { if (delta.total_pixops_ == 0) return "No stats yet"; @@ -84,11 +87,9 @@ std::string HwcDisplay::Dump() { return ss.str(); } -HwcDisplay::HwcDisplay(ResourceManager *resource_manager, - DrmDisplayPipeline *pipeline, hwc2_display_t handle, +HwcDisplay::HwcDisplay(DrmDisplayPipeline *pipeline, hwc2_display_t handle, HWC2::DisplayType type, DrmHwcTwo *hwc2) : hwc2_(hwc2), - resource_manager_(resource_manager), pipeline_(pipeline), handle_(handle), type_(type), @@ -99,6 +100,26 @@ HwcDisplay::HwcDisplay(ResourceManager *resource_manager, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0}; // clang-format on + + ChosePreferredConfig(); + Init(); + + hwc2_->ScheduleHotplugEvent(handle_, /*connected = */ true); +} + +HwcDisplay::~HwcDisplay() { + if (handle_ != kPrimaryDisplay) { + hwc2_->ScheduleHotplugEvent(handle_, /*connected = */ false); + } + + auto &main_lock = hwc2_->GetResMan().GetMainLock(); + /* Unlock to allow pending vsync callbacks to finish */ + main_lock.unlock(); + flattening_vsync_worker_.VSyncControl(false); + flattening_vsync_worker_.Exit(); + vsync_worker_.VSyncControl(false); + vsync_worker_.Exit(); + main_lock.lock(); } void HwcDisplay::ClearDisplay() { @@ -108,7 +129,7 @@ void HwcDisplay::ClearDisplay() { } AtomicCommitArgs a_args = {.clear_active_composition = true}; - GetPipe().compositor->ExecuteAtomicCommit(a_args); + pipeline_->compositor->ExecuteAtomicCommit(a_args); } HWC2::Error HwcDisplay::Init() { @@ -152,21 +173,29 @@ HWC2::Error HwcDisplay::Init() { return HWC2::Error::BadDisplay; } - ret = BackendManager::GetInstance().SetBackendForDisplay(this); - if (ret) { - ALOGE("Failed to set backend for d=%d %d\n", int(handle_), ret); - return HWC2::Error::BadDisplay; + if (!IsInHeadlessMode()) { + ret = BackendManager::GetInstance().SetBackendForDisplay(this); + if (ret) { + ALOGE("Failed to set backend for d=%d %d\n", int(handle_), ret); + return HWC2::Error::BadDisplay; + } } client_layer_.SetLayerBlendMode(HWC2_BLEND_MODE_PREMULTIPLIED); - return ChosePreferredConfig(); + return HWC2::Error::None; } HWC2::Error HwcDisplay::ChosePreferredConfig() { - HWC2::Error err = configs_.Update(*GetPipe().connector->Get()); - if (!IsInHeadlessMode() && err != HWC2::Error::None) + HWC2::Error err{}; + if (!IsInHeadlessMode()) { + err = configs_.Update(*pipeline_->connector->Get()); + } else { + configs_.FillHeadless(); + } + if (!IsInHeadlessMode() && err != HWC2::Error::None) { return HWC2::Error::BadDisplay; + } return SetActiveConfig(configs_.preferred_config_id); } @@ -185,8 +214,24 @@ HWC2::Error HwcDisplay::CreateLayer(hwc2_layer_t *layer) { } HWC2::Error HwcDisplay::DestroyLayer(hwc2_layer_t layer) { - if (!get_layer(layer)) + if (!get_layer(layer)) { + /* Primary display don't send unplug event, instead it replaces + * display to headless or to another one and sends Plug event to the + * SF. SF can't distinguish this case from virtualized display size + * change case and will destroy previously used layers. If we will return + * BadLayer, service will print errors to the logcat. + * + * Nevertheless VTS is trying to destroy 1st layer without adding any + * layers prior to that, than it checks for BadLayer result. So we + * numbering the layers starting from 2, and use index 1 to catch VTS client + * to return BadLayer, making VTS pass. + */ + if (layers_.empty() && layer != 1) { + return HWC2::Error::None; + } + return HWC2::Error::BadLayer; + } layers_.erase(layer); return HWC2::Error::None; @@ -226,12 +271,13 @@ HWC2::Error HwcDisplay::GetChangedCompositionTypes(uint32_t *num_elements, HWC2::Error HwcDisplay::GetClientTargetSupport(uint32_t width, uint32_t height, int32_t /*format*/, int32_t dataspace) { - std::pair min = GetPipe().device->GetMinResolution(); - std::pair max = GetPipe().device->GetMaxResolution(); if (IsInHeadlessMode()) { return HWC2::Error::None; } + std::pair min = pipeline_->device->GetMinResolution(); + std::pair max = pipeline_->device->GetMaxResolution(); + if (width < min.first || height < min.second) return HWC2::Error::Unsupported; @@ -261,7 +307,7 @@ HWC2::Error HwcDisplay::GetDisplayAttribute(hwc2_config_t config, int conf = static_cast(config); if (configs_.hwc_configs.count(conf) == 0) { - ALOGE("Could not find active mode for %d", conf); + ALOGE("Could not find mode #%d", conf); return HWC2::Error::BadConfig; } @@ -331,7 +377,11 @@ HWC2::Error HwcDisplay::GetDisplayConfigs(uint32_t *num_configs, HWC2::Error HwcDisplay::GetDisplayName(uint32_t *size, char *name) { std::ostringstream stream; - stream << "display-" << GetPipe().connector->Get()->GetId(); + if (IsInHeadlessMode()) { + stream << "null-display"; + } else { + stream << "display-" << GetPipe().connector->Get()->GetId(); + } std::string string = stream.str(); size_t length = string.length(); if (!name) { @@ -745,11 +795,18 @@ HWC2::Error HwcDisplay::SetContentType(int32_t contentType) { HWC2::Error HwcDisplay::GetDisplayIdentificationData(uint8_t *outPort, uint32_t *outDataSize, uint8_t *outData) { + if (IsInHeadlessMode()) { + return HWC2::Error::None; + } auto blob = GetPipe().connector->Get()->GetEdidBlob(); + *outPort = handle_ - 1; + if (!blob) { - ALOGE("Failed to get edid property value."); - return HWC2::Error::Unsupported; + if (outData == nullptr) { + *outDataSize = 0; + } + return HWC2::Error::None; } if (outData) { @@ -758,7 +815,6 @@ HWC2::Error HwcDisplay::GetDisplayIdentificationData(uint8_t *outPort, } else { *outDataSize = blob->length; } - *outPort = GetPipe().connector->Get()->GetId(); return HWC2::Error::None; } diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h index b73404a..42b000a 100644 --- a/hwc2_device/HwcDisplay.h +++ b/hwc2_device/HwcDisplay.h @@ -37,10 +37,10 @@ inline constexpr uint32_t kPrimaryDisplay = 0; class HwcDisplay { public: - HwcDisplay(ResourceManager *resource_manager, DrmDisplayPipeline *pipeline, - hwc2_display_t handle, HWC2::DisplayType type, DrmHwcTwo *hwc2); + HwcDisplay(DrmDisplayPipeline *pipeline, hwc2_display_t handle, + HWC2::DisplayType type, DrmHwcTwo *hwc2); HwcDisplay(const HwcDisplay &) = delete; - HWC2::Error Init(); + ~HwcDisplay(); HWC2::Error CreateComposition(AtomicCommitArgs &a_args); std::vector GetOrderLayersByZPos(); @@ -144,6 +144,10 @@ class HwcDisplay { const Backend *backend() const; void set_backend(std::unique_ptr backend); + auto GetHwc2() { + return hwc2_; + } + std::map &layers() { return layers_; } @@ -152,10 +156,6 @@ class HwcDisplay { return *pipeline_; } - ResourceManager *resource_manager() const { - return resource_manager_; - } - android_color_transform_t &color_transform_hint() { return color_transform_hint_; } @@ -193,8 +193,7 @@ class HwcDisplay { * https://source.android.com/devices/graphics/hotplug#handling-common-scenarios */ bool IsInHeadlessMode() { - return handle_ == kPrimaryDisplay && - !GetPipe().connector->Get()->IsConnected(); + return !pipeline_; } private: @@ -213,20 +212,22 @@ class HwcDisplay { HwcDisplayConfigs configs_; - DrmHwcTwo *hwc2_; + DrmHwcTwo *const hwc2_; std::optional staged_mode; - ResourceManager *resource_manager_; - DrmDisplayPipeline *const pipeline_; std::unique_ptr backend_; VSyncWorker vsync_worker_; - hwc2_display_t handle_; + + const hwc2_display_t handle_; HWC2::DisplayType type_; - uint32_t layer_idx_ = 0; + + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) + static uint32_t layer_idx_; + std::map layers_; HwcLayer client_layer_; int32_t color_mode_{}; @@ -237,6 +238,8 @@ class HwcDisplay { Stats total_stats_; Stats prev_stats_; std::string DumpDelta(HwcDisplay::Stats delta); + + HWC2::Error Init(); }; } // namespace android diff --git a/hwc2_device/HwcDisplayConfigs.cpp b/hwc2_device/HwcDisplayConfigs.cpp index f6ccdb7..c28ec9f 100644 --- a/hwc2_device/HwcDisplayConfigs.cpp +++ b/hwc2_device/HwcDisplayConfigs.cpp @@ -31,10 +31,10 @@ constexpr uint32_t kHeadlessModeDisplayVRefresh = 60; namespace android { -// NOLINTNEXTLINE (readability-function-cognitive-complexity): Fixme -HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) { - /* In case UpdateModes will fail we will still have one mode for headless - * mode*/ +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +int HwcDisplayConfigs::last_config_id = 1; + +void HwcDisplayConfigs::FillHeadless() { hwc_configs.clear(); last_config_id++; @@ -53,7 +53,13 @@ HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) { mm_width = kHeadlessModeDisplayWidthMm; mm_height = kHeadlessModeDisplayHeightMm; +} +// NOLINTNEXTLINE (readability-function-cognitive-complexity): Fixme +HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) { + /* In case UpdateModes will fail we will still have one mode for headless + * mode*/ + FillHeadless(); /* Read real configs */ int ret = connector.UpdateModes(); if (ret != 0) { @@ -190,8 +196,6 @@ HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) { } } - /* Set active mode to be valid mode */ - active_config_id = preferred_config_id; return HWC2::Error::None; } diff --git a/hwc2_device/HwcDisplayConfigs.h b/hwc2_device/HwcDisplayConfigs.h index 5bcf696..75852a6 100644 --- a/hwc2_device/HwcDisplayConfigs.h +++ b/hwc2_device/HwcDisplayConfigs.h @@ -40,13 +40,15 @@ struct HwcDisplayConfig { struct HwcDisplayConfigs { HWC2::Error Update(DrmConnector &conn); + void FillHeadless(); std::map hwc_configs; int active_config_id = 0; int preferred_config_id = 0; - int last_config_id = 1; + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) + static int last_config_id; uint32_t mm_width = 0; uint32_t mm_height = 0; diff --git a/hwc2_device/hwc2_device.cpp b/hwc2_device/hwc2_device.cpp index 57bc205..a6dedb4 100644 --- a/hwc2_device/hwc2_device.cpp +++ b/hwc2_device/hwc2_device.cpp @@ -390,12 +390,6 @@ static int HookDevOpen(const struct hw_module_t *module, const char *name, ctx->getCapabilities = HookDevGetCapabilities; ctx->getFunction = HookDevGetFunction; - HWC2::Error err = ctx->drmhwctwo.Init(); - if (err != HWC2::Error::None) { - ALOGE("Failed to initialize DrmHwcTwo err=%d\n", err); - return -EINVAL; - } - *dev = &ctx.release()->common; return 0; -- cgit v1.2.3 From 099c31156d4d916c1e18ec00ab163de77bd17a94 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Thu, 20 Jan 2022 11:50:54 +0200 Subject: drm_hwcomposer: Use single VSyncWorker per display Composer 2.4 will require another vsyncworker callback to send VsyncPeriodTimingChanged event. It makes sence to use single VSyncWorker and flags to indicate which actions are required to perform. This should also save some runtime resources. Signed-off-by: Roman Stratiienko --- hwc2_device/DrmHwcTwo.cpp | 17 +++++++++ hwc2_device/DrmHwcTwo.h | 3 ++ hwc2_device/HwcDisplay.cpp | 91 ++++++++++++++++++++++++++-------------------- hwc2_device/HwcDisplay.h | 27 +++----------- 4 files changed, 77 insertions(+), 61 deletions(-) diff --git a/hwc2_device/DrmHwcTwo.cpp b/hwc2_device/DrmHwcTwo.cpp index 2002b85..37901ee 100644 --- a/hwc2_device/DrmHwcTwo.cpp +++ b/hwc2_device/DrmHwcTwo.cpp @@ -203,4 +203,21 @@ void DrmHwcTwo::SendHotplugEventToClient(hwc2_display_t displayid, } } +void DrmHwcTwo::SendVsyncEventToClient( + hwc2_display_t displayid, int64_t timestamp, + [[maybe_unused]] uint32_t vsync_period) const { + /* vsync callback */ +#if PLATFORM_SDK_VERSION > 29 + if (vsync_2_4_callback_.first != nullptr && + vsync_2_4_callback_.second != nullptr) { + vsync_2_4_callback_.first(vsync_2_4_callback_.second, displayid, timestamp, + vsync_period); + } else +#endif + if (vsync_callback_.first != nullptr && + vsync_callback_.second != nullptr) { + vsync_callback_.first(vsync_callback_.second, displayid, timestamp); + } +} + } // namespace android diff --git a/hwc2_device/DrmHwcTwo.h b/hwc2_device/DrmHwcTwo.h index 83fbd79..5379cba 100644 --- a/hwc2_device/DrmHwcTwo.h +++ b/hwc2_device/DrmHwcTwo.h @@ -64,6 +64,9 @@ class DrmHwcTwo : public PipelineToFrontendBindingInterface { bool UnbindDisplay(DrmDisplayPipeline *pipeline) override; void FinalizeDisplayBinding() override; + void SendVsyncEventToClient(hwc2_display_t displayid, int64_t timestamp, + uint32_t vsync_period) const; + private: void SendHotplugEventToClient(hwc2_display_t displayid, bool connected); diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp index 7ca4549..656aa94 100644 --- a/hwc2_device/HwcDisplay.cpp +++ b/hwc2_device/HwcDisplay.cpp @@ -115,8 +115,6 @@ HwcDisplay::~HwcDisplay() { auto &main_lock = hwc2_->GetResMan().GetMainLock(); /* Unlock to allow pending vsync callbacks to finish */ main_lock.unlock(); - flattening_vsync_worker_.VSyncControl(false); - flattening_vsync_worker_.Exit(); vsync_worker_.VSyncControl(false); vsync_worker_.Exit(); main_lock.lock(); @@ -135,37 +133,16 @@ void HwcDisplay::ClearDisplay() { HWC2::Error HwcDisplay::Init() { int ret = vsync_worker_.Init(pipeline_, [this](int64_t timestamp) { const std::lock_guard lock(hwc2_->GetResMan().GetMainLock()); - /* vsync callback */ -#if PLATFORM_SDK_VERSION > 29 - if (hwc2_->vsync_2_4_callback_.first != nullptr && - hwc2_->vsync_2_4_callback_.second != nullptr) { - hwc2_vsync_period_t period_ns{}; + if (vsync_event_en_) { + uint32_t period_ns{}; GetDisplayVsyncPeriod(&period_ns); - hwc2_->vsync_2_4_callback_.first(hwc2_->vsync_2_4_callback_.second, - handle_, timestamp, period_ns); - } else -#endif - if (hwc2_->vsync_callback_.first != nullptr && - hwc2_->vsync_callback_.second != nullptr) { - hwc2_->vsync_callback_.first(hwc2_->vsync_callback_.second, handle_, - timestamp); + hwc2_->SendVsyncEventToClient(handle_, timestamp, period_ns); } - }); - if (ret) { - ALOGE("Failed to create event worker for d=%d %d\n", int(handle_), ret); - return HWC2::Error::BadDisplay; - } - - ret = flattening_vsync_worker_.Init(pipeline_, [this](int64_t /*timestamp*/) { - const std::lock_guard lock(hwc2_->GetResMan().GetMainLock()); - /* Frontend flattening */ - if (flattenning_state_ > ClientFlattenningState::ClientRefreshRequested && - --flattenning_state_ == - ClientFlattenningState::ClientRefreshRequested && - hwc2_->refresh_callback_.first != nullptr && - hwc2_->refresh_callback_.second != nullptr) { - hwc2_->refresh_callback_.first(hwc2_->refresh_callback_.second, handle_); - flattening_vsync_worker_.VSyncControl(false); + if (vsync_flattening_en_) { + ProcessFlatenningVsyncInternal(); + } + if (!vsync_event_en_ && !vsync_flattening_en_) { + vsync_worker_.VSyncControl(false); } }); if (ret) { @@ -700,7 +677,10 @@ HWC2::Error HwcDisplay::SetPowerMode(int32_t mode_in) { } HWC2::Error HwcDisplay::SetVsyncEnabled(int32_t enabled) { - vsync_worker_.VSyncControl(HWC2_VSYNC_ENABLE == enabled); + vsync_event_en_ = HWC2_VSYNC_ENABLE == enabled; + if (vsync_event_en_) { + vsync_worker_.VSyncControl(true); + } return HWC2::Error::None; } @@ -729,6 +709,13 @@ std::vector HwcDisplay::GetOrderLayersByZPos() { return ordered_layers; } +HWC2::Error HwcDisplay::GetDisplayVsyncPeriod( + uint32_t *outVsyncPeriod /* ns */) { + return GetDisplayAttribute(configs_.active_config_id, + HWC2_ATTRIBUTE_VSYNC_PERIOD, + (int32_t *)(outVsyncPeriod)); +} + #if PLATFORM_SDK_VERSION > 29 HWC2::Error HwcDisplay::GetDisplayConnectionType(uint32_t *outType) { if (IsInHeadlessMode()) { @@ -748,13 +735,6 @@ HWC2::Error HwcDisplay::GetDisplayConnectionType(uint32_t *outType) { return HWC2::Error::None; } -HWC2::Error HwcDisplay::GetDisplayVsyncPeriod( - hwc2_vsync_period_t *outVsyncPeriod /* ns */) { - return GetDisplayAttribute(configs_.active_config_id, - HWC2_ATTRIBUTE_VSYNC_PERIOD, - (int32_t *)(outVsyncPeriod)); -} - HWC2::Error HwcDisplay::SetActiveConfigWithConstraints( hwc2_config_t /*config*/, hwc_vsync_period_change_constraints_t *vsyncPeriodChangeConstraints, @@ -887,4 +867,37 @@ void HwcDisplay::set_backend(std::unique_ptr backend) { backend_ = std::move(backend); } +/* returns true if composition should be sent to client */ +bool HwcDisplay::ProcessClientFlatteningState(bool skip) { + int flattenning_state = flattenning_state_; + if (flattenning_state == ClientFlattenningState::Disabled) { + return false; + } + + if (skip) { + flattenning_state_ = ClientFlattenningState::NotRequired; + return false; + } + + if (flattenning_state == ClientFlattenningState::ClientRefreshRequested) { + flattenning_state_ = ClientFlattenningState::Flattened; + return true; + } + + vsync_flattening_en_ = true; + vsync_worker_.VSyncControl(true); + flattenning_state_ = ClientFlattenningState::VsyncCountdownMax; + return false; +} + +void HwcDisplay::ProcessFlatenningVsyncInternal() { + if (flattenning_state_ > ClientFlattenningState::ClientRefreshRequested && + --flattenning_state_ == ClientFlattenningState::ClientRefreshRequested && + hwc2_->refresh_callback_.first != nullptr && + hwc2_->refresh_callback_.second != nullptr) { + hwc2_->refresh_callback_.first(hwc2_->refresh_callback_.second, handle_); + vsync_flattening_en_ = false; + } +} + } // namespace android diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h index 42b000a..5a5c98e 100644 --- a/hwc2_device/HwcDisplay.h +++ b/hwc2_device/HwcDisplay.h @@ -83,7 +83,6 @@ class HwcDisplay { #endif #if PLATFORM_SDK_VERSION > 29 HWC2::Error GetDisplayConnectionType(uint32_t *outType); - HWC2::Error GetDisplayVsyncPeriod(hwc2_vsync_period_t *outVsyncPeriod); HWC2::Error SetActiveConfigWithConstraints( hwc2_config_t config, @@ -96,6 +95,7 @@ class HwcDisplay { HWC2::Error SetContentType(int32_t contentType); #endif + HWC2::Error GetDisplayVsyncPeriod(uint32_t *outVsyncPeriod); HWC2::Error GetDozeSupport(int32_t *support); HWC2::Error GetHdrCapabilities(uint32_t *num_types, int32_t *types, @@ -165,26 +165,8 @@ class HwcDisplay { } /* returns true if composition should be sent to client */ - bool ProcessClientFlatteningState(bool skip) { - int flattenning_state = flattenning_state_; - if (flattenning_state == ClientFlattenningState::Disabled) { - return false; - } - - if (skip) { - flattenning_state_ = ClientFlattenningState::NotRequired; - return false; - } - - if (flattenning_state == ClientFlattenningState::ClientRefreshRequested) { - flattenning_state_ = ClientFlattenningState::Flattened; - return true; - } - - flattening_vsync_worker_.VSyncControl(true); - flattenning_state_ = ClientFlattenningState::VsyncCountdownMax; - return false; - } + bool ProcessClientFlatteningState(bool skip); + void ProcessFlatenningVsyncInternal(); /* Headless mode required to keep SurfaceFlinger alive when all display are * disconnected, Without headless mode Android will continuously crash. @@ -206,7 +188,6 @@ class HwcDisplay { }; std::atomic_int flattenning_state_{ClientFlattenningState::NotRequired}; - VSyncWorker flattening_vsync_worker_; constexpr static size_t MATRIX_SIZE = 16; @@ -221,6 +202,8 @@ class HwcDisplay { std::unique_ptr backend_; VSyncWorker vsync_worker_; + bool vsync_event_en_{}; + bool vsync_flattening_en_{}; const hwc2_display_t handle_; HWC2::DisplayType type_; -- cgit v1.2.3 From d0c035b44a844af5017c0c3b2507af2f3907c36c Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Fri, 21 Jan 2022 15:12:56 +0200 Subject: drm_hwcomposer: Implement SetActiveConfigWithConstraints Enough to get 100% passes in Composer 2.4 VTS. Some SOCs require a VTS fix to pass [1] [1]: https://android-review.googlesource.com/c/platform/hardware/interfaces/+/1954544 Signed-off-by: Roman Stratiienko --- drm/ResourceManager.cpp | 8 ++++ drm/ResourceManager.h | 2 + hwc2_device/DrmHwcTwo.cpp | 22 +++++++++ hwc2_device/DrmHwcTwo.h | 4 ++ hwc2_device/HwcDisplay.cpp | 98 ++++++++++++++++++++++++++++----------- hwc2_device/HwcDisplay.h | 8 +++- hwc2_device/HwcDisplayConfigs.cpp | 16 +++---- hwc2_device/HwcDisplayConfigs.h | 12 ++--- 8 files changed, 127 insertions(+), 43 deletions(-) diff --git a/drm/ResourceManager.cpp b/drm/ResourceManager.cpp index 789eca3..b294180 100644 --- a/drm/ResourceManager.cpp +++ b/drm/ResourceManager.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #include "bufferinfo/BufferInfoGetter.h" @@ -114,6 +115,13 @@ int ResourceManager::AddDrmDevice(const std::string &path) { return ret; } +auto ResourceManager::GetTimeMonotonicNs() -> int64_t { + struct timespec ts {}; + clock_gettime(CLOCK_MONOTONIC, &ts); + constexpr int64_t kNsInSec = 1000000000LL; + return int64_t(ts.tv_sec) * kNsInSec + int64_t(ts.tv_nsec); +} + void ResourceManager::UpdateFrontendDisplays() { auto ordered_connectors = GetOrderedConnectors(); diff --git a/drm/ResourceManager.h b/drm/ResourceManager.h index c4c3edd..88ba878 100644 --- a/drm/ResourceManager.h +++ b/drm/ResourceManager.h @@ -56,6 +56,8 @@ class ResourceManager { return main_lock_; } + static auto GetTimeMonotonicNs() -> int64_t; + private: auto AddDrmDevice(std::string const &path) -> int; auto GetOrderedConnectors() -> std::vector; diff --git a/hwc2_device/DrmHwcTwo.cpp b/hwc2_device/DrmHwcTwo.cpp index 37901ee..f1a5490 100644 --- a/hwc2_device/DrmHwcTwo.cpp +++ b/hwc2_device/DrmHwcTwo.cpp @@ -174,6 +174,11 @@ HWC2::Error DrmHwcTwo::RegisterCallback(int32_t descriptor, vsync_2_4_callback_ = std::make_pair(HWC2_PFN_VSYNC_2_4(function), data); break; } + case HWC2::Callback::VsyncPeriodTimingChanged: { + period_timing_changed_callback_ = std:: + make_pair(HWC2_PFN_VSYNC_PERIOD_TIMING_CHANGED(function), data); + break; + } #endif default: break; @@ -220,4 +225,21 @@ void DrmHwcTwo::SendVsyncEventToClient( } } +void DrmHwcTwo::SendVsyncPeriodTimingChangedEventToClient( + [[maybe_unused]] hwc2_display_t displayid, + [[maybe_unused]] int64_t timestamp) const { +#if PLATFORM_SDK_VERSION > 29 + hwc_vsync_period_change_timeline_t timeline = { + .newVsyncAppliedTimeNanos = timestamp, + .refreshRequired = false, + .refreshTimeNanos = 0, + }; + if (period_timing_changed_callback_.first != nullptr && + period_timing_changed_callback_.second != nullptr) { + period_timing_changed_callback_ + .first(period_timing_changed_callback_.second, displayid, &timeline); + } +#endif +} + } // namespace android diff --git a/hwc2_device/DrmHwcTwo.h b/hwc2_device/DrmHwcTwo.h index 5379cba..0e72251 100644 --- a/hwc2_device/DrmHwcTwo.h +++ b/hwc2_device/DrmHwcTwo.h @@ -33,6 +33,8 @@ class DrmHwcTwo : public PipelineToFrontendBindingInterface { std::pair vsync_callback_{}; #if PLATFORM_SDK_VERSION > 29 std::pair vsync_2_4_callback_{}; + std::pair + period_timing_changed_callback_{}; #endif std::pair refresh_callback_{}; @@ -66,6 +68,8 @@ class DrmHwcTwo : public PipelineToFrontendBindingInterface { void SendVsyncEventToClient(hwc2_display_t displayid, int64_t timestamp, uint32_t vsync_period) const; + void SendVsyncPeriodTimingChangedEventToClient(hwc2_display_t displayid, + int64_t timestamp) const; private: void SendHotplugEventToClient(hwc2_display_t displayid, bool connected); diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp index 656aa94..d7b753a 100644 --- a/hwc2_device/HwcDisplay.cpp +++ b/hwc2_device/HwcDisplay.cpp @@ -141,7 +141,10 @@ HWC2::Error HwcDisplay::Init() { if (vsync_flattening_en_) { ProcessFlatenningVsyncInternal(); } - if (!vsync_event_en_ && !vsync_flattening_en_) { + if (vsync_tracking_en_) { + last_vsync_ts_ = timestamp; + } + if (!vsync_event_en_ && !vsync_flattening_en_ && !vsync_tracking_en_) { vsync_worker_.VSyncControl(false); } }); @@ -215,10 +218,10 @@ HWC2::Error HwcDisplay::DestroyLayer(hwc2_layer_t layer) { } HWC2::Error HwcDisplay::GetActiveConfig(hwc2_config_t *config) const { - if (configs_.hwc_configs.count(configs_.active_config_id) == 0) + if (configs_.hwc_configs.count(staged_mode_config_id_) == 0) return HWC2::Error::BadConfig; - *config = configs_.active_config_id; + *config = staged_mode_config_id_; return HWC2::Error::None; } @@ -321,7 +324,7 @@ HWC2::Error HwcDisplay::GetDisplayAttribute(hwc2_config_t config, case HWC2::Attribute::ConfigGroup: /* Dispite ConfigGroup is a part of HWC2.4 API, framework * able to request it even if service @2.1 is used */ - *value = hwc_config.group_id; + *value = int(hwc_config.group_id); break; #endif default: @@ -436,6 +439,26 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) { return HWC2::Error::None; } + int PrevModeVsyncPeriodNs = static_cast( + 1E9 / GetPipe().connector->Get()->GetActiveMode().v_refresh()); + + auto mode_update_commited_ = false; + if (staged_mode_ && + staged_mode_change_time_ <= ResourceManager::GetTimeMonotonicNs()) { + client_layer_.SetLayerDisplayFrame( + (hwc_rect_t){.left = 0, + .top = 0, + .right = static_cast(staged_mode_->h_display()), + .bottom = static_cast(staged_mode_->v_display())}); + + configs_.active_config_id = staged_mode_config_id_; + + a_args.display_mode = *staged_mode_; + if (!a_args.test_only) { + mode_update_commited_ = true; + } + } + // order the layers by z-order bool use_client_layer = false; uint32_t client_z_order = UINT32_MAX; @@ -498,9 +521,6 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) { } a_args.composition = composition; - if (staged_mode) { - a_args.display_mode = *staged_mode; - } ret = GetPipe().compositor->ExecuteAtomicCommit(a_args); if (ret) { @@ -509,8 +529,13 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) { return HWC2::Error::BadParameter; } - if (!a_args.test_only) { - staged_mode.reset(); + if (mode_update_commited_) { + staged_mode_.reset(); + vsync_tracking_en_ = false; + if (last_vsync_ts_ != 0) { + hwc2_->SendVsyncPeriodTimingChangedEventToClient( + handle_, last_vsync_ts_ + PrevModeVsyncPeriodNs); + } } return HWC2::Error::None; @@ -548,30 +573,24 @@ HWC2::Error HwcDisplay::PresentDisplay(int32_t *present_fence) { return HWC2::Error::None; } -HWC2::Error HwcDisplay::SetActiveConfig(hwc2_config_t config) { - int conf = static_cast(config); - - if (configs_.hwc_configs.count(conf) == 0) { - ALOGE("Could not find active mode for %d", conf); +HWC2::Error HwcDisplay::SetActiveConfigInternal(uint32_t config, + int64_t change_time) { + if (configs_.hwc_configs.count(config) == 0) { + ALOGE("Could not find active mode for %u", config); return HWC2::Error::BadConfig; } - auto &mode = configs_.hwc_configs[conf].mode; - - staged_mode = mode; - - configs_.active_config_id = conf; - - // Setup the client layer's dimensions - hwc_rect_t display_frame = {.left = 0, - .top = 0, - .right = static_cast(mode.h_display()), - .bottom = static_cast(mode.v_display())}; - client_layer_.SetLayerDisplayFrame(display_frame); + staged_mode_ = configs_.hwc_configs[config].mode; + staged_mode_change_time_ = change_time; + staged_mode_config_id_ = config; return HWC2::Error::None; } +HWC2::Error HwcDisplay::SetActiveConfig(hwc2_config_t config) { + return SetActiveConfigInternal(config, ResourceManager::GetTimeMonotonicNs()); +} + /* Find API details at: * https://cs.android.com/android/platform/superproject/+/android-11.0.0_r3:hardware/libhardware/include/hardware/hwcomposer2.h;l=1861 */ @@ -736,14 +755,37 @@ HWC2::Error HwcDisplay::GetDisplayConnectionType(uint32_t *outType) { } HWC2::Error HwcDisplay::SetActiveConfigWithConstraints( - hwc2_config_t /*config*/, + hwc2_config_t config, hwc_vsync_period_change_constraints_t *vsyncPeriodChangeConstraints, hwc_vsync_period_change_timeline_t *outTimeline) { if (vsyncPeriodChangeConstraints == nullptr || outTimeline == nullptr) { return HWC2::Error::BadParameter; } - return HWC2::Error::BadConfig; + uint32_t current_vsync_period{}; + GetDisplayVsyncPeriod(¤t_vsync_period); + + if (vsyncPeriodChangeConstraints->seamlessRequired) { + return HWC2::Error::SeamlessNotAllowed; + } + + outTimeline->refreshTimeNanos = vsyncPeriodChangeConstraints + ->desiredTimeNanos - + current_vsync_period; + auto ret = SetActiveConfigInternal(config, outTimeline->refreshTimeNanos); + if (ret != HWC2::Error::None) { + return ret; + } + + outTimeline->refreshRequired = true; + outTimeline->newVsyncAppliedTimeNanos = vsyncPeriodChangeConstraints + ->desiredTimeNanos; + + last_vsync_ts_ = 0; + vsync_tracking_en_ = true; + vsync_worker_.VSyncControl(true); + + return HWC2::Error::None; } HWC2::Error HwcDisplay::SetAutoLowLatencyMode(bool /*on*/) { diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h index 5a5c98e..e7ce7ef 100644 --- a/hwc2_device/HwcDisplay.h +++ b/hwc2_device/HwcDisplay.h @@ -195,7 +195,9 @@ class HwcDisplay { DrmHwcTwo *const hwc2_; - std::optional staged_mode; + std::optional staged_mode_; + int64_t staged_mode_change_time_{}; + uint32_t staged_mode_config_id_{}; DrmDisplayPipeline *const pipeline_; @@ -204,6 +206,8 @@ class HwcDisplay { VSyncWorker vsync_worker_; bool vsync_event_en_{}; bool vsync_flattening_en_{}; + bool vsync_tracking_en_{}; + int64_t last_vsync_ts_{}; const hwc2_display_t handle_; HWC2::DisplayType type_; @@ -223,6 +227,8 @@ class HwcDisplay { std::string DumpDelta(HwcDisplay::Stats delta); HWC2::Error Init(); + + HWC2::Error SetActiveConfigInternal(uint32_t config, int64_t change_time); }; } // namespace android diff --git a/hwc2_device/HwcDisplayConfigs.cpp b/hwc2_device/HwcDisplayConfigs.cpp index c28ec9f..6a3ed5a 100644 --- a/hwc2_device/HwcDisplayConfigs.cpp +++ b/hwc2_device/HwcDisplayConfigs.cpp @@ -32,7 +32,7 @@ constexpr uint32_t kHeadlessModeDisplayVRefresh = 60; namespace android { // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) -int HwcDisplayConfigs::last_config_id = 1; +uint32_t HwcDisplayConfigs::last_config_id = 1; void HwcDisplayConfigs::FillHeadless() { hwc_configs.clear(); @@ -77,15 +77,15 @@ HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) { mm_height = connector.GetMmHeight(); preferred_config_id = 0; - int preferred_config_group_id = 0; + uint32_t preferred_config_group_id = 0; - int first_config_id = last_config_id; - int last_group_id = 1; + uint32_t first_config_id = last_config_id; + uint32_t last_group_id = 1; /* Group modes */ for (const auto &mode : connector.GetModes()) { /* Find group for the new mode or create new group */ - int group_found = 0; + 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()) { @@ -128,7 +128,7 @@ HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) { preferred_config_group_id = 1; } - for (int group = 1; group < last_group_id; group++) { + for (uint32_t group = 1; group < last_group_id; group++) { bool has_interlaced = false; bool has_progressive = false; for (auto &hwc_config : hwc_configs) { @@ -179,8 +179,8 @@ HWC2::Error HwcDisplayConfigs::Update(DrmConnector &connector) { * otherwise android.graphics.cts.SetFrameRateTest CTS will fail */ constexpr float kMinFpsDelta = 1.0; // FPS - for (int m1 = first_config_id; m1 < last_config_id; m1++) { - for (int m2 = first_config_id; m2 < last_config_id; m2++) { + for (uint32_t m1 = first_config_id; m1 < last_config_id; m1++) { + 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() - diff --git a/hwc2_device/HwcDisplayConfigs.h b/hwc2_device/HwcDisplayConfigs.h index 75852a6..7c173d6 100644 --- a/hwc2_device/HwcDisplayConfigs.h +++ b/hwc2_device/HwcDisplayConfigs.h @@ -28,8 +28,8 @@ namespace android { class DrmConnector; struct HwcDisplayConfig { - int id{}; - int group_id{}; + uint32_t id{}; + uint32_t group_id{}; DrmMode mode; bool disabled{}; @@ -42,13 +42,13 @@ struct HwcDisplayConfigs { HWC2::Error Update(DrmConnector &conn); void FillHeadless(); - std::map hwc_configs; + std::map hwc_configs; - int active_config_id = 0; - int preferred_config_id = 0; + uint32_t active_config_id = 0; + uint32_t preferred_config_id = 0; // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) - static int last_config_id; + static uint32_t last_config_id; uint32_t mm_width = 0; uint32_t mm_height = 0; -- cgit v1.2.3 From 9362cef4c34b9d23018d75be0cbb6ef0486bf75b Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Wed, 2 Feb 2022 09:53:50 +0200 Subject: drm_hwcomposer: Rework KMS composition planner + plane sharing support Rewrite Layer-to-Plane planner. Get rid of ~200 redundant lines of code + added plane sharing functionality. Closes: https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/-/issues/11 Signed-off-by: Roman Stratiienko --- .ci/Makefile | 2 - Android.bp | 3 +- backend/Backend.cpp | 4 +- compositor/DrmDisplayComposition.cpp | 92 ---------------------------------- compositor/DrmDisplayComposition.h | 90 ---------------------------------- compositor/DrmDisplayCompositor.cpp | 27 +++------- compositor/DrmDisplayCompositor.h | 9 ++-- compositor/DrmKmsPlan.cpp | 59 ++++++++++++++++++++++ compositor/DrmKmsPlan.h | 44 +++++++++++++++++ compositor/Planner.cpp | 81 ------------------------------ compositor/Planner.h | 95 ------------------------------------ drm/DrmDisplayPipeline.cpp | 47 ++++++++++++------ drm/DrmDisplayPipeline.h | 4 +- hwc2_device/HwcDisplay.cpp | 34 +++++-------- hwc2_device/HwcDisplay.h | 2 + 15 files changed, 166 insertions(+), 427 deletions(-) delete mode 100644 compositor/DrmDisplayComposition.cpp delete mode 100644 compositor/DrmDisplayComposition.h create mode 100644 compositor/DrmKmsPlan.cpp create mode 100644 compositor/DrmKmsPlan.h delete mode 100644 compositor/Planner.cpp delete mode 100644 compositor/Planner.h diff --git a/.ci/Makefile b/.ci/Makefile index bca785c..a9b96d3 100644 --- a/.ci/Makefile +++ b/.ci/Makefile @@ -21,8 +21,6 @@ TIDY_FILES_OVERRIDE := \ bufferinfo/legacy/BufferInfoMaliMediatek.cpp:COARSE \ bufferinfo/legacy/BufferInfoMaliMeson.cpp:COARSE \ bufferinfo/legacy/BufferInfoMinigbm.cpp:COARSE \ - compositor/DrmDisplayComposition.cpp:COARSE \ - compositor/DrmDisplayComposition.h:COARSE \ compositor/DrmDisplayCompositor.cpp:COARSE \ drm/DrmFbImporter.h:FINE \ drm/DrmMode.h:COARSE \ diff --git a/Android.bp b/Android.bp index 9236b3e..2323c47 100644 --- a/Android.bp +++ b/Android.bp @@ -84,9 +84,8 @@ filegroup { "bufferinfo/BufferInfoGetter.cpp", "bufferinfo/BufferInfoMapperMetadata.cpp", - "compositor/DrmDisplayComposition.cpp", "compositor/DrmDisplayCompositor.cpp", - "compositor/Planner.cpp", + "compositor/DrmKmsPlan.cpp", "drm/DrmConnector.cpp", "drm/DrmCrtc.cpp", diff --git a/backend/Backend.cpp b/backend/Backend.cpp index d707192..f6d9c18 100644 --- a/backend/Backend.cpp +++ b/backend/Backend.cpp @@ -119,8 +119,8 @@ void Backend::MarkValidated(std::vector &layers, std::tuple Backend::GetExtraClientRange( HwcDisplay *display, const std::vector &layers, int client_start, size_t client_size) { - size_t avail_planes = 1 /* primary planes count*/ + - display->GetPipe().overlay_planes.size(); + auto planes = display->GetPipe().GetUsablePlanes(); + size_t avail_planes = planes.size(); /* * If more layers then planes, save one plane diff --git a/compositor/DrmDisplayComposition.cpp b/compositor/DrmDisplayComposition.cpp deleted file mode 100644 index e571b26..0000000 --- a/compositor/DrmDisplayComposition.cpp +++ /dev/null @@ -1,92 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#define LOG_TAG "hwc-drm-display-composition" - -#include "DrmDisplayComposition.h" - -#include -#include - -#include -#include -#include - -#include "DrmDisplayCompositor.h" -#include "Planner.h" -#include "drm/DrmDevice.h" -#include "utils/log.h" - -namespace android { - -DrmDisplayComposition::DrmDisplayComposition(DrmCrtc *crtc) - : crtc_(crtc) // Can be NULL if we haven't modeset yet -{ -} - -int DrmDisplayComposition::SetLayers(DrmHwcLayer *layers, size_t num_layers) { - for (size_t layer_index = 0; layer_index < num_layers; layer_index++) { - layers_.emplace_back(std::move(layers[layer_index])); - } - - return 0; -} - -int DrmDisplayComposition::AddPlaneComposition(DrmCompositionPlane plane) { - composition_planes_.emplace_back(std::move(plane)); - return 0; -} - -int DrmDisplayComposition::Plan(std::vector *primary_planes, - std::vector *overlay_planes) { - std::map to_composite; - - for (size_t i = 0; i < layers_.size(); ++i) - to_composite.emplace(std::make_pair(i, &layers_[i])); - - int ret = 0; - std::tie(ret, composition_planes_) = Planner::ProvisionPlanes(to_composite, - crtc_, - primary_planes, - overlay_planes); - if (ret) { - ALOGV("Planner failed provisioning planes ret=%d", ret); - return ret; - } - - // Remove the planes we used from the pool before returning. This ensures they - // won't be reused by another display in the composition. - for (auto &i : composition_planes_) { - if (!i.plane()) - continue; - - std::vector *container = nullptr; - if (i.plane()->GetType() == DRM_PLANE_TYPE_PRIMARY) - container = primary_planes; - else - container = overlay_planes; - for (auto j = container->begin(); j != container->end(); ++j) { - if (*j == i.plane()) { - container->erase(j); - break; - } - } - } - - return 0; -} - -} // namespace android diff --git a/compositor/DrmDisplayComposition.h b/compositor/DrmDisplayComposition.h deleted file mode 100644 index dcfd96e..0000000 --- a/compositor/DrmDisplayComposition.h +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef ANDROID_DRM_DISPLAY_COMPOSITION_H_ -#define ANDROID_DRM_DISPLAY_COMPOSITION_H_ - -#include -#include - -#include -#include - -#include "drm/DrmCrtc.h" -#include "drm/DrmPlane.h" -#include "drmhwcomposer.h" - -namespace android { - -class Importer; - -constexpr size_t kUndefinedSourceLayer = UINT16_MAX; - -class DrmCompositionPlane { - public: - DrmCompositionPlane() = default; - DrmCompositionPlane(DrmCompositionPlane &&rhs) = default; - DrmCompositionPlane &operator=(DrmCompositionPlane &&other) = default; - DrmCompositionPlane(DrmPlane *plane, size_t source_layer) - : plane_(plane), source_layer_(source_layer) { - } - - DrmPlane *plane() const { - return plane_; - } - - size_t source_layer() const { - return source_layer_; - } - - private: - DrmPlane *plane_ = nullptr; - size_t source_layer_ = kUndefinedSourceLayer; -}; - -class DrmDisplayComposition { - public: - DrmDisplayComposition(const DrmDisplayComposition &) = delete; - explicit DrmDisplayComposition(DrmCrtc *crtc); - ~DrmDisplayComposition() = default; - - int SetLayers(DrmHwcLayer *layers, size_t num_layers); - int AddPlaneComposition(DrmCompositionPlane plane); - - int Plan(std::vector *primary_planes, - std::vector *overlay_planes); - - std::vector &layers() { - return layers_; - } - - std::vector &composition_planes() { - return composition_planes_; - } - - DrmCrtc *crtc() const { - return crtc_; - } - - private: - DrmCrtc *crtc_ = nullptr; - - std::vector layers_; - std::vector composition_planes_; -}; -} // namespace android - -#endif // ANDROID_DRM_DISPLAY_COMPOSITION_H_ diff --git a/compositor/DrmDisplayCompositor.cpp b/compositor/DrmDisplayCompositor.cpp index bd0247d..5be2941 100644 --- a/compositor/DrmDisplayCompositor.cpp +++ b/compositor/DrmDisplayCompositor.cpp @@ -108,29 +108,18 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { new_frame_state.used_framebuffers.clear(); new_frame_state.used_planes.clear(); - std::vector &layers = args.composition->layers(); - std::vector &comp_planes = args.composition - ->composition_planes(); - - for (DrmCompositionPlane &comp_plane : comp_planes) { - DrmPlane *plane = comp_plane.plane(); - size_t source_layer = comp_plane.source_layer(); - - if (source_layer >= layers.size()) { - ALOGE("Source layer index %zu out of bounds %zu", source_layer, - layers.size()); - return -EINVAL; - } - DrmHwcLayer &layer = layers[source_layer]; + for (auto &joining : args.composition->plan) { + DrmPlane *plane = joining.plane->Get(); + DrmHwcLayer &layer = joining.layer; new_frame_state.used_framebuffers.emplace_back(layer.fb_id_handle); - new_frame_state.used_planes.emplace_back(plane); + new_frame_state.used_planes.emplace_back(joining.plane); /* Remove from 'unused' list, since plane is re-used */ auto &v = unused_planes; - v.erase(std::remove(v.begin(), v.end(), plane), v.end()); + v.erase(std::remove(v.begin(), v.end(), joining.plane), v.end()); - if (plane->AtomicSetState(*pset, layer, source_layer, crtc->GetId()) != + if (plane->AtomicSetState(*pset, layer, joining.z_pos, crtc->GetId()) != 0) { return -EINVAL; } @@ -143,8 +132,8 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { } if (args.clear_active_composition || args.composition) { - for (auto *plane : unused_planes) { - if (plane->AtomicDisablePlane(*pset) != 0) { + for (auto &plane : unused_planes) { + if (plane->Get()->AtomicDisablePlane(*pset) != 0) { return -EINVAL; } } diff --git a/compositor/DrmDisplayCompositor.h b/compositor/DrmDisplayCompositor.h index bff3de7..b556268 100644 --- a/compositor/DrmDisplayCompositor.h +++ b/compositor/DrmDisplayCompositor.h @@ -17,8 +17,6 @@ #ifndef ANDROID_DRM_DISPLAY_COMPOSITOR_H_ #define ANDROID_DRM_DISPLAY_COMPOSITOR_H_ -#include -#include #include #include @@ -27,7 +25,8 @@ #include #include -#include "DrmDisplayComposition.h" +#include "DrmKmsPlan.h" +#include "drm/DrmPlane.h" #include "drm/ResourceManager.h" #include "drm/VSyncWorker.h" #include "drmhwcomposer.h" @@ -39,7 +38,7 @@ struct AtomicCommitArgs { bool test_only = false; std::optional display_mode; std::optional active; - std::shared_ptr composition; + std::shared_ptr composition; /* 'clear' should never be used together with 'composition' */ bool clear_active_composition = false; @@ -68,7 +67,7 @@ class DrmDisplayCompositor { struct KmsState { /* Required to cleanup unused planes */ - std::vector used_planes; + std::vector>> used_planes; /* We have to hold a reference to framebuffer while displaying it , * otherwise picture will blink */ std::vector> used_framebuffers; diff --git a/compositor/DrmKmsPlan.cpp b/compositor/DrmKmsPlan.cpp new file mode 100644 index 0000000..966bd4e --- /dev/null +++ b/compositor/DrmKmsPlan.cpp @@ -0,0 +1,59 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "hwc-composition-drm-kms-plan" + +#include "DrmKmsPlan.h" + +#include "drm/DrmDevice.h" +#include "drm/DrmPlane.h" +#include "utils/log.h" + +namespace android { +auto DrmKmsPlan::CreateDrmKmsPlan(DrmDisplayPipeline &pipe, + std::vector composition) + -> std::unique_ptr { + auto plan = std::make_unique(); + + auto avail_planes = pipe.GetUsablePlanes(); + + int z_pos = 0; + for (auto &dhl : composition) { + std::shared_ptr> plane; + + /* Skip unsupported planes */ + do { + if (avail_planes.empty()) { + return {}; + } + + plane = *avail_planes.begin(); + avail_planes.erase(avail_planes.begin()); + } while (!plane->Get()->IsValidForLayer(&dhl)); + + LayerToPlaneJoining joining = { + .layer = std::move(dhl), + .plane = plane, + .z_pos = z_pos++, + }; + + plan->plan.emplace_back(std::move(joining)); + } + + return plan; +} + +} // namespace android diff --git a/compositor/DrmKmsPlan.h b/compositor/DrmKmsPlan.h new file mode 100644 index 0000000..35e66e9 --- /dev/null +++ b/compositor/DrmKmsPlan.h @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_DRM_KMS_PLAN_H_ +#define ANDROID_DRM_KMS_PLAN_H_ + +#include +#include + +#include "drmhwcomposer.h" + +namespace android { + +class DrmDevice; + +struct DrmKmsPlan { + struct LayerToPlaneJoining { + DrmHwcLayer layer; + std::shared_ptr> plane; + int z_pos; + }; + + std::vector plan; + + static auto CreateDrmKmsPlan(DrmDisplayPipeline &pipe, + std::vector composition) + -> std::unique_ptr; +}; + +} // namespace android +#endif diff --git a/compositor/Planner.cpp b/compositor/Planner.cpp deleted file mode 100644 index d875b4b..0000000 --- a/compositor/Planner.cpp +++ /dev/null @@ -1,81 +0,0 @@ -/* - * Copyright (C) 2016 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#define LOG_TAG "hwc-platform" - -#include "Planner.h" - -#include - -#include "drm/DrmDevice.h" -#include "utils/log.h" - -namespace android { - -std::vector Planner::GetUsablePlanes( - DrmCrtc *crtc, std::vector *primary_planes, - std::vector *overlay_planes) { - std::vector usable_planes; - std::copy_if(primary_planes->begin(), primary_planes->end(), - std::back_inserter(usable_planes), - [=](DrmPlane *plane) { return plane->IsCrtcSupported(*crtc); }); - std::copy_if(overlay_planes->begin(), overlay_planes->end(), - std::back_inserter(usable_planes), - [=](DrmPlane *plane) { return plane->IsCrtcSupported(*crtc); }); - return usable_planes; -} - -std::tuple> Planner::ProvisionPlanes( - std::map &layers, DrmCrtc *crtc, - std::vector *primary_planes, - std::vector *overlay_planes) { - std::vector composition; - std::vector planes = GetUsablePlanes(crtc, primary_planes, - overlay_planes); - if (planes.empty()) { - ALOGE("Crtc %d has no usable planes", crtc->GetId()); - return std::make_tuple(-ENODEV, std::vector()); - } - - // Go through the provisioning stages and provision planes - int ret = ProvisionPlanesInternal(&composition, layers, &planes); - if (ret != 0) { - ALOGV("Failed provision stage with ret %d", ret); - return std::make_tuple(ret, std::vector()); - } - - return std::make_tuple(0, std::move(composition)); -} - -int Planner::ProvisionPlanesInternal( - std::vector *composition, - std::map &layers, std::vector *planes) { - // Fill up the remaining planes - for (auto i = layers.begin(); i != layers.end(); i = layers.erase(i)) { - int ret = Emplace(composition, planes, std::make_pair(i->first, i->second)); - // We don't have any planes left - if (ret == -ENOENT) - break; - - if (ret != 0) { - ALOGV("Failed to emplace layer %zu, dropping it", i->first); - return ret; - } - } - - return 0; -} -} // namespace android diff --git a/compositor/Planner.h b/compositor/Planner.h deleted file mode 100644 index 7802d0c..0000000 --- a/compositor/Planner.h +++ /dev/null @@ -1,95 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef ANDROID_DRM_PLATFORM_H_ -#define ANDROID_DRM_PLATFORM_H_ - -#include -#include - -#include -#include -#include - -#include "compositor/DrmDisplayComposition.h" -#include "drmhwcomposer.h" - -namespace android { - -class DrmDevice; - -class Planner { - private: - // Removes and returns the next available plane from planes - static DrmPlane *PopPlane(std::vector *planes) { - if (planes->empty()) - return nullptr; - DrmPlane *plane = planes->front(); - planes->erase(planes->begin()); - return plane; - } - - // Inserts the given layer:plane in the composition at the back - static int Emplace(std::vector *composition, - std::vector *planes, - std::pair layer) { - DrmPlane *plane = PopPlane(planes); - std::vector unused_planes; - int ret = -ENOENT; - while (plane != nullptr) { - ret = plane->IsValidForLayer(layer.second) ? 0 : -EINVAL; - if (ret == 0) - break; - if (!plane->GetZPosProperty().is_immutable()) - unused_planes.push_back(plane); - plane = PopPlane(planes); - } - - if (ret == 0) { - composition->emplace_back(plane, layer.first); - planes->insert(planes->begin(), unused_planes.begin(), - unused_planes.end()); - } - - return ret; - } - - static int ProvisionPlanesInternal( - std::vector *composition, - std::map &layers, std::vector *planes); - - public: - // Takes a stack of layers and provisions hardware planes for them. If the - // entire stack can't fit in hardware, FIXME - // - // @layers: a map of index:layer of layers to composite - // @primary_planes: a vector of primary planes available for this frame - // @overlay_planes: a vector of overlay planes available for this frame - // - // Returns: A tuple with the status of the operation (0 for success) and - // a vector of the resulting plan (ie: layer->plane mapping). - static std::tuple> ProvisionPlanes( - std::map &layers, DrmCrtc *crtc, - std::vector *primary_planes, - std::vector *overlay_planes); - - private: - static std::vector GetUsablePlanes( - DrmCrtc *crtc, std::vector *primary_planes, - std::vector *overlay_planes); -}; -} // namespace android -#endif diff --git a/drm/DrmDisplayPipeline.cpp b/drm/DrmDisplayPipeline.cpp index 0ce1afc..31bc764 100644 --- a/drm/DrmDisplayPipeline.cpp +++ b/drm/DrmDisplayPipeline.cpp @@ -98,22 +98,6 @@ static auto TryCreatePipeline(DrmDevice &dev, DrmConnector &connector, return {}; } - char use_overlay_planes_prop[PROPERTY_VALUE_MAX]; - property_get("vendor.hwc.drm.use_overlay_planes", use_overlay_planes_prop, - "1"); - constexpr int kStrtolBase = 10; - bool use_overlay_planes = strtol(use_overlay_planes_prop, nullptr, - kStrtolBase) != 0; - - if (use_overlay_planes) { - for (auto *plane : overlay_planes) { - auto op = plane->BindPipeline(pipe.get()); - if (op) { - pipe->overlay_planes.emplace_back(op); - } - } - } - pipe->compositor = std::make_unique(pipe.get()); return pipe; @@ -173,4 +157,35 @@ auto DrmDisplayPipeline::CreatePipeline(DrmConnector &connector) return {}; } +static bool ReadUseOverlayProperty() { + char use_overlay_planes_prop[PROPERTY_VALUE_MAX]; + property_get("vendor.hwc.drm.use_overlay_planes", use_overlay_planes_prop, + "1"); + constexpr int kStrtolBase = 10; + return strtol(use_overlay_planes_prop, nullptr, kStrtolBase) != 0; +} + +auto DrmDisplayPipeline::GetUsablePlanes() + -> std::vector>> { + std::vector>> planes; + planes.emplace_back(primary_plane); + + static bool use_overlay_planes = ReadUseOverlayProperty(); + + if (use_overlay_planes) { + for (const auto &plane : device->GetPlanes()) { + if (plane->IsCrtcSupported(*crtc->Get())) { + if (plane->GetType() == DRM_PLANE_TYPE_OVERLAY) { + auto op = plane->BindPipeline(this, true); + if (op) { + planes.emplace_back(op); + } + } + } + } + } + + return planes; +} + } // namespace android diff --git a/drm/DrmDisplayPipeline.h b/drm/DrmDisplayPipeline.h index a3e2dd9..f055c85 100644 --- a/drm/DrmDisplayPipeline.h +++ b/drm/DrmDisplayPipeline.h @@ -72,13 +72,15 @@ struct DrmDisplayPipeline { static auto CreatePipeline(DrmConnector &connector) -> std::unique_ptr; + auto GetUsablePlanes() + -> std::vector>>; + DrmDevice *device; std::shared_ptr> connector; std::shared_ptr> encoder; std::shared_ptr> crtc; std::shared_ptr> primary_plane; - std::vector>> overlay_planes; std::unique_ptr compositor; }; diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp index d7b753a..f778e22 100644 --- a/hwc2_device/HwcDisplay.cpp +++ b/hwc2_device/HwcDisplay.cpp @@ -497,31 +497,21 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) { composition_layers.emplace_back(std::move(layer)); } - auto composition = std::make_shared( - GetPipe().crtc->Get()); - - // TODO(nobody): Don't always assume geometry changed - int ret = composition->SetLayers(composition_layers.data(), - composition_layers.size()); - if (ret) { - ALOGE("Failed to set layers in the composition ret=%d", ret); - return HWC2::Error::BadLayer; - } - - std::vector primary_planes; - primary_planes.emplace_back(pipeline_->primary_plane->Get()); - std::vector overlay_planes; - for (const auto &owned_plane : pipeline_->overlay_planes) { - overlay_planes.emplace_back(owned_plane->Get()); - } - ret = composition->Plan(&primary_planes, &overlay_planes); - if (ret) { - ALOGV("Failed to plan the composition ret=%d", ret); + /* Store plan to ensure shared planes won't be stolen by other display + * in between of ValidateDisplay() and PresentDisplay() calls + */ + current_plan_ = DrmKmsPlan::CreateDrmKmsPlan(GetPipe(), + std::move(composition_layers)); + if (!current_plan_) { + if (!a_args.test_only) { + ALOGE("Failed to create DrmKmsPlan"); + } return HWC2::Error::BadConfig; } - a_args.composition = composition; - ret = GetPipe().compositor->ExecuteAtomicCommit(a_args); + a_args.composition = current_plan_; + + int ret = GetPipe().compositor->ExecuteAtomicCommit(a_args); if (ret) { if (!a_args.test_only) diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h index e7ce7ef..ed29da6 100644 --- a/hwc2_device/HwcDisplay.h +++ b/hwc2_device/HwcDisplay.h @@ -221,6 +221,8 @@ class HwcDisplay { std::array color_transform_matrix_{}; android_color_transform_t color_transform_hint_; + std::shared_ptr current_plan_; + uint32_t frame_no_ = 0; Stats total_stats_; Stats prev_stats_; -- cgit v1.2.3 From 71f196fce021326fe9a72f6ee205bfb6b71c794f Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Fri, 4 Feb 2022 12:23:43 +0200 Subject: drm_hwcomposer: Remove RCAR-DU specific code. We are not testing it for more than year, therefore it's better to use generic logic for 'rcar-du' instead. Signed-off-by: Roman Stratiienko --- Android.bp | 1 - backend/BackendRCarDu.cpp | 47 ----------------------------------------------- backend/BackendRCarDu.h | 30 ------------------------------ 3 files changed, 78 deletions(-) delete mode 100644 backend/BackendRCarDu.cpp delete mode 100644 backend/BackendRCarDu.h diff --git a/Android.bp b/Android.bp index 2323c47..77a2644 100644 --- a/Android.bp +++ b/Android.bp @@ -105,7 +105,6 @@ filegroup { "backend/Backend.cpp", "backend/BackendClient.cpp", "backend/BackendManager.cpp", - "backend/BackendRCarDu.cpp", "hwc2_device/DrmHwcTwo.cpp", "hwc2_device/HwcDisplay.cpp", diff --git a/backend/BackendRCarDu.cpp b/backend/BackendRCarDu.cpp deleted file mode 100644 index 0750ee4..0000000 --- a/backend/BackendRCarDu.cpp +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright (C) 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "BackendRCarDu.h" - -#include "BackendManager.h" -#include "bufferinfo/BufferInfoGetter.h" -#include "drm_fourcc.h" - -namespace android { - -bool BackendRCarDu::IsClientLayer(HwcDisplay *display, HwcLayer *layer) { - hwc_drm_bo_t bo; - - int ret = BufferInfoGetter::GetInstance()->ConvertBoInfo(layer->GetBuffer(), - &bo); - if (ret != 0) - return true; - - if (bo.format == DRM_FORMAT_ABGR8888) - return true; - - if (layer->RequireScalingOrPhasing()) - return true; - - return Backend::IsClientLayer(display, layer); -} - -// clang-format off -// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables, cert-err58-cpp) -REGISTER_BACKEND("rcar-du", BackendRCarDu); -// clang-format on - -} // namespace android \ No newline at end of file diff --git a/backend/BackendRCarDu.h b/backend/BackendRCarDu.h deleted file mode 100644 index 1259c9f..0000000 --- a/backend/BackendRCarDu.h +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright (C) 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef HWC_DISPLAY_BACKEND_RCAR_DU_H -#define HWC_DISPLAY_BACKEND_RCAR_DU_H - -#include "Backend.h" - -namespace android { - -class BackendRCarDu : public Backend { - public: - bool IsClientLayer(HwcDisplay *display, HwcLayer *layer) override; -}; -} // namespace android - -#endif -- cgit v1.2.3 From ef5348b7a53ee7fc169956a95d959c2823aaf478 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Wed, 9 Feb 2022 17:19:56 +0200 Subject: drm_hwcomposer: Remove AtomicCommitArgs::clear_active_composition field Now we can use empty DrmKmsPlan to achieve the same goal. + Remove unused HwcDisplay::ClearDisplay() Signed-off-by: Roman Stratiienko --- compositor/DrmDisplayCompositor.cpp | 15 +++------------ compositor/DrmDisplayCompositor.h | 4 +--- hwc2_device/HwcDisplay.cpp | 10 ---------- 3 files changed, 4 insertions(+), 25 deletions(-) diff --git a/compositor/DrmDisplayCompositor.cpp b/compositor/DrmDisplayCompositor.cpp index 5be2941..e588b7f 100644 --- a/compositor/DrmDisplayCompositor.cpp +++ b/compositor/DrmDisplayCompositor.cpp @@ -58,11 +58,6 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { args.active = true; } - if (args.clear_active_composition && args.composition) { - ALOGE("%s: Invalid arguments", __func__); - return -EINVAL; - } - auto new_frame_state = NewFrameState(); auto *drm = pipe_->device; @@ -126,12 +121,7 @@ auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { } } - if (args.clear_active_composition) { - new_frame_state.used_framebuffers.clear(); - new_frame_state.used_planes.clear(); - } - - if (args.clear_active_composition || args.composition) { + if (args.composition) { for (auto &plane : unused_planes) { if (plane->Get()->AtomicDisablePlane(*pset) != 0) { return -EINVAL; @@ -176,7 +166,8 @@ auto DrmDisplayCompositor::ExecuteAtomicCommit(AtomicCommitArgs &args) -> int { pipe_->connector->Get()->GetName().c_str()); // Disable the hw used by the last active composition. This allows us to // signal the release fences from that composition to avoid hanging. - AtomicCommitArgs cl_args = {.clear_active_composition = true}; + AtomicCommitArgs cl_args{}; + cl_args.composition = std::make_shared(); if (CommitFrame(cl_args)) { ALOGE("Failed to clean-up active composition for pipeline %s", pipe_->connector->Get()->GetName().c_str()); diff --git a/compositor/DrmDisplayCompositor.h b/compositor/DrmDisplayCompositor.h index b556268..7e39eef 100644 --- a/compositor/DrmDisplayCompositor.h +++ b/compositor/DrmDisplayCompositor.h @@ -39,15 +39,13 @@ struct AtomicCommitArgs { std::optional display_mode; std::optional active; std::shared_ptr composition; - /* 'clear' should never be used together with 'composition' */ - bool clear_active_composition = false; /* out */ UniqueFd out_fence; /* helpers */ auto HasInputs() -> bool { - return display_mode || active || composition || clear_active_composition; + return display_mode || active || composition; } }; diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp index f778e22..e02019d 100644 --- a/hwc2_device/HwcDisplay.cpp +++ b/hwc2_device/HwcDisplay.cpp @@ -120,16 +120,6 @@ HwcDisplay::~HwcDisplay() { main_lock.lock(); } -void HwcDisplay::ClearDisplay() { - if (IsInHeadlessMode()) { - ALOGE("%s: Headless mode, should never reach here: ", __func__); - return; - } - - AtomicCommitArgs a_args = {.clear_active_composition = true}; - pipeline_->compositor->ExecuteAtomicCommit(a_args); -} - HWC2::Error HwcDisplay::Init() { int ret = vsync_worker_.Init(pipeline_, [this](int64_t timestamp) { const std::lock_guard lock(hwc2_->GetResMan().GetMainLock()); -- cgit v1.2.3 From 4e994055a3625e822dc04e659a1feba3017fffe6 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Wed, 9 Feb 2022 17:40:35 +0200 Subject: drm_hwcomposer: Rename DrmDisplayCompositor->DrmAtomicStateManager Primary responsibilities of this class are: 1. Send composition/mode/active state over DRM atomic commit IOCTL to the kernel 2. Track commit state and keep planes owned by the Pipeline while they are either displayed or staged for displaying. 3. Keep framebuffers alive while they are in use or staged. Not much related to composition itself, therefore rename it to DrmAtomicStateManager and move it to drm folder. Bump clang-tidy level of DrmAtomicStateManager.c to normal by fixing minor clang-tidy findings. Signed-off-by: Roman Stratiienko --- .ci/Makefile | 1 - Android.bp | 2 +- compositor/DrmDisplayCompositor.cpp | 191 ------------------------------------ compositor/DrmDisplayCompositor.h | 91 ----------------- drm/DrmAtomicStateManager.cpp | 191 ++++++++++++++++++++++++++++++++++++ drm/DrmAtomicStateManager.h | 89 +++++++++++++++++ drm/DrmDevice.cpp | 2 +- drm/DrmDisplayPipeline.cpp | 5 +- drm/DrmDisplayPipeline.h | 4 +- drm/ResourceManager.cpp | 2 +- hwc2_device/HwcDisplay.cpp | 6 +- hwc2_device/HwcDisplay.h | 2 +- 12 files changed, 292 insertions(+), 294 deletions(-) delete mode 100644 compositor/DrmDisplayCompositor.cpp delete mode 100644 compositor/DrmDisplayCompositor.h create mode 100644 drm/DrmAtomicStateManager.cpp create mode 100644 drm/DrmAtomicStateManager.h diff --git a/.ci/Makefile b/.ci/Makefile index a9b96d3..cadbde6 100644 --- a/.ci/Makefile +++ b/.ci/Makefile @@ -21,7 +21,6 @@ TIDY_FILES_OVERRIDE := \ bufferinfo/legacy/BufferInfoMaliMediatek.cpp:COARSE \ bufferinfo/legacy/BufferInfoMaliMeson.cpp:COARSE \ bufferinfo/legacy/BufferInfoMinigbm.cpp:COARSE \ - compositor/DrmDisplayCompositor.cpp:COARSE \ drm/DrmFbImporter.h:FINE \ drm/DrmMode.h:COARSE \ drm/DrmProperty.h:COARSE \ diff --git a/Android.bp b/Android.bp index 77a2644..8b6d295 100644 --- a/Android.bp +++ b/Android.bp @@ -84,9 +84,9 @@ filegroup { "bufferinfo/BufferInfoGetter.cpp", "bufferinfo/BufferInfoMapperMetadata.cpp", - "compositor/DrmDisplayCompositor.cpp", "compositor/DrmKmsPlan.cpp", + "drm/DrmAtomicStateManager.cpp", "drm/DrmConnector.cpp", "drm/DrmCrtc.cpp", "drm/DrmDevice.cpp", diff --git a/compositor/DrmDisplayCompositor.cpp b/compositor/DrmDisplayCompositor.cpp deleted file mode 100644 index e588b7f..0000000 --- a/compositor/DrmDisplayCompositor.cpp +++ /dev/null @@ -1,191 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#define ATRACE_TAG ATRACE_TAG_GRAPHICS -#define LOG_TAG "hwc-drm-display-compositor" - -#include "DrmDisplayCompositor.h" - -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include - -#include "drm/DrmCrtc.h" -#include "drm/DrmDevice.h" -#include "drm/DrmPlane.h" -#include "drm/DrmUnique.h" -#include "utils/log.h" - -namespace android { - -// NOLINTNEXTLINE (readability-function-cognitive-complexity): Fixme -auto DrmDisplayCompositor::CommitFrame(AtomicCommitArgs &args) -> int { - ATRACE_CALL(); - - if (args.active && *args.active == active_frame_state_.crtc_active_state) { - /* Don't set the same state twice */ - args.active.reset(); - } - - if (!args.HasInputs()) { - /* nothing to do */ - return 0; - } - - if (!active_frame_state_.crtc_active_state) { - /* Force activate display */ - args.active = true; - } - - auto new_frame_state = NewFrameState(); - - auto *drm = pipe_->device; - auto *connector = pipe_->connector->Get(); - auto *crtc = pipe_->crtc->Get(); - - auto pset = MakeDrmModeAtomicReqUnique(); - if (!pset) { - ALOGE("Failed to allocate property set"); - return -ENOMEM; - } - - int64_t out_fence = -1; - 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->GetActiveProperty().AtomicSet(*pset, *args.active) || - !connector->GetCrtcIdProperty().AtomicSet(*pset, crtc->GetId())) { - return -EINVAL; - } - } - - if (args.display_mode) { - new_frame_state.mode_blob = args.display_mode.value().CreateModeBlob(*drm); - - if (!new_frame_state.mode_blob) { - ALOGE("Failed to create mode_blob"); - return -EINVAL; - } - - if (!crtc->GetModeProperty().AtomicSet(*pset, *new_frame_state.mode_blob)) { - return -EINVAL; - } - } - - auto unused_planes = new_frame_state.used_planes; - - if (args.composition) { - new_frame_state.used_framebuffers.clear(); - new_frame_state.used_planes.clear(); - - for (auto &joining : args.composition->plan) { - DrmPlane *plane = joining.plane->Get(); - DrmHwcLayer &layer = joining.layer; - - new_frame_state.used_framebuffers.emplace_back(layer.fb_id_handle); - new_frame_state.used_planes.emplace_back(joining.plane); - - /* Remove from 'unused' list, since plane is re-used */ - auto &v = unused_planes; - v.erase(std::remove(v.begin(), v.end(), joining.plane), v.end()); - - if (plane->AtomicSetState(*pset, layer, joining.z_pos, crtc->GetId()) != - 0) { - return -EINVAL; - } - } - } - - if (args.composition) { - for (auto &plane : unused_planes) { - if (plane->Get()->AtomicDisablePlane(*pset) != 0) { - return -EINVAL; - } - } - } - - uint32_t flags = DRM_MODE_ATOMIC_ALLOW_MODESET; - if (args.test_only) - flags |= DRM_MODE_ATOMIC_TEST_ONLY; - - int err = drmModeAtomicCommit(drm->GetFd(), pset.get(), flags, drm); - if (err) { - if (!args.test_only) - ALOGE("Failed to commit pset ret=%d\n", err); - return err; - } - - if (!args.test_only) { - if (args.display_mode) { - /* TODO(nobody): we still need this for synthetic vsync, remove after - * vsync reworked */ - connector->SetActiveMode(*args.display_mode); - } - - active_frame_state_ = std::move(new_frame_state); - - if (crtc->GetOutFencePtrProperty()) { - args.out_fence = UniqueFd((int)out_fence); - } - } - - return 0; -} - -auto DrmDisplayCompositor::ExecuteAtomicCommit(AtomicCommitArgs &args) -> int { - int err = CommitFrame(args); - - if (!args.test_only) { - if (err) { - ALOGE("Composite failed for pipeline %s", - pipe_->connector->Get()->GetName().c_str()); - // Disable the hw used by the last active composition. This allows us to - // signal the release fences from that composition to avoid hanging. - AtomicCommitArgs cl_args{}; - cl_args.composition = std::make_shared(); - if (CommitFrame(cl_args)) { - ALOGE("Failed to clean-up active composition for pipeline %s", - pipe_->connector->Get()->GetName().c_str()); - } - return err; - } - } - - return err; -} // namespace android - -auto DrmDisplayCompositor::ActivateDisplayUsingDPMS() -> int { - return drmModeConnectorSetProperty(pipe_->device->GetFd(), - pipe_->connector->Get()->GetId(), - pipe_->connector->Get() - ->GetDpmsProperty() - .id(), - DRM_MODE_DPMS_ON); -} - -} // namespace android diff --git a/compositor/DrmDisplayCompositor.h b/compositor/DrmDisplayCompositor.h deleted file mode 100644 index 7e39eef..0000000 --- a/compositor/DrmDisplayCompositor.h +++ /dev/null @@ -1,91 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef ANDROID_DRM_DISPLAY_COMPOSITOR_H_ -#define ANDROID_DRM_DISPLAY_COMPOSITOR_H_ - -#include - -#include -#include -#include -#include -#include - -#include "DrmKmsPlan.h" -#include "drm/DrmPlane.h" -#include "drm/ResourceManager.h" -#include "drm/VSyncWorker.h" -#include "drmhwcomposer.h" - -namespace android { - -struct AtomicCommitArgs { - /* inputs. All fields are optional, but at least one has to be specified */ - bool test_only = false; - std::optional display_mode; - std::optional active; - std::shared_ptr composition; - - /* out */ - UniqueFd out_fence; - - /* helpers */ - auto HasInputs() -> bool { - return display_mode || active || composition; - } -}; - -class DrmDisplayCompositor { - public: - explicit DrmDisplayCompositor(DrmDisplayPipeline *pipe) : pipe_(pipe){}; - ~DrmDisplayCompositor() = default; - - auto ExecuteAtomicCommit(AtomicCommitArgs &args) -> int; - - DrmDisplayCompositor(const DrmDisplayCompositor &) = delete; - - auto ActivateDisplayUsingDPMS() -> int; - - private: - auto CommitFrame(AtomicCommitArgs &args) -> int; - - struct KmsState { - /* Required to cleanup unused planes */ - std::vector>> used_planes; - /* We have to hold a reference to framebuffer while displaying it , - * otherwise picture will blink */ - std::vector> used_framebuffers; - - DrmModeUserPropertyBlobUnique mode_blob; - - /* To avoid setting the inactive state twice, which will fail the commit */ - bool crtc_active_state{}; - } active_frame_state_; - - auto NewFrameState() -> KmsState { - return (KmsState){ - .used_planes = active_frame_state_.used_planes, - .used_framebuffers = active_frame_state_.used_framebuffers, - .crtc_active_state = active_frame_state_.crtc_active_state, - }; - } - - DrmDisplayPipeline *const pipe_; -}; -} // namespace android - -#endif // ANDROID_DRM_DISPLAY_COMPOSITOR_H_ diff --git a/drm/DrmAtomicStateManager.cpp b/drm/DrmAtomicStateManager.cpp new file mode 100644 index 0000000..65fb19e --- /dev/null +++ b/drm/DrmAtomicStateManager.cpp @@ -0,0 +1,191 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define ATRACE_TAG ATRACE_TAG_GRAPHICS +#define LOG_TAG "hwc-drm-atomic-state-manager" + +#include "DrmAtomicStateManager.h" + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#include "drm/DrmCrtc.h" +#include "drm/DrmDevice.h" +#include "drm/DrmPlane.h" +#include "drm/DrmUnique.h" +#include "utils/log.h" + +namespace android { + +// NOLINTNEXTLINE (readability-function-cognitive-complexity): Fixme +auto DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) -> int { + ATRACE_CALL(); + + if (args.active && *args.active == active_frame_state_.crtc_active_state) { + /* Don't set the same state twice */ + args.active.reset(); + } + + if (!args.HasInputs()) { + /* nothing to do */ + return 0; + } + + if (!active_frame_state_.crtc_active_state) { + /* Force activate display */ + args.active = true; + } + + auto new_frame_state = NewFrameState(); + + auto *drm = pipe_->device; + auto *connector = pipe_->connector->Get(); + auto *crtc = pipe_->crtc->Get(); + + auto pset = MakeDrmModeAtomicReqUnique(); + if (!pset) { + ALOGE("Failed to allocate property set"); + return -ENOMEM; + } + + int64_t out_fence = -1; + 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->GetActiveProperty().AtomicSet(*pset, *args.active ? 1 : 0) || + !connector->GetCrtcIdProperty().AtomicSet(*pset, crtc->GetId())) { + return -EINVAL; + } + } + + if (args.display_mode) { + new_frame_state.mode_blob = args.display_mode.value().CreateModeBlob(*drm); + + if (!new_frame_state.mode_blob) { + ALOGE("Failed to create mode_blob"); + return -EINVAL; + } + + if (!crtc->GetModeProperty().AtomicSet(*pset, *new_frame_state.mode_blob)) { + return -EINVAL; + } + } + + auto unused_planes = new_frame_state.used_planes; + + if (args.composition) { + new_frame_state.used_framebuffers.clear(); + new_frame_state.used_planes.clear(); + + for (auto &joining : args.composition->plan) { + DrmPlane *plane = joining.plane->Get(); + DrmHwcLayer &layer = joining.layer; + + new_frame_state.used_framebuffers.emplace_back(layer.fb_id_handle); + new_frame_state.used_planes.emplace_back(joining.plane); + + /* Remove from 'unused' list, since plane is re-used */ + auto &v = unused_planes; + v.erase(std::remove(v.begin(), v.end(), joining.plane), v.end()); + + if (plane->AtomicSetState(*pset, layer, joining.z_pos, crtc->GetId()) != + 0) { + return -EINVAL; + } + } + } + + if (args.composition) { + for (auto &plane : unused_planes) { + if (plane->Get()->AtomicDisablePlane(*pset) != 0) { + return -EINVAL; + } + } + } + + uint32_t flags = DRM_MODE_ATOMIC_ALLOW_MODESET; + if (args.test_only) + flags |= DRM_MODE_ATOMIC_TEST_ONLY; + + int err = drmModeAtomicCommit(drm->GetFd(), pset.get(), flags, drm); + if (err != 0) { + if (!args.test_only) + ALOGE("Failed to commit pset ret=%d\n", err); + return err; + } + + if (!args.test_only) { + if (args.display_mode) { + /* TODO(nobody): we still need this for synthetic vsync, remove after + * vsync reworked */ + connector->SetActiveMode(*args.display_mode); + } + + active_frame_state_ = std::move(new_frame_state); + + if (crtc->GetOutFencePtrProperty()) { + args.out_fence = UniqueFd((int)out_fence); + } + } + + return 0; +} + +auto DrmAtomicStateManager::ExecuteAtomicCommit(AtomicCommitArgs &args) -> int { + int err = CommitFrame(args); + + if (!args.test_only) { + if (err != 0) { + ALOGE("Composite failed for pipeline %s", + pipe_->connector->Get()->GetName().c_str()); + // Disable the hw used by the last active composition. This allows us to + // signal the release fences from that composition to avoid hanging. + AtomicCommitArgs cl_args{}; + cl_args.composition = std::make_shared(); + if (CommitFrame(cl_args) != 0) { + ALOGE("Failed to clean-up active composition for pipeline %s", + pipe_->connector->Get()->GetName().c_str()); + } + return err; + } + } + + return err; +} // namespace android + +auto DrmAtomicStateManager::ActivateDisplayUsingDPMS() -> int { + return drmModeConnectorSetProperty(pipe_->device->GetFd(), + pipe_->connector->Get()->GetId(), + pipe_->connector->Get() + ->GetDpmsProperty() + .id(), + DRM_MODE_DPMS_ON); +} + +} // namespace android diff --git a/drm/DrmAtomicStateManager.h b/drm/DrmAtomicStateManager.h new file mode 100644 index 0000000..08a1c13 --- /dev/null +++ b/drm/DrmAtomicStateManager.h @@ -0,0 +1,89 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_DRM_ATOMIC_STATE_MANAGER_H_ +#define ANDROID_DRM_ATOMIC_STATE_MANAGER_H_ + +#include + +#include +#include +#include +#include +#include + +#include "compositor/DrmKmsPlan.h" +#include "drm/DrmPlane.h" +#include "drm/ResourceManager.h" +#include "drm/VSyncWorker.h" +#include "drmhwcomposer.h" + +namespace android { + +struct AtomicCommitArgs { + /* inputs. All fields are optional, but at least one has to be specified */ + bool test_only = false; + std::optional display_mode; + std::optional active; + std::shared_ptr composition; + + /* out */ + UniqueFd out_fence; + + /* helpers */ + auto HasInputs() -> bool { + return display_mode || active || composition; + } +}; + +class DrmAtomicStateManager { + public: + explicit DrmAtomicStateManager(DrmDisplayPipeline *pipe) : pipe_(pipe){}; + DrmAtomicStateManager(const DrmAtomicStateManager &) = delete; + ~DrmAtomicStateManager() = default; + + auto ExecuteAtomicCommit(AtomicCommitArgs &args) -> int; + auto ActivateDisplayUsingDPMS() -> int; + + private: + auto CommitFrame(AtomicCommitArgs &args) -> int; + + struct KmsState { + /* Required to cleanup unused planes */ + std::vector>> used_planes; + /* We have to hold a reference to framebuffer while displaying it , + * otherwise picture will blink */ + std::vector> used_framebuffers; + + DrmModeUserPropertyBlobUnique mode_blob; + + /* To avoid setting the inactive state twice, which will fail the commit */ + bool crtc_active_state{}; + } active_frame_state_; + + auto NewFrameState() -> KmsState { + return (KmsState){ + .used_planes = active_frame_state_.used_planes, + .used_framebuffers = active_frame_state_.used_framebuffers, + .crtc_active_state = active_frame_state_.crtc_active_state, + }; + } + + DrmDisplayPipeline *const pipe_; +}; +} // namespace android + +#endif // ANDROID_DRM_DISPLAY_COMPOSITOR_H_ diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp index e5f41e8..fd4589e 100644 --- a/drm/DrmDevice.cpp +++ b/drm/DrmDevice.cpp @@ -26,7 +26,7 @@ #include #include -#include "compositor/DrmDisplayCompositor.h" +#include "drm/DrmAtomicStateManager.h" #include "drm/DrmPlane.h" #include "utils/log.h" #include "utils/properties.h" diff --git a/drm/DrmDisplayPipeline.cpp b/drm/DrmDisplayPipeline.cpp index 31bc764..f993d28 100644 --- a/drm/DrmDisplayPipeline.cpp +++ b/drm/DrmDisplayPipeline.cpp @@ -18,12 +18,12 @@ #include "DrmDisplayPipeline.h" +#include "DrmAtomicStateManager.h" #include "DrmConnector.h" #include "DrmCrtc.h" #include "DrmDevice.h" #include "DrmEncoder.h" #include "DrmPlane.h" -#include "compositor/DrmDisplayCompositor.h" #include "utils/log.h" #include "utils/properties.h" @@ -98,7 +98,8 @@ static auto TryCreatePipeline(DrmDevice &dev, DrmConnector &connector, return {}; } - pipe->compositor = std::make_unique(pipe.get()); + pipe->atomic_state_manager = std::make_unique( + pipe.get()); return pipe; } diff --git a/drm/DrmDisplayPipeline.h b/drm/DrmDisplayPipeline.h index f055c85..7ec619e 100644 --- a/drm/DrmDisplayPipeline.h +++ b/drm/DrmDisplayPipeline.h @@ -27,7 +27,7 @@ class DrmDevice; class DrmPlane; class DrmCrtc; class DrmEncoder; -class DrmDisplayCompositor; +class DrmAtomicStateManager; struct DrmDisplayPipeline; @@ -82,7 +82,7 @@ struct DrmDisplayPipeline { std::shared_ptr> crtc; std::shared_ptr> primary_plane; - std::unique_ptr compositor; + std::unique_ptr atomic_state_manager; }; } // namespace android diff --git a/drm/ResourceManager.cpp b/drm/ResourceManager.cpp index b294180..c8235e9 100644 --- a/drm/ResourceManager.cpp +++ b/drm/ResourceManager.cpp @@ -25,7 +25,7 @@ #include #include "bufferinfo/BufferInfoGetter.h" -#include "compositor/DrmDisplayCompositor.h" +#include "drm/DrmAtomicStateManager.h" #include "drm/DrmDevice.h" #include "drm/DrmDisplayPipeline.h" #include "drm/DrmPlane.h" diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp index e02019d..3f00fbf 100644 --- a/hwc2_device/HwcDisplay.cpp +++ b/hwc2_device/HwcDisplay.cpp @@ -501,7 +501,7 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) { a_args.composition = current_plan_; - int ret = GetPipe().compositor->ExecuteAtomicCommit(a_args); + int ret = GetPipe().atomic_state_manager->ExecuteAtomicCommit(a_args); if (ret) { if (!a_args.test_only) @@ -655,7 +655,7 @@ HWC2::Error HwcDisplay::SetPowerMode(int32_t mode_in) { * true, as the next composition frame will implicitly activate * the display */ - return GetPipe().compositor->ActivateDisplayUsingDPMS() == 0 + return GetPipe().atomic_state_manager->ActivateDisplayUsingDPMS() == 0 ? HWC2::Error::None : HWC2::Error::BadParameter; break; @@ -667,7 +667,7 @@ HWC2::Error HwcDisplay::SetPowerMode(int32_t mode_in) { return HWC2::Error::BadParameter; }; - int err = GetPipe().compositor->ExecuteAtomicCommit(a_args); + int err = GetPipe().atomic_state_manager->ExecuteAtomicCommit(a_args); if (err) { ALOGE("Failed to apply the dpms composition err=%d", err); return HWC2::Error::BadParameter; diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h index ed29da6..ae5d980 100644 --- a/hwc2_device/HwcDisplay.h +++ b/hwc2_device/HwcDisplay.h @@ -22,7 +22,7 @@ #include #include "HwcDisplayConfigs.h" -#include "compositor/DrmDisplayCompositor.h" +#include "drm/DrmAtomicStateManager.h" #include "drm/ResourceManager.h" #include "drm/VSyncWorker.h" #include "drmhwcomposer.h" -- cgit v1.2.3 From a8927b83ff9abe62a891725a059e50db5c46d160 Mon Sep 17 00:00:00 2001 From: Mauro Rossi Date: Sat, 4 Dec 2021 15:02:13 +0100 Subject: drm_hwcomposer: build with -std=c++17 cppflag Fixes the following building error: external/drm_hwcomposer/DrmHwcTwo.cpp:985:14: error: decomposition declarations are a C++17 extension [-Werror,-Wc++17-extensions] for (auto &[handle, layer] : layers_) { ^~~~~~~~~~~~~~~ 1 error generated. Signed-off-by: Mauro Rossi Signed-off-by: Roman Stratiienko Change-Id: I236f16969e4500ff2efb79c06500bc4d3a3d810c --- Android.bp | 1 + 1 file changed, 1 insertion(+) diff --git a/Android.bp b/Android.bp index 8b6d295..dd6225d 100644 --- a/Android.bp +++ b/Android.bp @@ -66,6 +66,7 @@ cc_defaults { cppflags: [ "-DHWC2_INCLUDE_STRINGIFICATION", "-DHWC2_USE_CPP11", + "-std=c++17", ], product_variables: { -- cgit v1.2.3 From cf6614db28b1c29472eddeb43391da1f92dc8e10 Mon Sep 17 00:00:00 2001 From: Mauro Rossi Date: Sun, 16 Jan 2022 16:11:46 +0100 Subject: drm_hwcomposer: fix sign-compare building error in uevent listener d26619b5 ("drm_hwcomposer: CI: Upgrade clang-* to v12") declared 'ret' as ssize_t but after commit 1e053b4e ("drm_hwcomposer: Make uevent listener standalone") drm/UEventListener.cpp is affected by the following builing error: external/drm_hwcomposer/drm/UEventListener.cpp:82:28: error: comparison of integers of different signs: 'uint32_t' (aka 'unsigned int') and 'ssize_t' (aka 'int') [-Werror,-Wsign-compare] for (uint32_t i = 0; i < ret;) { ~ ^ ~~~ 1 error generated. Fixes: 1e053b4e ("drm_hwcomposer: Make uevent listener standalone") Signed-off-by: Mauro Rossi [RomanS: Fixed CI nitpicks] Signed-off-by: Roman Stratiienko Change-Id: Ia97d9019c21ac68be386a627cb101f6e423bbfc7 --- drm/UEventListener.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drm/UEventListener.cpp b/drm/UEventListener.cpp index 44c503d..8d33ad2 100644 --- a/drm/UEventListener.cpp +++ b/drm/UEventListener.cpp @@ -79,7 +79,7 @@ void UEventListener::Routine() { bool drm_event = false; bool hotplug_event = false; - for (uint32_t i = 0; i < ret;) { + for (uint32_t i = 0; (ssize_t)i < ret;) { char *event = buffer + i; if (strcmp(event, "DEVTYPE=drm_minor") != 0) drm_event = true; -- cgit v1.2.3 From 2c63b3337380dd35ee0c658c05e27bcd89239b10 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Thu, 10 Feb 2022 10:45:06 +0200 Subject: drm_hwcomposer: Define DRM_FORMAT_XYUV8888 if missing Fixes drm_hwcomposer build for Android-9. Signed-off-by: Roman Stratiienko Change-Id: I14e931c37c3d09284dfd338e6482a27cf21e0e10 --- bufferinfo/legacy/BufferInfoLibdrm.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/bufferinfo/legacy/BufferInfoLibdrm.cpp b/bufferinfo/legacy/BufferInfoLibdrm.cpp index 4f942fe..6baf6bb 100644 --- a/bufferinfo/legacy/BufferInfoLibdrm.cpp +++ b/bufferinfo/legacy/BufferInfoLibdrm.cpp @@ -47,6 +47,11 @@ struct DroidYuvFormat { int fourcc; /* DRM_FORMAT_ */ }; +#ifndef DRM_FORMAT_XYUV8888 +#define DRM_FORMAT_XYUV8888 \ + fourcc_code('X', 'Y', 'U', 'V') /* [31:0] X:Y:Cb:Cr 8:8:8:8 little endian */ +#endif + /* The following table is used to look up a DRI image FourCC based * on native format and information contained in android_ycbcr struct. */ static const struct DroidYuvFormat kDroidYuvFormats[] = { -- cgit v1.2.3 From bd9731713b79f80b2fa1bf180b663649c9d357a5 Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Fri, 18 Feb 2022 16:51:53 +0200 Subject: drm_hwcomposer: Add test utility to listen for uevents Dumping uevents is useful for debugging purposes. 1. Extract logic related to uevent socket into utils/UEvent.h class. 2. Use it by both UEventListener.cpp and tests/uevent_print.cpp. Bump clang-tidy level of UEventListener.cpp to normal. Signed-off-by: Roman Stratiienko --- .ci/Makefile | 7 ++--- drm/UEventListener.cpp | 67 +++++++++------------------------------ drm/UEventListener.h | 4 +-- tests/Android.bp | 14 +++++++++ tests/uevent_print.cpp | 25 +++++++++++++++ utils/UEvent.h | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 143 insertions(+), 59 deletions(-) create mode 100644 tests/uevent_print.cpp create mode 100644 utils/UEvent.h diff --git a/.ci/Makefile b/.ci/Makefile index cadbde6..a0e4b73 100644 --- a/.ci/Makefile +++ b/.ci/Makefile @@ -26,7 +26,6 @@ TIDY_FILES_OVERRIDE := \ drm/DrmProperty.h:COARSE \ drm/DrmUnique.h:FINE \ drm/DrmProperty.cpp:COARSE \ - drm/UEventListener.cpp:COARSE \ drm/VSyncWorker.cpp:COARSE \ hwc2_device/DrmHwcTwo.cpp:COARSE \ hwc2_device/DrmHwcTwo.h:COARSE \ @@ -85,7 +84,7 @@ clean: # Build -BUILD_FILES_AUTO := $(shell find -L $(SRC_DIR) -not -path '*/\.*' -not -path '*/tests/*' -path '*.cpp') +BUILD_FILES_AUTO := $(shell find -L $(SRC_DIR) -not -path '*/\.*' -not -path '*/tests/test_include/*' -path '*.cpp') SKIP_FILES_path := $(foreach file,$(SKIP_FILES),$(SRC_DIR)/$(file)) BUILD_FILES := $(subst ./,,$(filter-out $(SKIP_FILES_path),$(BUILD_FILES_AUTO))) @@ -108,7 +107,7 @@ $(OUT_DIR)/%.d: $(SRC_DIR)/%.cpp $(CLANG) $(CXXARGS) $< -MM -MT $(OUT_DIR)/$(patsubst %.cpp,%.o,$<) -o $@ # TIDY -TIDY_FILES_AUTO := $(shell find -L $(SRC_DIR) -not -path '*/\.*' -not -path '*/tests/*' \( -path '*.cpp' -o -path '*.h' \)) +TIDY_FILES_AUTO := $(shell find -L $(SRC_DIR) -not -path '*/\.*' -not -path '*/tests/test_include/*' \( -path '*.cpp' -o -path '*.h' \)) TIDY_FILES_AUTO_filtered := $(filter-out $(SKIP_FILES_path),$(TIDY_FILES_AUTO)) @@ -145,7 +144,7 @@ $$(_TARG): _TARG := $$(_TARG) $$(_TARG): TIDY_ARGS := $$(TIDY_ARGS) $$(_TARG): $$(_DEP) mkdir -p $$(dir $$(_TARG)) - $$(CLANG_TIDY) $$(_DEP) $$(TIDY_ARGS) -- -x c++ $$(CXXARGS) + $$(CLANG_TIDY) $$(_DEP) $$(TIDY_ARGS) -- -x c++ $$(CXXARGS) -Wno-pragma-once-outside-header touch $$(_TARG) endef diff --git a/drm/UEventListener.cpp b/drm/UEventListener.cpp index 8d33ad2..b56b8e1 100644 --- a/drm/UEventListener.cpp +++ b/drm/UEventListener.cpp @@ -18,82 +18,43 @@ #include "UEventListener.h" -#include -#include -#include - -#include #include -#include #include "utils/log.h" -/* Originally defined in system/core/libsystem/include/system/graphics.h */ -#define HAL_PRIORITY_URGENT_DISPLAY (-8) +/* Originally defined in system/core/libsystem/include/system/graphics.h as + * #define HAL_PRIORITY_URGENT_DISPLAY (-8)*/ +constexpr int kHalPriorityUrgentDisplay = -8; namespace android { UEventListener::UEventListener() - : Worker("uevent-listener", HAL_PRIORITY_URGENT_DISPLAY){}; + : Worker("uevent-listener", kHalPriorityUrgentDisplay){}; int UEventListener::Init() { - uevent_fd_ = UniqueFd( - socket(PF_NETLINK, SOCK_DGRAM | SOCK_CLOEXEC, NETLINK_KOBJECT_UEVENT)); - if (!uevent_fd_) { - // NOLINTNEXTLINE(concurrency-mt-unsafe): Fixme - ALOGE("Failed to open uevent socket: %s", strerror(errno)); - return -errno; - } - - struct sockaddr_nl addr {}; - addr.nl_family = AF_NETLINK; - addr.nl_pid = 0; - addr.nl_groups = 0xFFFFFFFF; - - int ret = bind(uevent_fd_.Get(), (struct sockaddr *)&addr, sizeof(addr)); - if (ret) { - // NOLINTNEXTLINE(concurrency-mt-unsafe): Fixme - ALOGE("Failed to bind uevent socket: %s", strerror(errno)); - return -errno; + uevent_ = UEvent::CreateInstance(); + if (!uevent_) { + return -ENODEV; } return InitWorker(); } void UEventListener::Routine() { - char buffer[1024]; - ssize_t ret = 0; - while (true) { - ret = read(uevent_fd_.Get(), &buffer, sizeof(buffer)); - if (ret == 0) - return; + auto uevent_str = uevent_->ReadNext(); - if (ret < 0) { - ALOGE("Got error reading uevent %zd", ret); - return; - } - - if (!hotplug_handler_) + if (!hotplug_handler_ || !uevent_str) continue; - bool drm_event = false; - bool hotplug_event = false; - for (uint32_t i = 0; (ssize_t)i < ret;) { - char *event = buffer + i; - if (strcmp(event, "DEVTYPE=drm_minor") != 0) - drm_event = true; - else if (strcmp(event, "HOTPLUG=1") != 0) - hotplug_event = true; - - i += strlen(event) + 1; - } + bool drm_event = uevent_str->find("DEVTYPE=drm_minor") != std::string::npos; + bool hotplug_event = uevent_str->find("HOTPLUG=1") != std::string::npos; - if (drm_event && hotplug_event && hotplug_handler_) { - constexpr useconds_t delay_after_uevent_us = 200000; + if (drm_event && hotplug_event) { + constexpr useconds_t kDelayAfterUeventUs = 200000; /* We need some delay to ensure DrmConnector::UpdateModes() will query * correct modes list, otherwise at least RPI4 board may report 0 modes */ - usleep(delay_after_uevent_us); + usleep(kDelayAfterUeventUs); hotplug_handler_(); } } diff --git a/drm/UEventListener.h b/drm/UEventListener.h index 0724443..c8b8582 100644 --- a/drm/UEventListener.h +++ b/drm/UEventListener.h @@ -19,7 +19,7 @@ #include -#include "utils/UniqueFd.h" +#include "utils/UEvent.h" #include "utils/Worker.h" namespace android { @@ -39,7 +39,7 @@ class UEventListener : public Worker { void Routine() override; private: - UniqueFd uevent_fd_; + std::unique_ptr uevent_; std::function hotplug_handler_; }; diff --git a/tests/Android.bp b/tests/Android.bp index 56f8c4f..e56ff5c 100644 --- a/tests/Android.bp +++ b/tests/Android.bp @@ -33,3 +33,17 @@ cc_test { "external/drm_hwcomposer/include", ], } + +// Tool for listening and dumping uevents +cc_test { + name: "hwc-drm-uevent-print", + + srcs: ["uevent_print.cpp"], + + vendor: true, + header_libs: ["libhardware_headers"], + shared_libs: ["liblog"], + include_dirs: [ + "external/drm_hwcomposer", + ], +} diff --git a/tests/uevent_print.cpp b/tests/uevent_print.cpp new file mode 100644 index 0000000..6ffbbfb --- /dev/null +++ b/tests/uevent_print.cpp @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: Apache-2.0 + +#include + +#include "utils/UEvent.h" + +int main() { + auto uevent = android::UEvent::CreateInstance(); + if (!uevent) { + std::cout << "Can't initialize UEvent class" << std::endl; + return -ENODEV; + } + + int number = 0; + for (;;) { + auto msg = uevent->ReadNext(); + if (!msg) { + continue; + } + + std::cout << "New event #" << number++ << std::endl + << *msg << std::endl + << std::endl; + } +} diff --git a/utils/UEvent.h b/utils/UEvent.h new file mode 100644 index 0000000..17b3cab --- /dev/null +++ b/utils/UEvent.h @@ -0,0 +1,85 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include + +#include +#include +#include +#include + +#include "UniqueFd.h" +#include "log.h" + +namespace android { + +class UEvent { + public: + static auto CreateInstance() -> std::unique_ptr { + auto fd = UniqueFd( + socket(PF_NETLINK, SOCK_DGRAM | SOCK_CLOEXEC, NETLINK_KOBJECT_UEVENT)); + + if (!fd) { + ALOGE("Failed to open uevent socket: errno=%i", errno); + return {}; + } + + struct sockaddr_nl addr {}; + addr.nl_family = AF_NETLINK; + addr.nl_pid = 0; + addr.nl_groups = UINT32_MAX; + + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) + int ret = bind(fd.Get(), (struct sockaddr *)&addr, sizeof(addr)); + if (ret != 0) { + ALOGE("Failed to bind uevent socket: errno=%i", errno); + return {}; + } + + return std::unique_ptr(new UEvent(fd)); + } + + auto ReadNext() -> std::optional { + constexpr int kUEventBufferSize = 1024; + char buffer[kUEventBufferSize]; + ssize_t ret = 0; + ret = read(fd_.Get(), &buffer, sizeof(buffer)); + if (ret == 0) + return {}; + + if (ret < 0) { + ALOGE("Got error reading uevent %zd", ret); + return {}; + } + + for (int i = 0; i < ret - 1; i++) { + if (buffer[i] == '\0') { + buffer[i] = '\n'; + } + } + + return std::string(buffer); + } + + private: + explicit UEvent(UniqueFd &fd) : fd_(std::move(fd)){}; + UniqueFd fd_; +}; + +} // namespace android -- cgit v1.2.3 From bb594baa1c68607c4c393571250add5141dd647e Mon Sep 17 00:00:00 2001 From: Roman Stratiienko Date: Fri, 18 Feb 2022 16:52:03 +0200 Subject: drm_hwcomposer: Rework HwcDisplay disposal to avoid races The code prior to this commit has a flaw: HwcDisplay::~HwcDisplay() { ... auto &main_lock = hwc2_->GetResMan().GetMainLock(); /* Unlock to allow pending vsync callbacks to finish */ main_lock.unlock(); At this point display is no longer in displays_[] list. After lock is released, hwc2 API thread starts to process transactions which may fail with BAD_SIAPLAY responce and cause SF to crash. vsync_worker_.VSyncControl(false); vsync_worker_.Exit(); main_lock.lock(); } 1. Rework the logic in order to avoid such scenariuos: 1.a. Temporary switch non-primary unplugged displays to headless state allowing remaining transactions to succeed without impacting the pipeline. 1.b. Give 100mSec delay before destroying / removing display from the displays_[] list to allow all pending hwc2 transactions to complete. 2. Support hotswap of the DrmDisplayPipeline, which makes primary display reattaching process smoother. Now SF should be able to gracefully remove all layers. Signed-off-by: Roman Stratiienko --- hwc2_device/DrmHwcTwo.cpp | 75 +++++++++++++++++++++++++++++----------------- hwc2_device/DrmHwcTwo.h | 1 + hwc2_device/HwcDisplay.cpp | 56 +++++++++++++--------------------- hwc2_device/HwcDisplay.h | 11 +++---- 4 files changed, 75 insertions(+), 68 deletions(-) diff --git a/hwc2_device/DrmHwcTwo.cpp b/hwc2_device/DrmHwcTwo.cpp index f1a5490..e689419 100644 --- a/hwc2_device/DrmHwcTwo.cpp +++ b/hwc2_device/DrmHwcTwo.cpp @@ -30,26 +30,22 @@ DrmHwcTwo::DrmHwcTwo() : resource_manager_(this){}; /* Must be called after every display attach/detach cycle */ void DrmHwcTwo::FinalizeDisplayBinding() { if (displays_.count(kPrimaryDisplay) == 0) { - /* Create/update new headless display if no other displays exists - * or reattach different display to make it primary - */ + /* Primary display MUST always exist */ + ALOGI("No pipelines available. Creating null-display for headless mode"); + displays_[kPrimaryDisplay] = std::make_unique< + HwcDisplay>(kPrimaryDisplay, HWC2::DisplayType::Physical, this); + /* Initializes null-display */ + displays_[kPrimaryDisplay]->SetPipeline(nullptr); + } - if (display_handles_.empty()) { - /* Enable headless mode */ - ALOGI("No pipelines available. Creating null-display for headless mode"); - displays_[kPrimaryDisplay] = std::make_unique< - HwcDisplay>(nullptr, kPrimaryDisplay, HWC2::DisplayType::Physical, - this); - } else { - auto *pipe = display_handles_.begin()->first; - ALOGI("Primary display was disconnected, reattaching '%s' as new primary", - pipe->connector->Get()->GetName().c_str()); - UnbindDisplay(pipe); - BindDisplay(pipe); - if (displays_.count(kPrimaryDisplay) == 0) { - ALOGE("FIXME!!! Still no primary display after reattaching..."); - } - } + if (displays_[kPrimaryDisplay]->IsInHeadlessMode() && + !display_handles_.empty()) { + /* Reattach first secondary display to take place of the primary */ + auto *pipe = display_handles_.begin()->first; + ALOGI("Primary display was disconnected, reattaching '%s' as new primary", + pipe->connector->Get()->GetName().c_str()); + UnbindDisplay(pipe); + BindDisplay(pipe); } // Finally, send hotplug events to the client @@ -57,6 +53,24 @@ void DrmHwcTwo::FinalizeDisplayBinding() { SendHotplugEventToClient(dhe.first, dhe.second); } deferred_hotplug_events_.clear(); + + /* Wait 0.2s before removing the displays to flush pending HWC2 transactions + */ + auto &mutex = GetResMan().GetMainLock(); + mutex.unlock(); + const int kTimeForSFToDisposeDisplayUs = 200000; + usleep(kTimeForSFToDisposeDisplayUs); + mutex.lock(); + std::vector> for_disposal; + for (auto handle : displays_for_removal_list_) { + for_disposal.emplace_back( + std::unique_ptr(displays_[handle].release())); + displays_.erase(handle); + } + /* Destroy HwcDisplays while unlocked to avoid vsyncworker deadlocks */ + mutex.unlock(); + for_disposal.clear(); + mutex.lock(); } bool DrmHwcTwo::BindDisplay(DrmDisplayPipeline *pipeline) { @@ -73,18 +87,17 @@ bool DrmHwcTwo::BindDisplay(DrmDisplayPipeline *pipeline) { disp_handle = ++last_display_handle_; } - auto disp = std::make_unique(pipeline, disp_handle, - HWC2::DisplayType::Physical, this); - - if (disp_handle == kPrimaryDisplay) { - displays_.erase(disp_handle); + if (displays_.count(disp_handle) == 0) { + auto disp = std::make_unique(disp_handle, + HWC2::DisplayType::Physical, this); + displays_[disp_handle] = std::move(disp); } ALOGI("Attaching pipeline '%s' to the display #%d%s", pipeline->connector->Get()->GetName().c_str(), (int)disp_handle, disp_handle == kPrimaryDisplay ? " (Primary)" : ""); - displays_[disp_handle] = std::move(disp); + displays_[disp_handle]->SetPipeline(pipeline); display_handles_[pipeline] = disp_handle; return true; @@ -96,17 +109,25 @@ bool DrmHwcTwo::UnbindDisplay(DrmDisplayPipeline *pipeline) { return false; } auto handle = display_handles_[pipeline]; + display_handles_.erase(pipeline); ALOGI("Detaching the pipeline '%s' from the display #%i%s", pipeline->connector->Get()->GetName().c_str(), (int)handle, handle == kPrimaryDisplay ? " (Primary)" : ""); - display_handles_.erase(pipeline); if (displays_.count(handle) == 0) { ALOGE("%s, can't find the display, handle: %" PRIu64, __func__, handle); return false; } - displays_.erase(handle); + displays_[handle]->SetPipeline(nullptr); + + /* We must defer display disposal and removal, since it may still have pending + * HWC_API calls scheduled and waiting until ueventlistener thread releases + * main lock, otherwise transaction may fail and SF may crash + */ + if (handle != kPrimaryDisplay) { + displays_for_removal_list_.emplace_back(handle); + } return true; } diff --git a/hwc2_device/DrmHwcTwo.h b/hwc2_device/DrmHwcTwo.h index 0e72251..2b8a74f 100644 --- a/hwc2_device/DrmHwcTwo.h +++ b/hwc2_device/DrmHwcTwo.h @@ -81,6 +81,7 @@ class DrmHwcTwo : public PipelineToFrontendBindingInterface { std::string mDumpString; std::map deferred_hotplug_events_; + std::vector displays_for_removal_list_; uint32_t last_display_handle_ = kPrimaryDisplay; }; diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp index 3f00fbf..cedac19 100644 --- a/hwc2_device/HwcDisplay.cpp +++ b/hwc2_device/HwcDisplay.cpp @@ -20,6 +20,7 @@ #include "HwcDisplay.h" #include "DrmHwcTwo.h" +#include "backend/Backend.h" #include "backend/BackendManager.h" #include "bufferinfo/BufferInfoGetter.h" #include "utils/log.h" @@ -27,9 +28,6 @@ namespace android { -// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) -uint32_t HwcDisplay::layer_idx_ = 2; /* Start from 2. See destroyLayer() */ - std::string HwcDisplay::DumpDelta(HwcDisplay::Stats delta) { if (delta.total_pixops_ == 0) return "No stats yet"; @@ -87,10 +85,9 @@ std::string HwcDisplay::Dump() { return ss.str(); } -HwcDisplay::HwcDisplay(DrmDisplayPipeline *pipeline, hwc2_display_t handle, - HWC2::DisplayType type, DrmHwcTwo *hwc2) +HwcDisplay::HwcDisplay(hwc2_display_t handle, HWC2::DisplayType type, + DrmHwcTwo *hwc2) : hwc2_(hwc2), - pipeline_(pipeline), handle_(handle), type_(type), color_transform_hint_(HAL_COLOR_TRANSFORM_IDENTITY) { @@ -100,24 +97,26 @@ HwcDisplay::HwcDisplay(DrmDisplayPipeline *pipeline, hwc2_display_t handle, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0}; // clang-format on +} - ChosePreferredConfig(); - Init(); +HwcDisplay::~HwcDisplay() = default; - hwc2_->ScheduleHotplugEvent(handle_, /*connected = */ true); -} +void HwcDisplay::SetPipeline(DrmDisplayPipeline *pipeline) { + pipeline_ = pipeline; -HwcDisplay::~HwcDisplay() { - if (handle_ != kPrimaryDisplay) { - hwc2_->ScheduleHotplugEvent(handle_, /*connected = */ false); - } + if (pipeline != nullptr) { + ChosePreferredConfig(); + Init(); - auto &main_lock = hwc2_->GetResMan().GetMainLock(); - /* Unlock to allow pending vsync callbacks to finish */ - main_lock.unlock(); - vsync_worker_.VSyncControl(false); - vsync_worker_.Exit(); - main_lock.lock(); + hwc2_->ScheduleHotplugEvent(handle_, /*connected = */ true); + } else { + backend_.reset(); + vsync_worker_.Init(nullptr, [](int64_t) {}); + SetClientTarget(nullptr, -1, 0, {}); + if (handle_ != kPrimaryDisplay) { + hwc2_->ScheduleHotplugEvent(handle_, /*connected = */ false); + } + } } HWC2::Error HwcDisplay::Init() { @@ -138,7 +137,7 @@ HWC2::Error HwcDisplay::Init() { vsync_worker_.VSyncControl(false); } }); - if (ret) { + if (ret && ret != -EALREADY) { ALOGE("Failed to create event worker for d=%d %d\n", int(handle_), ret); return HWC2::Error::BadDisplay; } @@ -185,21 +184,6 @@ HWC2::Error HwcDisplay::CreateLayer(hwc2_layer_t *layer) { HWC2::Error HwcDisplay::DestroyLayer(hwc2_layer_t layer) { if (!get_layer(layer)) { - /* Primary display don't send unplug event, instead it replaces - * display to headless or to another one and sends Plug event to the - * SF. SF can't distinguish this case from virtualized display size - * change case and will destroy previously used layers. If we will return - * BadLayer, service will print errors to the logcat. - * - * Nevertheless VTS is trying to destroy 1st layer without adding any - * layers prior to that, than it checks for BadLayer result. So we - * numbering the layers starting from 2, and use index 1 to catch VTS client - * to return BadLayer, making VTS pass. - */ - if (layers_.empty() && layer != 1) { - return HWC2::Error::None; - } - return HWC2::Error::BadLayer; } diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h index ae5d980..98d8e9b 100644 --- a/hwc2_device/HwcDisplay.h +++ b/hwc2_device/HwcDisplay.h @@ -37,11 +37,13 @@ inline constexpr uint32_t kPrimaryDisplay = 0; class HwcDisplay { public: - HwcDisplay(DrmDisplayPipeline *pipeline, hwc2_display_t handle, - HWC2::DisplayType type, DrmHwcTwo *hwc2); + HwcDisplay(hwc2_display_t handle, HWC2::DisplayType type, DrmHwcTwo *hwc2); HwcDisplay(const HwcDisplay &) = delete; ~HwcDisplay(); + /* SetPipeline should be carefully used only by DrmHwcTwo hotplug handlers */ + void SetPipeline(DrmDisplayPipeline *pipeline); + HWC2::Error CreateComposition(AtomicCommitArgs &a_args); std::vector GetOrderLayersByZPos(); @@ -199,7 +201,7 @@ class HwcDisplay { int64_t staged_mode_change_time_{}; uint32_t staged_mode_config_id_{}; - DrmDisplayPipeline *const pipeline_; + DrmDisplayPipeline *pipeline_{}; std::unique_ptr backend_; @@ -212,8 +214,7 @@ class HwcDisplay { const hwc2_display_t handle_; HWC2::DisplayType type_; - // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) - static uint32_t layer_idx_; + uint32_t layer_idx_{}; std::map layers_; HwcLayer client_layer_; -- cgit v1.2.3