From ecf26a14682d06729dfb68f516172bb09b326646 Mon Sep 17 00:00:00 2001 From: Tom Englund Date: Fri, 31 Jan 2025 10:04:40 +0100 Subject: [PATCH 1/3] core: make SDMABUFAttrs use CFileDescriptor change SDMABUFAttrs use CFileDescriptor for its fd, change to use const references to the attrs, and its related usage. requires compositor patch. --- include/aquamarine/allocator/DRMDumb.hpp | 2 +- include/aquamarine/allocator/GBM.hpp | 3 ++- include/aquamarine/buffer/Buffer.hpp | 22 ++++++++++++---------- src/allocator/DRMDumb.cpp | 5 +++-- src/allocator/GBM.cpp | 17 +++++++---------- src/backend/Wayland.cpp | 8 ++++---- src/backend/drm/DRM.cpp | 16 ++++++++-------- src/backend/drm/Renderer.cpp | 12 ++++++------ src/backend/drm/impl/Legacy.cpp | 8 ++++---- src/buffer/Buffer.cpp | 4 ++-- 10 files changed, 49 insertions(+), 48 deletions(-) diff --git a/include/aquamarine/allocator/DRMDumb.hpp b/include/aquamarine/allocator/DRMDumb.hpp index a5eca2c..d087b66 100644 --- a/include/aquamarine/allocator/DRMDumb.hpp +++ b/include/aquamarine/allocator/DRMDumb.hpp @@ -16,7 +16,7 @@ namespace Aquamarine { virtual void update(const Hyprutils::Math::CRegion& damage); virtual bool isSynchronous(); virtual bool good(); - virtual SDMABUFAttrs dmabuf(); + virtual const SDMABUFAttrs& dmabuf() const; virtual std::tuple beginDataPtr(uint32_t flags); virtual void endDataPtr(); diff --git a/include/aquamarine/allocator/GBM.hpp b/include/aquamarine/allocator/GBM.hpp index 09e6cb9..1452064 100644 --- a/include/aquamarine/allocator/GBM.hpp +++ b/include/aquamarine/allocator/GBM.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include "Allocator.hpp" struct gbm_device; @@ -19,7 +20,7 @@ namespace Aquamarine { virtual void update(const Hyprutils::Math::CRegion& damage); virtual bool isSynchronous(); virtual bool good(); - virtual SDMABUFAttrs dmabuf(); + virtual const SDMABUFAttrs& dmabuf() const; virtual std::tuple beginDataPtr(uint32_t flags); virtual void endDataPtr(); diff --git a/include/aquamarine/buffer/Buffer.hpp b/include/aquamarine/buffer/Buffer.hpp index a96008f..0b52b6f 100644 --- a/include/aquamarine/buffer/Buffer.hpp +++ b/include/aquamarine/buffer/Buffer.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include "../misc/Attachment.hpp" namespace Aquamarine { @@ -19,15 +20,15 @@ namespace Aquamarine { }; struct SDMABUFAttrs { - bool success = false; - Hyprutils::Math::Vector2D size; - uint32_t format = 0; // fourcc - uint64_t modifier = 0; + bool success = false; + Hyprutils::Math::Vector2D size; + uint32_t format = 0; // fourcc + uint64_t modifier = 0; - int planes = 1; - std::array offsets = {0}; - std::array strides = {0}; - std::array fds = {-1, -1, -1, -1}; + int planes = 1; + std::array offsets = {0}; + std::array strides = {0}; + std::array fds; }; struct SSHMAttrs { @@ -50,7 +51,7 @@ namespace Aquamarine { virtual void update(const Hyprutils::Math::CRegion& damage) = 0; virtual bool isSynchronous() = 0; // whether the updates to this buffer are synchronous, aka happen over cpu virtual bool good() = 0; - virtual SDMABUFAttrs dmabuf(); + virtual const SDMABUFAttrs& dmabuf() const; virtual SSHMAttrs shm(); virtual std::tuple beginDataPtr(uint32_t flags); virtual void endDataPtr(); @@ -71,7 +72,8 @@ namespace Aquamarine { } events; private: - int locks = 0; + SDMABUFAttrs m_attrs{}; + int locks = 0; }; }; diff --git a/src/allocator/DRMDumb.cpp b/src/allocator/DRMDumb.cpp index ca4d911..7fbad36 100644 --- a/src/allocator/DRMDumb.cpp +++ b/src/allocator/DRMDumb.cpp @@ -14,6 +14,7 @@ using namespace Aquamarine; using namespace Hyprutils::Memory; +using namespace Hyprutils::OS; #define SP CSharedPointer #define WP CWeakPointer @@ -53,7 +54,7 @@ Aquamarine::CDRMDumbBuffer::CDRMDumbBuffer(const SAllocatorBufferParams& params, return; } - attrs.fds.at(0) = primeFD; + attrs.fds.at(0) = CFileDescriptor{primeFD}; attrs.success = true; @@ -94,7 +95,7 @@ bool Aquamarine::CDRMDumbBuffer::good() { return attrs.success && data; } -SDMABUFAttrs Aquamarine::CDRMDumbBuffer::dmabuf() { +const SDMABUFAttrs& Aquamarine::CDRMDumbBuffer::dmabuf() const { return attrs; } diff --git a/src/allocator/GBM.cpp b/src/allocator/GBM.cpp index d5f69db..fa53bc8 100644 --- a/src/allocator/GBM.cpp +++ b/src/allocator/GBM.cpp @@ -12,6 +12,7 @@ using namespace Aquamarine; using namespace Hyprutils::Memory; +using namespace Hyprutils::OS; #define SP CSharedPointer static SDRMFormat guessFormatFrom(std::vector formats, bool cursor, bool scanout) { @@ -195,13 +196,13 @@ Aquamarine::CGBMBuffer::CGBMBuffer(const SAllocatorBufferParams& params, Hypruti for (size_t i = 0; i < (size_t)attrs.planes; ++i) { attrs.strides.at(i) = gbm_bo_get_stride_for_plane(bo, i); attrs.offsets.at(i) = gbm_bo_get_offset(bo, i); - attrs.fds.at(i) = gbm_bo_get_fd_for_plane(bo, i); + attrs.fds.at(i) = CFileDescriptor{gbm_bo_get_fd_for_plane(bo, i)}; - if (attrs.fds.at(i) < 0) { + if (!attrs.fds.at(i).isValid()) { allocator->backend->log(AQ_LOG_ERROR, std::format("GBM: Failed to query fd for plane {}", i)); - for (size_t j = 0; j < i; ++j) { - close(attrs.fds.at(j)); - } + for (size_t j = 0; j < i; ++j) + attrs.fds.at(j).reset(); + attrs.planes = 0; return; } @@ -226,10 +227,6 @@ Aquamarine::CGBMBuffer::CGBMBuffer(const SAllocatorBufferParams& params, Hypruti } Aquamarine::CGBMBuffer::~CGBMBuffer() { - for (size_t i = 0; i < (size_t)attrs.planes; i++) { - close(attrs.fds.at(i)); - } - events.destroy.emit(); if (bo) { if (gboMapping) @@ -258,7 +255,7 @@ bool Aquamarine::CGBMBuffer::good() { return bo; } -SDMABUFAttrs Aquamarine::CGBMBuffer::dmabuf() { +const SDMABUFAttrs& Aquamarine::CGBMBuffer::dmabuf() const { return attrs; } diff --git a/src/backend/Wayland.cpp b/src/backend/Wayland.cpp index f7648cc..c8f2790 100644 --- a/src/backend/Wayland.cpp +++ b/src/backend/Wayland.cpp @@ -709,11 +709,11 @@ bool Aquamarine::CWaylandOutput::setCursor(Hyprutils::Memory::CSharedPointerdmabuf(); attrs.success) { + } else if (const auto& attrs = buffer->dmabuf(); attrs.success) { auto params = makeShared(backend->waylandState.dmabuf->sendCreateParams()); for (int i = 0; i < attrs.planes; ++i) { - params->sendAdd(attrs.fds.at(i), i, attrs.offsets.at(i), attrs.strides.at(i), attrs.modifier >> 32, attrs.modifier & 0xFFFFFFFF); + params->sendAdd(attrs.fds.at(i).get(), i, attrs.offsets.at(i), attrs.strides.at(i), attrs.modifier >> 32, attrs.modifier & 0xFFFFFFFF); } cursorState.cursorWlBuffer = makeShared(params->sendCreateImmed(attrs.size.x, attrs.size.y, attrs.format, (zwpLinuxBufferParamsV1Flags)0)); @@ -781,10 +781,10 @@ Aquamarine::CWaylandBuffer::CWaylandBuffer(SP buffer_, Hyprutils::Memor return; } - auto attrs = buffer->dmabuf(); + const auto& attrs = buffer->dmabuf(); for (int i = 0; i < attrs.planes; ++i) { - params->sendAdd(attrs.fds.at(i), i, attrs.offsets.at(i), attrs.strides.at(i), attrs.modifier >> 32, attrs.modifier & 0xFFFFFFFF); + params->sendAdd(attrs.fds.at(i).get(), i, attrs.offsets.at(i), attrs.strides.at(i), attrs.modifier >> 32, attrs.modifier & 0xFFFFFFFF); } waylandState.buffer = makeShared(params->sendCreateImmed(attrs.size.x, attrs.size.y, attrs.format, (zwpLinuxBufferParamsV1Flags)0)); diff --git a/src/backend/drm/DRM.cpp b/src/backend/drm/DRM.cpp index b7b123b..ddf6790 100644 --- a/src/backend/drm/DRM.cpp +++ b/src/backend/drm/DRM.cpp @@ -1650,9 +1650,9 @@ bool Aquamarine::CDRMOutput::commitState(bool onlyTest) { mgpu.swapchain = CSwapchain::create(backend->rendererState.allocator, backend.lock()); } - auto OPTIONS = swapchain->currentOptions(); - auto bufDma = STATE.buffer->dmabuf(); - OPTIONS.size = STATE.buffer->size; + auto OPTIONS = swapchain->currentOptions(); + const auto& bufDma = STATE.buffer->dmabuf(); + OPTIONS.size = STATE.buffer->size; if (OPTIONS.format == DRM_FORMAT_INVALID) OPTIONS.format = bufDma.format; OPTIONS.multigpu = false; // this is not a shared swapchain, and additionally, don't make it linear, nvidia would be mad @@ -1699,7 +1699,7 @@ bool Aquamarine::CDRMOutput::commitState(bool onlyTest) { // sometimes, our consumer could f up the swapchain format and change it without the state changing bool formatMismatch = false; if (data.mainFB) { - if (const auto params = data.mainFB->buffer->dmabuf(); params.success && params.format != STATE.drmFormat) { + if (const auto& params = data.mainFB->buffer->dmabuf(); params.success && params.format != STATE.drmFormat) { // formats mismatch. Update the state format and roll with it backend->backend->log(AQ_LOG_WARNING, std::format("drm: Formats mismatch in commit, buffer is {} but output is set to {}. Modesetting to {}", fourccToName(params.format), @@ -1988,7 +1988,7 @@ Aquamarine::CDRMFB::CDRMFB(SP buffer_, Hyprutils::Memory::CWeakPointer< } void Aquamarine::CDRMFB::import() { - auto attrs = buffer->dmabuf(); + const auto& attrs = buffer->dmabuf(); if (!attrs.success) { backend->backend->log(AQ_LOG_ERROR, "drm: Buffer submitted has no dmabuf or a drm handle"); return; @@ -2001,14 +2001,14 @@ void Aquamarine::CDRMFB::import() { // TODO: check format for (int i = 0; i < attrs.planes; ++i) { - int ret = drmPrimeFDToHandle(backend->gpu->fd, attrs.fds.at(i), &boHandles[i]); + int ret = drmPrimeFDToHandle(backend->gpu->fd, attrs.fds.at(i).get(), &boHandles[i]); if (ret) { backend->backend->log(AQ_LOG_ERROR, "drm: drmPrimeFDToHandle failed"); drop(); return; } - TRACE(backend->backend->log(AQ_LOG_TRACE, std::format("drm: CDRMFB: plane {} has fd {}, got handle {}", i, attrs.fds.at(i), boHandles.at(i)))); + TRACE(backend->backend->log(AQ_LOG_TRACE, std::format("drm: CDRMFB: plane {} has fd {}, got handle {}", i, attrs.fds.at(i).get(), boHandles.at(i)))); } id = submitBuffer(); @@ -2101,7 +2101,7 @@ uint32_t Aquamarine::CDRMFB::submitBuffer() { if (!buffer->dmabuf().success) return 0; - auto attrs = buffer->dmabuf(); + const auto& attrs = buffer->dmabuf(); std::array mods = {0, 0, 0, 0}; for (int i = 0; i < attrs.planes; ++i) { mods[i] = attrs.modifier; diff --git a/src/backend/drm/Renderer.cpp b/src/backend/drm/Renderer.cpp index b0c7203..c74d409 100644 --- a/src/backend/drm/Renderer.cpp +++ b/src/backend/drm/Renderer.cpp @@ -637,7 +637,7 @@ EGLImageKHR CDRMRenderer::createEGLImage(const SDMABUFAttrs& attrs) { for (int i = 0; i < attrs.planes; i++) { attribs.push_back(attrNames[i].fd); - attribs.push_back(attrs.fds[i]); + attribs.push_back(attrs.fds[i].get()); attribs.push_back(attrNames[i].offset); attribs.push_back(attrs.offsets[i]); attribs.push_back(attrNames[i].pitch); @@ -665,9 +665,9 @@ EGLImageKHR CDRMRenderer::createEGLImage(const SDMABUFAttrs& attrs) { } SGLTex CDRMRenderer::glTex(Hyprutils::Memory::CSharedPointer buffa) { - SGLTex tex; + SGLTex tex; - const auto dma = buffa->dmabuf(); + const auto& dma = buffa->dmabuf(); tex.image = createEGLImage(dma); if (tex.image == EGL_NO_IMAGE_KHR) { @@ -785,8 +785,8 @@ int CDRMRenderer::recreateBlitSync() { void CDRMRenderer::clearBuffer(IBuffer* buf) { setEGL(); - auto dmabuf = buf->dmabuf(); - GLuint rboID = 0, fboID = 0; + const auto& dmabuf = buf->dmabuf(); + GLuint rboID = 0, fboID = 0; if (!dmabuf.success) { backend->log(AQ_LOG_ERROR, "EGL (clear): cannot clear a non-dmabuf"); @@ -874,7 +874,7 @@ CDRMRenderer::SBlitResult CDRMRenderer::blit(SP from, SP to, i EGLImageKHR rboImage = nullptr; GLuint rboID = 0, fboID = 0; - auto toDma = to->dmabuf(); + const auto& toDma = to->dmabuf(); if (!verifyDestinationDMABUF(toDma)) { backend->log(AQ_LOG_ERROR, "EGL (blit): failed to blit: destination dmabuf unsupported"); diff --git a/src/backend/drm/impl/Legacy.cpp b/src/backend/drm/impl/Legacy.cpp index e9ac9a2..d51e24f 100644 --- a/src/backend/drm/impl/Legacy.cpp +++ b/src/backend/drm/impl/Legacy.cpp @@ -87,17 +87,17 @@ bool Aquamarine::CDRMLegacyImpl::commitInternal(Hyprutils::Memory::CSharedPointe if (data.cursorFB && connector->crtc->cursor && connector->output->cursorVisible && enable && (STATE.committed & COutputState::AQ_OUTPUT_STATE_CURSOR_SHAPE || STATE.committed & COutputState::AQ_OUTPUT_STATE_CURSOR_POS)) { - uint32_t boHandle = 0; - auto attrs = data.cursorFB->buffer->dmabuf(); + uint32_t boHandle = 0; + const auto& attrs = data.cursorFB->buffer->dmabuf(); - if (int ret = drmPrimeFDToHandle(connector->backend->gpu->fd, attrs.fds.at(0), &boHandle); ret) { + if (int ret = drmPrimeFDToHandle(connector->backend->gpu->fd, attrs.fds.at(0).get(), &boHandle); ret) { connector->backend->backend->log(AQ_LOG_ERROR, std::format("legacy drm: drmPrimeFDToHandle failed: {}", strerror(-ret))); return false; } connector->backend->backend->log(AQ_LOG_DEBUG, std::format("legacy drm: cursor fb: {} with bo handle {} from fd {}, size {}", connector->backend->gpu->fd, boHandle, - data.cursorFB->buffer->dmabuf().fds.at(0), data.cursorFB->buffer->size)); + data.cursorFB->buffer->dmabuf().fds.at(0).get(), data.cursorFB->buffer->size)); Vector2D cursorPos = connector->output->cursorPos; diff --git a/src/buffer/Buffer.cpp b/src/buffer/Buffer.cpp index 8197742..6d9b725 100644 --- a/src/buffer/Buffer.cpp +++ b/src/buffer/Buffer.cpp @@ -3,8 +3,8 @@ using namespace Aquamarine; -SDMABUFAttrs Aquamarine::IBuffer::dmabuf() { - return SDMABUFAttrs{}; +const SDMABUFAttrs& Aquamarine::IBuffer::dmabuf() const { + return m_attrs; } SSHMAttrs Aquamarine::IBuffer::shm() { From 5d7d34e4aa3ff03a39264300c141a35d26ee76ca Mon Sep 17 00:00:00 2001 From: Tom Englund Date: Mon, 3 Feb 2025 12:18:04 +0100 Subject: [PATCH 2/3] fences: make explicit fences use CFileDescriptor make explicit fences use CFileDescriptors. --- include/aquamarine/output/Output.hpp | 5 +++-- src/backend/Wayland.cpp | 2 +- src/backend/drm/DRM.cpp | 6 +++--- src/backend/drm/Renderer.cpp | 24 ++++++++++++------------ src/backend/drm/Renderer.hpp | 15 ++++++++------- src/backend/drm/impl/Atomic.cpp | 4 ++-- src/output/Output.cpp | 9 +++++---- 7 files changed, 34 insertions(+), 31 deletions(-) diff --git a/include/aquamarine/output/Output.hpp b/include/aquamarine/output/Output.hpp index c09f29b..2c9f869 100644 --- a/include/aquamarine/output/Output.hpp +++ b/include/aquamarine/output/Output.hpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include "../allocator/Swapchain.hpp" @@ -74,7 +75,7 @@ namespace Aquamarine { Hyprutils::Memory::CSharedPointer customMode; uint32_t drmFormat = DRM_FORMAT_INVALID; Hyprutils::Memory::CSharedPointer buffer; - int32_t explicitInFence = -1, explicitOutFence = -1; + Hyprutils::OS::CFileDescriptor explicitInFence, explicitOutFence; Hyprutils::Math::Mat3x3 ctm; bool wideColorGamut = false; hdr_output_metadata hdrMetadata; @@ -94,7 +95,7 @@ namespace Aquamarine { void setCustomMode(Hyprutils::Memory::CSharedPointer mode); void setFormat(uint32_t drmFormat); void setBuffer(Hyprutils::Memory::CSharedPointer buffer); - void setExplicitInFence(int32_t fenceFD); // -1 removes + void setExplicitInFence(Hyprutils::OS::CFileDescriptor&& fenceFD); void enableExplicitOutFenceForNextCommit(); void resetExplicitFences(); void setCTM(const Hyprutils::Math::Mat3x3& ctm); diff --git a/src/backend/Wayland.cpp b/src/backend/Wayland.cpp index c8f2790..f443de4 100644 --- a/src/backend/Wayland.cpp +++ b/src/backend/Wayland.cpp @@ -239,7 +239,7 @@ Aquamarine::CWaylandPointer::CWaylandPointer(SP pointer_, Hyprutils backend->backend->log(AQ_LOG_DEBUG, "New wayland pointer wl_pointer"); pointer->setMotion([this](CCWlPointer* r, uint32_t serial, wl_fixed_t x, wl_fixed_t y) { - const auto STATE = backend->focusedOutput->state->state(); + const auto& STATE = backend->focusedOutput->state->state(); if (!backend->focusedOutput || (!STATE.mode && !STATE.customMode)) return; diff --git a/src/backend/drm/DRM.cpp b/src/backend/drm/DRM.cpp index ddf6790..670685c 100644 --- a/src/backend/drm/DRM.cpp +++ b/src/backend/drm/DRM.cpp @@ -1665,7 +1665,7 @@ bool Aquamarine::CDRMOutput::commitState(bool onlyTest) { 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 : -1); + STATE.buffer, NEWAQBUF, (COMMITTED & COutputState::eOutputStateProperties::AQ_OUTPUT_STATE_EXPLICIT_IN_FENCE) ? STATE.explicitInFence.get() : -1); if (!blitResult.success) { backend->backend->log(AQ_LOG_ERROR, "drm: Backend requires blit, but blit failed"); return false; @@ -1675,9 +1675,9 @@ bool Aquamarine::CDRMOutput::commitState(bool onlyTest) { // if the commit doesn't have an explicit fence, don't use the one we created, just fallback to implicit static auto NO_EXPLICIT = envEnabled("AQ_MGPU_NO_EXPLICIT"); if (blitResult.syncFD.has_value() && !NO_EXPLICIT && (COMMITTED & COutputState::eOutputStateProperties::AQ_OUTPUT_STATE_EXPLICIT_IN_FENCE)) - state->setExplicitInFence(blitResult.syncFD.value()); + state->setExplicitInFence(std::move(blitResult.syncFD.value())); else - state->setExplicitInFence(-1); + state->setExplicitInFence({}); drmFB = CDRMFB::create(NEWAQBUF, backend, nullptr); // will return attachment if present } else diff --git a/src/backend/drm/Renderer.cpp b/src/backend/drm/Renderer.cpp index c74d409..733139d 100644 --- a/src/backend/drm/Renderer.cpp +++ b/src/backend/drm/Renderer.cpp @@ -13,6 +13,7 @@ using namespace Aquamarine; using namespace Hyprutils::Memory; using namespace Hyprutils::Math; +using namespace Hyprutils::OS; #define SP CSharedPointer #define WP CWeakPointer @@ -740,27 +741,26 @@ void CDRMRenderer::waitOnSync(int fd) { TRACE(backend->log(AQ_LOG_TRACE, "EGL (waitOnSync): failed to destroy sync")); } -int CDRMRenderer::recreateBlitSync() { +CFileDescriptor CDRMRenderer::recreateBlitSync() { TRACE(backend->log(AQ_LOG_TRACE, "EGL (recreateBlitSync): recreating blit sync")); if (egl.lastBlitSync) { - TRACE(backend->log(AQ_LOG_TRACE, std::format("EGL (recreateBlitSync): cleaning up old sync (fd {})", egl.lastBlitSyncFD))); + TRACE(backend->log(AQ_LOG_TRACE, std::format("EGL (recreateBlitSync): cleaning up old sync (fd {})", egl.lastBlitSyncFD.get()))); // cleanup last sync if (proc.eglDestroySyncKHR(egl.display, egl.lastBlitSync) != EGL_TRUE) TRACE(backend->log(AQ_LOG_TRACE, "EGL (recreateBlitSync): failed to destroy old sync")); - if (egl.lastBlitSyncFD >= 0) - close(egl.lastBlitSyncFD); + if (egl.lastBlitSyncFD.isValid()) + egl.lastBlitSyncFD.reset(); - egl.lastBlitSyncFD = -1; - egl.lastBlitSync = nullptr; + egl.lastBlitSync = nullptr; } EGLSyncKHR sync = proc.eglCreateSyncKHR(egl.display, EGL_SYNC_NATIVE_FENCE_ANDROID, nullptr); if (sync == EGL_NO_SYNC_KHR) { TRACE(backend->log(AQ_LOG_TRACE, "EGL (recreateBlitSync): failed to create an egl sync for explicit")); - return -1; + return {}; } // we need to flush otherwise we might not get a valid fd @@ -771,15 +771,15 @@ int CDRMRenderer::recreateBlitSync() { TRACE(backend->log(AQ_LOG_TRACE, "EGL (recreateBlitSync): failed to dup egl fence fd")); if (proc.eglDestroySyncKHR(egl.display, sync) != EGL_TRUE) TRACE(backend->log(AQ_LOG_TRACE, "EGL (recreateBlitSync): failed to destroy new sync")); - return -1; + return {}; } egl.lastBlitSync = sync; - egl.lastBlitSyncFD = fd; + egl.lastBlitSyncFD = CFileDescriptor{fd}; TRACE(backend->log(AQ_LOG_TRACE, std::format("EGL (recreateBlitSync): success, new fence exported with fd {}", fd))); - return fd; + return egl.lastBlitSyncFD.duplicate(); } void CDRMRenderer::clearBuffer(IBuffer* buf) { @@ -992,14 +992,14 @@ CDRMRenderer::SBlitResult CDRMRenderer::blit(SP from, SP to, i // get an explicit sync fd for the secondary gpu. // when we pass buffers between gpus we should always use explicit sync, // as implicit is not guaranteed at all - int explicitFD = recreateBlitSync(); + auto explicitFD = recreateBlitSync(); GLCALL(glBindFramebuffer(GL_FRAMEBUFFER, 0)); GLCALL(glBindRenderbuffer(GL_RENDERBUFFER, 0)); restoreEGL(); - return {.success = true, .syncFD = explicitFD == -1 ? std::nullopt : std::optional{explicitFD}}; + return {.success = true, .syncFD = explicitFD.isValid() ? std::nullopt : std::optional{std::move(explicitFD)}}; } void CDRMRenderer::onBufferAttachmentDrop(CDRMRendererBufferAttachment* attachment) { diff --git a/src/backend/drm/Renderer.hpp b/src/backend/drm/Renderer.hpp index af560d9..4fb18a9 100644 --- a/src/backend/drm/Renderer.hpp +++ b/src/backend/drm/Renderer.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include "FormatUtils.hpp" #include #include @@ -51,8 +52,8 @@ namespace Aquamarine { int drmFD = -1; struct SBlitResult { - bool success = false; - std::optional syncFD; + bool success = false; + std::optional syncFD; }; SBlitResult blit(Hyprutils::Memory::CSharedPointer from, Hyprutils::Memory::CSharedPointer to, int waitFD = -1); @@ -101,10 +102,10 @@ namespace Aquamarine { } exts; struct { - EGLDisplay display = nullptr; - EGLContext context = nullptr; - EGLSync lastBlitSync = nullptr; - int lastBlitSyncFD = -1; + EGLDisplay display = nullptr; + EGLContext context = nullptr; + EGLSync lastBlitSync = nullptr; + Hyprutils::OS::CFileDescriptor lastBlitSyncFD; } egl; struct { @@ -124,7 +125,7 @@ namespace Aquamarine { EGLImageKHR createEGLImage(const SDMABUFAttrs& attrs); bool verifyDestinationDMABUF(const SDMABUFAttrs& attrs); void waitOnSync(int fd); - int recreateBlitSync(); + Hyprutils::OS::CFileDescriptor recreateBlitSync(); void loadEGLAPI(); EGLDeviceEXT eglDeviceFromDRMFD(int drmFD); diff --git a/src/backend/drm/impl/Atomic.cpp b/src/backend/drm/impl/Atomic.cpp index 2ca4b01..7b5475b 100644 --- a/src/backend/drm/impl/Atomic.cpp +++ b/src/backend/drm/impl/Atomic.cpp @@ -128,8 +128,8 @@ void Aquamarine::CDRMAtomicRequest::addConnector(Hyprutils::Memory::CSharedPoint planeProps(connector->crtc->primary, data.mainFB, connector->crtc->id, {}); - if (connector->output->supportsExplicit && STATE.explicitInFence >= 0) - add(connector->crtc->primary->id, connector->crtc->primary->props.in_fence_fd, STATE.explicitInFence); + if (connector->output->supportsExplicit && STATE.explicitInFence.isValid()) + add(connector->crtc->primary->id, connector->crtc->primary->props.in_fence_fd, STATE.explicitInFence.get()); if (connector->crtc->primary->props.fb_damage_clips) add(connector->crtc->primary->id, connector->crtc->primary->props.fb_damage_clips, data.atomic.fbDamage); diff --git a/src/output/Output.cpp b/src/output/Output.cpp index 00262b2..ba924cc 100644 --- a/src/output/Output.cpp +++ b/src/output/Output.cpp @@ -1,6 +1,7 @@ #include using namespace Aquamarine; +using namespace Hyprutils::OS; Aquamarine::IOutput::~IOutput() { events.destroy.emit(); @@ -108,8 +109,8 @@ void Aquamarine::COutputState::setBuffer(Hyprutils::Memory::CSharedPointer Date: Mon, 3 Feb 2025 14:19:37 +0100 Subject: [PATCH 3/3] 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();