-
Notifications
You must be signed in to change notification settings - Fork 201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for commit-timing-v1. no wlroots #1230
base: master
Are you sure you want to change the base?
Add support for commit-timing-v1. no wlroots #1230
Conversation
src/wlserver.cpp
Outdated
|
||
#define NANOS_IN_SEC 1000000000 | ||
static uint64_t timer_get_target_present_nsec(uint64_t timestamp_nsec, uint32_t rounding_mode) { | ||
uint64_t refresh_nsec = NANOS_IN_SEC/(GetVBlankTimer().GetRefresh()/1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
We have utilities to do refresh -> cycle already
mHzToRefreshCycle
-
Doesn't all this rounding logic need to actually be where we check the timestamp in steamcompmgr?
Right now this problem will only manifest on hotplug, which isn't a big deal -- but some day we will support multiple displays, so we should move it there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I know that adding stuff to the ResListEntry whatever crap is painful, it is high on my to-do to rework that garbage soon and have wlserver create the commits directly).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hope it's better now.
src/wlserver.cpp
Outdated
target_nsec -= (target_nsec % refresh_nsec); | ||
} | ||
else if (rounding_mode == TIMER_ROUNDING_NOT_BEFORE) { | ||
target_nsec = ((target_nsec+(refresh_nsec-1))/refresh_nsec)*refresh_nsec; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get why we are touching anything for TIMER_ROUNDING_NOT_BEFORE
.
There is no world where we need to normalize it to a refresh cycle, and we probably shouldn't because multiple displays.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this logic to steamcompmgr, leaving out this case.
src/wlserver.cpp
Outdated
|
||
for (const auto& commit : *commits) | ||
{ | ||
if (commit.desired_present_time > target_present_nsec) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic doesn't work because some pending commits have already probably moved to steamcompmgr thread already.
Unfortunately Gamescope is not set up to really allow you to check something like this given it's architecture.
I am happy to just drop the implementation of the protocol error for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, you could just store the highest used target_present_nsec that gets updated at commit time on the wlserver_wl_surface_info instead of checking every commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should also probably do that for your actual wlroots implementation instead of checking over every commit ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, nice...
From now on whenever I find myself mindlessly proliferating cpu cycles, I'll ask myself if I could do any better :D
However I think I'll just leave a very trivial check for now, since it appears to be controversial what to actually consider an error in this case.
Signed-off-by: Sergio Gómez <[email protected]>
There might be other done commits which would be lost otherwise. Signed-off-by: Sergio Gómez <[email protected]>
652327a
to
c405f4a
Compare
Second WIP attempt at bringing support for commit-timing-v1 to gamescope.
Previous attempt uses wlroots' internal infrastructure.
The protocol offers 2 target stages: presentation and latching. I've ignored the distinction for now and just take the provided timestamp to be intended for "presentation".
I've also not tried to work through how a timestamp request would interact with the current implementation of fifo-v1 (something for a future iteration).