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.
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.
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.
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).
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
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.
The current mode was set to NULL to abuse it as state variable
persisting DRM suspend/resume, this results resulted in a segfault on
normal DPMS cycle.
This reverts that change and uses the wlr_output enabled variable, which
also persists and makes more sense.
Legacy gamma lut size now uses the new legacy_crtc member of
wlr_drm_crtc. This was Previously doen using old_crtc in
wlr_drm_connector, but since this refers to the crtc that was connected to
the ouput, this could give the wrong result.
On the drm output the wlr_drm_connector structs are reused.
This struct contains the wlr_output struct, which is reused as well.
The old code kept modes/edid and output state persistent over hotplug.
This nulls the relevant strings, reads newer edid data and removes old
modes on unplug.
since we're looking at pointer differences.
Otherwise the build fails on arm like
In file included from ../backend/drm/drm.c:19:0:
../include/wlr/util/log.h:34:17: error: format '%jd' expects argument of type 'intmax_t', but argument 7 has type 'int' [-Werror=format=]
_wlr_log(verb, "[%s:%d] " fmt, _strip_path(__FILE__), __LINE__, ##__VA_ARGS__)
^
../backend/drm/drm.c:462:2: note: in expansion of macro 'wlr_log'
wlr_log(L_DEBUG, "%s: crtc=%ju ovr=%jd pri=%jd cur=%jd", conn->output.name,
^~~~~~~
../backend/drm/drm.c:462:39: note: format string is defined here
wlr_log(L_DEBUG, "%s: crtc=%ju ovr=%jd pri=%jd cur=%jd", conn->output.name,
~~^
%d
This adds back `wlr_output::needs_swap`. This allows a backend to
request buffer swaps even if the output isn't damaged. This is
needed by the DRM backend to trigger pageflips when the cursor
moves.
The previous pixel format (RGBA8888) is not supported on all devices for cursors.
Weston uses ARGB8888 exclusively for its cursor buffer object which leads me to
believe this pixel format is better supported.
The gbm for the output might be null for the pageflip in the case that
the output has been disconnected. The gbm might be set to null by
wlr_drm_output_cleanup() in this case.
If the output is cleaned up before the pageflip, then a double free
will crash the compositor on the call to gbm_surface_release_buffer()
in the pageflip handler. The outputs buffer object bo[1] will point to
invalid memory.
Drew DeVault
Vincent Vanlaer
Brian Ashworth
emersion
Timidger
Scott Anderson
Rostislav Pehlivanov
sghctoma
Rouven Czerwinski
Scott Anderson
Guido Günther
Dominique Martinet
Markus Ongyerth
Versus Void
Heghedus Razvan
Tony Crisci
Calvin Lee
nyorain