In all codepaths, this var ends up assigned to the struct, except one:
a cleanup codepath, where the `close()` was removed, leading to fd leaks.
Remove the temp fd and assign to the struct field directly instead.
CovID: 1213930
Fixes: 7ec07beedf ("egl/drm: make use of the
dri2_display_destroy() helper")
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
v2: check for DRIimageExtension version 15 (Jason Ekstrand)
Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
query and return supported dmabuf format modifiers for
EGL_EXT_image_dma_buf_import_modifiers.
v2: move format check to the driver instead of making format queries
here and then checking.
v3: Check DRIimageExtension version before query (Daniel Stone)
v4:
- move to DRIimageExtension version 15, check queryDmaBufModifiers before
calling (Jason Ekstrand)
- pass external_only to the driver instead of setting as EGL_TRUE here
(Emil Velikov, Daniel Stone)
Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
allow egl clients to query the dmabuf formats supported on this platform.
v2: return EGLBoolean.
v3: Check DRIimageExtension version before querying (Daniel Stone).
v4: move to DRIimageExtension version 15, error checking (Jason Ekstrand).
Signed-off-by: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Allow creating EGLImages with dmabuf format modifiers when target is
EGL_LINUX_DMA_BUF_EXT for EGL_EXT_image_dma_buf_import_modifiers.
v2:
- clear modifier assembling and error label name (Eric Engestrom)
v3:
- remove goto jumps within switch-case (Emil Velikov)
- treat zero as valid modifier (Daniel Stone)
- ensure same modifier across all dmabuf planes (Emil Velikov)
v4:
- allow modifiers to add extra planes (Louis-Francis Ratté-Boulianne)
v5:
- fix error checking, some cleanups (Jason Ekstrand)
- pass single copy of the modifier to createImageFromDmaBufs2
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
Signed-off-by: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
The EGL_EXT_dma_buf_import_modifiers extension adds support for a
fourth plane, just like DRM KMS API does.
Bump maximum dma_buf plane count to four.
v2: prevent attribute tokens from being parsed if
EXT_image_dma_buf_import_modifiers is not suported. (Emil Velikov)
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Rather than hardcoding 3, use a #define. Makes it easier to bump this
later to 4.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
v2:
bump version
v3:
Add code comment
s/IsGlThread/IsThread/ (and variation)
Include X11/Xlibint.h protected by ifdef
v5: based on Daniel feedback
Move non X11 code outside of X11 define
Always return true for Wayland
Signed-off-by: Gregory Hainaut <gregory.hainaut@gmail.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Analogous to earlier commits - image_driver and image_loader are meant
to be used hand in hand.
v2: Rebase
Cc: Derek Foreman <derekf@osg.samsung.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Strictly speaking __DRI_DRI2 implies __DRI2_FLUSH. Although since we're
using the latter in the callback, we want to use the correct guard.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Rather than misleadingly depending on DRI2 for the WL_DRM vs WL_SHM
formats, use the wl_drm and wl_shm interface respectively.
Fixes: a1727aa75e ("egl/wayland: Don't use DRM format codes for SHM")
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Analogous to previous commit.
Cc: Chad Versace <chadversary@chromium.org>
Cc: Gurchetan Singh <gurchetansingh@chromium.org>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
They are meant to be used together. Otherwise we'll need workarounds
like egl/wayland. Namely register an image_loader_extension even thought
we should be using only DRI2.
v2: Add missing the bracket to fix the build (Tapani).
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Analogous to earlier commit.
Note that the dri2_x11_post_sub_buffer and dri2_x11_swap_buffers_region
paths already implicitly require __DRI2_FLUSH. The corresponding
extensions (NV_post_sub_buffer and NOK_swap_region) are enabled only
with DRI2.
v2: Split cosmetic changes into separate patch.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
The current __DRI_DRI2 imples __DRI2_FLUSH. At the same time, one can
use __DRI_IMAGE_DRIVER alongside the latter, so the current check is
confusing at best.
Check for what we use.
v2: Split out from whitespace changes
Reviewed-by: Chad Versace <chadversary@chromium.org> (v1)
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
With the final place that modifies the vtbl removed as of last commit we
can annotate the symbols accordingly.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
With previous commit we'll error out should one be using the extension
when it's not available. Thus we no longer need to modify the vtbl.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Currently f one does the silly thing by probing the entry point w/o
checking the extension they will attempt to use the extension even
though it cannot work.
That is due our of of an assert which gets removed in release builds.
Simply error out if the extension is not enabled. Thus we can
apply some cleanups with next commits.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Allows us to keep things in sync easier and lets us simplify the
interface between the two even further.
v2: Don't set GBM's extensions.
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Tested-by: Rob Herring <robh@kernel.org>
Split the create_screen into:
- create screen
- setup/bind extensions
- setup screen
This will allow us to reuse the latter two on egl/drm. Said platform
does create its own screen and attempts to reinvent the later two
functions itself.
Since the GBM ones tend to get out of sync quite often, and there is no
distinct reason why it does so we'll drop them with latter commits.
v2: disp -> dpy for the Android platform.
v3: use correct goto label (Rob)
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Tested-by: Rob Herring <robh@kernel.org>
Cc: Chad Versace <chadversary@chromium.org>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Rob Herring <robh@kernel.org>
Within dri2_display_release() we already tear down all the display
specifics. Within the platform specific dri initialize however we badly
and partially duplicate that.
Let's stop that by fleshing out the required functionality into a helper
and using it throughout the codebase.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Rob Herring <robh@kernel.org>
With later commits we'll split and reuse the destroy side of the
function for the initialize_foo error path.
In such cases, driver_configs may be NULL leading to a crash.
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
[Emil Velikov: reword commit message]
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Tested-by: Rob Herring <robh@kernel.org>
The former already keeps track of the DRI module opened, based on the
driver_name provided. So let's keep them together.
As a nice bonus this Will allows us to remove the gbm_drm_device all
together with next patch.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Tested-by: Rob Herring <robh@kernel.org>
The struct is a simple wraper around gbm_bo and brings no actual
benefit.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Tested-by: Rob Herring <robh@kernel.org>
We're already verified that 'window' wasn't NULL, I'm guessing this
allocation error is about the newly created queue.
CID: 1409754
Fixes: 03dd9a88b0 ("egl/wayland: Use per-surface event queues")
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Rather than having multiple places that define the macros, do it just
once in configure. Makes existing code a bit shorter and easier to
manage as we fix the VL targets with follow-up commits.
Cc: mesa-stable@lists.freedesktop.org
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
Rename the remaining references to omit the egl part.
Cc: mesa-stable@lists.freedesktop.org
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
Analogous to others earlier, these will be used to control the platform
for more than the EGL driver.
Cc: <mesa-stable@lists.freedesktop.org>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
Function droid_swap_buffers may get called without dri2_surf->buffer set,
in these cases we don't have a back buffer set either. Patch fixes segfault
seen with 3DMark that uses android.opengl.GLSurfaceView for rendering it's UI.
backtrace:
#00 pc 00013f88 /system/lib/egl/libGLES_mesa.so (droid_swap_buffers+104)
#01 pc 000117b2 /system/lib/egl/libGLES_mesa.so (dri2_swap_buffers+50)
#02 pc 000058b2 /system/lib/egl/libGLES_mesa.so (eglSwapBuffers+386)
#03 pc 00011329 /system/lib/libEGL.so (eglSwapBuffersWithDamageKHR+553)
#04 pc 000118e7 /system/lib/libEGL.so (eglSwapBuffers+55)
#05 pc 000754dc /system/lib/libandroid_runtime.so
Note, this is v1 as v2 caused dEQP regressions.
Fixes: 2acc69d ("EGL/Android: Add EGL_EXT_buffer_age extension")
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Acked-by: Emil Velikov <emil.velikov@collabora.com>
Cc: "17.1" <mesa-stable@lists.freedesktop.org>
Commit 9ca6711faa changed the Wayland winsys to only block for the
frame callback inside SwapBuffers, rather than get_back_bo. get_back_bo
would perform a single non-blocking Wayland event dispatch, to try to
find any release events which we had pulled off the wire but not
actually processed. The blocking dispatch was moved to SwapBuffers.
This removed a guarantee that we would've processed all events inside
get_back_bo(), and introduced a failure whereby the server could've sent
a buffer release event, but we wouldn't have read it. In clients
unconstrained by SwapInterval (rendering ~as fast as possible), which
were being displayed directly without composition (buffer release delayed),
this could lead to get_back_bo() failing because there were no free
buffers available to it.
The drawing rightly failed, but this was papered over because of the
path in eglSwapBuffers() which attempts to guarantee a BO, in order to
support calling SwapBuffers twice in a row with no rendering actually
having been performed.
Since eglSwapBuffers will perform a blocking dispatch of Wayland
events, a buffer release would have arrived by that point, and we
could then choose a buffer to post to the server. The effect was that
frames were displayed out-of-order, since we grabbed a frame with random
past content to display to the compositor.
Ideally get_back_bo() failing should store a failure flag inside the
surface and cause the next SwapBuffers to fail, but for the meantime,
restore the correct behaviour such that get_back_bo() no longer fails.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reported-by: Eero Tamminen <eero.t.tamminen@intel.com>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98833
Fixes: 9ca6711faa ("Revert "wayland: Block for the frame callback in get_back_bo not dri2_swap_buffers"")
During display initialisation, we need a separate event queue to handle
the registry events, which is correctly handled. But we also need
separate per-surface event queues to handle swapchain-related events,
such as surface frame events and buffer release events. This avoids two
surfaces from the same EGLDisplay, both current on separate threads,
dispatching each other's events.
Create separate per-surface event queues, create wl_surface and wl_drm
proxy wrapper objects per surface, so we eliminate the race around
sending events to the wrong queue. swrast buffers do not need a
dedicated proxy wrapper, as the wl_shm_pool used to create the
wl_buffers, being transient, can itself be assigned to a queue.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: 36b9976e1f ("egl/wayland: Avoid race conditions when on non-main thread")
Cc: mesa-stable@lists.freedesktop.org
wl_display_roundtrip_queue() exists and can replace roundtrip(). The
API was introduced with wayland 1.6, while we currently require 1.11.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: mesa-stable@lists.freedesktop.org
Fixes regressions in Android CtsVerifier.apk on Intel Chrome OS devices
due to incorrect error handling in eglMakeCurrent. See below on how to
confirm the regression is fixed.
This partially reverts
commit 23c86c74cc
Author: Chad Versace <chadversary@chromium.org>
Subject: egl: Emit error when EGLSurface is lost
The problem with commit 23c86c74 is that, once an EGLSurface became
lost, the app could never unbind the bad surface. Each attempt to unbind
the bad surface with eglMakeCurrent failed with EGL_BAD_CURRENT_SURFACE.
Specificaly, the bad commit added the error handling below. #2 and #3
were right, but #1 was wrong.
1. eglMakeCurrent emits EGL_BAD_CURRENT_SURFACE if the calling
thread has unflushed commands and either previous surface is no
longer valid.
2. eglMakeCurrent emits EGL_BAD_NATIVE_WINDOW if either new surface
is no longer valid.
3. eglSwapBuffers emits EGL_BAD_NATIVE_WINDOW if the swapped surface
is no longer valid.
Whe I wrote the bad commit, I misunderstood the EGL spec language
for #1. The correct behavior is, if I understand correctly now, is
below. This patch doesn't implement the correct behavior, though, it
just reverts the broken behavior.
- Assume a bound EGLSurface is no longer valid.
- Assume the bound EGLContext has unflushed commands.
- The app calls eglMakeCurrent. The spec requires eglMakeCurrent to
implicitly flush. After flushing, eglMakeCurrent emits
EGL_BAD_CURRENT_SURFACE and does *not* alter the thread's
current bindings.
- If the app calls eglMakeCurrent again, and the app inserts no
commands into the GL command stream between the two eglMakeCurrent
calls, then this second eglMakeCurrent succeeds without emitting an
error.
How to confirm this fixes the regression:
Download android-cts-verifier-7.1_r5-linux_x86-x86.zip from
source.android.com, unpack, and `adb install CtsVerifier.apk`.
Run test "Projection Cube". Click the Pass button (a
green checkmark). Then run test "Projection Widget". Confirm that
widgets are visible and that logcat does not complain about
eglMakeCurrent failure.
Then confirm there are no regressions in the cts-traded module that
commit 263243b1 fixed:
cts-tf > run cts --skip-preconditions --skip-device-info \
-m CtsCameraTestCases \
-t android.hardware.camera2.cts.RobustnessTest
Tested with Chrome OS board "reef".
Fixes: 23c86c74 (egl: Emit error when EGLSurface is lost)
Acked-by: Tapani Pälli <tapani.palli@intel.com>
Cc: "17.1" <mesa-stable@lists.freedesktop.org>
Cc: Tomasz Figa <tfiga@chromium.org>
Cc: Nicolas Boichat <drinkcat@chromium.org>
Cc: Emil Velikov <emil.velikov@collabora.com>
1ce5853 broken compilation since LOG_ERROR is not defined and also
macro expansion won't work as planned (expands to 'ANDROID_egl2alog[level]')
v2: append 'ANDROID' to egl2alog table and use LOG_PRI
(suggested by Chih-Wei Huang)
Fixes: 1ce5853 ("egl: simplify the Android logger")
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Drop the unsupported pre-JellyBean macros and use a simple egl2android
mapping. With this we loose the explicit abort() provided by LOG_FATAL,
although Mesa already already calls exit(1) in case of a fatal errors.
Suggested-by: Rob Herring <robh@kernel.org>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Rob Herring <robh@kernel.org>
src/gallium/targets/dri/Android.mk contains lots of conditional for
individual drivers. Let's move these details into the individual driver
makefiles.
In the process, align the make driver conditionals with automake
(i.e. HAVE_GALLIUM_*).
Signed-off-by: Rob Herring <robh@kernel.org>
[Emil Velikov: add the radeon winsys for radeonsi]
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
main/egldisplay.c: In function '_eglParseX11DisplayAttribList':
main/egldisplay.c:491:38: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
display->Options.Platform = (void *)value;
The fix: cast to uinptr_t before void*.
^
Fixes: ddb99127 egl/x11: Honor the EGL_PLATFORM_X11_SCREEN_EXT attribute
Cc: Adam Jackson <ajax@redhat.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
All the compilers used to build Mesa support them.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
Reviewed-by: Chad Versace <chadversary@chromium.org>
The array is local so we already know its size.
v2: Correct loop condition (Bartosz)
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
Reviewed-by: Chad Versace <chadversary@chromium.org>
The header is used only to provide STATIC_ASSERT. The latter is already
available in utils/macros.h so use that instead and kill of the header.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
Reviewed-by: Chad Versace <chadversary@chromium.org>
The variable level is already initialized to -1 which is already
interpreted as FALLBACK_LOG_LEVEL.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
Reviewed-by: Chad Versace <chadversary@chromium.org>
