Used as reference Hyujun's commit
5d3fdbc52b, that does the same for
turnip.
This commit also replaces in several cases alloc for zalloc, and adds
checks on more Destroy methods if the object to be free is NULL or
not. Most of them were needed to avoid crashes/weird behaviour due
trying to use un-initialized data. Note that now that vk_object_free
iterates over a array, making it more against un-initialized or just
NULL data.
Additionally, using zalloc we can also remove some memset to 0. In
fact we needed to remove them, as if not, they would override the
vk_object_base object to 0 (the alternative would me doing a memset
computing a pointer offset, but that's is not needed as we can just
use zalloc).
v2:
* Call memset(0) on reused descriptor sets when calling
ResetDescriptorPool, not when reallocating them (Iago)
* Add null check when calling DestroyImageView (detected by a full CTS run)
v3: Fixed rebase conflicts after last meta copy/clear changes
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7627>
Following a suggestion from Alejandro, since playout is a word on its own
and can be confusing. It also makes it more consistent with other
variable names that use an underscore.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7651>
This avoids redundant per-layer operations that are the same across
layers or that only need to do once. Namely:
- The sampler for the blit source is the same for all layers.
- The decision about whether we need to load TLB contents or not only
needs to be done once.
- Some command buffer state such as the pipeline, the viewport and the
scissor is the same for all layers and should only be bound once.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7651>
This removes the need to lock just to check if we have created them
due to the lazy allocation strategy we had in place.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7651>
This is much faster than the blit fallback (which requires to upload
the linear buffer to a tiled image) and the CPU path.
A simple stress test involving 100 buffer to image copies of a
single layer image with 10 mipmap levels provides the following
results:
Path | Recording Time | Execution Time |
-------------------------------------------------|
Texel Buffer | 2.954s | 0.137s |
-------------------------------------------------|
Blit | 10.732s | 0.148s |
-------------------------------------------------|
CPU | 0.002s | 1.453s |
-------------------------------------------------|
So generally speaking, this texel buffer copy path is the fastest
of the paths that can do partial copies, however, the CPU path might
provide better results in cases where command buffer recording is
important to overall performance. This is probably the reason why
the CPU path seems to provide slightly better results for vkQuake2.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7651>
By default we are using 32bit output type for texture operations,
16bit for shadow.
With this commit we also use the precision info from the sampler (that
is assigned if SPIR-V uses RelaxedPrecision decorator), in order to
use 16bit.
This is a first step as only take into account the precision of the
deref_vars used on the texture operation.
But the decoration can be also applied to other cases, like the result
of the operation. That means that there are ways to infer that the
texture operation can operate at relaxed precision. Those cases would
be handled on following patches.
v2:
* Add directly the return_size on the descriptor_map, instead of
shadow/relaxed_precision.
* Check relaxed precision for images too (Iago)
* Handle the return size for the default sampler
v3:
* Handle different output size for the case of not having a sampler.
* Comment fixes (Iago)
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7545>
Now that the v3d compiler has support for separated texture and
sampler indices, we can stop to combine them. Again, that's what
Vulkan allows after all.
As we are doing this we can't use anymore the texture format (coming
from the texture) to chose the return size (that is a sampling
parameter). We default for 32, and just go to 16 for shadow. We plan
to use SPIR-V RelaxedPrecision to use in more cases 16 bit. We would
do that on following patches.
v2 (from Iago feedback):
* Fix typos/bad grammar on comments.
* Move tex/sampler number assert to before the loop that fills
tex/sampler info.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7545>
So far the v3d compiler has them combined, as for OpenGL both are the
same. This change is intended to fit the v3d compiler better with
Vulkan, where they are separate concepts.
Note that NIR has them separate for a long time, both on nir_variable
and on some NIR lowerings.
v2: (from Iago feedback)
* Use key->num_tex/sampler_used to iterate through the array
* Fill up num_samplers_used on v3d, assert that is the same that
num_tex_used if possible.
v3: (Iago)
* Assert num_tex/samplers_used is smaller that tex/sampler array size.
v4: Update assert mentioned on v3 to use <= instead of < (detected by CI)
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
squash! broadcom/compiler: separate texture/sampler info from v3d_key
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7545>
In preparation to the changes that would allow to not need them.
It is worth to note that it is likely (we have some ideas in mind)
that we would need to bring back pre-generate variants on the
future. The approach is slightly different on v3dv_pipeline vs
v3dv_cmd_buffer:
* v3dv_pipeline: even after the clean-up, we had code for all the
functions they have, even if they were doing less things
(specifically, a second shader variant), so they still make sense
on their own, and serve as template for adding support of multiple
pre-generated shader variants in the future.
* v3dv_cmd_buffer: as we really don't need to fill up the key with
some after-pipeline data, we would end with some functions empty
(specifically cmd_buffer_populate_v3d_key). Even as a placeholder,
that would be odd. Additionally the current code has a lot of
boilerplate code (functions to fill up vs, cs and fs keys are
basically the same), and we already have in mind refactor them. So
it would be better to remove all of them, instead of keeping
around some code we would not be happy with. If in the future we
pregenerate more that one variant, hopefully the new code to chose
between them would be better.
v2: clarify the commit message, and fix typos on the comments (Iago)
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7545>
index is of type uint32_t.
Fix defects reported by Coverity Scan.
Macro compares unsigned to 0 (NO_EFFECT)
unsigned_compare: This greater-than-or-equal-to-zero comparison of
an unsigned value is always true. index >= 0U.
Signed-off-by: Vinson Lee <vlee@freedesktop.org>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7518>
So far, we have only been supporting X11, so we assumed that we were running
inside X11 and would always try to get an authenticated fd from Xorg during
device initialization. While this works for desktop Raspbian, it is not
really correct and it is not what we want to do when we start considering
other WSIs.
Initially, one could think we can still do this by guarding the WSI code
under the proper instance extension check. This, however, doesn't work
reliably, as the Vulkan loader can call vkEnumerateDevices without enabling
surface extensions on the instance, which then can lead to us not
initializing any display_fd and failing with VK_ERROR_INITIALIZATION_FAILED,
which is not correct, so while we can try to acquire the display_fd here,
it might not always work, and we should definitely not fail initialization
of the physical device for that.
Instead, with this change we move acquisition of display_fd to swapchain
creation time where required extensions need to be enabled in the instance.
This was also suggested by Daniel Stone during review of a work-in-progress
implementation for the Wayland WSI.
There is a special case to consider though: applications like Zink that
don't use Vulkan's swapchains at all but still allocate images that they
intend to use for WSI. We need to handle these by checking that we have
indeed acquired a display_fd before doing any memory allocation for WSI,
and acquiring one at that time if that's not the case.
This change also removes the render_fd and display_fd fields from the
logical device (which we were copying from the physical device), because
now there is no guarantee that we have acquired a display_fd at the
time we create a logical device. Instead, we now put a reference to the
physical device on the logical device from which we can access these.
Finally, this also fixes a regression introduced with VK_KHR_display, where
if that extension is enabled but we are running inside a compositor, we would
acquire a display_fd that is not authenticated and try to use that instead
of acquiring an authenticated display_fd from the display server.
Fixes: b1188c9451 (v3dv: VK_KHR_display extension support)
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7546>
This cleans up a bunch of gross sprintfs and keeps the caller from needing
to remember to ralloc_strdup. I added a couple of '"%s", name ? name :
""' to radv where I didn't fully trace through whether a non-null name was
being passed in.
I also took the liberty of adding a basic name to a few shaders (pan_blit,
unit tests)
Reviewed-by: Rhys Perry <pendingchaos02@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7323>
When calling GetPhysicalDeviceProperties2 we were ignoring and logging
the structures for extensions not supported. But for the case of
VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PCI_BUS_INFO_PROPERTIES_EXT we
already know that we are not going to support it, so let's just do
nothing (not even logging) when passed.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7497>
We already have vk_error to report errors, they add little specific
v3dv wrapping over a simple fprintf, and they are not used really
often.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7497>
subpass_idx is of type uint32_t.
Fix defects reported by Coverity Scan.
Macro compares unsigned to 0 (NO_EFFECT)
unsigned_compare: This greater-than-or-equal-to-zero comparison of
an unsigned value is always true. subpass_idx >= 0U.
Signed-off-by: Vinson Lee <vlee@freedesktop.org>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7325>
Now that the R/B swap mask for vertex attributes handles all the
attributes, ensure the right generic vertex attribute is marked.
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7196>
It is not used right now, so keeping it adds some noise/confusion.
So far configuring Z test are done through the CFG_BITS. See
v3dX(emit_state) at v3dx_emit.c for v3d, and pack_cfg_bits at
v3dv_pipeline.c for v3dv. There flags like z_updates_enable and others
are filled up.
That key field seems like a leftover coming from using vc4 as
reference, as that driver defines and uses a field with name name.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7421>
If a secondary command buffer has occlusion query inheritance then
draw calls recorded in it should update an active occlusion query
counter started in the primary command buffer.
If executing the secondary in a primary required to emit jobs and
not just a branch instruction, then we might need to create a new
job for the primary as well, and in that case we would lose the
occlusion query state, so we need to re-emit it at that point so
any additional draw calls recorded into the secondary that is being
executed continue to update the counter.
Fixes:
dEQP-VK.query_pool.concurrent_queries.secondary_command_buffer
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7373>
V3D doesn't provide any means to acquire timestamps from the GPU
so we have to implement these in the CPU.
v2: enable timestampComputeAndGraphics and set timestampPeriod (Piñeiro)
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7373>
For 3D images we should take the slice to copy from or to, from
the Z coordinate of the corresponding offset, not the base array
layer.
Fixes VK_KHR_maintenance1 tests:
dEQP-VK.api.copy_and_blit.core.image_to_image.3d_images.3d_to_2d_by_slices
dEQP-VK.api.copy_and_blit.core.image_to_image.3d_images.2d_to_3d_by_layers
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7364>
So far for the formats E5B9G9R9_UFLOAT_PACK32 and
B10G11R11_UFLOAT_PACK32 we were using a XYZW swizzle. But from Vulkan
spec those are three-component, without alpha, formats. So we should
use XYZ1 instead, as we were already doing for other three-component
formats.
Curiously the only case where this raised a problem were when using
clamp to border with transparent black. This change allows us to
remove the code that handled only that specific case.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7355>
This is another case of a feature that is implemented in the compiler
and that only required that we set the shader key properly from the
pipeline state, which we were already doing.
I verified we pass the tests in dEQP-VK.pipeline.multisample.alpha_to_one.*
(we only support 4x multisampling, so we can only pass a single test there),
however, the tests seem to have a bug by which they always pass, even if
the driver doesn't actually implement alpha to one correctly. I submitted
a fix to Khronos and verified that we also pass the fixed tests (and that
we failed them if we don't actually set te shader key correctly).
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7336>
Our blit shader path allocates a descriptor pool to create
combined image sampler descriptors for blit source images. So
far, we had sized this pool statically and the driver would
fail if we ever need to allocate more descriptors than that.
With this change, we switch to using a dynamic allocation
mechanism instead where we allocate as many pools as we need to
meet descriptor set allocation requirements for the command buffer.
Also, every time a new pool needs to be created, we double its
size (up to a limit), so we can start small and avoid wasting
memory for command buffers that only have a small number of blits,
while trying to keep allocation overhead low for command buffers
that record a lot of blits.
v2: use existing framework for automatic destruction of private
driver objects to free allocated pools.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7311>
For us this is mostly handled in the compiler by a NIR lowering so
for the Vulkan driver we only need to make sure that we program our
shader key correctly from the pipeline state, which we were already
doing.
It doesn't look like CTS has any coverage for this yet so it has only
been smoke tested, but it seems to be working correctly, as expected.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7313>
This key field is only used for V3D versions before 4.1, so it
should not be set for v3dv. Also, it is a mask not a boolean.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7271>
We were computing this too early based on the view's format alone
which is not correct, since we need to consider the view's swizzle
as well.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7271>
If we are blitting to tile boundaries we don't need to emit
tile loads. The exception to this is the case where we are
blitting only a subset of the pixel components in the image
(which we do for single aspect blits of D24S8), since in that
case we need to preserve the components we are not writing.
There is a corner case where some times we create framebuffers
that alias subregions of a larger image. In that case the edge
tiles are not padded and we can't skip the loads.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7247>
Fix defect reported by Coverity Scan.
Assign instead of compare (PW.ASSIGN_WHERE_COMPARE_MEANT)
assign_where_compare_meant: use of "=" where "==" may have been intended
Fixes: ca86c7c65a ("v3dv: assert command buffers are executable when submitting to a queue")
Signed-off-by: Vinson Lee <vlee@freedesktop.org>
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7156>
Fix defect reported by Coverity Scan.
Uninitialized scalar variable (UNINIT)
uninit_use_in_call: Using uninitialized value time.tv_sec when calling free_stale_bos.
Fixes: f78c99f357 ("v3dv/bo: add a maximum size for the bo_cache and a envvar to configure it")
Signed-off-by: Vinson Lee <vlee@freedesktop.org>
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7154>
index is of type uint32_t.
Fix defect reported by Coverity Scan.
Macro compares unsigned to 0 (NO_EFFECT)
unsigned_compare: This greater-than-or-equal-to-zero comparison of
an unsigned value is always true. index >= 0U.
Signed-off-by: Vinson Lee <vlee@freedesktop.org>
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7153>
Fix defect reported by Coverity Scan.
Assign instead of compare (PW.ASSIGN_WHERE_COMPARE_MEANT)
assign_where_compare_meant: use of "=" where "==" may have been intended
Fixes: c75846e674 ("v3dv: implement DRM modifier setup for WSI")
Signed-off-by: Vinson Lee <vlee@freedesktop.org>
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7124>
