-
Notifications
You must be signed in to change notification settings - Fork 342
Conversation
902c709
to
0bce5a9
Compare
Hey, I just tried to try out this PR. I have a Ryzen 3700U with vega graphics. But I failed to get it running. Having installed all the vulkan*-git and mesa-git packages on ArchLinux as well as the validation layers with your patch and libdrm-git, I get a
So I think the radeonsi driver should be there 🤔 Do you have any ideas what I'm missing? |
Do other gl/vulkan programs work for you? Try e.g. |
Ahh I think I found the issue, I install the git packages from chaotic-aur as building everything on the laptop is quite painful and it seems they built the last mesa-git package against llvm-libs v12, but llvm-libs is already v13. I'll try to get that working tomorrow, maybe after chaotic-aur rebuilt their packages it all works. In the meantime, anything specific you would like me to test? |
You can try running tinywl with the wayland backend (if that works without validation errors you can try the drm backend, i.e. from tty, exit via alt + escape) with |
Alright, got to it before tomorrow ^^
|
Oh that one is to be expected, I got it too and think it's a validation layer error but didn't have time to dig into it yet. The message does not make sense, it even says we have an effective API version of 1.1.0 (and we definitely do) but then outputs incorrect properties and complains that the called function requires 1.1.0. This one shouldn't be a problem, just ignore it. Do things work other than that? |
Nope, after that it crashes, but in the Vulkan-ValidationLayers. I just glanced over your pull request but my guess would be that there is some other place where GetPhysicalDeviceFormatProperties2 is called in a wrong way.
Don't get confused by the fact that the validationlayers are in the directory of the extra repo, I modified the PKGBUILD to pull your repo. |
Ah damn I remember encountering problems with the MESA_device_select layer as well. You could try to disable it by using |
With that I get a little bit further, but now it crashes with:
|
Alright! This means that no formats/modifiers at all are detected as supporting dmabuf imports which also means we can't render anything. Which is weird. Thank you for your help so far, I'll add more detailed debug logging regarding format querying, maybe we can get further then :) |
For reference, this is the output for anv. |
Somehow the format querying changes seem to have fixed my issue, I can start tinywl now and weston-simple-dmabuf-egl works like a charm.
After sway failed, I ran firefox in tinywl, which seemed to work so I wanted to write this comment in there, it always crashed when I got to the 2FA part of github though with The only errors I see from tinywl are:
But they appear wihtout the vulkan renderer, too, so nothing special apparently. |
Trying the DRM backend somehow doesn't work at all, the screen just freezes after running the tinywl command (still showing the tty), ctrl+alt+F1 doesn't work then, but ALT+ESC to quit tinywl. I attached the log with all the validation errors (quite a few).
This appears for both screens (eDP-1 is my internal screen, DP-1 an external one). Log: drm.log. |
If you're using amdgpu, you'll need a pretty recent kernel (5.11+). You can check whether the kernel has |
ah gotcha, I was staying on 5.10 because 5.11 has issues suspending for me. Will try 5.11 in the next few days. |
Thanks for the detailed report @David96.
Yeah, well, I found a minor issue with the code looking over it again (that wasn't a problem on anv but apparently is on radv).
Yes I didn't test the renderer with sway as sway seems to assume the gl renderer is used (e.g. here). Will gladly add support in sway down the line if there is interest. The error you are getting seems unrelated though, I might look into it.
I got that one as well on both renderers. I looked into it and it's not related to the renderer. (EDIT: see #2554). |
I'd be happy to do that, but I'm not sure how to find a program that uses this format. I tried using v4l by running wf-recorder on a v4l2loopback device setting the pixel format to nv12 but then running weston-simple-dmabuf-v4l with -f NV12 -d NV12 gives me Edit - update on the drm backend: |
Alright, good to hear it works with the DRM backend! I investigated the lag you mentioned and it's likely because the mesa egl implementation currently falls back to shm buffers instead of using dmabufs. This happens because it requires the wl_drm protocol (legacy protocol, it only really needs it to get a device fd anyways) which isn't supported by the compositor when using this renderer. I completely forgot about this issue, am only now starting to seriously dig into linux graphic stack developments again. It is possible to hack support for wl_drm into the vulkan renderer (just the device-fd part basically), I think I got it working the last time, it still lives on a branch, but it's terrible and ugly. The wp_linux_dmabuf_hints protocol is the better replacement, this mesa patch removes the hard wl_drm dependency from the egl driver. Only vulkan clients and clients using linux_dmabuf_v1 directly (like weston-simple-dmabuf-egl) will make use of the dmabuf protocol with this renderer at the moment. |
#2708 is basically this. Yeah, it isn't great. |
Given that you already have a finished implementation, maybe it's worth integrating it here after all for easier testing and as a workaround until wp_linux_dmabuf_hints lands. I'd prefer just using wp_linux_dmabuf_hints though, if there's any way to help with making that land, let me know. |
Yeah, I don't think we'll be able to get rid of I've also seen Xwayland falling back to |
Note, this kernel patch might be of interest, to be able to properly synchronize client buffers: https://lists.freedesktop.org/archives/dri-devel/2021-March/299766.html |
Alright, I will pull your implementation in here after I split off the ycbcr support part.
Interesting read, good to hear the explicit sync infrastructure is made as efficient as possible. Explicit sync is another rework (see #1685) that would be needed to make this renderer efficient and really use the advantages of vulkan. The compositor -> client direction is "solved" at the moment by waiting on the rendering fence in the compositor before releasing the buffer back to the client. For the client -> compositor direction we rely on the driver to enable implicit sync for buffers submitted to the compositor (which is weird in vulkan as the API is designed with explicit sync all the way). Explicit sync for wlr_buffers on the drm backend via IN_FENCE_FD would also be nice to have down the road (another reason we have to wait for rendering on the CPU at the moment). |
Some of the texture uploading transitions @cyanreg commented on were indeed messy, this should be better now. On the other issues, I agree with @emersion. I'm personally still not convinced that compute shaders bring a huge advantage here so I won't put my time into it. You can just do a new PR after this if you want it I guess. |
0ae5b0b breaks
|
Oops, fixed. |
Doesn't work with
Debug log: sway-1.7 |
It seems like your X11 server doesn't support DRM format modifiers. It should work if you run it via Xwayland ( There's no way around it, Vulkan requires DRM format modifiers. However the error message could really be improved. The reason it doesn't just fail to start is that wlroots has an implicit assumption that the modifier-less codepath will always work. That's wrong and #2815 tries to fix that, it should help with this case. |
Do you plan to enable Vulkan on FreeBSD CI (to avoid regressing Clang builds)? According to downstream recipe the extra dependencies are:
|
Done! |
Thanks for the review @Joshua-Ashton. |
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.
Looks like a good start. Let's iterate in follow-up pull requests.
Thanks a lot for your hard work!
Squashed everything since I don't think there's a reasonable way to split these additions up. Added Alpine CI. |
This new renderer is implemented with the existing wlr_renderer API (which is known to be sub-optimal for some operations). It's not used by default, but users can opt-in by setting WLR_RENDERER=vulkan. The renderer depends on VK_EXT_image_drm_format_modifier and VK_EXT_physical_device_drm. Co-authored-by: Simon Ser <[email protected]> Co-authored-by: Jan Beich <[email protected]>
Having some issues with it on AMD, looking into right now. |
(doesn't need to block it being merged though, if there is an issue I will PR to here or Mesa or whatever.) |
Thanks for your contribution! |
Created a few follow-up issues:
Feel free to open more issues if I missed anything. |
As I understand it VK_EXT_image_drm_format_modifier is only available on AMD GFX9 and later? Is there going to be any solution to this? I use a Polaris GPU, and I should think many others do too, they're still sold new after all... |
The solution is to add format modifiers support to GFX8. It's not possible to use a Vulkan compositor without this extension and without resorting to Mesa-specific hacks. |
I've left a comment asking about this on the mesa pr for gfx9 support: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6176#note_1123860 Hopefully, this wlroots Vulkan renderer is important enough for the amdgpu devs to address the lack of support. There must be more GFX8 AMD GPUs out there than any other AMD architecture. |
Reworked version of #1214, building upon the new wlr_allocator abstraction. A lot of the old discussion points have been resolved.
wlr_renderer_read_pixels
,(functions have been removed from wlroots). We can implement all of them but implementations would be poor, I would much rather propose interface changes to implement them efficiently, without completely blocking the compositor. These functions being unimplemented should only have an impact on multi-GPU support and wlr_screencopy_v1 not working under vulkan.wlr_renderer_blit_dmabuf
,wlr_texture_to_dmabuf
As this isn't perfect and needs a lot of testing on all possible hardware, the renderer currently always enables the vulkan validation layers.
You will need this patch as there were errors in the validation layers for VK_EXT_image_drm_format_modifier.You will need to build the vulkan validation layers from master as there was an error with validation of a used extension just fixed recently.I tested this on an intel 8th gen GPU with the anv mesa MR (there isn't support for importing YCbCr dmabufs yet), using tinywl with the wayland and drm backend, running the sample weston-* applications as well as firefox and some GTK applications. Sadly, I don't have a >=GFX9 (vega) AMD GPU, and since radv already has support for the vulkan extension on those GPUs I'd be thankful for people testing it. Tests on any other driver/platform where the extension is available would certainly be useful as well.
If you can think of applications that this should be tested with, let me know (something that uses YCbCr shm buffers would be nice).