I'm pretty sure this doesn't fix anything because the WSI code only
use a single VkSubmitInfo, but better be safe.
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: ccb7d606f1 ("anv: Use submit-time implicit sync instead of allocate-time")
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8934>
(cherry picked from commit 64cb03a52d)
We can skip CCS ambiguate if followed by a fast clear within render
pass.
v2: (Jason)
- Check array layer as well since we only fast clear first layer and
first LOD.
- Don't drop fast clear check while doing resolve operation.
Fixes: d5849bc840 "anv: Skip HiZ and CCS ambiguates which preceed fast-clears"
Signed-off-by: Sagar Ghuge <sagar.ghuge@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6988>
(cherry picked from commit 001722b3a3)
According to the BSpec page for MEDIA_VFE_STATE, on Gen12 platforms
"if a fused configuration has fewer threads than the native POR
configuration, the scratch space allocation is based on the number of
threads in the base native POR configuration". However we currently
use the subslice count from devinfo->num_subslices[0], which only
includes the subslices currently enabled by the platform fusing. This
leads to scratch space underallocation and occasional hangs.
The problem is likely to affect most Gen12 GPUs with less than 96 EUs.
GFXBench5 Aztec Ruins is able to reproduce the issue fairly reliably.
Fixes: 9e5ce30da7 "intel: fix the gen 12 compute shader scratch IDs"
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8636>
(cherry picked from commit e2c5ef6cd6)
While invalidating the AUX-TT entries, we have to consider the surface
offset as well otherwise, we will end up invalidating another surface's
CCS portion.
For eg. when we have HiZ+CCS and STC_CCS enabled, both will use the CCS
portion allocated at the end of BO. While invalidating the CCS portion
of stencil buffer, we will end up invalidating the CCS portion that
belongs to the depth main surface and vice-versa, if the surface offset
is not considered.
Cc: mesa-stable
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4123
Signed-off-by: Sagar Ghuge <sagar.ghuge@intel.com>
Acked-by: Nanley Chery <nanley.g.chery@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8677>
(cherry picked from commit dab229ef69)
We incorrectly utilize the stencil layouts structures even if we
should stick to the depth_stencil ones if the layout includes stencil.
v2: Don't forget stencil only layout (Nanley)
Simplify callers of new helper functions (Nanley)
v3: Store VK_IMAGE_LAYOUT_UNDEFINED when no stencil is available (Nanley)
Use a switch statement (Nanley)
v4: Consider all layouts but depth only to be potential stencil layouts (Lionel)
v5: Refactor helper in vk_image_layout_depth_only() and discard
VkAttachmentDescriptionStencilLayoutKHR in
VkAttachmentDescription2KHR if format is not depth/stencil.
v5: s/LAYOUT_COLOR_ATTACHMENT_OPTIMAL/LAYOUT_DEPTH_ATTACHMENT_OPTIMAL/ (Nanley)
v6: Fix overly harsh assert()
Fixes: c1c346f166 ("anv: implement VK_KHR_separate_depth_stencil_layouts")
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4084
Reviewed-by: Nanley Chery <nanley.g.chery@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8475>
(cherry picked from commit 28207669d0)
We sometimes use anv_layout_to_aux_state() to compute the aux state of
an image during the resolve operations at the end of a render
(sub)pass.
If we're dealing with a multisampled image that is created without a
transfer usage, our internal code might trigger a resolve using the
transfer layout (see genX_cmd_buffer.c:cmd_buffer_end_subpass), for
which the image doesn't the usage bit. The current code tries to AND
the 2 usages which won't have any bit in common, thus skipping all
checks below.
v2: Add the transfer usages depending on attachment usage (Lionel)
v3: Limit to samples > 1 (Jason) && DEPTH_STENCIL_ATTACHMENT_BIT (Lionel)
v4: Add transfer usage at image creation (Jason)
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: 54b525caf0 ("anv: Rework anv_layout_to_aux_state")
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4037
Reviewed-by: Reviewed-by: Tapani Pälli <tapani.palli@intel.com> (v1)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8307>
(cherry picked from commit d4b4d69d4d)
The DRM_RDWR flag is needed for mmap with PROT_WRITE to work.
Cc: mesa-stable
Signed-off-by: Robin Ole Heinemann <robin.ole.heinemann@gmail.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8075>
VK spec got clarification about the pSizes parameter.
Fixes set of new tests:
dEQP-VK.pipeline.extended_dynamic_state*with_offset*
v2: move offset subtract to be part of size calculation (Jason)
CC: mesa-stable
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/3871
Fixes: b9a05447a1 ("anv: dynamic vertex input binding stride and size support")
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7439>
When DEBUG is not defined, no error reporting is done, the error is
just returned back. The current definition a couple of warnings in
anv_formats.c. First when the return value is intentionally ignored
../src/intel/vulkan/anv_formats.c:989:48: warning: statement with no effect [-Wunused-value]
989 | vk_errorfi(instance, physical_device, VK_ERROR_FORMAT_NOT_SUPPORTED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/intel/vulkan/anv_private.h:486:55: note: in definition of macro ‘vk_errorfi’
486 | #define vk_errorfi(instance, obj, error, format, ...) error
| ^~~~~
and also when an argument is used only
../src/intel/vulkan/anv_formats.c:908:25: warning: unused variable ‘instance’ [-Wunused-variable]
908 | struct anv_instance *instance = physical_device->instance;
| ^~~~~~~~
../src/intel/vulkan/anv_formats.c: In function ‘anv_GetPhysicalDeviceImageFormatProperties2’:
../src/intel/vulkan/anv_formats.c:1231:25: warning: unused variable ‘instance’ [-Wunused-variable]
1231 | struct anv_instance *instance = physical_device->instance;
| ^~~~~~~~
to avoid both issues, use a static inline function that just returns
it's argument but can consume other input. Ignoring the return value
of a function is OK, and the extra input can be tagged as UNUSED
getting rid of both warnings.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7860>
Quoting the spec :
"When a pool is destroyed, all descriptor sets allocated from the
pool are implicitly freed and become invalid. Descriptor sets
allocated from a given pool do not need to be freed before
destroying that descriptor pool."
This implies we might leak nodes allocated in the vma object.
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: 0a6d2593b8 ("anv: Allocate descriptor buffers from the BO cache")
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7796>
Simple refactor. No intended change in behavior.
Replace each derivation of aux address with anv_image_get_aux_addr().
The function will soon do more in support of
VK_EXT_image_drm_format_modifier, where the image bo and aux bo may be
disjoint.
v2:
- Replace param 'aspect' with 'plane'.
v3:
- Workaround for stencil ccs. If no aux surface, then return
ANV_NULL_ADDRESS.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v2)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> (v3)
Pre-patch, we checked the offsets once per aspect after adding all
surfaces for the aspect. The additional checks will make it easier to
diagnose layout bugs.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Pure refactor. No intended change in behavior.
This makes the code infinitely easier to understand. And it uncovers
a potential bug (marked with XXX comment).
v2: Fix narrowing conversions on 32-bit arch. s/size_t/uintmax_t/.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v1)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> (v2)
Months ago, make_surface() added *all* surfaces required for the given
aspect. It was a monster monolithic function, and difficult to reason
about its correctness. In commit c652ff8c (2020-03-06), I split the code
for aux surfaces into its own function, add_aux_surface_if_supported().
This patch continues the splitting, therefore making bugs easier to
identify.
Code changes:
- Move the code that adds the shadow surface from make_surface() to
a new function add_shadow_surface(), called from
add_all_surfaces().
- Move the call to add_aux_surface_if_supported() from make_surface()
to add_all_surfaces().
- To preserve correctness of the assertions on image layout in
make_surface(), move them to the loop in add_all_surfaces() after
all the aspect's surfaces have been added.
- Rename make_surface() to add_primary_surface() because now that's
what it does.
Pure refactor, no intended change in behavior.
v2:
- Rebase onto "anv: Fix isl_surf_usage_flags for stencil images".
- Sanitize the image's extent earlier.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v2)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> (v2)
This deduplicates the loops in anv_image_create() and
resolve_ahw_image().
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
In anv_get_image_format_properties(), the special-case code for
VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT is tiny. It is mostly a detached
'case' in the 'switch' block for VkImageType. So move the special-case
code to immediately follow the 'switch' block.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
In vkGetPhysicalDeviceImageFormatProperties.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
The check in anv_get_image_format_properties() is already handled in
anv_get_image_format_features().
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
When filling VkImageFormatProperties, anv_get_image_format_properties()
checks the requested VkImageUsageFlags and VkImageCreateFlags against
the VkFormatFeatureFlags available to the queried VkFormat. However, we
neglected to consider if any formats given in
VkImageFormatListCreateInfo
further restricted the available VkFormatFeatureFlags.
The image view formats are more likely to introduce additional
restrictions when DRM format modifiers are present.
v2:
- Do not drop anv_formats_ccs_e_compatible().
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v2)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> (v2)
If anv_get_image_format_features reports that the inputs are
unsupported, fail immediately.
Without the early fail, I have less confidence in the function's
correctness when a DRM format modifier is present.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
The code in anv_get_image_format_properties() that set sampleCounts
appears correct, but weirdly inconsistent. Clean the code to
consistently set sampleCounts in the same location as
maxExtent/maxMipLevels/maxArraySize.
Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Rename it to get_drm_format_modifier_properties_list() because it is now
independent of WSI.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
In vkGetPhysicalDeviceImageFormatProperties2, we advertised support for
VK_IMAGE_TILING_LINEAR and VK_IMAGE_TILING_OPTIMAL for all memory
handles.
However, when importing or exporting an image, there must exist a method
that enables the app and driver to agree on the image's memory layout.
If no method exists, then we should reject image creation.
v2:
- Reduce copy-paste for Lionel.
v3:
- Treat tiling LINEAR and DRM_FORMAT_MODIFIER as identical when
determing compatible memory handles.
- Improve comments.
v4:
- Remove DMA_BUF from opaque_fd_only_props.
v5:
- Minor changes to code style for `if`. (for jekstrand)
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v4)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> (v4)
The code asserted that we supported no more than 4 formats with
modifiers: /VK_FORMAT_B8G8R8(A8)?_(SRGB|UNORM)/.
Strangely, 2 of the 4 were non-power-of-two formats, which were rejected
elsewhere.
The assertion's comment suggested that we use a hard-coded list of
formats because the driver was not yet able to determine if a given
format was compatible with a given modifier. Therefore, the list only
contained formats that were compatible with *all* modifiers. That code
deficiency no longer exists: anv_get_image_format_features() can check
format/modifier compatibility.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Refactor in get_wsi_format_modifier_properties_list().
Instead of iterating over a function-local hard-coded list, iterate over
all modifiers in isl_drm.c.
This will improve agreement in behavior between
VkDrmFormatModifierPropertiesListEXT
VkPhysicalDeviceImageDrmFormatModifierInfoEXT.
The future disagreement this patch attempts to prevent is the
combination of:
a. VkDrmFormatModifierPropertiesListEXT neglects to return a valid
modifier because its hard-coded list of modifiers drifts
out-of-sync with hard-coded lists elsewhere in the code. (Already
today, the list in get_wsi_format_modifier_properties_list() does
not match the list in isl_drm.c; though, this has produced no bug
yet).
b. vkGetPhysicalDeviceImageFormatProperties2 accepts, via
VkPhysicalDeviceImageDrmFormatModifierInfoEXT, the modifier
overlooked in (a), because it does not use the same hard-coded
list in get_wsi_format_modifier_properties_list(). (Recall that
the spec requires vkGetPhysicalDeviceImageFormatProperties2 to
correctly accept/reject any int that the app provides, even when
the int is an invalid modifier).
c. The Bug. The driver told the app in (b) that it can legally
create an image with format+modifier, but the app cannot query
the VkFormatFeatureFlags of the format+modifier due to (a).
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Fill VkDrmFormatModifierPropertiesEXT::drmFormatModifierTilingFeatures
with anv_get_image_format_features().
anv_formats.c:get_wsi_format_modifier_properties_list() incorrectly left
it uninitialized.
v2: Increment drmFormatModifierPlaneCount if modifier support aux.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v2)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> (v2)
Because anv_get_image_format_features() now understands modifiers, also
relocate most of the modifier compatibility checks from
anv_get_format_plane() into anv_get_image_format_features() in order to
avoid duplication.
The new signature forces some code movement in
anv_get_image_format_properties().
v2:
- Reject VK_FORMAT_B4G4R4A4_UNORM_PACK16 with modifiers on HSW.
v3:
- Revert the v2 change.
- Query isl_format_layout instead of pipe_format. (for jekstrand)
- Drop misguided comments. (for jekstrand)
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v2)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> (v3)
The code did not return error when VK_IMAGE_CREATE_DISJOINT_BIT was
incompatible with the other input params.
If the Vulkan spec forbids a set of input params for vkCreateImage,
but permits them for vkGetPhysicalDeviceImageFormatProperties2,
then vkGetPhysicalDeviceImageFormatProperties2 must reject those input
params with failure.
- v2: Clearer commit message.
CC: <mesa-stable@lists.freedesktop.org>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v2)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
v2: Apply the workaround to all gen hardawre
Ref: GEN:BUG:1409725701
Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Reviewed-by: Nanley Chery <nanley.g.chery@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7463>
We are limited to 64 threads per dispatched group, regardless of what
num_cs_threads claims, so advertise that limit correctly.
Fixes (on TGL and up):
dEQP-VK.subgroups.size_control.compute.required_subgroup_size_min
and other *.required_subgroup_size_min tests.
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7453>
pool can't be NULL at this point, because it was already
dereferenced earlier.
Addresses "Dereference before null check" issue reported by Coverity.
Signed-off-by: Marcin Ślusarz <marcin.slusarz@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7449>
NIR derefs currently have exactly one variable mode. This is about to
change so we can handle OpenCL generic pointers. In order to transition
safely, we need to audit every deref->mode check. This commit adds a
set of helpers that provide more nuanced mode checks and converts most
of NIR to use them.
For simple cases, we add nir_deref_mode_is and nir_deref_mode_is_one_of
helpers. These can be used in passes which don't have to bother with
generic pointers and just want to know what mode a thing is. If the
pass ever encounters generic pointers in a way that this check would be
unsafe, it will assert-fail to alert developers that they need to think
harder about things and fix the pass.
For more complex passes which require a more nuanced understanding of
modes, we add nir_deref_mode_may_be and nir_deref_mode_must_be helpers
which accurately describe the compiler's best knowledge about the given
deref. Unfortunately, we may not be able to exactly identify the mode
in a generic pointers scenario so we have to be very careful when we use
these. Conversion of these passes is left to later commits.
For the case of mass lowering of a particular mode (nir_lower_explicit_io
is one good example), we add nir_deref_mode_is_in_set. This is also
pretty assert-happy like nir_deref_mode_is but is for a set containment
comparison on deref modes where you expect the deref to either be all-in
or all-out.
Reviewed-by: Jesse Natalie <jenatali@microsoft.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6332>
anv_bo_pool_alloc expects that the memory returned by and_gem_mmap
was annotated using VALGRIND_MALLOCLIKE_BLOCK, but anv_gem_mmap_offset
didn't do that. Move annotation from anv_gem_mmap_legacy to common
code.
Fixes: 4abf0837cd ("anv: Add support for new MMAP_OFFSET ioctl.")
Signed-off-by: Marcin Ślusarz <marcin.slusarz@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7381>
In many cases those revision happened every before the first public
release of the spec and we just forgot to update our numbers.
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Acked-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7136>
