backend/drm: move primary wlr_swapchain handling out of backend #2903

emersion · 2021-04-29T14:46:21Z

This is #2505 but with the DRM backend changes too. This essentially completes the plan laid out in #1352, while keeping the existing compositor-facing wlr_output API intact.

Implement get_primary_formats
Allocate an empty buffer on modeset, if we don't have one
Allow direct scan-out with legacy KMS API as long as the format/modifier doesn't change
Perform a test-only commit with the buffer when mode-setting an output
Fallback to modifier-less buffers

~~Depends on: #2505~~
~~Depends on: #2829~~
~~Depends on: #3035~~

emersion · 2021-06-14T10:20:35Z

Seems like I'm hitting synchronization issues with this PR, on both AMD and Intel. The buffers seem to be scanned out by KMS before the draw is complete, or cleared before KMS is done with them.

emersion · 2021-07-08T13:36:08Z

Seems like I'm hitting synchronization issues with this PR

Fixed by #3027

emersion · 2021-07-12T16:08:20Z

This is now ready to review.

bl4ckb0ne · 2021-07-15T14:34:38Z

Overall LGTM, I don't think I understand fully everything but I got the gist of it. Do you want another reviewer on it?

emersion · 2021-07-19T14:40:05Z

@kennylevinsen, if you had a bit of time to double-check this PR, that would be helpful! Let me know if you don't, no worries. :)

@bl4ckb0ne, let me know if you have any specific questions about this PR.

kennylevinsen

Commit message nit: "we don't need a renderer not an allocator." - I suspect you meant "nor".

Have this been run on multi-GPU for testing? I only have boring single-GPU machines.

backend/drm/legacy.c

types/wlr_output.c

kennylevinsen · 2021-07-20T10:21:10Z

types/wlr_output.c

@@ -706,6 +789,9 @@ static bool output_basic_test(struct wlr_output *output) {
 }

 bool wlr_output_test(struct wlr_output *output) {
+	if (!output_ensure_buffer(output)) {


Do we want to attach buffers to the output before doing the basic output test? And do we even want wlr_output_test to attach buffers? Seems a bit intrusive.

That's a good question. The reason a buffer is attached is that some backends might only be able to test a modeset if they have a buffer. This is the case of the DRM backend for instance, previously an empty black buffer was attached on modeset if none was provided by the compositor.

For the X11 and Wayland backends performing a modeset without a buffer doesn't really make sense either.

Oh yeah, it's also needed to test the modeset.

However, does it make sense to perform output_basic_test before ensuring a buffer? This would let us bail out early from an obviously bad configuration without wasting time allocating and clearing buffers (possibly twice if we fall back to modifierless). I don't think output_basic_test relies on the output from output_ensure_buffer—the buffer tests have matching conditions in output_ensure_buffer, and the direct scan-out resolution test shouldn't be relevant if we had to allocate the buffer.

It should also be noted that we now issue up to 3 tests towards the output impl from wlr_output_test, as output_ensure_buffer performs 1 or 2 tests of its own. This shouldn't be a problem if the test is sufficiently fast.

However, does it make sense to perform output_basic_test before ensuring a buffer?

I've been wondering about this as well. Updated to allocate the buffer only if the basic test succeeds, we can always change this in the future if necessary.

Note, once the first test has allocated a swapchain+buffer, these are re-used in subsequent tests and commits, so the overhead shouldn't be too bad.

kennylevinsen · 2021-07-20T13:27:50Z

Fails in test:

00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1096] Reallocating CRTCs
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1107] State before reallocation:
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1113]   'DP-1' crtc=0 state=1 desired_enabled=1
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1113]   'HDMI-A-1' crtc=-1 state=0 desired_enabled=0
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1113]   'DP-2' crtc=1 state=1 desired_enabled=1
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1113]   'HDMI-A-2' crtc=-1 state=0 desired_enabled=0
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1113]   'DP-3' crtc=-1 state=0 desired_enabled=0
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1113]   'HDMI-A-3' crtc=-1 state=0 desired_enabled=0
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1164] State after reallocation:
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1171]   'DP-1' crtc=0 state=1 desired_enabled=1
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1171]   'HDMI-A-1' crtc=-1 state=0 desired_enabled=0
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1171]   'DP-2' crtc=1 state=1 desired_enabled=1
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1171]   'HDMI-A-2' crtc=-1 state=0 desired_enabled=0
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1171]   'DP-3' crtc=-1 state=0 desired_enabled=0
00:00:00.450 [DEBUG] [wlr] [backend/drm/drm.c:1171]   'HDMI-A-3' crtc=-1 state=0 desired_enabled=0
00:00:00.450 [INFO] [wlr] [backend/drm/drm.c:1453] connector DP-1: Requesting modeset
00:00:00.450 [DEBUG] [sway/desktop/output.c:839] New output 0x1ec6230: DP-1
00:00:00.450 [DEBUG] [sway/config/output.c:351] Turning on output DP-1
00:00:00.450 [DEBUG] [sway/config/output.c:329] Output DPI: 162.560000x161.364706
00:00:00.450 [DEBUG] [sway/config/output.c:382] Auto-detected output scale: 1.000000
00:00:00.450 [DEBUG] [sway/config/output.c:412] Committing output DP-1
00:00:00.450 [DEBUG] [wlr] [types/wlr_output.c:694] Attaching empty buffer to output for modeset
00:00:00.450 [DEBUG] [wlr] [types/wlr_output.c:513] Choosing primary buffer format 0x34325241 for output 'DP-1'
00:00:00.450 [DEBUG] [wlr] [render/swapchain.c:105] Allocating new swapchain buffer
00:00:00.454 [DEBUG] [wlr] [render/gbm_allocator.c:127] Allocated 3840x2160 GBM buffer (format 0x34325241, modifier 0x100000000000004)
00:00:00.454 [DEBUG] [wlr] [render/gles2/renderer.c:139] Created GL FBO for buffer 3840x2160
00:00:00.455 [DEBUG] [wlr] [backend/drm/atomic.c:35] connector DP-1: Atomic test failed (modeset): Invalid argument
00:00:00.455 [DEBUG] [wlr] [types/wlr_output.c:712] Output modeset test failed, retrying without modifiers
00:00:00.455 [DEBUG] [wlr] [types/wlr_output.c:513] Choosing primary buffer format 0x34325241 for output 'DP-1'
sway: types/wlr_output.c:661: output_attach_empty_buffer: Assertion `!(output->pending.committed & WLR_OUTPUT_STATE_BUFFER)' failed.

emersion · 2021-07-26T15:48:38Z

Fixed up multi-GPU issues: with drm_surface_lock gone, the multi-GPU copy was gone too, and we were trying to use direct scanout without copying. Also we shouldn't strip LINEAR from the format if allow_modifiers = false.

kennylevinsen

LGTM. I'll take your word for multi-GPU being fixed. :)

backend/drm/drm.c

This will be necessary for the primary buffer.

This allows the swapchain to be created with the correct resolution during a mode change.

Some backends need a buffer in order to be able to perform a modeset.

Sometimes we allocate a buffer with modifiers but then fail to perform a modeset with it. This can happen on Intel because of bandwidth limitations. To mitigate this issue, it's possible to re-allocate the buffer with modifiers. Add the logic to do so in wlr_output.

Historically we haven't allowed direct scan-out for legacy KMS, because legacy misses the functionality to make sure a buffer can be scanned out. However with renderer v6 the backend can't figure out anymore whether the buffer comes from its internal swap-chain, because the backend doesn't have an internal swap-chain. The legacy KMS API guarantees that the driver won't reject a buffer as long as it's been allocated with the same parameters as the previous one. Let's check this in legacy_crtc_test.

We can't nuke it completely, we still need it for multi-GPU.

We only accept SCANOUT, the buffer type should never be set to RENDER.

We can now just rely on the common code for this.

Unless we're dealing with a multi-GPU setup and the backend being initialized is secondary, we don't need a renderer nor an allocator. Stop initializing these.

emersion mentioned this pull request Apr 29, 2021

Move primary wlr_swapchain handling out of backends #2505

Merged

10 tasks

emersion force-pushed the output-swapchain-drm branch 2 times, most recently from 0519f1a to 1b0bf91 Compare May 1, 2021 09:48

zccrs mentioned this pull request Jun 8, 2021

render: add renderer-specific functions #2940

Merged

emersion mentioned this pull request Jun 8, 2021

Simpler screencopy #2615

Merged

emersion force-pushed the output-swapchain-drm branch 3 times, most recently from 5e048fb to e3905e9 Compare June 13, 2021 12:06

emersion force-pushed the output-swapchain-drm branch from efb4864 to 46f7a55 Compare July 8, 2021 13:38

emersion mentioned this pull request Jul 8, 2021

backend/drm: move legacy-specific checks to legacy.c #3035

Merged

emersion force-pushed the output-swapchain-drm branch 10 times, most recently from ace1aae to fc05b76 Compare July 12, 2021 15:46

emersion mentioned this pull request Jul 12, 2021

Drop legacy output APIs #3044

Merged

4 tasks

emersion marked this pull request as ready for review July 12, 2021 16:08

bl4ckb0ne approved these changes Jul 15, 2021

View reviewed changes

kennylevinsen reviewed Jul 20, 2021

View reviewed changes

emersion force-pushed the output-swapchain-drm branch 2 times, most recently from 3911291 to 6dd3b25 Compare July 26, 2021 08:59

emersion mentioned this pull request Jul 26, 2021

Support DRM connector tiling #2961

Draft

emersion force-pushed the output-swapchain-drm branch 3 times, most recently from 3edd5a1 to b7ad57e Compare July 26, 2021 15:43

kennylevinsen approved these changes Jul 27, 2021

View reviewed changes

bl4ckb0ne reviewed Jul 27, 2021

View reviewed changes

backend/drm/drm.c Outdated Show resolved Hide resolved

emersion added 11 commits July 28, 2021 22:49

output: fallback to XRGB in output_pick_format

1b8ce0a

This will be necessary for the primary buffer.

backend/drm: implement get_primary_formats

7d69c59

output: use pending resolution when allocating swapchain

b2c2f92

This allows the swapchain to be created with the correct resolution during a mode change.

output: allocate and attach empty buffer on modeset

cee127e

Some backends need a buffer in order to be able to perform a modeset.

backend/drm: remove primary swapchain

f0437bc

We can't nuke it completely, we still need it for multi-GPU.

backend/drm: remove SCANOUT check in drm_connector_test

fa3e30f

We only accept SCANOUT, the buffer type should never be set to RENDER.

backend/drm: drop drm_surface_{make,unset}_current

e9a0991

backend/drm: drop get_renderer implementation

84efa8f

We can now just rely on the common code for this.

backend/drm: stop initializing renderer for parent backend

5b9a931

Unless we're dealing with a multi-GPU setup and the backend being initialized is secondary, we don't need a renderer nor an allocator. Stop initializing these.

emersion force-pushed the output-swapchain-drm branch from b7ad57e to 5b9a931 Compare July 28, 2021 20:50

emersion enabled auto-merge (rebase) July 28, 2021 20:51

emersion merged commit c55f70c into swaywm:master Jul 28, 2021

emersion deleted the output-swapchain-drm branch July 28, 2021 20:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

backend/drm: move primary wlr_swapchain handling out of backend #2903

backend/drm: move primary wlr_swapchain handling out of backend #2903

emersion commented Apr 29, 2021 •

edited

Loading

emersion commented Jun 14, 2021

emersion commented Jul 8, 2021

emersion commented Jul 12, 2021

bl4ckb0ne commented Jul 15, 2021

emersion commented Jul 19, 2021

kennylevinsen left a comment

kennylevinsen Jul 20, 2021

emersion Jul 26, 2021

kennylevinsen Jul 26, 2021

emersion Jul 26, 2021

kennylevinsen commented Jul 20, 2021

emersion commented Jul 26, 2021

kennylevinsen left a comment

backend/drm: move primary wlr_swapchain handling out of backend #2903

backend/drm: move primary wlr_swapchain handling out of backend #2903

Conversation

emersion commented Apr 29, 2021 • edited Loading

emersion commented Jun 14, 2021

emersion commented Jul 8, 2021

emersion commented Jul 12, 2021

bl4ckb0ne commented Jul 15, 2021

emersion commented Jul 19, 2021

kennylevinsen left a comment

Choose a reason for hiding this comment

kennylevinsen Jul 20, 2021

Choose a reason for hiding this comment

emersion Jul 26, 2021

Choose a reason for hiding this comment

kennylevinsen Jul 26, 2021

Choose a reason for hiding this comment

emersion Jul 26, 2021

Choose a reason for hiding this comment

kennylevinsen commented Jul 20, 2021

emersion commented Jul 26, 2021

kennylevinsen left a comment

Choose a reason for hiding this comment

emersion commented Apr 29, 2021 •

edited

Loading