This fixes a heap-use-after-free when the session is destroyed before
the backend during wl_display_destroy:
==1085==ERROR: AddressSanitizer: heap-use-after-free on address 0x614000000180 at pc 0x7f88e3590c2d bp 0x7ffdc4e33f90 sp 0x7ffdc4e33f80
READ of size 8 at 0x614000000180 thread T0
#0 0x7f88e3590c2c in find_device ../subprojects/wlroots/backend/session/session.c:192
#1 0x7f88e3590e85 in wlr_session_close_file ../subprojects/wlroots/backend/session/session.c:204
#2 0x7f88e357b80c in libinput_close_restricted ../subprojects/wlroots/backend/libinput/backend.c:24
#3 0x7f88e21af274 (/lib64/libinput.so.10+0x28274)
#4 0x7f88e21aff1d (/lib64/libinput.so.10+0x28f1d)
#5 0x7f88e219ddac (/lib64/libinput.so.10+0x16dac)
#6 0x7f88e21b415d in libinput_unref (/lib64/libinput.so.10+0x2d15d)
#7 0x7f88e357c9d6 in backend_destroy ../subprojects/wlroots/backend/libinput/backend.c:130
#8 0x7f88e3545a09 in wlr_backend_destroy ../subprojects/wlroots/backend/backend.c:50
#9 0x7f88e358981a in multi_backend_destroy ../subprojects/wlroots/backend/multi/backend.c:54
#10 0x7f88e358a059 in handle_display_destroy ../subprojects/wlroots/backend/multi/backend.c:107
#11 0x7f88e314acde (/lib64/libwayland-server.so.0+0x8cde)
#12 0x7f88e314b466 in wl_display_destroy (/lib64/libwayland-server.so.0+0x9466)
#13 0x559fefb52385 in main ../main.c:67
#14 0x7f88e2639152 in __libc_start_main (/lib64/libc.so.6+0x27152)
#15 0x559fefb4297d in _start (/home/simon/src/glider/build/glider+0x2297d)
0x614000000180 is located 320 bytes inside of 416-byte region [0x614000000040,0x6140000001e0)
freed by thread T0 here:
#0 0x7f88e3d0a6b0 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:122
#1 0x7f88e35b51fb in logind_session_destroy ../subprojects/wlroots/backend/session/logind.c:270
#2 0x7f88e35905a4 in wlr_session_destroy ../subprojects/wlroots/backend/session/session.c:156
#3 0x7f88e358f440 in handle_display_destroy ../subprojects/wlroots/backend/session/session.c:65
#4 0x7f88e314acde (/lib64/libwayland-server.so.0+0x8cde)
previously allocated by thread T0 here:
#0 0x7f88e3d0acd8 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:153
#1 0x7f88e35b911c in logind_session_create ../subprojects/wlroots/backend/session/logind.c:746
#2 0x7f88e358f6b4 in wlr_session_create ../subprojects/wlroots/backend/session/session.c:91
#3 0x559fefb51ea6 in main ../main.c:20
#4 0x7f88e2639152 in __libc_start_main (/lib64/libc.so.6+0x27152)
This requires functions without a prototype definition to be static.
This allows to detect dead code, export less symbols and put shared
functions in headers.
This is set to the value of wlr_output.commit_seq when the frame has
been submitted. This allows tracking presentation with more then 1 full
frame of latency.
References: https://github.com/swaywm/wlroots/issues/1917
Instead of checking that the path property is not 0 to determine if the
connector is an MST connector, check if the path contains the mst:
string.
Fixes#1813
Instead of waiting for the next pageflip, destroy the output immediately
since we can now handle flips for outputs which no longer exist.
Also demote the missing crtc on flip to debug.
Fixes#1739
This commit makes more output properties (mode, enabled, scale and transform)
atomic. This means that they are double-buffered and only applied on commit.
Compositors now need to call wlr_output_commit after setting any of those
properties.
Internally, backends still apply properties sequentially. The behaviour should
be exactly the same as before. Future commits will update some backends to take
advantage of the atomic interface. Some backends are non-atomic by design, e.g.
the X11 backend or the legacy DRM backend.
Updates: https://github.com/swaywm/wlroots/issues/1640
The documentation for wayland-server.h says:
> Use of this header file is discouraged. Prefer including
> wayland-server-core.h instead, which does not include the server protocol
> header and as such only defines the library PI, excluding the deprecated API
> below.
Replacing wayland-server.h with wayland-server-core.h allows us to drop the
WL_HIDE_DEPRECATED declaration.
atomic and legacy now both pass the backend as the user data for the
pageflip event. We than retrieve the correct connector by matching on
the crtc_id passed to the page_flip_handler2.
Wlroots also requires the DRM_CRTC_IN_VBLANK_EVENT capability now.
Fixes#1297
The backend doesn't need to handle transform changes, since everything is done
in software. In fact, all of the implementations were all identical and just
set the transform.
We could add support for hardware transforms, but:
- This would require a different field (something like hardware_transform)
- Not all combinations are possible because there often are hardware
limitations
- The Wayland protocol isn't ready for this (in particular xdg-output, see [1])
This belongs to a different patch series anyway.
[1]: https://patchwork.freedesktop.org/series/52324/
The DRM subsystem needs a NULL modeset for connectors which disappear
from the system to disable the hardware pipes, otherwise the pixels get
rendered but are sent nowhere.
The atomic backend does the equivalent by removing the properties and
issuing a commit.
Fixes#1706
This updates the backend part of the output API. This is mostly renaming:
make_current becomes attach_render and swap_buffers becomes commit.
This also fixes the RDP backend to support NULL damage.
The deleted includes are redundant, because other headers will include
the necessary files. Additionally, they cause build failures, because
including EGL/egl.h or EGL/eglext.h directly, instead of through
wlr/render/egl.h or wlr/render/interface.h, will mean that
MESA_EGL_NO_X11_HEADERS will not have been defined, and so the EGL
headers will attempt to pull in unnecessary X11 headers that may not
exist on the system.
For the headers produced by glgen.sh, the includes couldn't simply be
deleted, because no other header would include the EGL headers. Neither
wlr/render/egl.h or wlr/render/interface.h felt appropriate to include,
so I opted instead to copy the MESA_EGL_NO_X11_HEADERS definition before
the EGL includes.
* wlr_output: Indicate modes link
* wlr_output: Introduce preferred flag
This indicates an outputs preferred mode.
* drm: Set preferred flag for an outputs preferred mode
If *changed_outputs is not supplied by the calling function, track the local
allocation with a bool variable and free the allocation at the end of the
function.
On DRM resume, such as switching back to a TTY, the output needs to be
modeset to the current mode. However, wlr_output_set_mode will return
early when attempting to set the mode to the current mode. This just
steps around wlr_output_set_mode and calls drm_connector_set_mode
directly.
There is no point in modesetting an output to a mode that it is already
set to. Modesetting will cause the output to briefly flicker which is
undesirable for a noop. This returns early in `drm_connector_set_mode`
when attempting to modeset to the current mode.
In order for a surface to be used as a cursor plane framebuffer, it
appears that requiring the buffer to be linear is sufficient.
GBM_BO_USE_SCANOUT is added in case GBM_BO_USE_LINEAR isn't sufficient
on untested hardware.
Fixes#1323
Removed wlr_drm_plane.cursor_bo as it does not serve any purpose
anymore.
Relevant analysis (taken from the PR description):
While trying to implement a fix for #1323, I found that when exporting
the rendered surface into a DMA-BUF and reimporting it with
`GBM_BO_USE_CURSOR`, the resulting object does not appear to be valid.
After some digging (turning on drm-kms debugging and switching to legacy
mode), I managed to extract the following error: ```
[drm:__setplane_check.isra.1 [drm]] Invalid pixel format AR24
little-endian (0x34325241), modifier 0x100000000000001 ``` The format
itself refers to ARGB8888 which is the same format as
`renderer->gbm_format` used in master to create the cursor bo. However,
using `gbm_bo_create` with `GBM_BO_USE_CURSOR` results in a modifier of
0. A modifier of zero represents a linear buffer while the modifier of
the surface that is rendered to is `I915_FORMAT_MOD_X_TILED` (see
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/uapi/drm/drm_fourcc.h?h=v4.20.6#n263).
In order to fix this mismatch in modifier, I added the
`GBM_BO_USE_LINEAR` to the render surface and everything started to work
just fine. I wondered however, whether the export and import is really
necessary. I then decided to test if the back buffer of the render
surface works as well, and at least on my hardware (Intel HD 530 and
Intel UHD 620) it does. This is the patch in this PR and this requires
no exporting and importing.
I have to note that I cheated in order to import DMA_BUFs into a cursor
bo when doing the first tests, since on import the Intel drivers check
that the cursor is 64x64. This is strange since cursor sizes other than
64x64 have been around for quite some time now
(https://lists.freedesktop.org/archives/mesa-commit/2014-June/050268.html).
Removing this check made everything work fine. I later (while writing
this PR) found out that `__DRI_IMAGE_USE_CURSOR` (to which
`GBM_BO_USE_CURSOR` translates) has been deprecated in mesa
(https://gitlab.freedesktop.org/mesa/mesa/blob/master/include/GL/internal/dri_interface.h#L1296),
which makes me wonder what the usecase of `GBM_BO_USE_CURSOR` is. The
reason we never encountered this is that when specifying
`GBM_BO_USE_WRITE`, a dumb buffer is created trough DRM and the usage
flag never reaches the Intel driver directly. The relevant code is in
https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gbm/backends/dri/gbm_dri.c#L1011-1089
. From this it seems that as long as the size, format and modifiers are
right, any surface can be used as a cursor.
We create the EGL config with GBM_FORMAT_ARGB8888, but then initialize GBM BOs
with GBM_FORMAT_XRGB8888. This mismatch confuses Mesa.
Instead, we can always use GBM_FORMAT_ARGB8888, and use DRM_FORMAT_XRGB8888
when calling drmModeAddFB2.
Fixes https://github.com/swaywm/wlroots/issues/1438
When there aren't enough CRTCs for all outputs, we try to move a CRTC from a
disabled output to an enabled one. When this happens, the old output's state
wasn't changed, so the compositor thought it was still enabled and rendering.
This commit marks the old output as WLR_DRM_CONN_NEEDS_MODESET and sets its
current mode to NULL.
Values from libdrm are likely more reliable than raw values from the EDID. We
were already using values from libdrm, but they were overwritten by parse_edid.
See drm.c:
wlr_conn->output.phys_width = drm_conn->mmWidth;
wlr_conn->output.phys_height = drm_conn->mmHeight;
This commit changes `scan_drm_connectors` to add new outputs to the end of the
list. That way, it's easier to understand what's going on with indices.
When we need to destroy outputs, we now walk the list in reverse order. This
ensures indices remain correct while iterating and removing items from the
list.
We now also make outputs without a CRTC disappear (those are in
WLR_DRM_CONN_NEEDS_MODESET state).
There was a missing copy_drm_surface_mgpu call in drm_connector_schedule_frame
so we asked for a pageflip with an unknown BO, resulting in ENOENT.
Additionally, this commit makes schedule_frame return a bool indicating
failures. This allows schedule_frame_handle_idle_timer to only set
frame_pending to true if a frame has been successfully scheduled. Thus, if a
pageflip fails, rendering won't be blocked forever anymore.
In case a pageflip is already pending, true is returned because a frame has
already been scheduled and will be sent sometime soon.
This allows the legacy interface to be used instead of atomic if needed. This
is a workaround to make some Intel GPUs work (if this variable is unset) as
well as to make some AMD GPUs work (if this variable is set).
If a pageflip is pending before cleanup, it's still pending after. This
is used line 1177: drm_connector_cleanup is called and
conn->pageflip_pending is checked afterwards.
Fixes#1297
This prevents the idle event to be activated on a destroyed
output.
This also makes the backend responsible for free-ing modes, as it
is the one allocating them and adding them to the list. Note that
the DRM backend (the only one using modes) already frees them.
This desynchronizes our rendering loop with the vblank cycle.
In case a compositor doesn't swap buffers but schedules a frame,
emitting a frame event immediately enters a busy-loop.
Instead, ask the backend to send a frame when appropriate. On
Wayland we can just register a frame callback on our surface. On
DRM we can do a no-op pageflip.
Fixes#617Fixesswaywm/sway#2748
When a pageflip is pending, we'll get a DRM event for the connector
in the future. We don't want to free the connector immediately
otherwise we'll use-after-free in the pageflip handler.
This commit adds a new state, "DISAPPEARED". This asks the pageflip
handler to destroy the output after it's done pageflipping.
Sessions can now be retrieved from a backend in a more general manner.
Multi-backend gets back its `session` field that contains the session
if one was created, removing the interfacing from multi backend with the
drm backend directly. This adds the possibility to use sessions even
without the drm backend.
It additionally fixes the bug that 2 session objects got created when
WLR_BACKENDS were set to "libinput,drm".
To allow vt switching without drm backend (and drm fd) on logind, start
listening to PropertiesChanged signals from dbus and parse the session
"Active" property when no master fd was created (this does not change
current drm backend behaviour in any way).
This commit allows outputs that need a CRTC to steal it from
user-disabled outputs. Note that in the case there are enough
CRTCs, disabled outputs don't loose it (so there's no modeset
and plane initialization needed after DPMS). CRTC allocation
still prefers to keep the old configuration, even if that means
allocating an extra CRTC to a disabled output.
CRTC reallocation now happen when enabling/disabling an output as
well as when trying to modeset. When enabling an output without a
CRTC, we realloc to try to steal a CRTC from a disabled output
(that doesn't really need the CRTC). When disabling an output, we
try to give our CRTC to an output that needs one. Modesetting is
similar to enabling.
A new DRM connector field has been added: `desired_enabled`.
Outputs without CRTCs get automatically disabled. This field keeps
track of the state desired by the user, allowing to automatically
re-enable outputs when a CRTC becomes free.
This required some changes to the allocation algorithm. Previously,
the algorithm tried to keep the previous configuration even if a
new configuration with a better score was possible (it only changed
configuration when the old one didn't work anymore). This is now
changed and the old configuration (still preferred) is only
retained without considering new possibilities when it's perfect
(all outputs have CRTCs).
User-disabled outputs now have `possible_crtcs` set to 0, meaning
they can only retain a previous CRTC (not acquire a new one). The
allocation algorithm has been updated to do not bump the score
when assigning a CRTC to a disabled output.
This commit handles better situations in which the number of
connected outputs is greater than the number of available CRTCs.
It'll enable as many outputs as possible, and transfer CRTCs to
outputs that need one on unplug.
This changes CRTC and plane reallocation to happen after scanning
DRM connectors instead of on modeset.
This cleanups CRTCs and planes on unplug to allow them to be
re-used for other outputs.
On modeset, if an output doesn't have a CRTC, the desired mode is
saved and used later when the output gains a CRTC.
Future work includes giving priority to enabled outputs over
disabled ones for CRTC allocation. This requires the compositor to
know about all outputs (even outputs without CRTCs) to properly
modeset outputs enabled in the compositor config file and disable
outputs disabled in the config file.
This prevents receiving modesetting requests from the compositor
while we don't have the whole picture (ie. while we haven't yet
scanned all connectors).
This also makes connectors without CRTCs disabled (they can't be
enabled yet even if some CRTCs are free'd -- this is future work).
All screens on secondary GPU in multiple GPU configurations
was flipped 180.
The flipped screens was always on secondary card (the primary card
was always correct).
Tested on nouveau with:
WLR_DRM_DEVICES=/dev/dri/card1:/dev/dri/card2
WLR_DRM_DEVICES=/dev/dri/card2:/dev/dri/card1
The commit is fixing this problem. Now all screens are "normal".
backend_get_renderer() is now returning the renderer of the primary GPU, instead
of its own renderer, since that's the thing which actually does all of the "real"
rendering
wlr_multi_backend_add() is now adding all subbackends (otherwise only one GPU
is handled).
credits: @ascent12
These operations are done in 32-bit arithmetics before being casted to 64-bit,
thus can overflow before the cast.
Casting early fixes the issue.
Found through static analysis
Compositors now have more control over how the backend creates its
renderer. Currently all backends create an EGL/GLES2 renderer, so
the necessary attributes for creating the context are passed to a
user-provided callback function. It is responsible for initializing
provided wlr_egl and to return a renderer. On fail, return 0.
Fixes#987
Updates the projection matrix for the cursor plane in the DRM backend,
when the cursor is set, so new cursor are uploaded with the correct
transformation.
This changes the `wlr_output_impl.set_cursor` function to take a
`wlr_texture` instead of a byte buffer. This simplifies the
DRM and Wayland backends since they were creating textures from
the byte buffer anyway.
With this commit, performance should be improved when moving the
cursor since outputs don't need to be re-rendered anymore.
- Textures are now immutable (apart from those created from raw
pixels), no more invalid textures
- Move all wl_drm stuff in wlr_renderer
- Most of wlr_texture fields are now private
- Remove some duplicated DMA-BUF code in the DRM backend
- Add more assertions
- Stride is now always given as bytes rather than pixels
- Drop wl_shm functions
Fun fact: this patch has been written 10,000 meters up in the air.
