Replace assert() with proper error checking for realloc() failure.
If realloc fails, free any existing modifiers, clean up resources,
and return NULL instead of potentially crashing or leaking memory.
Fixes a potential memory leak when memory allocation fails.
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39215>
In this case, do the wrapping logic on our end and normalize right away
to host time domain.
Signed-off-by: Hans-Kristian Arntzen <post@arntzen-software.no>
Reviewed-by: Emma Anholt <emma@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38770>
Deal with VRR vs FRR as well.
Loosely based on earlier work by Keith Packard and Emma Anholt
(MR 38472 for reference).
Signed-off-by: Hans-Kristian Arntzen <post@arntzen-software.no>
Reviewed-by: Emma Anholt <emma@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38770>
Looks like the "new" page flip handler was in 2.4.78 in 2017. Mesa
requires at least 2.4.109, so we can retire this.
Reviewed-by: Hans-Kristian Arntzen <post@arntzen-software.no>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38770>
This is mostly provided for convenience, but it's not implementable by
applications when we're using blit queues for PRIME, so it's quite useful
to have.
This is reworked from previous GOOGLE_display_timing
MRs by Keith Packard and Emma Anholt.
See MR 38472 for reference.
Rather than exposing PRESENT_STAGE_LOCAL, we expose all timestamps in
one unified domain to simplify the implementation.
Signed-off-by: Hans-Kristian Arntzen <post@arntzen-software.no>
Reviewed-by: Emma Anholt <emma@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38770>
Only weakness right now is that we cannot implement VRR vs FRR query reliably.
Signed-off-by: Hans-Kristian Arntzen <post@arntzen-software.no>
Reviewed-by: Derek Foreman <derek.foreman@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38770>
We get a display fd passed in to us through wsi_display_init_wsi(), and
when that was the first open of the display device with no previous DRM
master, it got master privs and we saved that as the display fd to use for
KHR_display. However, that meant that no other client can get DRM master,
preventing things like vkAcquireDRMDisplayEXT() users from getting a
master fd to pass in to us.
Instead, we can drop master at device init time, and pick it back up when
a VK_KHR_display swapchain is created that uses that fd.
This allows dEQP-VK.wsi.acquire_drm and dEQP-VK.wsi.direct_drm CTS tests
to run, which was previously impossible (those tests try to create a
custom VK instance, while the CTS already has an instance that had been
created with KHR_display enabled, so they're not the first open of the
fd). It also means that you could successfully implement VT switching
between a KHR_display client and other userspace DRM clients. Also, we
can finally implement the text about vkAcquireDRMDisplayEXT's drmFd
needing to match the device's fd.
The risk of this change, though, is if you're implementing a compositor,
and your clients have a chance to open the DRM fd before you've created
your swapchain, they may inadvertently have master and DOS you. However,
this is no different than the previous situation, where someone with
permissions to open DRM could hold master and DOS you already.
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38502>
Use wl_display_roundtrip() instead of wl_display_flush() when freeing
a swapchain to ensure the compositor has processed buffer release
events before continuing.
wl_display_flush() only sends pending requests without waiting for
the compositor to process them. When rapidly creating and destroying
large swapchain buffers, buffer references may not be released quickly
enough (e.g., during CTS testing), causing memory to accumulate.
Using wl_display_roundtrip() ensures synchronization with the
compositor, allowing buffers to be released promptly.
Signed-off-by: Wei Zhao <Wei.Zhao@amd.com>
Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39475>
Common wsi should only create that semaphore if present wait is enabled.
Drivers may only advertise KHR_present_wait2 w/o KHR_present_wait. So
the common wsi core has to check for both.
Fixes: bf285c3be9 ("vulkan/wsi: Add basic support for PresentWait2")
Reviewed-by: Hans-Kristian Arntzen <post@arntzen-software.no>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39322>
For mitigating the below VVL violation:
> VUID-vkCmdPipelineBarrier-srcStageMask-09634
>
> vkCmdPipelineBarrier(): dstStageMask is VK_PIPELINE_STAGE_HOST_BIT
> but srcQueueFamilyIndex (0) != dstQueueFamilyIndex (4294967293).
>
> The Vulkan spec states: If either srcStageMask or dstStageMask
> includes VK_PIPELINE_STAGE_HOST_BIT, for any element of
> pBufferMemoryBarriers, srcQueueFamilyIndex and dstQueueFamilyIndex
> must be equal
Fixes: 153857fb64 ("vulkan/wsi: amend barriers for blit dst buffer going to foreign queue")
Reviewed-by: Hans-Kristian Arntzen <post@arntzen-software.no>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39322>
We need to support this if we advertise KHR_swapchain_maintenance1
Cc: mesa-stable
Signed-off-by: llyyr <llyyr.public@gmail.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39218>
Similar to last MR, since we advertise support for
KHR_swapchain_maintenance1, we need to advertise this
Cc: mesa-stable
Signed-off-by: llyyr <llyyr.public@gmail.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39218>
Currently the image creation failure handling codepath of
wsi_headless_surface_create_swapchain() just calls
wsi_headless_swapchain_destroy() , which will try to destroy all
`image_count` of images. However, some of these images might never be
successfully created because of the failure, which leads to double-free.
Set image_count to the number of successfully created images before
calling wsi_headless_swapchin_destroy() to prevent over-destroying.
Fixes dEQP-VK.wsi.headless.swapchain.simulate_oom.* on lavapipe and pvr,
although some of the tests got QualityWarning saying "Creating swapchain
did not succeed, callback limit exceeded" on lavapipe (Pass on pvr).
Cc: mesa-stable
Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Reviewed-by: Frank Binns <frank.binns@imgtec.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39039>
Since we use atomic mode setting now, the wsi->fd we use needs to have
the atomic client cap.
There are several different code paths where wsi can acquire a file
descriptor. For drm masters, the atomic client cap is set in
wsi_display_init_wsi. For leased drm fds, there are AcquireDrmDisplayEXT
and AcquireXlibDisplayEXT.
According to a comment we previously assumed wsi_display_get_connector
is common among all code paths, and that's why the atomic client cap was
set there. But that assumption can be broken based on the particular
order which the application invokes vulkan APIs in.
This commit simply push the drmSetClientCap to all entrypoints where a
drm fd comes through.
Fixes: 513ffea1d3 ("wsi/display: use atomic mode setting")
Signed-off-by: Yuxuan Shui <yshui@codeweavers.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38987>
Replace the duplicated swapchain image detection pattern across all
Vulkan drivers with the new wsi_common_is_swapchain_image() helper.
Since the swapchain handle can be extracted from VkImageCreateInfo's
pNext chain inside wsi_common_create_swapchain_image(), remove the
now-redundant VkSwapchainKHR parameter from that function.
This removes the #ifdef guards for Android/WSI platforms from each
driver, as the helper now handles this uniformly.
Signed-off-by: Christian Gmeiner <cgmeiner@igalia.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
Reviewed-by: Faith Ekstrand <faith.ekstrand@collabora.com>
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Yonggang Luo <luoyonggang@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38541>
Add a helper function to check if a VkImageCreateInfo represents a
swapchain image by looking for VkImageSwapchainCreateInfoKHR in the
pNext chain.
This consolidates the swapchain detection logic that is currently
duplicated across all Vulkan drivers, and handles the Android case
in one place.
Signed-off-by: Christian Gmeiner <cgmeiner@igalia.com>
Suggested-by: Faith Ekstrand <faith.ekstrand@collabora.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
Reviewed-by: Faith Ekstrand <faith.ekstrand@collabora.com>
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Yonggang Luo <luoyonggang@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38541>
Current extent can be 0xFFFFFFFF (-1) which indicates there is no current extent, and the swapchain will inherit that of which the application provides.
Check this before applying the hack.
Signed-off-by: Joshua Ashton <joshua@froggi.es>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/31134>
CTA-861-G section 6.9.1 Static Metadata Type 1 declares that zero values
for different groups of HDR Metadata properties are allowed, including
zero nits values for max display mastering luminance, max content light
level, max frame-average light level and min display mastering luminance.
A zero value is meant to be treated by the video sink as "undefined" /
"unknown", and handled accordingly. This is common for dynamically
generated visual content.
The is_hdr_metadata_legal() function in the Vulkan/WSI/Wayland HDR backend
currently declares HDR light level metadata as invalid if the mastering
display min_luminance and max_luminance light levels are set to the legal
level of zero nits. This causes valid HDR metadata as set by the client
via vkSetHdrMetadata() to be not sent to the compositor.
Fix this by skipping checks that don't apply if min_luminance or
max_luminance are zero. If max_luminance is zero then we skip sending
of mastering display min/max luminance to Wayland, as sending a a
max_luminance <= min_luminance would trigger a protocol error. All
other valid data is still send, ie. color primaries, white-point,
content light levels.
Fixes: cb7726bb2c ("vulkan/wsi: validate HDR metadata to not cause protocol errors")
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Co-authored-by: Michel Dänzer <michel@daenzer.net>
Reviewed-by: Xaver Hugl <xaver.hugl@kde.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38326>
GTK is missing a semaphore between QueueSubmit() and QueuePresent()
causing the WSI submit to be "unordered" and to immediately signal the
semaphores (because it's missing a wait semaphore in QueuePresent()).
The workaround is to disable unordered WSI submits until GTK fixes it
properly.
Cc: "25.3"
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/14087
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38351>
At least some drivers need a full modeset to change the Colorspace
property or to en-/disable HDR mode. E.g., at least amdgpu-kms as
tested under Linux 6.8 on Polaris needs it. Otherwise the atomic
commit for disabling HDR in _wsi_display_cleanup_state() will fail,
and the connector stays stuck in HDR mode after vkDestroySwapchainKHR().
Fixes: 1ed78dd7ec ("wsi/display: Clean up DRM hdr/color state on swapchain destruction")
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Reviewed-by: Autumn Ashton <misyl@froggi.es>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/37880>
For a selected non-default imageColorSpace during swapchain creation,
make sure that proper HDR setup also works even if a client app does not
explicitly call vkSetHdrMetadataEXT() in time.
Assign the EDID provided metadata here, so the 1st atomic commit will
set Colorspace and HDR metadata properties on the connector, to make sure
HDR or other wide color gamut modes get enabled.
Without this, the chain->color_outcome_serial would stay at zero and
the properties would not ever get assigned during drm_atomic_commit(),
leaving HDR disabled on the display sink.
Fixes: 13137393f6 ("wsi/display: Expose HDR10 colorspace based on EDID")
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Reviewed-by: Autumn Ashton <misyl@froggi.es>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/37880>
CTA-861-G section 6.9.1 Static Metadata Type 1 declares that zero values
for different groups of HDR Metadata properties are allowed, including
zero nits values for max display mastering luminance, max content light
level, max frame-average light level and min display mastering luminance.
A zero value is meant to be treated by the video sink as "undefined" /
"unknown", and handled accordingly. This is common for dynamically
generated visual content.
Therefore don't assert on some minimum nits level > 0, but only check for
a non-negative level.
Fixes: b4176393a0 ("wsi/display: Implement VK_EXT_hdr_metadata on KHR_display swapchain")
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Reviewed-by: Autumn Ashton <misyl@froggi.es>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/37880>
The standard way to query options in mesa is `os_get_option()` which
abstracts platform-specific mechanisms to get config variables.
However in quite a few places `getenv()` is still used and this may
preclude controlling some options on some systems.
For instance it is not generally possible to use `MESA_DEBUG` on
Android.
So replace most `getenv()` occurrences with `os_get_option()` to
support configuration options more consistently across different
platforms.
Do the same with `secure_getenv()` replacing it with
`os_get_option_secure()`.
The bulk of the proposed changes are mechanically performed by the
following script:
-----------------------------------------------------------------------
#!/bin/sh
set -e
replace() {
# Don't replace in some files, for example where `os_get_option` is defined,
# or in external files
EXCLUDE_FILES_PATTERN='(src/util/os_misc.c|src/util/u_debug.h|src/gtest/include/gtest/internal/gtest-port.h)'
# Don't replace some "system" variables
EXCLUDE_VARS_PATTERN='("XDG|"DISPLAY|"HOME|"TMPDIR|"POSIXLY_CORRECT)'
git grep "[=!( ]$1(" -- src/ | cut -d ':' -f 1 | sort | uniq | \
grep -v -E "$EXCLUDE_FILES_PATTERN" | \
while read -r file;
do
# Don't replace usages of XDG_* variables or HOME
sed -E -e "/$EXCLUDE_VARS_PATTERN/!s/([=!\( ])$1\(/\1$2\(/g" -i "$file";
done
}
# Add const to os_get_option results, to avoid warning about discarded qualifier:
# warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
# but also errors in some cases:
# error: invalid conversion from ‘const char*’ to ‘char*’ [-fpermissive]
add_const_results() {
git grep -l -P '(?<!const )char.*os_get_option' | \
while read -r file;
do
sed -e '/^\s*const/! s/\(char.*os_get_option\)/const \1/g' -i "$file"
done
}
replace 'secure_getenv' 'os_get_option_secure'
replace 'getenv' 'os_get_option'
add_const_results
-----------------------------------------------------------------------
After this, the `#include "util/os_misc.h"` is also added in files where
`os_get_option()` was not used before.
And since the replacements from the script above generated some new
`-Wdiscarded-qualifiers` warnings, those have been addressed as well,
generally by declaring `os_get_option()` results as `const char *` and
adjusting some function declarations.
Finally some replacements caused new errors like:
-----------------------------------------------------------------------
../src/gallium/auxiliary/gallivm/lp_bld_misc.cpp:127:31: error: no matching function for call to 'strtok'
127 | for (n = 0, option = strtok(env_llc_options, " "); option; n++, option = strtok(NULL, " ")) {
| ^~~~~~
/android-ndk-r27c/toolchains/llvm/prebuilt/linux-x86_64/bin/../sysroot/usr/include/string.h:124:17: note: candidate function not viable: 1st argument ('const char *') would lose const qualifier
124 | char* _Nullable strtok(char* _Nullable __s, const char* _Nonnull __delimiter);
| ^ ~~~~~~~~~~~~~~~~~~~
-----------------------------------------------------------------------
Those have been addressed too, copying the const string returned by
`os_get_option()` so that it could be modified.
In particular, the error above has been fixed by copying the `const
char *env_llc_options` variable in
`src/gallium/auxiliary/gallivm/lp_bld_misc.cpp` to a `char *` which can
be tokenized using `strtok()`.
Reviewed-by: Eric Engestrom <eric@igalia.com>
Reviewed-by: Yonggang Luo <luoyonggang@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38128>
Most of the time, we can infer the type to append in
util_dynarray_append using __typeof__, which is standardized in C23 and
support in Jesse's MSMSVCV. This patch drops the type argument most of
the time, making util_dynarray a little more ergonomic to use.
This is done in four steps.
First, rename util_dynarray_append -> util_dynarray_append_typed
bash -c "find . -type f -exec sed -i -e 's/util_dynarray_append(/util_dynarray_append_typed(/g' \{} \;"
Then, add a new append that infers the type. This is much more ergonomic
for what you want most of the time.
Next, use type-inferred append as much as possible, via Coccinelle
patch (plus manual fixup):
@@
expression dynarray, element;
type type;
@@
-util_dynarray_append_typed(dynarray, type, element);
+util_dynarray_append(dynarray, element);
Finally, hand fixup cases that Coccinelle missed or incorrectly
translated, of which there were several because we can't used the
untyped append with a literal (since the sizeof won't do what you want).
All four steps are squashed to produce a single patch changing every
util_dynarray_append call site in tree to either drop a type parameter
(if possible) or insert a _typed suffix (if we can't infer). As such,
the final patch is best reviewed by hand even though it was
tool-assisted.
No Long Linguine Meals were involved in the making of this patch.
Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@intel.com>
Acked-by: Faith Ekstrand <faith.ekstrand@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38038>
It's not really clear whether or not it should use gamma 2.2 or the piece-wise
transfer function, or how clients would use it for wider gamut in general.
Currently no compositors I know of support ext_srgb, so this shouldn't affect
applications in practice.
Signed-off-by: Xaver Hugl <xaver.hugl@kde.org>
Fixes: 4b663d56 ("vulkan/wsi: implement support for VK_EXT_hdr_metadata on Wayland")
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/36444>
This reverts commit a219308867.
It's failing most of the tests on Anv :
$ ./deqp-vk -n dEQP-VK.wsi.xlib.maintenance1.scaling.*
Test run totals:
Passed: 88/2422 (3.6%)
Failed: 576/2422 (23.8%)
Not supported: 1758/2422 (72.6%)
Warnings: 0/2422 (0.0%)
Waived: 0/2422 (0.0%)
The only passing tests seem to be with this pattern :
dEQP-VK.wsi.xlib.maintenance1.scaling.*.same_size_and_aspect
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/37904>
Monado calls vkGetRandROutputDisplayEXT, which calls wsi_display_get_output, which then calls
wsi_display_alloc_connector with fd = wsi->fd, which at this point is still -1 as the display is not leased yet.
Signed-off-by: Torge Matthies <openglfreak@googlemail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/36325>
