Semaphore waits in a command buffer only affect the first jobs we execute
in each hardware queue since jobs in the same queue are serialized against
each other. Binning syncs in particular, only affect CL jobs.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17594>
We only want to cleant BCL barrier flags if we consume a BCL barrier.
For example, if the client records a barrier for an index buffer
it should apply to the next draw call that uses an index buffer
which may not be the current draw call but one coming after it.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17594>
The trigger for this commit was when we found that we were not calling
nir_metadata_preserve when lowering the layout code. But then I found
that it would be better to just update the code to use
nir_shader_instructions_pass, so we can avoid to manually:
* Initialize the nir_builder
* Call nir_foreach functions (we pass the callback)
* Call nir_metadata_preserve functions (that as mentioned we were not calling)
We also get a nice cleanup of several functions by reducing the number
of parameters (we pass a state struct).
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17609>
This represents an offset from the actual start of the image data,
not from the start of the memory allocation bound to the image.
Fixes:
dEQP-VK.image.subresource_layout.*
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17648>
From the Vulkan spec:
"If poolSizeCount is not 0, pPoolSizes must be a valid pointer to an
array of poolSizeCount valid VkDescriptorPoolSize structures"
So 0 is actually allowed and there is a CTS to check it is handled gracefully.
Fixes:
dEQP-VK.api.descriptor_pool.zero_pool_size_count
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17648>
This feature allows shaders to use pointers to buffers which may
not be bound via descriptor sets. Access to these buffers is done
via global intrinsics.
Because the buffers are not accessed through descriptor sets, any
live buffer flagged with VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT_KHR
can be accessed by any shader using global intrinsics, so the driver
needs to make sure all these buffers are mapped by the kernel when
it submits the job for execution.
We handle this by tracking if any draw call or compute dispatch in
a job uses a pipeline that has any such shaders. If so, the job is
flagged as using buffer device address and the kernel submission
for that job will add all live BOs bound to buffers flagged with the
buffer device address usage flag.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17275>
If we have 2 pipelines that consume the same push constant data
but where one of them only uses direct access and the other has
indirect access, a draw with the first pipeline would clear the
dirty flag without updating the UBO and by the time we bind and
draw with the second pipeline we won't upload the constants either
because the first draw cleared the dirty flag.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17536>
Since we allocate this ourselves we can immediately add it to the
job at the time we allocate it.
This also fixes a bug we introduced when we implemented inline
uniforms because since that commit, if we had an inline uniform
buffer at index 1 which happend to have indirect access we would
track it in slot 0 instead of slot 1, potentially overwriting
the push constant buffer reference.
Fixes: ea3223e7a4 ('v3dv: implement VK_EXT_inline_uniform_block')
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17536>
We have code in there to allocate various segments of MAX_PUSH_CONSTANTS_SIZE
to handle the case of various draw calls in the same command buffer requiring
different push constants, so we are implicitly expecting it to be larger than
this. In fact, this only works now because when we allocate a BO we are always
at least allocating a full page, so the least we ever allocate is 4096 bytes,
so be explicit about it to avoid confusion.
Also, since we were always mapping MAX_PUSH_CONSTANTS_SIZE and the mapping
always starts at the beginning of the BO, it looks like after the first copy
when the resource offset is not zero, we would be writing outside the mapped
range. Always map the full size of the BO instead to ensure this doesn't
happen.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17536>
We have been always uploading MAX_PUSH_CONSTANTS_SIZE but now that
we track the actual size of the push constant buffer we can use
this instead.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17536>
If the command buffer didn't have any push constants or the meta
operation didn't write any new constants we don't need to restore
the state.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17536>
Also, remove the FIXME to pre-compute this in images. We only use
this helper from copy/clear operations where we may be working
with a compatible framebuffer format instead of the original image.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17509>
This intrinsic is only produced when the compiler is instructed
to handle layer id as a system value, which we don't use. Also,
we have been supporting layered rendering for a while and passing
all the relevant tests which would've failed if we were hitting
this lowering.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17483>
When using the texel buffer copy path to copy a buffer we need to
sample from the buffer and for that we need a texture shader state
record where we specify the base offset of the texture (the buffer).
If the copy operation has a start offset we can't add that offset
to the base address of the buffer because the texture state record
requires the base pointer to be 64-byte aligned, so it would only
work for offsets that are multiple of 64B. Instead, we pass the
offset (in elements) to the shader and we use that to shift the
indices into the buffer when selecting the source texel to copy.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17482>
We can produce slightly better code for these in the backend, so
do that. For this we need to:
1. Fix our implementation of uadd_carry (which wasn't used) to return
an integer instead of a boolean value.
2. Add an implementation of usub_borrow.
Notice these are only used in Vulkan. In GL these instructions are
always unconditionally lowered by the state tracker in GLSL IR so
we never get to see them in the backend.
Shader-db stats from a collection of Vulkan samples:
total instructions in shared programs: 122351 -> 122345 (<.01%)
instructions in affected programs: 196 -> 190 (-3.06%)
helped: 2
HURT: 0
total uniforms in shared programs: 18670 -> 18672 (0.01%)
uniforms in affected programs: 59 -> 61 (3.39%)
helped: 0
HURT: 2
total max-temps in shared programs: 13145 -> 13147 (0.02%)
max-temps in affected programs: 27 -> 29 (7.41%)
helped: 0
HURT: 2
total inst-and-stalls in shared programs: 123052 -> 123046 (<.01%)
inst-and-stalls in affected programs: 197 -> 191 (-3.05%)
helped: 2
HURT: 0
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17372>
This only works if the framebuffer config is exactly the same so
testing both subpasses have the same attachments is not enough,
they also need to be exactly in the same order.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17358>
Right now we had two methods that tries to optimize the nir shader,
nir_optimize and st_nir_opts. The latter is being used when we are
linking, but again, it has basically the same purpose that
nir_optimize.
So this commit adds more lowerings to nir_optimize_nir, add some extra
comments on the method, and replaces st_nir_opts with nir_optimize.
Ideally we would like to just use the already existing
v3d_optimize_nir that we have at the backend But:
* Using it leads to some regressions on Vulkan CTS tests, due some
lowerings that are already there.
* We would need to move to the backend some additional
lowerings/optimizations that are used on the Vulkan
frontend. That would require to check that we are not getting any
regression or performance drop on OpenGL
So for now we are keeping a Vulkan specific nir_optimize method.
Additionally this fixes the following test:
dEQP-VK.graphicsfuzz.cov-loop-condition-clamp-vec-of-ones
Shaderdb stats, using some well known Vulkan apps (ue4 demos, Quake3e,
etc):
total instructions in shared programs: 124974 -> 125108 (0.11%)
instructions in affected programs: 50328 -> 50462 (0.27%)
helped: 4
HURT: 79
total uniforms in shared programs: 19019 -> 19020 (<.01%)
uniforms in affected programs: 60 -> 61 (1.67%)
helped: 0
HURT: 1
total max-temps in shared programs: 13438 -> 13444 (0.04%)
max-temps in affected programs: 85 -> 91 (7.06%)
helped: 0
HURT: 2
total inst-and-stalls in shared programs: 125715 -> 125849 (0.11%)
inst-and-stalls in affected programs: 50429 -> 50563 (0.27%)
helped: 4
HURT: 79
total nops in shared programs: 8203 -> 8204 (0.01%)
nops in affected programs: 732 -> 733 (0.14%)
helped: 7
HURT: 9
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16986>
That is what most others Vulkan drivers do (radv, anv, turnip at
least).
The origin of this change cames from a CTS test where the loop
unrolling converted a ubo index defined inside a loop from constant to
non constant. That is not desiderable on any driver, but a problem on
v3dv, as v3dv doesn't support that case.
Although we initially tried to fix it on the loop unroll, we discarded
that approach, and focused on the existing nir lowerings/optimizations
as this was not happening with other drivers.
We noted that in other drivers this case of a ubo index going from
const to non-const were also happening with nir_lower_explicit_io, but
in that case it was able to be converted back to a const on following
lowerings. The only difference with other drivers is that we were
calling it before the first nir optimization loop.
So this change helps with fixing the following CTS test (for that we
also need to run additional lowerings, which we do in a later patch):
dEQP-VK.graphicsfuzz.cov-loop-condition-clamp-vec-of-ones
You can get further details on the following issue and RFC merge
request, specially the merge request:
https://gitlab.freedesktop.org/mesa/mesa/-/issues/6051https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15391
We also made some shaderdb stats with our usual Vulkan apps (ue4
demos, quake3, etc):
Total instructions in shared programs: 125014 -> 124974 (-0.03%)
instructions in affected programs: 7544 -> 7504 (-0.53%)
helped: 7
HURT: 4
total uniforms in shared programs: 19026 -> 19019 (-0.04%)
uniforms in affected programs: 514 -> 507 (-1.36%)
helped: 5
HURT: 0
total max-temps in shared programs: 13430 -> 13438 (0.06%)
max-temps in affected programs: 270 -> 278 (2.96%)
helped: 0
HURT: 8
total sfu-stalls in shared programs: 739 -> 741 (0.27%)
sfu-stalls in affected programs: 30 -> 32 (6.67%)
helped: 0
HURT: 2
total inst-and-stalls in shared programs: 125753 -> 125715 (-0.03%)
inst-and-stalls in affected programs: 7685 -> 7647 (-0.49%)
helped: 7
HURT: 4
total nops in shared programs: 8228 -> 8203 (-0.30%)
nops in affected programs: 546 -> 521 (-4.58%)
helped: 9
HURT: 2
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16986>
Since we only consume barriers at the beginning of a new job, if
a command buffer ends with a barrier we will not handle it. Fix
this by emitting a noop job in that case to consume it. Ideally,
we could do better and check the pending barrier state to fine
tune the noop job so we don't wait on all queues, but for now
this fixes flakyness with some CTS pipeline barrier tests that
started to show up after we optimized binning sync barriers. It
is likely that the additional sync we had before that change was
enough to prevent the problem from showing up.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17020>
When we switched to using structs to track barrier state we made a mistake
and started to overwrite barrier state in primary command buffers with
the pending state from secondary command buffers executed inside them, when we
should've been merging the state instead.
Fixes flakyness with some CTS barrier tests.
Fixes: f7ce42636c ('v3dv: use an explicit struct type to track barrier state')
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17020>
The only reason for the wrapper was so that we could dummy signal the
semaphore and fence. Now that the WSI code always dos this for us, we
can drop our wrapper.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037>
Until now we would only disable EZ globally if we had a depth or stencil
load operation or if we had no draw calls at all, but even if we have draw
calls if all of them disable EZ we should also us the global disable.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16794>
When we have a draw call that is incompatible with EZ we should only
disable EZ for the remaining of the job in the case that both of the
following conditions are met:
1. The cause for the incompatibility is an incompatible depth test
direction.
2. The pipeline does Z writes.
Otherwise it is enough to disable EZ temporarily only for draw calls
with the incompatible pipeline.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16794>
Until know when we consumed a barrier we would implement it by
setting the serialize flag on a job, which would cause it to
be serialized across all hardware queues (CL, CSD, TFU). However,
now that we track the source(s) of the barrier, we can restrict this
to only the relevant queue(s) instead (multisync path only).
It should be noted that we can implement transfers via TFU or CL
jobs, so if the source of a barrier is a transfer, we currently
synchronize against both the TFU and the CL queues, however, we
may be able to more effectively track this in the future to
restrict this to just one of the queues.
Also, for secondary command buffers we are taking the easy way
out and always synchronize against all queues, but we should
be able to do the same for secondaries without too much effort.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16743>
Until now we have been tracking the dstStageMask of barriers (where they
are consumed) but not where they are produced (the srcStageMask). With
this change we extend our barrier state to keep track of this as well.
This allows the driver to have better knowledge of the intended barrier
semantics so it can limit the amount of synchronization it does only
to the source stages involved with a barrier. We will do this in a
later patch.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16743>
