This is easily noticed as a leaked fd on every VC switch.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Tested-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Fix a regression with VT-switching away from Weston and then back
causing drmModePageFlip() to fail with ENOSPC or EINVAL, leaving one or
more outputs not updated. The regression appeared in
47224cc931:
compositor-drm: Delete drm_backend_set_modes
Fix it by forcing a drmModeSetCrtc() on all outputs both initially
created and after VT-switch in.
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
v2: moved state_invalid=true from create_output_for_connector() to
drm_output_enable()
Reviewed-by: Daniel Stone <daniels@collabora.com>
We still need to close fds passed to us - or we leak quite a few fds
on VC switch.
Regression, originally fixed in 8f5acc2f3a
and re-broken in commit 72dea06d79
but only for the logind launcher.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
This is a simple wrapper for casting the user data of a wl_resource into
a struct weston_output pointer. Using the wrapper clearly marks all the
places where a wl_output protocol object is used.
Replace ALL wl_output related calls to wl_resource_get_user_data() with
a call to weston_output_from_resource().
v2: add type assert in weston_output_from_resource().
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
It's a little awkward to try to keep the weston_output::region and
weston_output::previous_damage allocate exactly only when the output is
enabled. There was also a leak: weston_output_move() was calling
weston_output_init_geometry() on an already allocated regions without
fini in between.
Fix both issues by allocating the regions in weston_output_init(),
always fini/init'ing them in weston_output_init_geometry(), and fini'ing
for good in weston_output_destroy().
This nicely gets rid of weston_output_enable_undo() so I do not need to
try to figure out what to do with it later.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Move the wl_output global management into weston_compositor_add_output()
and weston_compositor_remove_output().
If weston_output_enable() fails, there is no need to clean up the global
and the clients will not see a wl_output come and go.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Move the output id management into weston_compositor_add_output() and
weston_compositor_remove_output(). This is a more logical place, and
works towards assimilating weston_output_enable_undo().
The output id is no longer available to the backend enable() vfuncs, but
it was not used there to begin with.
v2: moved assert earlier in weston_compositor_add_output()
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Enabling an already enabled output is an error, at least with the
current implementation.
However, disabling an output that has not been enabled is ok.
Cope with the first and document the second.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
It was ambiguous what this flag meant - it did not mean whether the
backend is considering this output to be enabled, because
weston_output_destroy() unsets it while deliberately not calling the
backend disable() vfunc.
Perhaps the most clear definition is with respect to the output's
assignment in the pending vs. enabled output lists. There is also a whole
bunch of variables that are allocated only when enabled is true.
Since the flag is related to the list membership, set and clear the flag
only when manipulating the lists.
Assert that weston_compositor_add_output() and
weston_compositor_remove_output() are not called in a wrong state.
v2:
- talk about "list of enabled outputs"
- clear 'enabled' in weston_compositor_remove_output() earlier
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
weston_compositor_add_pending_output() is the point through which all
backends must go when creating a new output. The enable and disable
vfuns are essential for anything to be done with the output, so it makes
sense to check them here, rather than when actually enabling or
disabling.
Particularly the disable vfunc is rarely called, so this gets the check
better excercised.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Only used internally in core. Needs to happen automatically when
something changes, so there should no need to call it from outside.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Only used by weston_output_enable().
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Document two more functions of the weston_output API.
Exported functions marked internal are meant for backends only.
Exported functions not marked internal are meant for libweston users.
v2: talk about "list of enabled outputs".
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
A weston_output available to the compositor should always be either in
the list of pending outputs or the list of enabled outputs. Let
weston_compositor_add_output() and weston_compositor_remove_output()
handle the moves between the lists.
This way weston_output_enable() does not need to remove and
oops-it-failed-add-it-back. weston_output_disable() does not need to
manually re-add the output back to the pending list.
To make everything nicely symmetric and fix any unbalancing caused by
this:
- weston_output_destroy() explicitly wl_list_remove()s
- weston_compositor_add_pending_output() first removes then inserts, as
we have the assumption that the link is always valid, even if empty.
Update the documentations, too.
v2:
- talk about "list of enabled outputs"
- keep wl_list_remove in weston_compositor_remove_output in its old
place
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Trying to make it more readable. Things that happen in the same step are
kept in the same paragraph.
v2: talk about "list of enabled outputs"
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
We need to make sure the client bound dmabuf with a high enough
version to receive modifier events before sending them or the
client will crash.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
We shouldn't free &modifier_invalid because it wasn't allocated
with malloc()
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Some log messages weren't terminated with a newline.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
It looks like there are some code paths where this has been forgotten, so
it likely doesn't work as is. It's probable that nobody has actually
used this in a very long time, so it's not worth the maintenance burden
of keeping xkbcommon vs raw keyboard code anymore.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
EGL_EXT_image_dma_buf_import_modifiers supports importing upto four dmabuf
planes into an EGLImage.
v2: correct PLANE3_PITCH token (Daniel Stone)
Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
pass over the modifier attributes to EGL.
v2: ensure same modifier is passed for all planes (Daniel Stone)
Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
implement 'modifier' event to communicate available formats and modifiers
to the client and support zwp_linux_dmabuf_v1 interface version 3.
v2: handle zero modifiers case, deprecate 'format' event.
Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
EGL_EXT_image_dma_buf_import_modifiers allows querying the formats
and modifiers supported by the platform. expose these to the compositor.
v2:
- change calloc args (Daniel Stone)
- check for modifier support before querying formats (Daniel Stone)
Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
handle create_immed() dmabuf import requests and support
zwp_linux_dmabuf_v1_interface version 2.
v2: terminate client with INVALID_WL_BUFFER when reason
for create_immed failure is unknown.
[daniels: Bump wayland-protocols dependency.]
Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
I ran Weston on a Nexus 4 mobile phone, with a native GNU/Linux userland,
and the latest Android kernel for that device from LineageOS [1].
calculate_refresh_rate() returned 1 (mHz), which gets rounded to 0 Hz later
and results in nothing being drawn to the screen.
This patch makes sure, that there is at least a refresh rate of 1 Hz, because
it returns the default refresh rate of 60 Hz otherwise.
[1]: https://github.com/LineageOS/lge-kernel-mako
Signed-off-by: Oliver Smith <ollieparanoid@bitmessage.ch>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
wl_surface_damage_buffer landed ages ago, but in order for GL to
use it the client must bind a wl_compositor version >= 4 (the
version where damage_buffer was introduced).
This patch updates the bind version and allows
eglSwapBuffersWithDamage to actually use the provided damage
rectangles instead of performing full surface damage.
Reviewed-by: Daniel Stone <daniels@collabora.com>
At the bottom of weston_output_finish_frame(), code exists to account
for flips which have missed the repaint window, by shifting them to lock
on to the next repaint window rather than repainting immediately.
This code only accounted for flips which missed their target by one
repaint window. If they miss by multiples of the repaint window, adjust
them until the next repaint timestamp is in the future. This will only
happen in fairly extreme situations, such as Weston being scheduled out
for a punitively long period of time. Nevertheless, try to help recovery
by still aiming for more predictable timings.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
commit a7cba1d4cd changed the way
the cursor plane is setup. Previously it was pre-emptively set
disabled for the next frame, and that would be changed at next
frame time if the cursor plane was to be used. It was changed
to be disabled at plane assignment time.
We disable the use of planes entirely by setting disable_planes to
a non-zero value, which bypasses all calls to assign_planes - so
if the plane was set-up in the previous frame it will retain its
state post-disable.
This leads to desktop zoom leaving the cursor plane in place when
it sets disable_planes.
This patch clears any stale cursor plane state from the redraw
handler if disable_planes is set so drm_output_set_cursor()
will do the right thing.
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reported-by: Emmanuel Gil Peyrot <emmanuel.peyrot@collabora.com>
Reorder some paragraphs to be more logically ordered. Rewrite the
description of the backend-specific disable function to explain the
semantics instead of the mechanics. Remove the paragraph about
pending_output_list as unnecessary details.
Add a big fat comment on why we call output->disable() always instead of
only for actually enabled outputs.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
We make the differentiation where planes are an abstract framebuffer
with a position within a CRTC/output, and sprites are special cases of
planes that are neither the primary (base/framebuffer) nor cursor plane.
drm_sprite, OTOH, contains nothing that's actually specific to sprites,
and we end up duplicating a lot of code to deal with them, especially
when we come to use an entirely plane-based interface with atomic
modesetting.
Rename drm_sprite to drm_plane, to reflect that it's actually generic.
No functional changes.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
[Pekka: dropped the removal of an unrelated comment]
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
page_flip_pending is only be set when do a pageflip to a newly-rendered
buffer; if the flag is not set, we have landed in the start_repaint_loop
path where the vblank query fails, and thus we must pageflip to the same
buffer.
This test was not sufficient for what it was supposed to guard:
releasing framebuffers back. When using client-supplied framebuffers, it
is possible to reuse the same buffer multiple times, and we would send a
framebuffer-release event too early.
However, since we have a properly reference-counted drm_fb now, we can
just drop this test, and rely on the reference counting to prevent
too-early release of client framebuffers.
page_flip_pending now becomes exactly what the name suggests: a flag
which indicates whether or not we are expecting a pageflip event. Add
asserts here to verify that we never receive a pageflip event we weren't
expecting.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
vblank_pending is currently a bool, which is reset on every vblank
requests (i.e. sprite pageflip). This can occur more than once per
frame, so turn it into a callback, so we only fire frame-done when we've
collected all the events.
This fixes unexpected behaviour when multiple views per output have been
promoted to DRM planes.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Previously, framebuffers were stored as fb_current and fb_pending.
In this scheme, current was the last buffer that the kernel/hardware had
acknowledged displaying: a framebuffer would be created, set as
fb_pending, and Weston would request the kernel display it. When the
kernel signals that the request was completed and the hardware had made
the buffer current (i.e. page_flip_handler / vblank_handler), we would
unreference the old fb_current, and promote fb_pending to fb_current.
In other words, the view is 'which buffer has turned to light?'.
This patch changes them to a tristate of fb_last, fb_current and
fb_pending, based around the kernel's view of the current state.
fb_pending is used purely as a staging area for request construction;
when the kernel acknowledges a request (e.g. drmModePageFlip returns 0),
the previous buffer is moved to fb_last, and this new buffer to
fb_current. When the kernel signals that the request has completed and
the hardware has made the buffer current, we simply unreference and
clear fb_last, without touching fb_current/fb_pending.
The view here is now 'which state is current in the kernel?'.
As all state changes are incremental on the last state submitted to the
kernel, even if the hardware has not yet been able to make it current,
this simplifies state tracking: all state submissions will always be
relative to fb_current, rather than the previous
(fb_pending) ? fb_pending : fb_current.
The use of fb_pending is strictly bounded between a repaint cycle
(including a grouped set of repaints) beginning, and those repaints
being flushed to the kernel.
fb_current will always be valid between an output's first repaint
flush, and when a disable/destroy request has been processed. For a
plane, it will be valid when a repaint cycle enabling that plane has
been flushed, and when a repaint cycle disabling that plane has been
flushed.
fb_last is only present when a repaint request for the output/plane has
been submitted, but not yet completed by the hardware.
This is the same set of constructs which will be used for storing
plane/output state objects in future patches.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
We implement v2 so use that instead of the DRM_EVENT_CONTEXT_VERSION
macro.
The latter defines the version of the drmEventContext struct declared in
the header [used in the current build] and can be 2, 3 or even 1000.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Instead of setting state members directly in the drm_output_render
functions (to paint using Pixman or GL), just return a drm_fb, and let
the core function place it in state.
This brings damage handling in line with repaint state, so we do not
clear damage if repaint fails.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Call drm_output_render unconditionally, doing an early exit if we're
already rendering a client buffer on the primary plane.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
'next' is used as a framebuffer which has either been rendered but not
had a configuration request (pageflip or CRTC set) applied to it, or
when for a framebuffer that has had configuration requested but not
applied (delayed pageflip where the event has not been applied).
'current' is used as the last framebuffer for which we know
configuration has been fully applied, i.e. CRTC set executed or pageflip
requested and event received.
Rename these members to fb_current and fb_pending, doing some small
reordering of drm_output whilst in the vicinity.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Now that we have better types in drm_fb, use it for cursor buffers as
well. This gives us easier refcounting for our cursors, as well as a
unified buffer-destruction path.
Currently this makes no difference, as the KMS legacy cursor update API
uses GEM names directly, and never touches DRM FBs. However, the cursor
plane becomes a regular KMS plane under atomic, at which point we
require DRM FBs.
Take the opportunity to move to drm_fb ahead of time.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Sometimes we need to duplicate an existing drm_fb, e.g. when
pageflipping to the same buffer to kickstart the repaint loop. To handle
situations like these, and simplify resource management for dumb and
cursor buffers, refcount drm_fb.
drm_fb_get_from_bo has a path where it may reuse a drm_fb, if the BO has
been imported and not released yet. As drm_fb_unref now relies on actual
refcounting (backed up by asserts), we add a balancing drm_fb_ref() to
the path where we return a reused drm_fb.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
We only need it for the GBM surface the FB was originally created
against; a mismatch here is very bad indeed, so no reason to pass it in
explictly every time rather than store it.
Following patches change drm_fb to be explicitly reference counted; in
order to reduce churn, rename drm_output_release_fb to drm_fb_unref
whilst changing its call signature here, even though it does not yet
actually perform reference counting.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Armin Krezović <krezovic.armin@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
The drm_fb destroy callback to mostly the same thing regardless of
whether the buffer is a dumb buffer or gbm buffer. This patch refactors
the common parts into a new function that can be called for both cases.
[daniels: Rebased on top of fb->fd changes, cosmetic changes.]
Signed-off-by: Tomohito Esaki <etom@igel.co.jp>
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
This uses the new pixel-format helpers, so we can also replace depth/bpp
with these.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalaneN@collabora.co.uk>
Rather than magically trying to infer what the buffer is and what we
should do with it when we go to destroy it, add an explicit type
instead.
In doing so, the test for dumb images (destroying them, but only if
they're not the 'live' ones) is removed. This was dead code, as the only
path which could cause us to shuffle images is drm_output_switch_mode.
This calls drm_output_release_fb before the images are reallocated in
drm_output_fini_pixman / drm_output_init_pixman, with the reallocation
unconditionally destroying the images, so can never be hit.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Make drm_output_set_cursor more deterministic, by calculating more state
and performing more plane manipulation, inside
drm_output_prepare_cursor_view.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Rather than duplicating knowledge of pixel formats across several
components, create a custom central repository.
Signed-off-by: Daniel Stone <daniels@collabora.com>
[Pekka: fix include paths and two copy-pastas]
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
HAVE_LIBDRM was used as a condition for the launcher infrastructure to
call libdrm.so functions. It was set by an independent test for libdrm,
which would silently continue if libdrm was not found. It was assumed
that if you enabled a feature that used libdrm at runtime, the test for
that feature would imply that HAVE_LIBDRM is also set. This was quite
subtle.
The only feature that actually uses libdrm.so at runtime is the DRM
backend. No other backend needs the libdrm calls in the launcher
infrastructure.
Therefore to simplify things, stop using HAVE_LIBDRM and use
BUILD_DRM_COMPOSITOR instead. If you enable the DRM compositor, you
automatically also get libdrm support in the launchers.
There are still things depending on LIBDRM_CFLAGS and LIBDRM_LIBS, so
the test cannot be removed completely.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
