From 273300ea81405dce36e060897a1c7e1b7f15bdc8 Mon Sep 17 00:00:00 2001 From: Tom Englund Date: Mon, 3 Feb 2025 14:19:37 +0100 Subject: [PATCH] allocators: make some internal FD use CFileDescriptors make some internal AQ fd handling using CFileDescriptor instead. --- include/aquamarine/allocator/GBM.hpp | 6 ++--- include/aquamarine/backend/Backend.hpp | 3 ++- include/aquamarine/backend/DRM.hpp | 1 + src/allocator/GBM.cpp | 24 ++++++++---------- src/backend/Backend.cpp | 35 +++++++++++++------------- src/backend/drm/DRM.cpp | 8 +++--- src/backend/drm/Renderer.cpp | 19 +++++++------- src/backend/drm/Renderer.hpp | 4 +-- 8 files changed, 50 insertions(+), 50 deletions(-) diff --git a/include/aquamarine/allocator/GBM.hpp b/include/aquamarine/allocator/GBM.hpp index 1452064..7e057a9 100644 --- a/include/aquamarine/allocator/GBM.hpp +++ b/include/aquamarine/allocator/GBM.hpp @@ -41,7 +41,7 @@ namespace Aquamarine { class CGBMAllocator : public IAllocator { public: ~CGBMAllocator(); - static Hyprutils::Memory::CSharedPointer create(int drmfd_, Hyprutils::Memory::CWeakPointer backend_); + static Hyprutils::Memory::CSharedPointer create(Hyprutils::OS::CFileDescriptor&& drmfd_, Hyprutils::Memory::CWeakPointer backend_); virtual Hyprutils::Memory::CSharedPointer acquire(const SAllocatorBufferParams& params, Hyprutils::Memory::CSharedPointer swapchain_); virtual Hyprutils::Memory::CSharedPointer getBackend(); @@ -53,12 +53,12 @@ namespace Aquamarine { Hyprutils::Memory::CWeakPointer self; private: - CGBMAllocator(int fd_, Hyprutils::Memory::CWeakPointer backend_); + CGBMAllocator(Hyprutils::OS::CFileDescriptor&& fd_, Hyprutils::Memory::CWeakPointer backend_); // a vector purely for tracking (debugging) the buffers and nothing more std::vector> buffers; - int fd = -1; + Hyprutils::OS::CFileDescriptor fd; Hyprutils::Memory::CWeakPointer backend; // gbm stuff diff --git a/include/aquamarine/backend/Backend.hpp b/include/aquamarine/backend/Backend.hpp index 7da8039..1916657 100644 --- a/include/aquamarine/backend/Backend.hpp +++ b/include/aquamarine/backend/Backend.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -115,7 +116,7 @@ namespace Aquamarine { void removeIdleEvent(Hyprutils::Memory::CSharedPointer> pfn); // utils - int reopenDRMNode(int drmFD, bool allowRenderNode = true); + Hyprutils::OS::CFileDescriptor reopenDRMNode(int drmFD, bool allowRenderNode = true); // called when a new DRM card is hotplugged void onNewGpu(std::string path); diff --git a/include/aquamarine/backend/DRM.hpp b/include/aquamarine/backend/DRM.hpp index 9f7f718..d74e1ae 100644 --- a/include/aquamarine/backend/DRM.hpp +++ b/include/aquamarine/backend/DRM.hpp @@ -5,6 +5,7 @@ #include "../output/Output.hpp" #include "../input/Input.hpp" #include +#include #include #include #include diff --git a/src/allocator/GBM.cpp b/src/allocator/GBM.cpp index fa53bc8..df75d19 100644 --- a/src/allocator/GBM.cpp +++ b/src/allocator/GBM.cpp @@ -287,44 +287,40 @@ CGBMAllocator::~CGBMAllocator() { if (!gbmDevice) return; - int fd = gbm_device_get_fd(gbmDevice); + CFileDescriptor fd{gbm_device_get_fd(gbmDevice)}; gbm_device_destroy(gbmDevice); - - if (fd < 0) - return; - - close(fd); } -SP Aquamarine::CGBMAllocator::create(int drmfd_, Hyprutils::Memory::CWeakPointer backend_) { +SP Aquamarine::CGBMAllocator::create(CFileDescriptor&& drmfd_, Hyprutils::Memory::CWeakPointer backend_) { uint64_t capabilities = 0; - if (drmGetCap(drmfd_, DRM_CAP_PRIME, &capabilities) || !(capabilities & DRM_PRIME_CAP_EXPORT)) { + if (drmGetCap(drmfd_.get(), DRM_CAP_PRIME, &capabilities) || !(capabilities & DRM_PRIME_CAP_EXPORT)) { backend_->log(AQ_LOG_ERROR, "Cannot create a GBM Allocator: PRIME export is not supported by the gpu."); return nullptr; } - auto allocator = SP(new CGBMAllocator(drmfd_, backend_)); + auto allocator = SP(new CGBMAllocator(std::move(drmfd_), backend_)); if (!allocator->gbmDevice) { backend_->log(AQ_LOG_ERROR, "Cannot create a GBM Allocator: gbm failed to create a device."); return nullptr; } - backend_->log(AQ_LOG_DEBUG, std::format("Created a GBM allocator with drm fd {}", drmfd_)); + backend_->log(AQ_LOG_DEBUG, std::format("Created a GBM allocator with drm fd {}", allocator->fd.get())); allocator->self = allocator; return allocator; } -Aquamarine::CGBMAllocator::CGBMAllocator(int fd_, Hyprutils::Memory::CWeakPointer backend_) : fd(fd_), backend(backend_), gbmDevice(gbm_create_device(fd_)) { +Aquamarine::CGBMAllocator::CGBMAllocator(CFileDescriptor&& fd_, Hyprutils::Memory::CWeakPointer backend_) : + fd(std::move(fd_)), backend(backend_), gbmDevice(gbm_create_device(fd.get())) { if (!gbmDevice) { - backend->log(AQ_LOG_ERROR, std::format("Couldn't open a GBM device at fd {}", fd_)); + backend->log(AQ_LOG_ERROR, std::format("Couldn't open a GBM device at fd {}", fd.get())); return; } gbmDeviceBackendName = gbm_device_get_backend_name(gbmDevice); - auto drmName_ = drmGetDeviceNameFromFd2(fd_); + auto drmName_ = drmGetDeviceNameFromFd2(fd.get()); drmName = drmName_; free(drmName_); } @@ -352,7 +348,7 @@ Hyprutils::Memory::CSharedPointer Aquamarine::CGBMAllocator::getBacken } int Aquamarine::CGBMAllocator::drmFD() { - return fd; + return fd.get(); } eAllocatorType Aquamarine::CGBMAllocator::type() { diff --git a/src/backend/Backend.cpp b/src/backend/Backend.cpp index 8bb9f47..74ced7d 100644 --- a/src/backend/Backend.cpp +++ b/src/backend/Backend.cpp @@ -12,6 +12,7 @@ #include using namespace Hyprutils::Memory; +using namespace Hyprutils::OS; using namespace Aquamarine; #define SP CSharedPointer @@ -145,12 +146,12 @@ bool Aquamarine::CBackend::start() { for (auto const& b : implementations) { if (b->drmFD() >= 0) { auto fd = reopenDRMNode(b->drmFD()); - if (fd < 0) { + if (!fd.isValid()) { // this is critical, we cannot create an allocator properly log(AQ_LOG_CRITICAL, "Failed to create an allocator (reopenDRMNode failed)"); return false; } - primaryAllocator = CGBMAllocator::create(fd, self); + primaryAllocator = CGBMAllocator::create(std::move(fd), self); break; } } @@ -291,17 +292,17 @@ void Aquamarine::CBackend::onNewGpu(std::string path) { // Yoinked from wlroots, render/allocator/allocator.c // Ref-counting reasons, see https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110 -int Aquamarine::CBackend::reopenDRMNode(int drmFD, bool allowRenderNode) { +CFileDescriptor Aquamarine::CBackend::reopenDRMNode(int drmFD, bool allowRenderNode) { if (drmIsMaster(drmFD)) { // Only recent kernels support empty leases - uint32_t lesseeID = 0; - int leaseFD = drmModeCreateLease(drmFD, nullptr, 0, O_CLOEXEC, &lesseeID); - if (leaseFD >= 0) { + uint32_t lesseeID = 0; + CFileDescriptor leaseFD{drmModeCreateLease(drmFD, nullptr, 0, O_CLOEXEC, &lesseeID)}; + if (leaseFD.isValid()) { return leaseFD; - } else if (leaseFD != -EINVAL && leaseFD != -EOPNOTSUPP) { + } else if (leaseFD.get() != -EINVAL && leaseFD.get() != -EOPNOTSUPP) { log(AQ_LOG_ERROR, "reopenDRMNode: drmModeCreateLease failed"); - return -1; + return {}; } log(AQ_LOG_DEBUG, "reopenDRMNode: drmModeCreateLease failed, falling back to open"); } @@ -316,34 +317,32 @@ int Aquamarine::CBackend::reopenDRMNode(int drmFD, bool allowRenderNode) { if (!name) { log(AQ_LOG_ERROR, "reopenDRMNode: drmGetDeviceNameFromFd2 failed"); - return -1; + return {}; } } log(AQ_LOG_DEBUG, std::format("reopenDRMNode: opening node {}", name)); - int newFD = open(name, O_RDWR | O_CLOEXEC); - if (newFD < 0) { + CFileDescriptor newFD{open(name, O_RDWR | O_CLOEXEC)}; + if (!newFD.isValid()) { log(AQ_LOG_ERROR, std::format("reopenDRMNode: failed to open node {}", name)); free(name); - return -1; + return {}; } free(name); // We need to authenticate if we are using a DRM primary node and are the master - if (drmIsMaster(drmFD) && drmGetNodeTypeFromFd(newFD) == DRM_NODE_PRIMARY) { + if (drmIsMaster(drmFD) && drmGetNodeTypeFromFd(newFD.get()) == DRM_NODE_PRIMARY) { drm_magic_t magic; - if (int ret = drmGetMagic(newFD, &magic); ret < 0) { + if (int ret = drmGetMagic(newFD.get(), &magic); ret < 0) { log(AQ_LOG_ERROR, std::format("reopenDRMNode: drmGetMagic failed: {}", strerror(-ret))); - close(newFD); - return -1; + return {}; } if (int ret = drmAuthMagic(drmFD, magic); ret < 0) { log(AQ_LOG_ERROR, std::format("reopenDRMNode: drmAuthMagic failed: {}", strerror(-ret))); - close(newFD); - return -1; + return {}; } } diff --git a/src/backend/drm/DRM.cpp b/src/backend/drm/DRM.cpp index 670685c..f95246f 100644 --- a/src/backend/drm/DRM.cpp +++ b/src/backend/drm/DRM.cpp @@ -37,6 +37,7 @@ extern "C" { using namespace Aquamarine; using namespace Hyprutils::Memory; using namespace Hyprutils::Math; +using namespace Hyprutils::OS; #define SP CSharedPointer Aquamarine::CDRMBackend::CDRMBackend(SP backend_) : backend(backend_) { @@ -1663,9 +1664,10 @@ bool Aquamarine::CDRMOutput::commitState(bool onlyTest) { return false; } - auto NEWAQBUF = mgpu.swapchain->next(nullptr); - auto blitResult = backend->rendererState.renderer->blit( - STATE.buffer, NEWAQBUF, (COMMITTED & COutputState::eOutputStateProperties::AQ_OUTPUT_STATE_EXPLICIT_IN_FENCE) ? STATE.explicitInFence.get() : -1); + auto NEWAQBUF = mgpu.swapchain->next(nullptr); + const CFileDescriptor defaultFd; + auto blitResult = backend->rendererState.renderer->blit( + STATE.buffer, NEWAQBUF, (COMMITTED & COutputState::eOutputStateProperties::AQ_OUTPUT_STATE_EXPLICIT_IN_FENCE) ? STATE.explicitInFence : defaultFd); if (!blitResult.success) { backend->backend->log(AQ_LOG_ERROR, "drm: Backend requires blit, but blit failed"); return false; diff --git a/src/backend/drm/Renderer.cpp b/src/backend/drm/Renderer.cpp index 733139d..f28752f 100644 --- a/src/backend/drm/Renderer.cpp +++ b/src/backend/drm/Renderer.cpp @@ -706,27 +706,28 @@ inline const float fullVerts[] = { 0, 1, // bottom left }; -void CDRMRenderer::waitOnSync(int fd) { - TRACE(backend->log(AQ_LOG_TRACE, std::format("EGL (waitOnSync): attempting to wait on fd {}", fd))); +void CDRMRenderer::waitOnSync(const CFileDescriptor& fd) { + TRACE(backend->log(AQ_LOG_TRACE, std::format("EGL (waitOnSync): attempting to wait on fd {}", fd.get()))); std::vector attribs; - int dupFd = fcntl(fd, F_DUPFD_CLOEXEC, 0); - if (dupFd < 0) { + //int dupFd = fcntl(fd, F_DUPFD_CLOEXEC, 0); + auto dupFd = fd.duplicate(); + if (!dupFd.isValid()) { backend->log(AQ_LOG_TRACE, "EGL (waitOnSync): failed to dup fd for wait"); return; } attribs.push_back(EGL_SYNC_NATIVE_FENCE_FD_ANDROID); - attribs.push_back(dupFd); + attribs.push_back(dupFd.get()); attribs.push_back(EGL_NONE); EGLSyncKHR sync = proc.eglCreateSyncKHR(egl.display, EGL_SYNC_NATIVE_FENCE_ANDROID, attribs.data()); if (sync == EGL_NO_SYNC_KHR) { TRACE(backend->log(AQ_LOG_TRACE, "EGL (waitOnSync): failed to create an egl sync for explicit")); - if (dupFd >= 0) - close(dupFd); return; } + if (dupFd.isValid()) + dupFd.take(); // eglCreateSyncKHR takes ownership on success. // we got a sync, now we just tell egl to wait before sampling if (proc.eglWaitSyncKHR(egl.display, sync, 0) != EGL_TRUE) { @@ -828,7 +829,7 @@ void CDRMRenderer::clearBuffer(IBuffer* buf) { restoreEGL(); } -CDRMRenderer::SBlitResult CDRMRenderer::blit(SP from, SP to, int waitFD) { +CDRMRenderer::SBlitResult CDRMRenderer::blit(SP from, SP to, const CFileDescriptor& waitFD) { setEGL(); if (from->dmabuf().size != to->dmabuf().size) { @@ -836,7 +837,7 @@ CDRMRenderer::SBlitResult CDRMRenderer::blit(SP from, SP to, i return {}; } - if (waitFD >= 0) { + if (waitFD.isValid()) { // wait on a provided explicit fence waitOnSync(waitFD); } diff --git a/src/backend/drm/Renderer.hpp b/src/backend/drm/Renderer.hpp index 4fb18a9..e8b0aef 100644 --- a/src/backend/drm/Renderer.hpp +++ b/src/backend/drm/Renderer.hpp @@ -56,7 +56,7 @@ namespace Aquamarine { std::optional syncFD; }; - SBlitResult blit(Hyprutils::Memory::CSharedPointer from, Hyprutils::Memory::CSharedPointer to, int waitFD = -1); + SBlitResult blit(Hyprutils::Memory::CSharedPointer from, Hyprutils::Memory::CSharedPointer to, const Hyprutils::OS::CFileDescriptor& waitFD = {}); // can't be a SP<> because we call it from buf's ctor... void clearBuffer(IBuffer* buf); @@ -124,7 +124,7 @@ namespace Aquamarine { EGLImageKHR createEGLImage(const SDMABUFAttrs& attrs); bool verifyDestinationDMABUF(const SDMABUFAttrs& attrs); - void waitOnSync(int fd); + void waitOnSync(const Hyprutils::OS::CFileDescriptor& fd); Hyprutils::OS::CFileDescriptor recreateBlitSync(); void loadEGLAPI();