Implement Explicit Sync #104
Conversation
|
Hi, Thanks for this PR! Looking forward for the driver release. Log: Used wayland-protocols PKGBUILD: Edit: |
|
Thanks, sorry I had missed the meson bits for this. With the updated version it should build again. Note that it still isn't guaranteed to work on currently released drivers, so your mileage may vary. |
Thanks for your fast response. It is working now. :) We just want to have everything ready for the upcoming driver release. |
Can we expect this in the first non-beta 550 release, or the major version # bump after that? |
Based on what I'm seeing here, it is the EGL_ANDROID_native_fence_sync extension that is missing from the current stable 535 drivers and the 545.23.06/08 drivers. 545.29.02/06 and 550.40.07 have it available if you set nvidia-drm.modeset=1. Somebody here could corroborate it, but that is what I'm seeing looking at all related mr's. |
This implements the explicit sync linux-drm-syncobj-v1 protocol for EGL. Most of this change involves wayland-protocol handling boilerplate. The protocol works by allowing the creation of timelines (i.e. DRM syncobjs) and per-surface states. We can then specify acquire and release points during our surface state configuration which will tell the compositor when it can access the buffer or notify us when it is finished accessing the buffer. Sync point signaling takes place during acquire_surface_image. We choose our two release point values and send them to the compositor. In the acquire case, the EGLSync will be created first and will be populated with a fence representing the GPU work. We can extract its syncfd and import it at the acquire point. We choose the release point during image acquisition, but don't actually create an EGLSync for it until later when the compositor has added a fence to the release point. We check if this has taken place after every surface commit. Separate timelines are used for the acquire and release points. Each stream image will have its own timeline, otherwise our signaling of acquire points would implicitly signal the still-pending release points.
is this |
|
Thanks Austin, the updated code looks good to me. We'll wait until https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/90 is merged before we merge this, though, just to be safe. |
It's done! 🎉 |
|
Very nice, finally things are going forward. @erik-kz Is there any ETA on the 555 driver? |
Beta release is currently targeted for May 15. It will include support for both the Wayland explicit sync protocol for EGL applications and the counterpart X11 explicit sync protocol for GLX and Vulkan X11 applications.
Both protocols will only be used if the server advertises support for them, otherwise it will just be dead code. So there's no need for a delay. Older compositor or Xwayland versions will continue to work as they currently do. |
Is there any chance the explicit sync protocol fixes will be backported to 550? |
We can only hope. It seems possible but only if Nvidia is willing to. Backporting to 550 would help out so many people it's not even funny. I have friends building PCs who went with AMD instead after seeing the troubles I was going through with this stuff lol. Other stuff like multimonitor gsync can wait tbh, for 555 or maybe later if Nvidia hasn't implemented it yet. But we need WORKING DRIVERS at the least. p.s. by multimonitor gsync I am talking about having one monitor not gsync and the main one gsync and it still not working Overall, I have hope that Nvidia will backport to 550. They themselves have seen the lengthy wait time required for all this stuff to merge, and now that it's finally going through we can only hope they can empathize with their users who've been stuck for months either on older drivers or on objectively inferior drivers. |
|
For this driver support to do anything useful, XWayland support for the protocol has to be included in 24.1 and that has to make it to distros. This might take longer than the 555 driver anyway, even on bleeding edge distros. Here's the XWayland MR: https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/967 |
Completely agree here. But these changes can already be used by installing a patched version of XWayland. I think most if not all people experiencing this issue will be willing to compile from source for a bit until a release. Some distros provide patched versions, AUR packages exist. For those not willing to install from source, it's exactly like you said. It will have ZERO effect. Nvidia backporting the explicit sync patches will only bring good. |
@erik-kz first, sorry.. I know this is not the place to ask.. but want to ask to NV (VK) devs if NV plans on supporting extension VK_KHR_incremental_present.. for full findings, see: seems Zink "requires" since very recently (https://www.phoronix.com/news/Zink-Partial-Updates-Damage) and RADV and ANV Mesa drivers support since 2017 EDIT: Phoronix: EDIT2: forgot to ask about borderColorSwizzleFromImage but that may be a HW limitation to expose on NV GPUs? |
|
As far as I can tell, VK_KHR_incremental_present is not a strict requirement for zink. It will use it if it's available for the potential performance improvement but it also should work without it. We don't have any plans at the moment to add support for it, but that could change. For the Wayland WSI in particular, it looks like the effort wouldn't be all that large so maybe it's worth doing. VkPhysicalDeviceBorderColorSwizzleFeaturesEXT::borderColorSwizzleFromImage is indeed unsupported do to hardware limitations. I'm don't think I'm allowed to disclose any details beyond that. |
|
thanks for the information Erik!
…On Mon, Mar 25, 2024 at 7:51 PM Erik Kurzinger ***@***.***> wrote:
As far as I can tell, VK_KHR_incremental_present is not a strict
requirement for zink. It will use it if it's available for the potential
performance improvement but it also should work without it.
We don't have any plans at the moment to add support for it, but that
could change. For the Wayland WSI in particular, it looks like the effort
wouldn't be all that large so maybe it's worth doing.
VkPhysicalDeviceBorderColorSwizzleFeaturesEXT::borderColorSwizzleFromImage
is indeed unsupported do to hardware limitations. I'm don't think I'm
allowed to disclose any details beyond that.
—
Reply to this email directly, view it on GitHub
<#104 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFDM6PHHU6V63QQ7IV3RETY2BWZ7AVCNFSM6AAAAABDTKIEQOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJYGY4DCNRQHA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
@erik-kz I'm assuming this means explicit sync for Wayland vulkan programs will only be coming with a later driver release? That's something that needs to be handled on Nvidia's Vulkan Wayland WSI implementation, right? (Sorry for replying to a 2 week old comment on a merged PR) |
|
Correct. Explicit sync for the Vulkan Wayland WSI is implemented but I haven't back-ported it to the 555 branch (which is currently undergoing QA testing). At this point I think it would be a bit too late to do that given the size of the change. So it will not be available until the following release, 560. |
|
Thank you for being open about these changes to end users. You have no idea how much it helps. Good luck to you and your team for the QA testing! |
Merged |
Hi Erik, thank you very much for your delivery and commitment. |
I think the forums are probably a better venue for this kind of discussion |
Apologies if this would more suited for the NVIDIA forum. I commented on a thread there also. Do you think there's any chance it could be back-ported post release, or released as the only feature in 560, so it could be quicker perhaps? I feel like Wayland is really close to being stable as daily driver with NVIDIA hardware, but the explicit sync issue seems like the main blocker. And great work by the way, seeing a lot of progress over the last several years. |
|
@jjones0293 With the 555 driver explicit sync will ship everything youre looking for. |
Hey @erik-kz, is this ETA still accurate? |
|
Its 9 AM in California, so please wait the day. Edit: |
The E in ETA already says it: "estimated" - the time is irrelevant. And it could be tomorrow in your timezone. We are given ETAs (and I appreciate that), and then people start pinning that down to the minute precision and request arrival (not pointing at you, that's a general problem, your question was probably not badly intended). Please don't stop giving such information - but maybe be a bit more vague like "by end of week/month" and add some buffer to your internal ETA. If it arrives early then, everyone will be happy. :-) Thanks. |
|
@erik-kz I know this isn't the appropriate place to say this, but thank you so freaking much for everything you do, you absolute monster of a genius! I saw you fighting Xwayland for 2+ years to get them to accept Explicit Sync. I saw you fighting years ago with your presentations at XDC to get X to accept Explicit Sync. You have been fighting non-stop for us, and I deeply appreciate everything you've been doing for Linux. The thing is, NVIDIA was right. As usual (you were right about EGL vs GBM too, even if GBM has gotten better with time). Explicit Sync is objectively the best way to synchronize rendering. But people were so happy to lazily sit in the status quo, babbling about "we already have Implicit Sync, so who cares..." all these years. There's no doubt that explicit is better. Windows uses Explicit Sync since Windows 7 or Vista. macOS uses explicit sync. Google wrote their own renderer for Android to use Explicit Sync (since implicit/wayland/x11 was so awful). I've seen both AMD and Intel engineers on Linux mailing lists, expressing how they wished Linux used Explicit Sync since it simplifies drivers and improves performance. And yet it always fell to NVIDIA to push this thing forwards. And to finally see the day arriving, when all your hard work pays off, is amazing. Your hard work and genius has not gone unnoticed. Thank you so much - to you and everyone involved in finally giving Linux the fast and sane Explicit Sync rendering that it has needed for so long! |
Thank You, that answers my question I had earlier. |
To save Erik from being the bad guy, no, this is no longer accurate. Sorry. We know you're all excited. We're excited too. We're on it, sit tight, it's coming very soon! Release dates generally shift around over time (It looks like Erik shared that date 2 months ago) and the above comments are indeed why we don't generally share specific target dates. Note this is a closed/merged pull request, not a driver release announcement/discussion forum. |
|
While I really hate it when communication gets stifled, perhaps it's time to lock to collaborators so that people who are subscribed here only get a notification when there's real news from NVIDIA? |
This is a merged request, if it's going to be closed then it should be closed to everyone I do agree that discussions not related to the explicit sync stuff should be avoided. Edit: the nvidia forum does not have a post for beta releases, only stable. I'm sorry everyone |
It's not that shifting a release date makes someone a bad guy, but the lack of communication and transparency is a bit disappointing. It's not only here that people are anxiously waiting for this much needed fix, also on many subreddits (r/linux, r/linuxgaming, r/nvidia to name a few) people were talking about yesterday being "the day". We know you do good work and there is a lot going on. We respect that 100%. Please leave the link here where we can follow along, so we can keep this PR clean from release date discussions. |
|
so when is it gonna come out? |
just be patient |
| @@ -1353,6 +1381,7 @@ WlEglDisplay *wlEglAcquireDisplay(EGLDisplay dpy) { | |||
| static void wlEglUnrefDisplay(WlEglDisplay *display) { | |||
| if (--display->refCount == 0) { | |||
There was a problem hiding this comment.
I love non-atomic refcount decrements
Some update(s) would be nice. Surely the devs/managers have some excpectation of when they will release it. |
I will agree with you I need explicit sync so I can finally use Wayland peacefully with my Nvidia and if explicit sync turns out bad I am gonna hold them hostage in my basement |
Last time they gave one, a pull request on Github was flooded because the prediction turned out wrong. The sheer amount of spam in this PR validates Nvidia's decision of not giving concrete dates. We've waited several years for explicit sync. A few days or weeks make no difference whatsoever. Now please - stop sending replies asking for a timeline. It's worthless noise. There are people with notifications enabled for this PR who would like to receive relevant, on-topic information. Jeez. |
|
The beta driver ( |
|
Please note that additional sources of "flickering" in XWayland were addressed as part of the XWayland 24.1 release. No NVidia driver updates needed for that one. |
|
Hi, Actually user reporting that this patch together with the 555 NVIDIA Driver does cause crashes in several applications, when using in KDE. Sadly, the user did not provide any special log yet, but following I could see in the journalctl: Full log: Downgrading the egl-wayland package to an unpatched one, does fix the issue. Edit: |
|
Explicit sync is used ? 🤔 Is strange because is available for plasma 6.1 in June. Not now, if user want use it now, it require https://invent.kde.org/plasma/kwin/-/merge_requests/5511 this |
Yes, we patch it in our kwin. There is a backport patch provided by KDE for plasma 6.0.4 |
|
I get that this is a great spot to get a lot of eyes on an issue, but this PR is closed and now shipping. I'm pretty sure there are also a LOT of people subscribed here. Any further discussion should be a new issue or on the forums. P.S. The KWin patch is not needed for explicit sync support, just for optimal performance and behavior. |
I agree; might I ask if there's a proper thread on the forums for this? I can see the GRD 555.85 feedback thread, but that doesn't look like the same thing. I think most people (myself included) are just posting here because they don't know where else to look EDIT: found the discussion thread on the developer forums |
| * points without attaching a buffer in the same commit. | ||
| */ | ||
| if (!send_explicit_sync_points(surface->wlEglDpy, surface, image)) { | ||
| return EGL_FALSE; |
There was a problem hiding this comment.
This is a bug. There is no turning back after wl_surface_attach has been called. You must prepare the entire state before modifying any of it.
| attribs[2] = EGL_NONE; | ||
| eglSync = data->egl.createSync(dpy, EGL_SYNC_NATIVE_FENCE_ANDROID, | ||
| attribs); | ||
| close (syncFd); |
| EGL_NO_SYNC_KHR)) { | ||
| return EGL_BAD_SURFACE; | ||
| acquireSync)) { | ||
| if (acquireSync != EGL_NO_SYNC_KHR) { |
This implements the explicit sync linux-drm-syncobj-v1 protocol for EGL.
This pull request exists to publicize our EGL implementation of explicit sync that we have been using to test the Mutter and Xwayland implementations. Erik and I have done a good amount of testing with this so it's in a fairly polished state and isn't just a first prototype.
Currently the code as proposed here will not build or run on existing drivers. To build it would need an updated wayland-protocols that contains linux-drm-syncobj, and to run it would require a newer (next upcoming release) NVIDIA driver that has the necessary prerequisites. Whenever this lands the minimum required NVIDIA driver version will also be bumped.
This change finally does away with the background threads which have given us issues in the past. Both the damage and the buffer release threads are disabled when explicit sync is supported.
Most of this change involves wayland-protocol handling boilerplate. The protocol works by allowing the creation of timelines (i.e. DRM syncobjs) and per-surface states. We can then specify acquire and release points during our surface state configuration which will tell the compositor when it can access the buffer or notify us when it is finished accessing the buffer.
Sync point signaling takes place during acquire_surface_image. We choose our two release point values and send them to the compositor. In the acquire case, the EGLSync will be created first and will be populated with a fence representing the GPU work. We can extract its syncfd and import it at the acquire point. We choose the release point during image acquisition, but don't actually create an EGLSync for it until later when the compositor has added a fence to the release point. We check if this has taken place after every surface commit.
Separate timelines are used for the acquire and release points. Each stream image will have its own timeline, otherwise our signaling of acquire points would implicitly signal the still-pending release points.