We need to access the pipeline layout to compute correct dynamic
offsets for dyamic UBO/SSBO descriptors when we emit draw commands.
Instead of taking it from the pipeline object, store the layout
in the command buffer pipeline state.
Suggested-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
The Vulkan spec states that VkPipelineLayout objects must not be
destroyed while any command buffer that uses them is in the recording
state, but it permits them to be destroyed otherwise. This means that
applications are allowed to free pipeline layouts after command recording
is finished even if there are pipeline objects that still exist and were
created with these layouts.
There are two solutions to this, one is to use reference counting on
pipeline layout objects. The other is to avoid holding references to
pipeline layouts where they are not really needed.
This patch takes a step towards the second option by making the
pipeline shader compile code take pipeline layout from the
VkGraphicsPipelineCreateInfo provided rather than the pipeline
object.
A follow-up patch will remove any remaining uses of the layout field
so we can remove it from the pipeline object and avoid the need
for reference counting.
v2: Use ANV_FROM_HANDLE, remove unnecessary braces (Jason)
Suggested-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
The spec states that descriptor set layouts can be destroyed almost
at any time:
"VkDescriptorSetLayout objects may be accessed by commands that
operate on descriptor sets allocated using that layout, and those
descriptor sets must not be updated with vkUpdateDescriptorSets
after the descriptor set layout has been destroyed. Otherwise,
descriptor set layouts can be destroyed any time they are not in
use by an API command."
v2: allocate off the device allocator with DEVICE scope (Jason)
Fixes the following work-in-progress CTS tests:
dEQP-VK.api.descriptor_set.descriptor_set_layout_lifetime.graphics
dEQP-VK.api.descriptor_set.descriptor_set_layout_lifetime.compute
Suggested-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Without this, we may end up dereferencing blend before we check for
binding->index != UINT32_MAX. However, Vulkan allows the blend state to
be NULL so long as you don't have any color attachments. This fixes a
segfault when running The Talos Principal.
Fixes: 12f4e00b69
Cc: mesa-stable@lists.freedesktop.org
Reviewed-by: Alex Smith <asmith@feralinteractive.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Sort the output to ensure build reproducibility
Signed-off-by: Maxin B. John <maxin.john@intel.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Fixes: 0ab04ba979 ("anv: Use python to generate ICD json files")
Reviewed-by: Dylan Baker <dylan@pnwbakers.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Looks like checking both sources was intended, instead of the first one
twice. Found with Coccinelle, coccinellery/xand/xand.cocci semantic patch.
Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
While we're here, make it an anv_address.
Tested-by: Józef Kucia <joseph.kucia@gmail.com>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Cc: "18.0" <mesa-stable@lists.freedesktop.org>
We were already doing this for some packets to keep the lines shorter.
We may as well just do it for all of them.
Tested-by: Józef Kucia <joseph.kucia@gmail.com>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Cc: "18.0" <mesa-stable@lists.freedesktop.org>
The Vulkan spec says:
"pipelineBindPoint is a VkPipelineBindPoint indicating whether the
descriptors will be used by graphics pipelines or compute pipelines.
There is a separate set of bind points for each of graphics and
compute, so binding one does not disturb the other."
Up until now, we've been ignoring the pipeline bind point and had just
one bind point for everything. This commit separates things out into
separate bind points.
Tested-by: Józef Kucia <joseph.kucia@gmail.com>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102897
Cc: "18.0" <mesa-stable@lists.freedesktop.org>
This lets us unify some code between push descriptors and regular
descriptors. It doesn't do much for us yet but it will.
Tested-by: Józef Kucia <joseph.kucia@gmail.com>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Cc: "18.0" <mesa-stable@lists.freedesktop.org>
It's now a function which returns the push descriptor set. Since we set
the error on the command buffer, returning the error is a little
redundant. Returning the descriptor set (or NULL on error) is more
convenient.
Tested-by: Józef Kucia <joseph.kucia@gmail.com>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Cc: "18.0" <mesa-stable@lists.freedesktop.org>
With the semicolons, they can't be used in a function argument without
throwing syntax errors.
Tested-by: Józef Kucia <joseph.kucia@gmail.com>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Cc: "18.0" <mesa-stable@lists.freedesktop.org>
Initially, these just contain the pipeline in a base struct.
Tested-by: Józef Kucia <joseph.kucia@gmail.com>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Cc: "18.0" <mesa-stable@lists.freedesktop.org>
There are several places where we'd already saved the pipeline off to a
temporary variable but, due to an artifact of history, weren't actually
using that temporary everywhere. No functional change.
Tested-by: Józef Kucia <joseph.kucia@gmail.com>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Cc: "18.0" <mesa-stable@lists.freedesktop.org>
This splits anv_cmd_state_reset into separate init and finish functions.
This lets us share init code with cmd_buffer_create. This potentially
fixes subtle bugs where we may have missed some bit of state that needs
to get initialized on command buffer creation.
Tested-by: Józef Kucia <joseph.kucia@gmail.com>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Cc: "18.0" <mesa-stable@lists.freedesktop.org>
Meta has been gone for a long time.
Tested-by: Józef Kucia <joseph.kucia@gmail.com>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Cc: "18.0" <mesa-stable@lists.freedesktop.org>
This is a legacy left-over from the mechanism we used to use to handle
scratch. The new (and better) mechanism doesn't use this.
Tested-by: Józef Kucia <joseph.kucia@gmail.com>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Cc: "18.0" <mesa-stable@lists.freedesktop.org>
This prevents an assert when running one unreleased Vulkan game.
Tested-by: Józef Kucia <joseph.kucia@gmail.com>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Cc: "18.0" <mesa-stable@lists.freedesktop.org>
Technically, the Vulkan spec requires that we return valid entrypoints
for all core functionality and any available device extensions. This
means that, for gen-specific functions, we need to return a trampoline
which looks at the device and calls the right device function. In 99%
of cases, the loader will do this for us but, aparently, we're supposed
to do it too. It's a tiny increase in binary size for us to carry this
around but really not bad.
Before:
text data bss dec hex filename
3541775 204112 6136 3752023 394057 libvulkan_intel.so
After:
text data bss dec hex filename
3551463 205632 6136 3763231 396c1f libvulkan_intel.so
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
The Vulkan spec annoyingly requires us to track what core version and
what all extensions are enabled and only advertise those entrypoints.
Any call to vkGet*ProcAddr for an entrypoint for an extension the client
has not explicitly enabled is supposed to return NULL.
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
This lets us move a bunch of stuff out of codegen and back into
anv_device.c which is a bit nicer.
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
This removes some redundant code between libanv_common, libvulkan_intel,
and libvulkan_intel_test.
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
The new anv_extensions_gen.py is the code generator while the old
anv_extensions.py file is purely declarative.
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
vk_error() is a macro that calls __vk_errorf() with instance == NULL.
Then, __vk_errorf() passes a pointer to instance->debug_report_callbacks
to vk_debug_error(), which segfaults as this pointer is invalid but not
NULL.
Fixes: e5b1bd6ab8 "vulkan: move anv VK_EXT_debug_report implementation to common code."
Signed-off-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
From the Vulkan spec with KHX extensions:
"If queries are used while executing a render pass instance that has
multiview enabled, the query uses N consecutive query indices
in the query pool (starting at query) where N is the number of bits
set in the view mask in the subpass the query is used in.
How the numerical results of the query are distributed among the
queries is implementation-dependent. For example, some implementations
may write each view's results to a distinct query, while other
implementations may write the total result to the first query and write
zero to the other queries. However, the sum of the results in all the
queries must accurately reflect the total result of the query summed
over all views. Applications can sum the results from all the queries to
compute the total result."
In our case we only really emit a single query (in the first query index)
that stores the aggregated result for all views, but we still need to manage
availability for all the other query indices involved, even if we don't
actually use them.
This is relevant when clients call vkGetQueryPoolResults and pass all N
queries to retrieve the results. In that scenario, without this patch,
we will never see queries other than the first being available since we
never emit them.
v2: we need the same treatment for timestamp queries.
v3 (Jason):
- Better an if instead of an early return.
- We can't write to this memory in the CPU, we should use
MI_STORE_DATA_IMM and emit_query_availability (Jason).
v4 (Jason):
- No need to take the value to write as parameter, just hard code it to 0.
Fixes test failures in some work-in-progress CTS multiview+query tests.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
For also using it in radv. I moved the remaining stubs back to
anv_device.c as they were just trivial.
This does not move the vk_errorf/anv_perf_warn or the object
type macros, as those depend on anv types and logging.
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
From Vulkan spec:
"descriptorCount is the number of descriptors contained in the binding,
accessed in a shader as an array. If descriptorCount is zero this
binding entry is reserved and the resource must not be accessed from
any stage via this binding within any pipeline using the set layout."
Fixes:
dEQP-VK.binding_model.descriptor_update.empty_descriptor.uniform_buffer
Signed-off-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: mesa-stable@lists.freedesktop.org
This creates two new internal dependencies, idep_nir_headers and
idep_nir. The former encapsulates the generation of nir_opcodes.h and
nir_builder_opcodes.h and adding src/compiler/nir as an include path.
This ensures that any target that needs nir headers will have the
includes and that the generated headers will be generated before the
target is build. The second, idep_nir, includes the first and
additionally links to libnir.
This is intended to make it easier to avoid race conditions in the build
when using nir, since the number of consumers for libnir and it's
headers are quite high.
Acked-by: Eric Engestrom <eric.engestrom@imgtec.com>
Signed-off-by: Dylan Baker <dylan.c.baker@intel.com>
For things like:
loop
x = func()
list += x
end
just do:
loop
list += func()
end
Acked-by: Eric Engestrom <eric.engestrom@imgtec.com>
Signed-off-by: Dylan Baker <dylan.c.baker@intel.com>
