From 4c6000b3e1829da5f0ccb81bbccfee37fb28cd24 Mon Sep 17 00:00:00 2001 From: Xaver Hugl Date: Sat, 6 Apr 2024 18:51:01 +0200 Subject: [PATCH] core/syncobjtimeline: make explicit sync use SYNC_IOC_MERGE instead of waiting on the CPU side This brings some performance benefits, because the application can potentially reuse the buffer earlier, and it simplifies the code a bit --- src/core/syncobjtimeline.cpp | 63 +++++++++++++++++++++++++------ src/core/syncobjtimeline.h | 28 ++++---------- src/scene/itemrenderer_opengl.cpp | 5 ++- src/wayland/surface.h | 6 ++- 4 files changed, 67 insertions(+), 35 deletions(-) diff --git a/src/core/syncobjtimeline.cpp b/src/core/syncobjtimeline.cpp index 1918a0475c..437902644e 100644 --- a/src/core/syncobjtimeline.cpp +++ b/src/core/syncobjtimeline.cpp @@ -6,8 +6,24 @@ #include "syncobjtimeline.h" #include +#include #include +#if defined(Q_OS_LINUX) +#include +#else +struct sync_merge_data +{ + char name[32]; + __s32 fd2; + __s32 fence; + __u32 flags; + __u32 pad; +}; +#define SYNC_IOC_MAGIC '>' +#define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 3, struct sync_merge_data) +#endif + namespace KWin { @@ -19,7 +35,39 @@ SyncReleasePoint::SyncReleasePoint(const std::shared_ptr &timeline SyncReleasePoint::~SyncReleasePoint() { - m_timeline->signal(m_timelinePoint); + if (m_releaseFence.isValid()) { + m_timeline->moveInto(m_timelinePoint, m_releaseFence); + } else { + m_timeline->signal(m_timelinePoint); + } +} + +static FileDescriptor mergeSyncFds(const FileDescriptor &fd1, const FileDescriptor &fd2) +{ + struct sync_merge_data data + { + .name = "merged release fence", + .fd2 = fd2.get(), + .fence = -1, + }; + int err = -1; + do { + err = ioctl(fd1.get(), SYNC_IOC_MERGE, &data); + } while (err == -1 && (errno == EINTR || errno == EAGAIN)); + if (err < 0) { + return FileDescriptor{}; + } else { + return FileDescriptor(data.fence); + } +} + +void SyncReleasePoint::addReleaseFence(const FileDescriptor &fd) +{ + if (m_releaseFence.isValid()) { + m_releaseFence = mergeSyncFds(m_releaseFence, fd); + } else { + m_releaseFence = fd.duplicate(); + } } SyncTimeline *SyncReleasePoint::timeline() const @@ -60,17 +108,8 @@ void SyncTimeline::signal(uint64_t timelinePoint) drmSyncobjTimelineSignal(m_drmFd, &m_handle, &timelinePoint, 1); } -SyncReleasePointHolder::SyncReleasePointHolder(FileDescriptor &&requirement, std::unordered_set> &&releasePoints) - : m_fence(std::move(requirement)) - , m_notifier(m_fence.get(), QSocketNotifier::Type::Read) - , m_releasePoints(std::move(releasePoints)) -{ - connect(&m_notifier, &QSocketNotifier::activated, this, &SyncReleasePointHolder::signaled); - m_notifier.setEnabled(true); -} - -void SyncReleasePointHolder::signaled() +void SyncTimeline::moveInto(uint64_t timelinePoint, const FileDescriptor &fd) { - delete this; + drmSyncobjImportSyncFile(m_drmFd, m_handle, fd.get()); } } diff --git a/src/core/syncobjtimeline.h b/src/core/syncobjtimeline.h index cffdb75e33..4ced3e0627 100644 --- a/src/core/syncobjtimeline.h +++ b/src/core/syncobjtimeline.h @@ -7,11 +7,8 @@ #include "kwin_export.h" #include "utils/filedescriptor.h" -#include -#include #include #include -#include namespace KWin { @@ -30,9 +27,16 @@ public: SyncTimeline *timeline() const; uint64_t timelinePoint() const; + /** + * Adds the fence of a graphics job that this release point should wait for + * before the timeline point is signaled + */ + void addReleaseFence(const FileDescriptor &fd); + private: const std::shared_ptr m_timeline; const uint64_t m_timelinePoint; + FileDescriptor m_releaseFence; }; class KWIN_EXPORT SyncTimeline @@ -47,26 +51,10 @@ public: FileDescriptor eventFd(uint64_t timelinePoint) const; void signal(uint64_t timelinePoint); + void moveInto(uint64_t timelinePoint, const FileDescriptor &fd); private: const int32_t m_drmFd; const uint32_t m_handle; }; - -class SyncReleasePointHolder : public QObject -{ - Q_OBJECT -public: - /** - * @param requirement the filedescriptor that needs to be readable before the release points may be signalled. Once that's happened, this object deletes itself!' - */ - explicit SyncReleasePointHolder(FileDescriptor &&requirement, std::unordered_set> &&releasePoints); - -private: - void signaled(); - - const FileDescriptor m_fence; - QSocketNotifier m_notifier; - const std::unordered_set> m_releasePoints; -}; } diff --git a/src/scene/itemrenderer_opengl.cpp b/src/scene/itemrenderer_opengl.cpp index 90e287ead1..8e9f351810 100644 --- a/src/scene/itemrenderer_opengl.cpp +++ b/src/scene/itemrenderer_opengl.cpp @@ -53,7 +53,10 @@ void ItemRendererOpenGL::endFrame() if (m_eglDisplay) { EGLNativeFence fence(m_eglDisplay); if (fence.isValid()) { - new SyncReleasePointHolder(std::move(fence.fileDescriptor()), std::move(m_releasePoints)); + for (const auto &releasePoint : m_releasePoints) { + releasePoint->addReleaseFence(fence.fileDescriptor()); + } + m_releasePoints.clear(); } } m_releasePoints.clear(); diff --git a/src/wayland/surface.h b/src/wayland/surface.h index 80d1fa5570..152f1accc1 100644 --- a/src/wayland/surface.h +++ b/src/wayland/surface.h @@ -344,8 +344,10 @@ public: void setPreferredColorDescription(const ColorDescription &descr); /** - * @returns the release point that should be referenced as long as the buffer on this surface - * is, or may still be used by the compositor + * Returns the current release point for the buffer on this surface. The buffer keeps the + * release point referenced as long as it's referenced itself; for synchronization on the + * GPU side, the compositor has to either keep the release point referenced as long as the + * GPU task is running, or add a fence for each GPU task to the release point */ std::shared_ptr bufferReleasePoint() const;