Right now, we're assuming that even when deep nesting involved, the proc
vector is always set to a valid function. One the one hand it requires
extra dummy procs in some cases, OTOH it's making upcoming refactoring
of the code flow unnecessarily complex.
The big plot (of subsequent commits) is splitting out the extension's
(and possibly subsystem's) special logic out of the wrapping chain and
let them be executed independently from the DDX/drivers - when applicable
even only when the pixmap is really destroyed (not just unref'ed).
(At some later point, it might even become be actually a valid situation
that DestroyPixmap vector really being NULL.)
See: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1754
See: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1755
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1709>
This way the caller knows if the conversion failed.
While at it, check for width/height at the same time.
Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1764>
We alread have several of these calls, that aren't interested in result value,
explicitly casting to void. Fixing this up for the remaining ones.
This is helpful for the human reader as well as quality analysis tools.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1648>
This has been nothing but an alias for two decades now (somewhere in R6.6),
so there doesn't seem to be any practical need for this indirection.
The macro still needs to remain, as long as (external) drivers still using it.
Fixes: ded6147bfb
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1529>
Clears -Wcalloc-transposed-args warnings from gcc 14.1, such as:
../dix/main.c:165:42: warning: ‘calloc’ sizes specified with ‘sizeof’ in the
earlier argument and not in the later argument [-Wcalloc-transposed-args]
165 | serverClient = calloc(sizeof(ClientRec), 1);
| ^~~~~~~~~
../dix/main.c:165:42: note: earlier argument should specify number of
elements, later size of each element
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1606>
With the potential modeset vs. modifiers issue covered by
commit 899c87af1f ("modesetting: unflip before any setcrtc() calls")
we can safely enable modifiers by default, at least on Intel
hardware where we know that things work properly.
I suppose the one open question is whether everything will work
correctly with wonky multi-GPU setups? I don't have one to test
myself.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
On certain system deployments, /dev/dri/card* nodes aren't directly
accessible to the currently logged in user, but the display server only
access it by asking systemd-logind to open the device for it. This
causes the X server to fail when trying to re-open the card* device
directly, causing all use of DRI3 to fail.
Fix this by using the render device path instead where possible.
This allows Xorg to use Glamor GLX when Glamor is requested,
and eliminates usage of DRI2 in case of Glamor.
Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Acked-by: Emma Anholt <emma@anholt.net>
This commit adds an ability to store a glvnd vendor in Glamor
structures, which can be used for initialize some vendor-based values
without hooking into DDX internals. Also this adds setting this value
into Xorg and Xwayland
Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Acked-by: Emma Anholt <emma@anholt.net>
It is useful to know on what context we are running, and
we need to show it into xorg.log
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Konstantin <ria.freelander@gmail.com>
This allows to choose between Glamor on OpenGL and Glamor on OpenGL ES
via an option.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Konstantin <ria.freelander@gmail.com>
There are systems where softpipe is the default renderer,
e.g. when llvmpipe is not is not available. Using glamor
on such systems is never a good idea.
This mirrors what commit 0a9415cf79
did for llvmpipe.
Closes: #1417
Signed-off-by: Ivan A. Melnikov <iv@altlinux.org>
The virgl driver exposes the name of the host renderer which might be llvmpipe.
In this case we still need glamor to be initialized.
Only check if the renderer starts with llvmpipe (which is what llvmpipe exposes).
Signed-off-by: Corentin Noël <corentin.noel@collabora.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Multiplanar GBM buffers can point to different objects from each plane.
Use the _for_plane API when possible to retrieve the correct prime FD
for each plane.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Simon Ser <contact@emersion.fr>
Tested-by: Guido Günther <agx@sigxcpu.org>
Check the fd for validity before giving a success return code.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Simon Ser <contact@emersion.fr>
Tested-by: Guido Günther <agx@sigxcpu.org>
This is not actually a change for xwayland with gbm, or for xfree86 with
big-GL, but we do change them as well to use EGL_NO_CONFIG_KHR
explicitly.
Reviewed-by: Emma Anholt <emma@anholt.net>
This reverts commit 9b89994110.
Turns out that defaulting glamor_egl->dmabuf_capable = TRUE
breaks kms page-flipping on various Mesa+Linux/DRM-KMS+hardware
combos, resulting in broken presentation timing, degraded performance
and awful tearing. E.g., my testing shows that X-Server master +
Mesa 21.2 + Linux 5.3 on Intel Kabylake has broken pageflipping.
Similar behaviour was observed in the past on RaspberryPi 4/400
with VideoCore-6 by myself and others, and iirc by myself on some
AMD gpu's, although my memories of the latter are a bit dim.
Cfe. https://gitlab.freedesktop.org/mesa/mesa/-/issues/3601 and
possibly https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/254
for related problems.
The reason for pageflip failure on the modesetting-ddx under
DRI3/Present seems to be the following sequence:
1. Atomic modesetting for the modesetting-ddx is broken and therefore
both disabled by default in the modesetting-ddx itself and also
force-disabled by the Linux kernel since quite a while. If the kernel
detects drmSetClientCap(fd, DRM_CLIENT_CAP_ATOMIC, 1); from the
X-Server, it will reject the request, as a countermeasure to all the
past and current brokeness.
2. Without DRM_CLIENT_CAP_ATOMIC we don't get the implied universal
planes support (DRM_CLIENT_CAP_UNIVERSAL_PLANES).
3. Without DRM_CLIENT_CAP_UNIVERSAL_PLANES, drmModeGetPlaneResources()
will only return overlay planes, but not primary- or cursor planes.
4. As modesetting-ddx drmmode_crtc_create_planes() function can only
operate on primary planes, but can't get any from drmModeGetPlaneResources(),
the drmmode_crtc_create_planes() mostly turns into a no-op, never
executes populate_format_modifiers() and therefore the Linux kernels
DRM-KMS driver is not ever queried for the list of scanout/pageflip
capable DRM format modifiers. Iow. the drmmode_crtc->formats[i].modifiers
list stays empty with zero drmmode_crtc->formats[i].num_modifiers.
5. The list from step 4 provides the format+modifiers for intersection
which would get returned by the X-Servers DRI3 backend as response to
a xcb_dri3_get_supported_modifiers_window_modifiers() request. Given
an empty list was returned in step 4, this will lead to return of an
empty modifiers list by xcb_dri3_get_supported_modifiers_window_modifiers().
6. Both Mesa's DRI3/Present OpenGL backbuffer allocation logic and iirc
Mesa/Vulkan/WSI/X11's swapchain image allocation logic use the list
from xcb_dri3_get_supported_modifiers_window_modifiers() for format+
modifier selection for scanout/pageflip capable buffers. Cfe. Mesa's
dri3_alloc_render_buffer() function.
Due to the empty list, the Mesa code falls back to the format+modifiers
reported by xcb_dri3_get_supported_modifiers_screen_modifiers()
instead. This list contains all modifiers reported by GLAMOR as
result of glamor_get_formats() and glamor_get_modifiers(), which
in turn are query results from Mesa eglQueryDmaBufFormatsEXT()
and eglQueryDmaBufModifiersEXT(). Iow. all format+modifiers which
are supported for rendering are considered for the OpenGL backbuffers
and Vulkan swapchain buffers.
7. Depending on kms driver + gpu combo and Mesa version, such buffers
are often not direct-scanout / pageflip capable, and so pageflipping
can't be used for DRI3/Present of fullscreen windows. Whenever the
system has to fallback to copies instead of pageflips, the results
are broken presentation timing, degraded performance and quite
horrible tearing, as the current DRI3/Present implementation does not
perform any hardware synchronization of copy presents to the start
of vblank or similar.
By defaulting glamor_egl->dmabuf_capable = FALSE instead, as the server
1.20 branch does, we avoid this failure:
1. glamor_get_modifiers() turns into a no-op and returns false, not
reporting any supported dmabuf modifiers to the servers DRI3 code,
ie. the servers cache_formats_and_modifiers() function can't retrieve
and cache any format+modifiers. Therefore the servers DRI3 code now
also reports an empty format+modifiers list when Mesa does a
xcb_dri3_get_supported_modifiers_screen_modifiers() query.
2. Mesa's buffer allocation code therefore falls back to using the old
DRI image extensions createImage() function to allocate buffers
with use flags __DRI_IMAGE_USE_SCANOUT | __DRI_IMAGE_USE_BACKBUFFER
and our OpenGL backbuffers / Vulkan swapchain images get allocated
in a direct-scanout / pageflip capable format. Pageflipping works,
timing and performance is good, presentation is tear-free.
Please consider merging this for branching the X-Server 1.21 branch.
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
When making a pixmap exportable, glamor will currently create a temporary
exported pixmap backed by a GBM bo, with the devKind updated to the stride of
the bo. However, when the backing of the exported pixmap is swapped into the
original, the devKind of the original is not updated.
Some GBM bos may get implicitly padded, in which case the devKind of the pixmap
will not match the stride of the backing bo. For example, an 800x600 pixmap will
have a devKind of 3200, but the bo's stride will be 3328. This can cause
corruption with PRIME, when the sink uses the wrong stride to display the shared
pixmap.
This commit changes glamor_make_pixmap_exportable() to update the devKind of the
original pixmap after it swaps exported pixmap's backing into it, keeping
everything consistent.
Fixes issue #1018.
Signed-off-by: Alex Goins <agoins@nvidia.com>
Signed-off-by: Aaron Plattner <aplattner@nvidia.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
The Etnaviv driver on GC2000 reports desktop OpenGL 1.3 but also OpenGL ES 2.0.
However, with the modesetting driver, GLES2 never gets a chance:
[ 11233.393] Require OpenGL version 2.1 or later.
[ 11233.393] (EE) modeset(0): Failed to initialize glamor at ScreenInit() time.
[ 11233.393] (EE)
Fatal server error:
[ 11233.395] (EE) AddScreen/ScreenInit failed for driver 0
Let's reject old desktop GL early on, just like XWayland seems to do.
This is perhaps a slightly bit more complicated that one would expect, since we
need to call eglMakeCurrent() before we query the GL version.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
DDX such as Xorg, Xwayland & Xephyr do not destroy the pixmap before
they call into CloseScreen. At the same time Xvfb (support for glamor
coming with later commit) do.
As such the pixmap will be NULL and we'll crash out.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Currently we wrap the EGL CloseScreen/DestroyPixmap callbacks after the
glamor ones. Thus upon teardown, we'll end calling things in the wrong
order.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
As of last commit, all of glamor_egl ix xf86 agnostic, so adjust the
includes and drop the GLAMOR_FOR_XORG instances.
Note the macro is still used for glamor_xv_init() which pulls xf86.
v2: Drop GLAMOR_FOR_XORG guards (Michel Dänzer)
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Currently we parse through xf86Info.debug to check if we the modifiers
should be disabled. Handle that within DDX and pass GLAMOR_NO_MODIFIERS
into the glamor_init() flags.
This allows individual DDX control over the setting - say when modifiers
are woking OK with one implementation and not the other.
Most importantly, this removes the final xf86 piece from the codebase.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Move from the xf86 specific ScrnInfoRec::privates, to the dix private
handling. Since there's no FreeScreen function in ScreenPtr, fold the
former within the existing CloseScreen.
Users, such as modesetting are updated, and out of tree drivers will
need equivalent, yet trivial, patch.
Note: we need to ensure that the screen private is unset and the screen
callbacks are restored in our CloseScreen function.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
The following two members of glamor_egl_screen_private has been unused
for a little while now.
CreateScreenResources
CloseScreen
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Much of glamor already use LogMessage() so we might as well be
consistent. This effectively paves the way of making glamor-egl xf86
agnostic.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
If a pixmap is not exportable, `glamor_gbm_bo_from_pixmap()` would fail
and the modesettings driver would consequently fail to do its page flip,
which both prevents Present from working and also fill up the logs with
error messages such as:
(EE) modeset(0): Failed to get GBM bo for flip to new front.
(EE) modeset(0): present flip failed
Refactor the code so that `glamor_gbm_bo_from_pixmap()` takes care of
making the pixmap exportable.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Signed-off-by: Yuxuan Shui yshui@hadean.com
See-also: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/131
Closes: https://gitlab.freedesktop.org/xorg/xserver/issues/68
Fixes: 86b2d8740a "glamor: Reallocate pixmap storage without modifiers
if necessary"
0a9415cf apparently can tickle bugs in the GL stack where glGetString
returns NULL, presumably because the eglMakeCurrent() didn't manage to
actually install a dispatch table and you're hitting a stub function.
That's clearly not our bug, but if it happens we should at least not
crash. Notice this case and fail gently.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Mesa started supporting GL_OES_EGL_image on llvmpipe in 17.3, after this
commit:
commit bbdeddd5fd0b797e1e281f058338b3da4d98029d
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Tue Aug 1 14:49:33 2017 -0700
st/dri: add drisw image extension
That's pretty cool, but it means glamor now thinks it can initialize on
llvmpipe. This is almost certainly not what anyone wants, as glamor on
llvmpipe is pretty much uniformly slower than fb.
This fixes both Xorg and Xwayland to refuse glamor in such a setup.
Xephyr is left alone, both because glamor is not the default there and
because Xephyr+glamor+llvmpipe is one of the easier ways to get xts to
exercise glamor.
The (very small) downside of this change is that you lose DRI3 support.
This wouldn't have helped you very much (since an lp glamor blit is
slower than a pixman blit), but it would eliminate the PutImage overhead
for llvmpipe's glXSwapBuffers. A future change should add DRI3 support
for the fb-only case.
Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Adam Jackson <ajax@redhat.com>
With a patch to mesa to expose rgb565 pbuffers even on a server with
only depth 24 and 32 visuals, fixes
dEQP-EGL.functional.render.single_context.gles2.rgb565_pbuffer. Those
pbuffers (or at least something renderable with 565) are required by
the current CTS for GLES3, and having the server support DRI3 on those
pixmaps means that we can avoid having a different path for EGL
pbuffers compared to pixmaps.
Signed-off-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
KMS drivers are not required to support GEM. In particular, vmwgfx
doesn't support flink and handles and names are identical.
Getting a bo name should really be part of a lower level API, if needed,
but in the mean time work around this by setting the name identical to
the handle if GEM isn't supported.
This fixes modesetting driver dri2 on vmwgfx.
Reviewed-by: Deepak Rawat <drawat@vmware.com>
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
When support for allocating GBM BOs with modifiers was added,
glamor_fd_from_pixmap() was changed so that it would return an error if
it got a bo with modifiers set from glamor_fds_from_pixmap(). The
problem is that on systems that support BOs with modifiers,
glamor_fds_from_pixmap() will always return BOs with modifiers.
This means that glamor_fd_from_pixmap() was broken entirely, which broke
a number of other things including glamor_shareable_fd_from_pixmap(),
which meant that modesetting using multiple GPUs with the modesetting
DDX was also broken. Easy reproducer:
- Find a laptop with DRI prime that has outputs connected to the
dedicated GPU and integrated GPU
- Try to enable one display on each using the modesetting DDX
- Fail
Since there isn't a way to ask for no modifiers from
glamor_fds_from_pixmap, we create a shared _glamor_fds_from_pixmap()
function used by both glamor_fds_from_pixmap() and
glamor_fd_from_pixmap() that calls down to the appropriate
glamor_egl_fd*_from_pixmap() function.
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Cc: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Fixes: c8c276c956 ("glamor: Implement PixmapFromBuffers and BuffersFromPixmap")
This was left disabled in 1.20.0, it's time to start being sure it
works.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Acked-by: Keith Packard <keithp@keithp.com>
Acked-by: Daniel Stone <daniels@collabora.com>
We were mixing stdint and CARD* types, causing compiler warnings on
32-bit. Just switch over to stdint, which is what we'd like the server
to be using long term, anyway.
Reviewed-by: Adam Jackson <ajax@redhat.com>
If dmabuf_capable is false, because the server "dmabuf_capable"
debug flag isn't set, treat it as successfull query with zero
returned formats, instead of failure.
This allows the servers cache_formats_and_modifiers() function
to cache the fact that formats are not supported during the
current server generation, instead of pointless retesting at
every invocation.
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Enrico Weigelt, metux IT consult
Pierre-Eric Pelloux-Prayer
Alan Coopersmith
Michel Dänzer
Ville Syrjälä
msizanoen1
Konstantin
Ivan A. Melnikov
Corentin Noël
Lucas Stach
Adam Jackson
Mario Kleiner
Zoltán Böszörményi
Alex Goins
Lubomir Rintel
Kenneth Graunke
Emil Velikov
Olivier Fourdan
Rohan Garg
Michel Dänzer
Eric Anholt
Thomas Hellstrom
Lyude Paul