When creating an image out of a swapchain on Android, the android
layer call will detect a VkBindImageMemorySwapchainInfoKHR in the
pNext chain of the vkBindImageMemory2() call and add a
VkNativeBufferANDROID in the chain. This is what we should use as
backing memory for that image.
v2: Fix a couple of obvious mistakes (Tapani)
v3: Silence build warning (Lionel)
Fix invalid object argument to vk_error() (Lionel)
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: bc3c71b87a ("anv: don't try to access Android swapchains")
Cc: mesa-stable
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/5180
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12244>
drm_i915_query_perf_config::data is an unsized array and declaring a
struct containing an unsized array that isn't at the end is a GNU
extension which trips up Android builds. Instead, stuff both into a
char array of the appropriate size. This emulates what you'd normally
do to allocate one of these with malloc only on the stack.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12308>
The gfx6_gs_visitor overrides emit_urb_write_opcode but with a different
function signature. This causes warnings with -Woverloaded-virtual.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12308>
They require expat which we don't have on Android.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12308>
On Gfx4 and Gfx5, sel.l (for min) and sel.ge (for max) are implemented
using a separte cmpn and sel instruction. This lowering occurs in
fs_vistor::lower_minmax which is called very, very late... a long, long
time after the first calls to opt_cmod_propagation. As a result,
conditional modifiers can be incorrectly propagated across sel.cond on
those platforms.
No tests were affected by this change, and I find that quite shocking.
After just changing flags_written(), all of the atan tests started
failing on ILK. That required the change in cmod_propagatin (and the
addition of the prop_across_into_sel_gfx5 unit test).
Shader-db results for ILK and GM45 are below. I looked at a couple
before and after shaders... and every case that I looked at had
experienced incorrect cmod propagation. This affected a LOT of apps!
Euro Truck Simulator 2, The Talos Principle, Serious Sam 3, Sanctum 2,
Gang Beasts, and on and on... :(
I discovered this bug while working on a couple new optimization
passes. One of the passes attempts to remove condition modifiers that
are never used. The pass made no progress except on ILK and GM45.
After investigating a couple of the affected shaders, I noticed that
the code in those shaders looked wrong... investigation led to this
cause.
v2: Trivial changes in the unit tests.
v3: Fix type in comment in unit tests. Noticed by Jason and Priit.
v4: Tweak handling of BRW_OPCODE_SEL special case. Suggested by Jason.
Fixes: df1aec763e ("i965/fs: Define methods to calculate the flag subset read or written by an fs_inst.")
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Tested-by: Dave Airlie <airlied@redhat.com>
Iron Lake
total instructions in shared programs: 8180493 -> 8181781 (0.02%)
instructions in affected programs: 541796 -> 543084 (0.24%)
helped: 28
HURT: 1158
helped stats (abs) min: 1 max: 1 x̄: 1.00 x̃: 1
helped stats (rel) min: 0.35% max: 0.86% x̄: 0.53% x̃: 0.50%
HURT stats (abs) min: 1 max: 3 x̄: 1.14 x̃: 1
HURT stats (rel) min: 0.12% max: 4.00% x̄: 0.37% x̃: 0.23%
95% mean confidence interval for instructions value: 1.06 1.11
95% mean confidence interval for instructions %-change: 0.31% 0.38%
Instructions are HURT.
total cycles in shared programs: 239420470 -> 239421690 (<.01%)
cycles in affected programs: 2925992 -> 2927212 (0.04%)
helped: 49
HURT: 157
helped stats (abs) min: 2 max: 284 x̄: 62.69 x̃: 70
helped stats (rel) min: 0.04% max: 6.20% x̄: 1.68% x̃: 1.96%
HURT stats (abs) min: 2 max: 48 x̄: 27.34 x̃: 24
HURT stats (rel) min: 0.02% max: 2.91% x̄: 0.31% x̃: 0.20%
95% mean confidence interval for cycles value: -0.80 12.64
95% mean confidence interval for cycles %-change: -0.31% <.01%
Inconclusive result (value mean confidence interval includes 0).
GM45
total instructions in shared programs: 4985517 -> 4986207 (0.01%)
instructions in affected programs: 306935 -> 307625 (0.22%)
helped: 14
HURT: 625
helped stats (abs) min: 1 max: 1 x̄: 1.00 x̃: 1
helped stats (rel) min: 0.35% max: 0.82% x̄: 0.52% x̃: 0.49%
HURT stats (abs) min: 1 max: 3 x̄: 1.13 x̃: 1
HURT stats (rel) min: 0.12% max: 3.90% x̄: 0.34% x̃: 0.22%
95% mean confidence interval for instructions value: 1.04 1.12
95% mean confidence interval for instructions %-change: 0.29% 0.36%
Instructions are HURT.
total cycles in shared programs: 153827268 -> 153828052 (<.01%)
cycles in affected programs: 1669290 -> 1670074 (0.05%)
helped: 24
HURT: 84
helped stats (abs) min: 2 max: 232 x̄: 64.33 x̃: 67
helped stats (rel) min: 0.04% max: 4.62% x̄: 1.60% x̃: 1.94%
HURT stats (abs) min: 2 max: 48 x̄: 27.71 x̃: 24
HURT stats (rel) min: 0.02% max: 2.66% x̄: 0.34% x̃: 0.14%
95% mean confidence interval for cycles value: -1.94 16.46
95% mean confidence interval for cycles %-change: -0.29% 0.11%
Inconclusive result (value mean confidence interval includes 0).
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12191>
Changes:
- removal of attr_wa_state (it's passed directly)
- nir_metadata_preserve(..., nir_metadata_all) is called when pass doesn't
make progress
Signed-off-by: Marcin Ślusarz <marcin.slusarz@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12324>
Setting it to GPU can cause an L3$ flush in certain cases. That's not
what we want as we really only care about coherency within the GPU.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Sagar Ghuge <sagar@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12291>
It's called anv_image_* so it really should take an anv_image. For the
couple of cases where we really want to pass in a set of aspects, we
leave an anv_aspect_to_plane() helper. anv_image_aspect_to_plane() is
then just a wrapper around it which grabs the aspects from the image.
While we're in the area, sprinkle some const around.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12141>
The new versions should have identical output, just a simpler (and
probably faster) implementation and more/better asserts.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12141>
The Vulkan 1.2.184 spec says:
"When creating a VkImageView, if sampler Y′CBCR conversion is
enabled in the sampler, the aspectMask of a subresourceRange used by
the VkImageView must be VK_IMAGE_ASPECT_COLOR_BIT.
When creating a VkImageView, if sampler Y′CBCR conversion is not
enabled in the sampler and the image format is multi-planar, the
image must have been created with
VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT, and the aspectMask of the
VkImageView’s subresourceRange must be VK_IMAGE_ASPECT_PLANE_0_BIT,
VK_IMAGE_ASPECT_PLANE_1_BIT or VK_IMAGE_ASPECT_PLANE_2_BIT."
Previously, for YCbCr images, we were flipping this around. For single-
plane views where VK_IMAGE_ASPECT_PLANE_N_BIT would be passed in by the
app, we would store VK_IMAGE_ASPECT_COLOR_BIT. For multi-plane views
where the client says VK_IMAGE_ASPECT_COLOR_BIT, we would store a all of
the planes. (There was also an extra bit of remapping that would
compact the planes in the non-existent case of a format with a non-
contiguous set of planes.) The idea behind this was that for things
like rendering or single-plane sampling, storage, or compute, we want it
to look as much like a single-plane image as possible but we wanted the
multi-plane case to be the awkward one.
This commit changes it around so that iview->aspects is always exactly
the subset of image->vk.aspects represented by the view. This is
identical to how aspects work for depth/stencil so it gains us some
consistency.
This commit also changes anv_image_view::aspect_mask to aspects to force
a full audit of the field. As can be seen, there are only a few uses of
this field and they're all mostly fine:
- A bunch of them are used to check for depth/stencil. That hasn't
changed.
- Most of the checks for color already used ANY_COLOR_BIT, only one
needed fixing.
- There's a check that both src/depth are color for MSAA resolves.
However, we don't support MSAA on YCbCr so there's no point in
checking for ANY_COLOR_BIT.
There is a hidden usage of planes in anv_descriptor_set_write_image_view
that's not as obvious. However, this function simply looks at
anv_image_view::n_planes and blindly fills out the descriptor
accordingly. As long as image views with a single plane continue to
claim n_planes == 1, this will be fine.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12141>
Previously, we initialized vplane in anv_CreateImageView to 0 and
incremented it every iteration of the aspect loop. This only works
because planes are guaranteed to be in aspect-bit-order which wasn't
documented anywhere. Instead, drop this assumption and burn a couple
CPU cycles properly calculating vplane.
While we're here, make iplane const as well.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12141>
When creating a single-plane view of a multi-plane image, we were
relying on vplane_aspect to be VK_IMAGE_ASPECT_COLOR_BIT so that
anv_get_format_plane of the single-plane view format would work.
Instead of relying on this quirk, we can drop vplane_aspect and rely
entirely on vplane to only be 0 in this case. In the case of depth or
stencil images, we still need to grab the format aspect but we can use
the actual aspect and don't need the vplane_aspect trickery.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12141>
Once we get past depth/stencil, what we really want is plane 0 not the
color aspect. A bunch of those formats don't have a single color
aspect.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12141>
Unlike anv_get_format_aspect, this takes a plane number which is
relative to the set of aspects on the format. There are a number of
cases where we already have the plane and so re-fetching it is useless.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12141>
The comment about modifiers is bogus because we check the modifier
before this check and return early. Also, there's no reason why we need
to check the requested aspect when we could check the format itself.
anv_image_aspect_to_plane will ensure that the requested aspect is one
that actually exists.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12141>
Vulkan allows us to, in theory, support ycbcr on single-plane formats if
the client really wants it. Also, these functions should work on a
multi-plane color image as long as the client specifies the right
aspect. This gets rid of our usage of can_ycbcr outside of anv_image.c.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12141>
This commit reverts 58e9371141 as now iris driver can import stencil.
This makes ext_external_objects-vk-stencil-display pass and X-Plane 11
vulkan rendering backend to work with anv + iris.
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Eleni Maria Stea <elene.mst@gmail.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Nanley Chery <nanley.g.chery@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10609>
This makes import easier on different gfx generations and we don't
have to lock down on a certain aux layout just yet.
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Nanley Chery <nanley.g.chery@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10609>
Many places need to know the maximum or minimum possible value for a
given size integer... so everyone just open-codes their favorite
version. There is some potential to hit either undefined or
implementation-defined behavior, so having one version that Just Works
seems beneficial.
v2: Fix copy-and-pasted bug (INT64_MAX instead of INT64_MIN) in
u_intmin. Noticed by CI. Lol. Rename functions
`s/u_(uint|int)(min|max)/u_\1N_\2/g`. Suggested by Jason. Add some
unit tests that would have caught the copy-and-paste bug before wasting
CI time. Change the implementation of u_intN_min to use the same
pattern as stdint.h. This avoids the integer division. Noticed by
Jason.
v3: Add changes to convert_clear_color
(src/gallium/drivers/iris/iris_clear.c). Suggested by Nanley.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Suggested-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12177>
Also switch to array of strings to show high/low dwords.
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: daba2894ff ("intel/disasm: decode/describe more send messages")
Acked-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12183>
This is where it should be rather than having to pass it into the
optimisation pass every time.
It also allows us to call the loop analysis pass without having to
duplicate these options which we will do later in this series.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12064>
This fixes a hang in the following piglit test when GCM moves a
UBO load outside of the loop.
tests/shaders/ssa/fs-if-def-else-break.shader_test
The end NIR ends up looking like this:
vec2 32 ssa_3 = intrinsic load_ubo (ssa_2, ssa_0) (0, 1073741824, 0, 0, 8)
vec1 32 ssa_4 = mov ssa_3.x
vec1 32 ssa_5 = inot ssa_3.y
/* succs: block_1 */
loop {
...
if ssa_5 { }
}
Fixes: 1edf67fc3f ("intel/fs: Generate if instructions with inverted conditions")
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12064>
Add support for driconf overrides on a per-device level, for cases
where we don't want to override behavior for all devices supported
by a particular driver.
Signed-off-by: Rob Clark <robdclark@chromium.org>
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12135>
There are two problems with the current architecture.
In OpenGL, the id is supposed to be a unique identifier for a particular
log source. This is done so that applications can (theoretically)
filter particular log messages. The debug callback infrastructure in
Mesa assigns a uniqe value when a value of 0 is passed in. This causes
the id to get set once to a unique value for each message.
By passing a stack variable that is initialized to 0 on every call,
every time the same message is logged, it will have a different id.
This isn't great, but it's also not catastrophic.
When threaded shader compiles are used, the id *pointer* is saved and
dereferenced at a possibly much later time on a possibly different
thread. This causes one thread to access the stack from a different
thread... and that stack frame might not be valid any more. :(
I have not observed any crashes related to this particular issue.
Reviewed-by: Matt Turner <mattst88@gmail.com>
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12136>
There are two problems with the current architecture.
In OpenGL, the id is supposed to be a unique identifier for a particular
log source. This is done so that applications can (theoretically)
filter particular log messages. The debug callback infrastructure in
Mesa assigns a uniqe value when a value of 0 is passed in. This causes
the id to get set once to a unique value for each message.
By passing a stack variable that is initialized to 0 on every call,
every time the same message is logged, it will have a different id.
This isn't great, but it's also not catastrophic.
When threaded shader compiles are used, the id *pointer* is saved and
dereferenced at a possibly much later time on a possibly different
thread. This causes one thread to access the stack from a different
thread... and that stack frame might not be valid any more. :(
This fixes shader-db crashes of various kinds on Iris with threaded
shader compiles enabled.
Fixes: 42c34e1ac8 ("iris: Enable threaded shader compilation")
Reviewed-by: Matt Turner <mattst88@gmail.com>
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12136>
If the passed VkPipelineRasterizationLineStateCreateInfoEXT wasn't zero
initialized, we copy garbage values that are later on used to set the
state and may end up crashing when they are beyond the limits of the HW.
v2 (Lionel): Simplify if condition
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12121>
If we have 2 command buffers back to back, one with a query pool, one
without, we don't want to retain the second query pool value (NULL).
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: 0a7224f3ff ("anv: group as many command buffers into a single execbuf")
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12107>
All vulkan drivers have been copying anv's code to convert
VkSpecializationInfo into nir_spirv_specialization.
Recently there was a Vulkan spec change on allowed values for
VkSpecializationInfo, and all drivers got affected.
This commits creates a new helper, and uses it on all Vulkan Mesa
drivers.
v2: use (uint8_t*) castings, instead of void*, to avoid C2036 with
MSVC (detected by the CI, inspired on what radv was doing)
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-By: Mike Blumenkrantz <michael.blumenkrantz@gmail.com>
Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12047>
The current code we have for this is a bit of a mess, likely due to
trying too hard to put it in anv_android.c. The external_format bit in
anv_image, for instance, really means "quit creation early" which is
something we want to do for AHardwareBuffer imports regardless of
whether or not they use a native format. It gets set both by declaring
an AHardwareBuffer external handle type and by VkExternalFormatANDROID.
However, VkExternalFormatANDROID is only allowed for AHardwareBuffer
imports. If we ever did get an external format outside the context of
an AHardwareBuffer import, we would end up with a useless partially
created image.
When we detect an AHardwareBuffer import, we punt off to a function in
anv_android.c that does nothing interesting but call anv_create_image
with AUX disabled and external_format = true. The aux disable here is
useless because the actual isl_surf layout is done by resolve_ahw_image
which also sets ISL_SURF_USAGE_DISABLE_AUX_BIT. As far as external
formats go, anv_image_from_external() sets it regardless of whether or
not there is actually an external format.
This commit replaces anv_image::external_format with anv_image::from_ahb
which is the thing we actually want to track for this. We delete
anv_image_from_external and a bunch of the external_format handling
because it's all useless. The end result is massively simpler and,
while it appears to blur the boundary between Android code and the rest
of the driver, it makes the whole flow more obvious.
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12040>
Only fragment and some compute shaders support implicit derivatives.
They're totally meaningless without helper invocations and some
understanding of the dispatch pattern. We've got code to lower
nir_texop_tex in these shader stages to use an explicit derivative of 0
but it was pretty badly broken:
1. It only handled nir_texop_tex, not nir_texop_txb or nir_texop_lod.
2. It didn't take min_lod into account
3. It was conflated with adding a missing LOD parameter to opcodes
which expect one such as nir_texop_txf. While not really a bug,
this does make it way harder to reason about the code.
4. Unless you set a flag (which most drivers don't), it left the
opcode nir_texop_tex instead of nir_texop_txl which it should have
been.
This reworks it to go through roughly the same path as other LOD
lowering only with a constant lod of 0 instead of calling out to
nir_texop_lod. We also get rid of the lower_tex_without_implicit_lod
flag because most drivers set it and those that don't are probably
subtly broken. If someone really wants to get nir_texop_tex in their
vertex shaders, they can write a new patch to add the flag back in.
Fixes: e382890e25 "nir: set default lod to texture opcodes that..."
Fixes: d5ac5d6e83 "nir: Add option to lower tex to txl when..."
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11775>
It's intel-specific, used to get at MSAA compression information.
Reviewed-by: Emma Anholt <emma@anholt.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11775>
Eric Engestrom
Lionel Landwerlin
Jason Ekstrand
Dave Airlie
Ian Romanick
Marcin Ślusarz
Tapani Pälli
Timothy Arceri
Rob Clark
Dave Airlie
Sagar Ghuge
Iván Briano
Alejandro Piñeiro