Dedicated BOs waste memory and are also a significant cause of CPU
overhead when applications use hundreds of them per frame due to
all the work the kernel has to do to page in all these BOs for a job.
The UE4 Vehicle demo was hitting this causing it to freeze and stutter
under 1fps.
The hardware allows us to setup groups of 16 queries in consecutive
4-byte addresses, requiring only that each group of 16 queries is
aligned to a 1024 byte boundary. With this change, we allocate all
the queries in a pool in a single BO and we assign them different
offsets based on the above restriction. This eliminates the freezes
and stutters in the Vehicle sample.
One caveat of this solution is that we can only wait or test for
completion of a query by testing if the GPU is still using its BO,
which basically means that we can only wait for all active queries
in a pool to complete and not just the ones being requested by the
API. Since the Vulkan recommendation is to use a different query
pool per frame this should not be a big issue though.
If this ever becomes a problem (for example if an application does't
follow the recommendation and instead allocates a single pool and
splits its queries between frames), we could try to group queries
in a pool into a number of BOs to try and find a balance, but for
now this should work fine in most cases.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10253>
In these cases we know that the BO has not been added to the job
before, so we can skip the usual process for adding the BO where
we check if we had already added it before to avoid duplicates.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10210>
64 was a temporary and conservative "big enough" value, but we can do
better.
Note that as mentioned on the FIXME, we could be even more detailed,
adding a descriptor map allocate method based on the descriptor
type. That would mean more individual allocations, and slightly more
complexity.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10207>
Across every driver...
v2: Add casts to appease -fpermissive used on CI.
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9477>
We are providing a BO with the default attribute values for the
GL_SHADER_STATE_RECORD, that contains 16 vec4. Such default value for
each vec4 is (0, 0, 0, 1). As the attribute format could be int or
float, the "1" value needs to take into account the attribute format.
But in the practice, the most common case is all floats. So we create
one default attribute values BO assuming that all attributes will be
floats, and we store it at v3dv_device and only create a new one if a
int format type is defined. That allows to reduce the amount of BOs
needed.
Note that we could still try to reduce the amount of BOs used by the
pipelines if we create a bigger BO, and we just play with the
offsets. But as mentioned, that's not the usual, and would add an
extra complexity,so it is not a priority right now.
This makes the following test passing when disabling the pipeline
cache support:
dEQP-VK.api.object_management.max_concurrent.graphics_pipeline
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9845>
Quoting Jason's commit message (afa8f5892), that also applies here:
"The Vulkan API provides a mechanism for applications to cache their
own shaders and manage on-disk pipeline caching themselves.
Generally, this is what I would recommend to application developers
and I've resisted implementing driver-side transparent caching in the
Vulkan driver for a long time. However, not all applications do this
and, for some use-cases, it's just not practical."
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9403>
Until now we were always doing a two-step cache lookup, as we were
using the NIR shaders to fill up the key to lookup for the compiled
shaders. But since we were already generating the sha1 key with the
original SPIR-V shader (or its internal NIR representation) any info
we were collecting from from NIR is already implicit in the original
shader, so we can avoid using the NIR in most cases.
Because the v3d_key that is used to compile a shader is populated with
data coming directly from the NIR shader or produced during NIR
lowerings, we can't use it directly as part of the pipeline cache
entry. We could split them, but that would be confusing, so we add a
new struct, v3dv_pipeline_key used specifically to search for the
compiled shaders on the pipeline cache. v3d_key would be still used to
compile the shaders.
As we are using the same sha1 key for all compiled shaders in a
pipeline, we can also group all of them in the same cache entry, so we
don't need a lookup for each stage. This also allows to cache pipeline
data shared by all the stages (like the descriptor maps).
While we are here, we also create a single BO to store the assembly
for all the pipeline stages.
Finally, we remove the link to the variant on the pipeline stage
struct, to avoid the confusion of having two links to the same
data. This mostly means that we stop to use the pipeline stage
structures after the pipeline is created, so we can freed them.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9403>
Mostly the same that main mesa gl_shader_stage, but including the
coordinate shader. This would allow to loop over all the available
stages (for example if we need to free them, compute the max spill
size, etc).
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9403>
We stopped to re-use them after pippeline creation long ago, so let's
reduce the size of both structs, and avoid serialize/deserialize for
the variant case.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9403>
squashed changes from Alejandro Piñeiro <apinheiro@igalia.com>:
Add call to vk_object_base_init on internal shader_module: we have
some cases where internally we have some shader modules that we don't
create through CreateShaderModule, so in this case we need to manually
call base_init. Not sure why this wasn't needed before.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9508>
We do this by converting them to a compatible color copy and using a
destination color mask as well as a source component swizzle to handle
D24 format semantics according to the V3D hardware requirements,
similar to what we do with our blit shader interface.
This path is faster than the terrible copy_buffer_to_image_blit,
which requires to copy the source buffer to a tiled image first
and should be avoided as much as possible, since it is slow and
can also quickly increase device memory usage.
This fixes occasional OOM errors when loading traces in renderdoc.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8875>
This moves v3dv over to using the new common dispatch layer code.
v2 (Jason Ekstrand):
- Remove some now dead function declarations
Acked-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8676>
As we already have a reference to vk_instance at vk_physical_device,
that we are setting when calling vk_physical_device_init.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8676>
This moves to using the common base structs for these two objects, but
doesn't use any of the new features yet.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8676>
Things are going to start getting more complicated so let's avoid the
single mega-file approach.
Reviewed-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8676>
From vkCmdBindPipeline spec:
"pipelineBindPoint is a VkPipelineBindPoint value specifying to
which bind point the pipeline is bound. Binding one does not disturb
the others."
But internally we were only handling one pipeline per command buffer,
so binding a pipeline of one type would override an alredy bound
pipeline of other type.
Note that for push constants, in the same way that we were keeping one
client array and one bo for the values, for all stages, independently
of the stageFlags specified by vkCmdPushConstants, we are keeping the
same idea here, so such client array and bo is still tied to the
command buffer, and used by the two pipeline bind points. That makes
far easier tracking the push constants. We could revisit in the future
if we want a more fine grained tracking.
Fixes the following crashes:
dEQP-VK.pipeline.push_constant.lifetime.pipeline_change_diff_range_bind_push_vert_and_comp
dEQP-VK.pipeline.push_constant.lifetime.pipeline_change_same_range_bind_push_vert_and_comp
v2 (from Iago review)
* Move removal of v3dv_resource definition to a different commit.
* Use the new v3dv_cmd_pipeline_state on the cmd buffer meta
sub-struct, call it gfx for consistency
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8613>
The documentation states that if we disable Early Z for the whole
frame in the RCL Tile Rendering Mode packet, then we should not
emit any draw calls with it enabled (which we can do by enabling
it in the CFG_BITS packet).
Since we emit our RCL after recording our draw calls in the BCL
and we were not considering there if any condition for global disable
would be met, it was possible that we end up with an incorrect
configuration when we decide for a global disable in the RCL, which
can cause rendering artifacts. This can be easily observed by simply
forcing the RCL bit to disable early Z in applications that are known
to enable it in CFG_BITS (such as the UE Shooter demo for example).
With this change we keep track of this scenario when we record
draw calls in the BCL and if decide that we need to disable EZ for
the entire job, we make sure we never enable it for any draw calls
in the frame.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8589>
This is an optimization that should make Z/S clears faster. To enable
this we can't have any Z/S loads or stores in the job. Also, it seems
that enabling early Z/S clearing is independent of whether early Z/S
testing is enabled.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8589>
I saw this while inspecting CL dumps from the UE Shooter demo,
where they disable Z writes for occlusion queries. The hardware
is probably doing this internally, but it doesn't hurt
to do this explicitly and make CL traces consistent with intended
behavior.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8571>
If we have dirty descriptor set state we have to update our uniform
data to reference the new resources such as addresses for textures
or UBOs. This is known to have a high CPU cost, so we want to limit
this as much as we can.
It is a common rendering pattern in applications to render many objects
using the same pipeline, but modifying the descriptor sets bound to update
textures, UBOs, etc. In this scenario, we would be incurring in unnecessary
uniform stream updates for stages that don't access descriptor sets at all.
This change makes it so we track which shader stages in a pipeline
use descriptor set state and skips updating uniform streams for them
when dirty descriptor set state is the only reason requiring us to
generate new uniform streams for a draw call.
v2: reuse shader stage information from the pipeline set layouts
to track shader stages that use descriptor state.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8555>
dep_valgrind gives you -I/usr/include/valgrind (or whatever) so if
valgrind/ wasn't in the search path anyway, these includes would fail.
Found in CI when adding valgrind to the build images.
Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7936>
Writing uniform streams is performance sensitive so we should try our
best to avoid writing new uniforms if they have not changed. Particularly,
if only the vertex buffers have changed, we should not write new uniforms.
This improves performance in vkQuake2 by about 11.15%.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7683>
We still need a fallback for the case where the application makes
WSI allocations without a surface (Zink), but for the general case,
this is the right way to do this, as it would ensure that we use
the same display connection that was used to create the surface.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7631>
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 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>
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>
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>
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>
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>
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>
Iago Toral Quiroga
Alejandro Piñeiro
Alyssa Rosenzweig
Mike Blumenkrantz
Jason Ekstrand
Eric Anholt
James Park