Skip to content

Commit

Permalink
syncobj: use eventfd instead of stalling fd checks
Browse files Browse the repository at this point in the history
use eventfd and add it to the event loop and when it recieves a signal
release the buffer, this means we dont stall entire compositor when
waiting for materilisation of the fd. and change its related usage, this
also means we need to store release points that can popup in a container
and signal them all on buffer release.

im not sure why directscanout previously manually tried to signal
releasepoints. remove that and let the buffers releaser handle it.
  • Loading branch information
gulafaran committed Feb 22, 2025
1 parent f4b148d commit 46dbd22
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 78 deletions.
16 changes: 2 additions & 14 deletions src/helpers/Monitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1315,10 +1315,10 @@ bool CMonitor::attemptDirectScanout() {
auto explicitOptions = g_pHyprRenderer->getExplicitSyncSettings(output);

// wait for the explicit fence if present, and if kms explicit is allowed
bool DOEXPLICIT = PSURFACE->syncobj && PSURFACE->syncobj->current.acquireTimeline && PSURFACE->syncobj->current.acquireTimeline->timeline && explicitOptions.explicitKMSEnabled;
bool DOEXPLICIT = PSURFACE->syncobj && PSURFACE->syncobj->acquire.resource && PSURFACE->syncobj->acquire.resource->timeline && explicitOptions.explicitKMSEnabled;
CFileDescriptor explicitWaitFD;
if (DOEXPLICIT) {
explicitWaitFD = PSURFACE->syncobj->current.acquireTimeline->timeline->exportAsSyncFileFD(PSURFACE->syncobj->current.acquirePoint);
explicitWaitFD = PSURFACE->syncobj->acquire.resource->timeline->exportAsSyncFileFD(PSURFACE->syncobj->acquire.point);
if (!explicitWaitFD.isValid())
Debug::log(TRACE, "attemptDirectScanout: failed to acquire an explicit wait fd");
}
Expand Down Expand Up @@ -1352,18 +1352,6 @@ bool CMonitor::attemptDirectScanout() {
lastScanout = PCANDIDATE;
Debug::log(LOG, "Entered a direct scanout to {:x}: \"{}\"", (uintptr_t)PCANDIDATE.get(), PCANDIDATE->m_szTitle);
}

// delay explicit sync feedback until kms release of the buffer
if (DOEXPLICIT) {
Debug::log(TRACE, "attemptDirectScanout: Delaying explicit sync release feedback until kms release");
PSURFACE->current.buffer->releaser->drop();

PSURFACE->current.buffer->buffer->hlEvents.backendRelease2 = PSURFACE->current.buffer->buffer->events.backendRelease.registerListener([PSURFACE](std::any d) {
const bool DOEXPLICIT = PSURFACE->syncobj && PSURFACE->syncobj->current.releaseTimeline && PSURFACE->syncobj->current.releaseTimeline->timeline;
if (DOEXPLICIT)
PSURFACE->syncobj->current.releaseTimeline->timeline->signal(PSURFACE->syncobj->current.releasePoint);
});
}
} else {
Debug::log(TRACE, "attemptDirectScanout: failed to scanout surface");
lastScanout.reset();
Expand Down
4 changes: 0 additions & 4 deletions src/helpers/sync/SyncReleaser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,3 @@ CSyncReleaser::~CSyncReleaser() {
void CSyncReleaser::addReleaseSync(SP<CEGLSync> sync_) {
sync = sync_;
}

void CSyncReleaser::drop() {
timeline.reset();
}
3 changes: 0 additions & 3 deletions src/helpers/sync/SyncReleaser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ class CSyncReleaser {
CSyncReleaser(WP<CSyncTimeline> timeline_, uint64_t point_);
~CSyncReleaser();

// drops the releaser, will never signal anymore
void drop();

// wait for this gpu job to finish before releasing
void addReleaseSync(SP<CEGLSync> sync);

Expand Down
17 changes: 17 additions & 0 deletions src/helpers/sync/SyncTimeline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ SP<CSyncTimeline> CSyncTimeline::create(int drmFD_, int drmSyncobjFD) {
}

CSyncTimeline::~CSyncTimeline() {
for (auto& w : waiters) {
if (w->source) {
wl_event_source_remove(w->source);
w->source = nullptr;
}
}
if (handle == 0)
return;

Expand Down Expand Up @@ -124,6 +130,17 @@ void CSyncTimeline::removeWaiter(SWaiter* w) {
std::erase_if(waiters, [w](const auto& e) { return e.get() == w; });
}

void CSyncTimeline::removeAllWaiters() {
for (auto& w : waiters) {
if (w->source) {
wl_event_source_remove(w->source);
w->source = nullptr;
}
}

waiters.clear();
}

CFileDescriptor CSyncTimeline::exportAsSyncFileFD(uint64_t src) {
int sync = -1;

Expand Down
1 change: 1 addition & 0 deletions src/helpers/sync/SyncTimeline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class CSyncTimeline {

bool addWaiter(const std::function<void()>& waiter, uint64_t point, uint32_t flags);
void removeWaiter(SWaiter*);
void removeAllWaiters();
Hyprutils::OS::CFileDescriptor exportAsSyncFileFD(uint64_t src);
bool importFromSyncFileFD(uint64_t dst, Hyprutils::OS::CFileDescriptor& fd);
bool transfer(SP<CSyncTimeline> from, uint64_t fromPoint, uint64_t toPoint);
Expand Down
75 changes: 39 additions & 36 deletions src/protocols/DRMSyncobj.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(SP<CWpLinuxDrmSyncobjSurf
}

auto timeline = CDRMSyncobjTimelineResource::fromResource(timeline_);
pending.acquireTimeline = timeline;
pending.acquirePoint = ((uint64_t)hi << 32) | (uint64_t)lo;
pendingAcquire.resource = timeline;
pendingAcquire.point = ((uint64_t)hi << 32) | (uint64_t)lo;
});

resource->setSetReleasePoint([this](CWpLinuxDrmSyncobjSurfaceV1* r, wl_resource* timeline_, uint32_t hi, uint32_t lo) {
Expand All @@ -35,12 +35,13 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(SP<CWpLinuxDrmSyncobjSurf
}

auto timeline = CDRMSyncobjTimelineResource::fromResource(timeline_);
pending.releaseTimeline = timeline;
pending.releasePoint = ((uint64_t)hi << 32) | (uint64_t)lo;
pendingRelease.resource = timeline;
pendingRelease.point = ((uint64_t)hi << 32) | (uint64_t)lo;
releasePoints.emplace_back(makeUnique<CSyncReleaser>(pendingRelease.resource->timeline, pendingRelease.point));
});

listeners.surfacePrecommit = surface->events.precommit.registerListener([this](std::any d) {
if ((pending.acquireTimeline || pending.releaseTimeline) && !surface->pending.texture) {
if ((pendingAcquire.resource || pendingRelease.resource) && !surface->pending.texture) {
resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_BUFFER, "Missing buffer");
surface->pending.rejected = true;
return;
Expand All @@ -49,55 +50,57 @@ CDRMSyncobjSurfaceResource::CDRMSyncobjSurfaceResource(SP<CWpLinuxDrmSyncobjSurf
if (!surface->pending.newBuffer)
return; // this commit does not change the state here

if (!!pending.acquireTimeline != !!pending.releaseTimeline) {
resource->error(pending.acquireTimeline ? WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_RELEASE_POINT : WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_ACQUIRE_POINT,
if (!!pendingAcquire.resource != !!pendingRelease.resource) {
resource->error(pendingAcquire.resource ? WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_RELEASE_POINT : WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_NO_ACQUIRE_POINT,
"Missing timeline");
surface->pending.rejected = true;
return;
}

if (!pending.acquireTimeline)
return;

if (pending.acquireTimeline && pending.releaseTimeline && pending.acquireTimeline == pending.releaseTimeline) {
if (pending.acquirePoint >= pending.releasePoint) {
if (pendingAcquire.resource && pendingRelease.resource && pendingAcquire.resource == pendingRelease.resource) {
if (pendingAcquire.point >= pendingRelease.point) {
resource->error(WP_LINUX_DRM_SYNCOBJ_SURFACE_V1_ERROR_CONFLICTING_POINTS, "Acquire and release points are on the same timeline, and acquire >= release");
surface->pending.rejected = true;
return;
}
}

// wait for the acquire timeline to materialize
auto materialized = pending.acquireTimeline->timeline->check(pending.acquirePoint, DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE);
if (!materialized.has_value()) {
LOGM(ERR, "Failed to check the acquire timeline");
resource->noMemory();
if (!pendingAcquire.resource)
return;

if (acquireWaiting)
return; // wait for acquire waiter to signal.

if (pendingAcquire.resource->timeline->addWaiter(
[this]() {
if (surface.expired())
return;

surface->unlockPendingState();
surface->commitPendingState();
acquireWaiting = false;
},
pendingAcquire.point, 0u)) {
surface->lockPendingState();
acquireWaiting = true;
}
});

if (materialized.value())
return;
listeners.surfaceRoleCommit = surface->events.roleCommit.registerListener([this](std::any d) {
if (pendingAcquire.resource)
acquire = std::exchange(pendingAcquire, {});

surface->lockPendingState();
pending.acquireTimeline->timeline->addWaiter([this]() { surface->unlockPendingState(); }, pending.acquirePoint, DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE);
if (pendingRelease.resource)
release = std::exchange(pendingRelease, {});
});
}

listeners.surfaceCommit = surface->events.roleCommit.registerListener([this](std::any d) {
// apply timelines if new ones have been attached, otherwise don't touch
// the current ones
if (pending.releaseTimeline) {
current.releaseTimeline = pending.releaseTimeline;
current.releasePoint = pending.releasePoint;
}
CDRMSyncobjSurfaceResource::~CDRMSyncobjSurfaceResource() {
if (acquire.resource)
acquire.resource->timeline->removeAllWaiters();

if (pending.acquireTimeline) {
current.acquireTimeline = pending.acquireTimeline;
current.acquirePoint = pending.acquirePoint;
}

pending.releaseTimeline.reset();
pending.acquireTimeline.reset();
});
if (pendingAcquire.resource)
pendingAcquire.resource->timeline->removeAllWaiters();
}

bool CDRMSyncobjSurfaceResource::good() {
Expand Down
22 changes: 17 additions & 5 deletions src/protocols/DRMSyncobj.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <vector>
#include "WaylandProtocol.hpp"
#include "helpers/sync/SyncReleaser.hpp"
#include "linux-drm-syncobj-v1.hpp"
#include "../helpers/signal/Signal.hpp"
#include <hyprutils/os/FileDescriptor.hpp>
Expand All @@ -13,21 +14,32 @@ class CSyncTimeline;
class CDRMSyncobjSurfaceResource {
public:
CDRMSyncobjSurfaceResource(SP<CWpLinuxDrmSyncobjSurfaceV1> resource_, SP<CWLSurfaceResource> surface_);
~CDRMSyncobjSurfaceResource();

bool good();

WP<CWLSurfaceResource> surface;
struct {
WP<CDRMSyncobjTimelineResource> acquireTimeline, releaseTimeline;
uint64_t acquirePoint = 0, releasePoint = 0;
} current, pending;
struct STimeLineState {
WP<CDRMSyncobjTimelineResource> resource;
uint64_t point = 0;

STimeLineState() = default;
STimeLineState(STimeLineState&&) noexcept = default;
STimeLineState& operator=(STimeLineState&&) noexcept = default;
STimeLineState(const STimeLineState&) = delete;
STimeLineState& operator=(const STimeLineState&) = delete;
~STimeLineState() = default;
} acquire, release, pendingAcquire, pendingRelease;

std::vector<UP<CSyncReleaser>> releasePoints;

private:
SP<CWpLinuxDrmSyncobjSurfaceV1> resource;
bool acquireWaiting = false;

struct {
CHyprSignalListener surfacePrecommit;
CHyprSignalListener surfaceCommit;
CHyprSignalListener surfaceRoleCommit;
} listeners;
};

Expand Down
16 changes: 8 additions & 8 deletions src/protocols/core/Compositor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ CWLSurfaceResource::CWLSurfaceResource(SP<CWlSurface> resource_) : resource(reso
return;
}

if (stateLocks <= 0)
commitPendingState();
commitPendingState();
});

resource->setDamage([this](CWlSurface* r, int32_t x, int32_t y, int32_t w, int32_t h) { pending.damage.add(CBox{x, y, w, h}); });
Expand Down Expand Up @@ -412,16 +411,17 @@ CRegion CWLSurfaceResource::accumulateCurrentBufferDamage() {
}

void CWLSurfaceResource::lockPendingState() {
stateLocks++;
stateLocked = true;
}

void CWLSurfaceResource::unlockPendingState() {
stateLocks--;
if (stateLocks <= 0)
commitPendingState();
stateLocked = false;
}

void CWLSurfaceResource::commitPendingState() {
if (stateLocked && syncobj)
return;

static auto PDROP = CConfigValue<Hyprlang::INT>("render:allow_early_buffer_release");
auto const previousBuffer = current.buffer;
current = pending;
Expand All @@ -433,8 +433,8 @@ void CWLSurfaceResource::commitPendingState() {

events.roleCommit.emit();

if (syncobj && syncobj->current.releaseTimeline && syncobj->current.releaseTimeline->timeline && current.buffer && current.buffer->buffer)
current.buffer->releaser = makeShared<CSyncReleaser>(syncobj->current.releaseTimeline->timeline, syncobj->current.releasePoint);
if (syncobj && syncobj->release.resource && syncobj->release.resource->timeline && current.buffer && current.buffer->buffer)
current.buffer->releaser = std::move(syncobj->releasePoints);

if (current.texture)
current.texture->m_eTransform = wlTransformToHyprutils(current.transform);
Expand Down
4 changes: 2 additions & 2 deletions src/protocols/core/Compositor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ class CWLSurfaceResource {
void presentFeedback(timespec* when, PHLMONITOR pMonitor, bool discarded = false);
void lockPendingState();
void unlockPendingState();
void commitPendingState();

// returns a pair: found surface (null if not found) and surface local coords.
// localCoords param is relative to 0,0 of this surface
Expand All @@ -144,13 +145,12 @@ class CWLSurfaceResource {
// this stupid-ass hack is used
WP<IHLBuffer> lastBuffer;

int stateLocks = 0;
bool stateLocked = false;

void destroy();
void releaseBuffers(bool onlyCurrent = true);
void dropPendingBuffer();
void dropCurrentBuffer();
void commitPendingState();
void bfHelper(std::vector<SP<CWLSurfaceResource>> const& nodes, std::function<void(SP<CWLSurfaceResource>, const Vector2D&, void*)> fn, void* data);
void updateCursorShm(CRegion damage = CBox{0, 0, INT16_MAX, INT16_MAX});

Expand Down
4 changes: 2 additions & 2 deletions src/protocols/types/Buffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ class CHLBufferReference {
CHLBufferReference(SP<IHLBuffer> buffer, SP<CWLSurfaceResource> surface);
~CHLBufferReference();

WP<IHLBuffer> buffer;
SP<CSyncReleaser> releaser;
WP<IHLBuffer> buffer;
std::vector<UP<CSyncReleaser>> releaser;

private:
WP<CWLSurfaceResource> surface;
Expand Down
5 changes: 3 additions & 2 deletions src/render/Renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1574,10 +1574,11 @@ bool CHyprRenderer::commitPendingAndDoExplicitSync(PHLMONITOR pMonitor) {
Debug::log(TRACE, "Explicit: can't add sync, EGLSync failed");
else {
for (auto const& e : explicitPresented) {
if (!e->current.buffer || !e->current.buffer->releaser)
if (!e->current.buffer || e->current.buffer->releaser.empty())
continue;

e->current.buffer->releaser->addReleaseSync(sync);
for (auto& r : e->current.buffer->releaser)
r->addReleaseSync(sync);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/render/pass/SurfacePassElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ void CSurfacePassElement::draw(const CRegion& damage) {
return;

// explicit sync: wait for the timeline, if any
if (data.surface->syncobj && data.surface->syncobj->current.acquireTimeline) {
if (!g_pHyprOpenGL->waitForTimelinePoint(data.surface->syncobj->current.acquireTimeline->timeline, data.surface->syncobj->current.acquirePoint)) {
if (data.surface->syncobj && data.surface->syncobj->acquire.resource) {
if (!g_pHyprOpenGL->waitForTimelinePoint(data.surface->syncobj->acquire.resource->timeline, data.surface->syncobj->acquire.point)) {
Debug::log(ERR, "Renderer: failed to wait for explicit timeline");
return;
}
Expand Down

0 comments on commit 46dbd22

Please sign in to comment.