drm_hwcomposer: Introduce SharedFd, use standard c++ RAII for UniqueFd
We use too much dup() system calls for present fence propagating.
Also when propagating acquire fence we use additional logic for
skipping such propagation for the validate/test cycle.
Both issues can be solved by introducing SharedFd, which will track
reference count of fd object.
After that the UniqueFd is used very rarely and can be simplified by
wrapping it into std::unique_ptr without caring too much of adding
an extra malloc/free operation.
Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>
diff --git a/drm/DrmAtomicStateManager.cpp b/drm/DrmAtomicStateManager.cpp
index b5e4629..58a5523 100644
--- a/drm/DrmAtomicStateManager.cpp
+++ b/drm/DrmAtomicStateManager.cpp
@@ -141,7 +141,7 @@
uint32_t flags = DRM_MODE_ATOMIC_ALLOW_MODESET;
if (args.test_only) {
- return drmModeAtomicCommit(drm->GetFd(), pset.get(),
+ return drmModeAtomicCommit(*drm->GetFd(), pset.get(),
flags | DRM_MODE_ATOMIC_TEST_ONLY, drm);
}
@@ -150,10 +150,10 @@
ATRACE_NAME("WaitPriorFramePresented");
constexpr int kTimeoutMs = 500;
- const int err = sync_wait(last_present_fence_.Get(), kTimeoutMs);
+ const int err = sync_wait(*last_present_fence_, kTimeoutMs);
if (err != 0) {
- ALOGE("sync_wait(fd=%i) returned: %i (errno: %i)",
- last_present_fence_.Get(), err, errno);
+ ALOGE("sync_wait(fd=%i) returned: %i (errno: %i)", *last_present_fence_,
+ err, errno);
}
CleanupPriorFrameResources();
@@ -163,17 +163,19 @@
flags |= DRM_MODE_ATOMIC_NONBLOCK;
}
- auto err = drmModeAtomicCommit(drm->GetFd(), pset.get(), flags, drm);
+ auto err = drmModeAtomicCommit(*drm->GetFd(), pset.get(), flags, drm);
if (err != 0) {
ALOGE("Failed to commit pset ret=%d\n", err);
return err;
}
+ args.out_fence = MakeSharedFd(out_fence);
+
if (nonblock) {
{
const std::unique_lock lock(mutex_);
- last_present_fence_ = UniqueFd::Dup(out_fence);
+ last_present_fence_ = args.out_fence;
staged_frame_state_ = std::move(new_frame_state);
frames_staged_++;
}
@@ -182,8 +184,6 @@
active_frame_state_ = std::move(new_frame_state);
}
- args.out_fence = UniqueFd(out_fence);
-
return 0;
}
@@ -193,7 +193,7 @@
auto &main_mutex = pipe_->device->GetResMan().GetMainLock();
for (;;) {
- UniqueFd present_fence;
+ SharedFd present_fence;
{
std::unique_lock lk(mutex_);
@@ -207,7 +207,7 @@
tracking_at_the_moment = frames_staged_;
- present_fence = UniqueFd::Dup(last_present_fence_.Get());
+ present_fence = last_present_fence_;
if (!present_fence)
continue;
}
@@ -216,10 +216,10 @@
// NOLINTNEXTLINE(misc-const-correctness)
ATRACE_NAME("AsyncWaitForBuffersSwap");
constexpr int kTimeoutMs = 500;
- auto err = sync_wait(present_fence.Get(), kTimeoutMs);
+ auto err = sync_wait(*present_fence, kTimeoutMs);
if (err != 0) {
- ALOGE("sync_wait(fd=%i) returned: %i (errno: %i)", present_fence.Get(),
- err, errno);
+ ALOGE("sync_wait(fd=%i) returned: %i (errno: %i)", *present_fence, err,
+ errno);
}
}
@@ -272,7 +272,7 @@
} // namespace android
auto DrmAtomicStateManager::ActivateDisplayUsingDPMS() -> int {
- return drmModeConnectorSetProperty(pipe_->device->GetFd(),
+ return drmModeConnectorSetProperty(*pipe_->device->GetFd(),
pipe_->connector->Get()->GetId(),
pipe_->connector->Get()
->GetDpmsProperty()
diff --git a/drm/DrmAtomicStateManager.h b/drm/DrmAtomicStateManager.h
index 5f19bcc..e456a91 100644
--- a/drm/DrmAtomicStateManager.h
+++ b/drm/DrmAtomicStateManager.h
@@ -37,7 +37,7 @@
std::shared_ptr<DrmKmsPlan> composition;
/* out */
- UniqueFd out_fence;
+ SharedFd out_fence;
/* helpers */
auto HasInputs() -> bool {
@@ -95,7 +95,7 @@
void CleanupPriorFrameResources();
KmsState staged_frame_state_;
- UniqueFd last_present_fence_;
+ SharedFd last_present_fence_;
int frames_staged_{};
int frames_tracked_{};
diff --git a/drm/DrmConnector.cpp b/drm/DrmConnector.cpp
index 07c5d74..f625563 100644
--- a/drm/DrmConnector.cpp
+++ b/drm/DrmConnector.cpp
@@ -63,7 +63,7 @@
auto DrmConnector::CreateInstance(DrmDevice &dev, uint32_t connector_id,
uint32_t index)
-> std::unique_ptr<DrmConnector> {
- auto conn = MakeDrmModeConnectorUnique(dev.GetFd(), connector_id);
+ auto conn = MakeDrmModeConnectorUnique(*dev.GetFd(), connector_id);
if (!conn) {
ALOGE("Failed to get connector %d", connector_id);
return {};
@@ -109,7 +109,7 @@
return {};
}
- return MakeDrmModePropertyBlobUnique(drm_->GetFd(), *blob_id);
+ return MakeDrmModePropertyBlobUnique(*drm_->GetFd(), *blob_id);
}
bool DrmConnector::IsInternal() const {
@@ -158,7 +158,7 @@
}
int DrmConnector::UpdateModes() {
- auto conn = MakeDrmModeConnectorUnique(drm_->GetFd(), 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 b54f14b..3b749b1 100644
--- a/drm/DrmCrtc.cpp
+++ b/drm/DrmCrtc.cpp
@@ -35,7 +35,7 @@
auto DrmCrtc::CreateInstance(DrmDevice &dev, uint32_t crtc_id, uint32_t index)
-> std::unique_ptr<DrmCrtc> {
- auto crtc = MakeDrmModeCrtcUnique(dev.GetFd(), 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 33adf2a..1d6b62e 100644
--- a/drm/DrmDevice.cpp
+++ b/drm/DrmDevice.cpp
@@ -55,46 +55,46 @@
auto DrmDevice::Init(const char *path) -> int {
/* TODO: Use drmOpenControl here instead */
- fd_ = UniqueFd(open(path, O_RDWR | O_CLOEXEC));
+ fd_ = MakeSharedFd(open(path, O_RDWR | O_CLOEXEC));
if (!fd_) {
// NOLINTNEXTLINE(concurrency-mt-unsafe): Fixme
ALOGE("Failed to open dri %s: %s", path, strerror(errno));
return -ENODEV;
}
- int ret = drmSetClientCap(GetFd(), DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+ int ret = drmSetClientCap(*GetFd(), DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
if (ret != 0) {
ALOGE("Failed to set universal plane cap %d", ret);
return ret;
}
- ret = drmSetClientCap(GetFd(), DRM_CLIENT_CAP_ATOMIC, 1);
+ ret = drmSetClientCap(*GetFd(), DRM_CLIENT_CAP_ATOMIC, 1);
if (ret != 0) {
ALOGE("Failed to set atomic cap %d", ret);
return ret;
}
#ifdef DRM_CLIENT_CAP_WRITEBACK_CONNECTORS
- ret = drmSetClientCap(GetFd(), DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
+ ret = drmSetClientCap(*GetFd(), DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
if (ret != 0) {
ALOGI("Failed to set writeback cap %d", ret);
}
#endif
uint64_t cap_value = 0;
- if (drmGetCap(GetFd(), DRM_CAP_ADDFB2_MODIFIERS, &cap_value) != 0) {
+ 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(GetFd());
- if (drmIsMaster(GetFd()) == 0) {
+ drmSetMaster(*GetFd());
+ if (drmIsMaster(*GetFd()) == 0) {
ALOGE("DRM/KMS master access required");
return -EACCES;
}
- auto res = MakeDrmModeResUnique(GetFd());
+ auto res = MakeDrmModeResUnique(*GetFd());
if (!res) {
ALOGE("Failed to get DrmDevice resources");
return -ENODEV;
@@ -136,7 +136,7 @@
}
}
- auto plane_res = MakeDrmModePlaneResUnique(GetFd());
+ auto plane_res = MakeDrmModePlaneResUnique(*GetFd());
if (!plane_res) {
ALOGE("Failed to get plane resources");
return -ENOENT;
@@ -161,7 +161,7 @@
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast)
create_blob.data = (__u64)data;
- auto ret = drmIoctl(GetFd(), DRM_IOCTL_MODE_CREATEPROPBLOB, &create_blob);
+ auto ret = drmIoctl(*GetFd(), DRM_IOCTL_MODE_CREATEPROPBLOB, &create_blob);
if (ret != 0) {
ALOGE("Failed to create mode property blob %d", ret);
return {};
@@ -171,7 +171,7 @@
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;
- auto err = drmIoctl(GetFd(), DRM_IOCTL_MODE_DESTROYPROPBLOB,
+ auto err = drmIoctl(*GetFd(), DRM_IOCTL_MODE_DESTROYPROPBLOB,
&destroy_blob);
if (err != 0) {
ALOGE("Failed to destroy mode property blob %" PRIu32 "/%d", *it,
@@ -186,7 +186,7 @@
const char *prop_name, DrmProperty *property) const {
drmModeObjectPropertiesPtr props = nullptr;
- props = drmModeObjectGetProperties(GetFd(), obj_id, obj_type);
+ props = drmModeObjectGetProperties(*GetFd(), obj_id, obj_type);
if (props == nullptr) {
ALOGE("Failed to get properties for %d/%x", obj_id, obj_type);
return -ENODEV;
@@ -195,7 +195,7 @@
bool found = false;
for (int i = 0; !found && (size_t)i < props->count_props; ++i) {
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
- drmModePropertyPtr p = drmModeGetProperty(GetFd(), props->props[i]);
+ 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]);
@@ -209,9 +209,9 @@
}
std::string DrmDevice::GetName() const {
- auto *ver = drmGetVersion(GetFd());
+ auto *ver = drmGetVersion(*GetFd());
if (ver == nullptr) {
- ALOGW("Failed to get drm version for fd=%d", GetFd());
+ ALOGW("Failed to get drm version for fd=%d", *GetFd());
return "generic";
}
@@ -221,12 +221,12 @@
}
auto DrmDevice::IsKMSDev(const char *path) -> bool {
- auto fd = UniqueFd(open(path, O_RDWR | O_CLOEXEC));
+ auto fd = MakeUniqueFd(open(path, O_RDWR | O_CLOEXEC));
if (!fd) {
return false;
}
- auto res = MakeDrmModeResUnique(fd.Get());
+ auto res = MakeDrmModeResUnique(*fd);
if (!res) {
return false;
}
diff --git a/drm/DrmDevice.h b/drm/DrmDevice.h
index bb515e0..39d0c88 100644
--- a/drm/DrmDevice.h
+++ b/drm/DrmDevice.h
@@ -23,7 +23,7 @@
#include "DrmConnector.h"
#include "DrmCrtc.h"
#include "DrmEncoder.h"
-#include "utils/UniqueFd.h"
+#include "utils/fd.h"
namespace android {
@@ -38,8 +38,8 @@
static auto CreateInstance(std::string const &path, ResourceManager *res_man)
-> std::unique_ptr<DrmDevice>;
- auto GetFd() const {
- return fd_.Get();
+ auto &GetFd() const {
+ return fd_;
}
auto &GetResMan() {
@@ -101,7 +101,7 @@
static auto IsKMSDev(const char *path) -> bool;
- UniqueFd fd_;
+ SharedFd fd_;
std::vector<std::unique_ptr<DrmConnector>> connectors_;
std::vector<std::unique_ptr<DrmConnector>> writeback_connectors_;
diff --git a/drm/DrmEncoder.cpp b/drm/DrmEncoder.cpp
index eed5b5f..21ca693 100644
--- a/drm/DrmEncoder.cpp
+++ b/drm/DrmEncoder.cpp
@@ -29,7 +29,7 @@
auto DrmEncoder::CreateInstance(DrmDevice &dev, uint32_t encoder_id,
uint32_t index) -> std::unique_ptr<DrmEncoder> {
- auto e = MakeDrmModeEncoderUnique(dev.GetFd(), 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 f0be32f..a91a52b 100644
--- a/drm/DrmFbImporter.cpp
+++ b/drm/DrmFbImporter.cpp
@@ -50,7 +50,7 @@
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.GetFd(), 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],
@@ -74,11 +74,11 @@
/* Create framebuffer object */
if (!has_modifiers) {
- err = drmModeAddFB2(drm.GetFd(), bo->width, bo->height, bo->format,
+ err = drmModeAddFB2(*drm.GetFd(), bo->width, bo->height, bo->format,
local->gem_handles_.data(), &bo->pitches[0],
&bo->offsets[0], &local->fb_id_, 0);
} else {
- err = drmModeAddFB2WithModifiers(drm.GetFd(), bo->width, bo->height,
+ err = drmModeAddFB2WithModifiers(*drm.GetFd(), bo->width, bo->height,
bo->format, local->gem_handles_.data(),
&bo->pitches[0], &bo->offsets[0],
&bo->modifiers[0], &local->fb_id_,
@@ -97,7 +97,7 @@
ATRACE_NAME("Close FB and dmabufs");
/* Destroy framebuffer object */
- if (drmModeRmFB(drm_->GetFd(), fb_id_) != 0) {
+ if (drmModeRmFB(*drm_->GetFd(), fb_id_) != 0) {
ALOGE("Failed to rm fb");
}
@@ -118,7 +118,7 @@
continue;
}
gem_close.handle = gem_handles_[i];
- auto err = drmIoctl(drm_->GetFd(), DRM_IOCTL_GEM_CLOSE, &gem_close);
+ auto 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);
}
@@ -129,7 +129,8 @@
-> std::shared_ptr<DrmFbIdHandle> {
/* Lookup DrmFbIdHandle in cache first. First handle serves as a cache key. */
GemHandle first_handle = 0;
- auto err = drmPrimeFDToHandle(drm_->GetFd(), bo->prime_fds[0], &first_handle);
+ auto 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);
diff --git a/drm/DrmPlane.cpp b/drm/DrmPlane.cpp
index 4d03840..228e3dd 100644
--- a/drm/DrmPlane.cpp
+++ b/drm/DrmPlane.cpp
@@ -31,7 +31,7 @@
auto DrmPlane::CreateInstance(DrmDevice &dev, uint32_t plane_id)
-> std::unique_ptr<DrmPlane> {
- auto p = MakeDrmModePlaneUnique(dev.GetFd(), plane_id);
+ auto p = MakeDrmModePlaneUnique(*dev.GetFd(), plane_id);
if (!p) {
ALOGE("Failed to get plane %d", plane_id);
return {};
@@ -261,7 +261,7 @@
}
if (layer.acquire_fence &&
- !in_fence_fd_property_.AtomicSet(pset, layer.acquire_fence.Get())) {
+ !in_fence_fd_property_.AtomicSet(pset, *layer.acquire_fence)) {
return -EINVAL;
}
diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp
index 77b9d98..8a251c7 100644
--- a/drm/VSyncWorker.cpp
+++ b/drm/VSyncWorker.cpp
@@ -40,7 +40,7 @@
if (pipe != nullptr) {
vsw->high_crtc_ = pipe->crtc->Get()->GetIndexInResArray()
<< DRM_VBLANK_HIGH_CRTC_SHIFT;
- vsw->drm_fd_ = UniqueFd::Dup(pipe->device->GetFd());
+ vsw->drm_fd_ = pipe->device->GetFd();
}
std::thread(&VSyncWorker::ThreadFn, vsw.get(), vsw).detach();
@@ -143,7 +143,7 @@
DRM_VBLANK_HIGH_CRTC_MASK));
vblank.request.sequence = 1;
- ret = drmWaitVBlank(drm_fd_.Get(), &vblank);
+ ret = drmWaitVBlank(*drm_fd_, &vblank);
if (ret == -EINTR)
continue;
}
diff --git a/drm/VSyncWorker.h b/drm/VSyncWorker.h
index 2a0b084..031a561 100644
--- a/drm/VSyncWorker.h
+++ b/drm/VSyncWorker.h
@@ -52,7 +52,7 @@
VSyncWorkerCallbacks callbacks_;
- UniqueFd drm_fd_;
+ SharedFd drm_fd_;
uint32_t high_crtc_ = 0;
bool enabled_ = false;