Skip to content

core: fix hyplock on proprietary nvidia drivers#654

Closed
PointerDilemma wants to merge 3 commits intohyprwm:mainfrom
PointerDilemma:main
Closed

core: fix hyplock on proprietary nvidia drivers#654
PointerDilemma wants to merge 3 commits intohyprwm:mainfrom
PointerDilemma:main

Conversation

@PointerDilemma
Copy link
Collaborator

Goal: Make it work with proprietary nvidia drivers

  • Removed timerThr and pollThr in favour of polling directly in the event loop
  • Never call wl_display_dispatch directly

TODO:

  • Fix background:path=screenshot

@PointerDilemma PointerDilemma force-pushed the main branch 2 times, most recently from 9c5a667 to da72e51 Compare January 19, 2025 10:24
@PointerDilemma
Copy link
Collaborator Author

Initially i though our problem was described by this comment: NVIDIA/egl-wayland#98 (comment)

So i made sure we always prepare_read to sync our reads,
but that does not seam to work.

Not blocking in poll immediately resolves all stutters. (for example adding POLLOUT to the pollfd of the wayland display). But this makes it a busy loop :(.

It seems like sometimes we are not getting a POLLIN wl event for a frameCallback. Weirdly enough this takes a few frames to become a problem.

@PointerDilemma
Copy link
Collaborator Author

I also tried using the presentation_time protocol instead of frame callbacks, but that has the exact same problems.

@PointerDilemma
Copy link
Collaborator Author

PointerDilemma commented Jan 19, 2025

This is problematic because egl-wayland creates a background thread that also reads from the Wayland socket.

To me it seems like this is either a problem that we can't fix when we want to use poll for the purpose of waking on wayland events, or there is some weird interaction with some egl stuff that we do.

Maybe we can set the poll timeout to 0 for as long as we expect a frame callback to arrive.

@PointerDilemma
Copy link
Collaborator Author

Yeah actually something like that seems to work

bool expectingFrameCallback = std::find_if(m_vOutputs.begin(), m_vOutputs.end(),
                                           [](const auto& o) { return o->sessionLockSurface && o->sessionLockSurface->expectingFrameCallback(); }) != m_vOutputs.end();

int  ret = poll(pollfds, fdcount, (expectingFrameCallback) ? 0 : timeout);

@PointerDilemma PointerDilemma force-pushed the main branch 4 times, most recently from 9cf9c79 to f576bbd Compare January 19, 2025 11:46
@PointerDilemma
Copy link
Collaborator Author

Yeah i mean... stutters are gone and still idles properly.

@PointerDilemma
Copy link
Collaborator Author

@vaxerski anything against removing timerThr and pollThr?

Comment on lines +736 to +739
//TODO: if we call this from the same thread that we handle timers, everything is fine,
// but if we are currently in poll, and call this from another thread, we need a way to wake up the poll
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk how to handle that though.
Maybe an extra pollFd for waking internally??

@PointerDilemma
Copy link
Collaborator Author

Also works smoothly on my old laptop with 3rd gen intel integrated graphics

@PointerDilemma PointerDilemma force-pushed the main branch 5 times, most recently from 3031f61 to fdd75c0 Compare January 19, 2025 16:28
@PointerDilemma
Copy link
Collaborator Author

this is a bad try to work around the problem. cpu is busy when are not idle.
gonna check again if i can find the reason why it blocks.
there are many other applications that poll on the display fd in their event loop and don't seem to have any problems

This was done, so that we can

  wl_display_prepare_read -> poll -> wl_display_read_events

Fixes synchronization issues, that happen on nvidia proprietary drivers.
@PointerDilemma
Copy link
Collaborator Author

Ok so i found that doing wl_display_prepare_read -> poll -> wl_display_read fixes it.
There is probably some nvidia egl thread reading from the display when we call poll and for some reason that causes a deadlock.
Preparing before poll makes sure that does not happen.

To allow for that I moved all of the wayland event reading to the poll thread. Dispatch obviously still only in the event loop thread. Seems to work nicely.

@PointerDilemma
Copy link
Collaborator Author

Happy with it now. Gonna close this in favour of 2 seperate patches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant