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>
Until now, we have always consumed barriers with the next GPU job
recorded into the command buffer after the barrier even if the job
was not the target of the barrier itself. This works based on the
idea that when we consume a barrier in a job we serialize it against
all queues, so effectively we are ensuring that whatever came
before it has completed, so if the barrier was intended for an
even later job, it would have served its purpose anyway.
It should be noted that CL jobs are special because they are actually
split in two different queues: the binning queue and the render
queue, with a dependency between them to ensure render runs after
binning. With our current implementation, if we have 3 jobs (A, B,
C) and we have a barrier after job A which is intended to block job C
on A's completion, with our implementation we would instead block
B on A's completion. If C is a CL job, and the barrier was targetting
the binning stage then we can have the following scenarios:
1. If B) is a CL job, it will consume the barrier at its binning
stage, so we know that B's binning will not start until A has
completed. Then C's binning will not start until B's binning
has completed, and thus, will not start until A has completed,
as intended.
2. If B) is not a CL job, it will consume the barrier and will not
start until A has completed, however, C's binning job will be
submitted to the binning queue without any sync requirements
and since B did not put any jobs in the binning queue it will
start as soon as A's binning has completed, but not A's render,
which would be incorrect.
Further, since a981ac0539 we now skip consumming BCL barriers if
a job does not have draw calls that can be affected by them. In the
same scenarios as before, now case 1) would also be problematic,
since B may skip the binning sync in that case and start immediately,
and since C's binning would be allowe to start immediately after B's
binning, there is no guarantee that this doesn't happen in parallel
with A's render.
With this patch we fix this situation by tracking the intended
consumer of each barrier: graphics, compute or transfer, and we make
sure to consume them only with jobs that match those semantics.
This fixes flakyness in dEQP-VK.device_group.*
Fixes: a981ac0539 ('v3dv: skip binning sync if binning shaders don't access external resources')
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16743>
VK_KHR_format_feature_flags2 is mostly about define a new 64-bit
VkFormatFeatureFlagBits2KHR format feature flag type, as 29 bits of
the 32-bit VkFormatFeatureFlagBits are already in use.
So all the bits from VkFormatFeatureFlagBits are being replicated, and
most of the work here consist on switch to the new flags.
From the new (not replicated from VkFormatFeatureFlagBits) flag bits,
we don't support
VK_FORMAT_FEATURE_2_STORAGE_READ_WITHOUT_FORMAT_BIT_KHR or
VK_FORMAT_FEATURE_2_STORAGE_WRITE_WITHOUT_FORMAT_BIT_KHR, as right now
we require the format on the shader for doing the read and stores.
We use now VK_FORMAT_FEATURE_2_SAMPLED_IMAGE_DEPTH_COMPARISON_BIT_KHR,
but only applying it for depth formats.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16718>
As it could happens that when a bo is reused from the cache, it is
being mapped with a smaller size that needed. So let's just unmap it,
and let be remapped with the needed size.
Even if we could try to be smarter when deciding when to unmap or not,
to avoid uneeded re-mappings later, it is also true that doing the
unmap would help to reduce the memory used.
This fixes an assert when running the following tests in a row (same
deqp-vk execution):
dEQP-VK.pipeline.creation_feedback.graphics_tests.vertex_stage_fragment_stage
dEQP-VK.pipeline.executable_properties.graphics.vertex_stage_geometry_stage_fragment_stage
dEQP-VK.pipeline.executable_properties.graphics.vertex_stage_fragment_stage_internal_representations
That hits the following assertion:
assert(qpu_bo && qpu_bo->map_size >= variant->assembly_offset +
variant->qpu_insts_size);
at v3dv_pipeline.c, pipeline_get_qpu.
v2: use just one call to v3dv_bo_unmap (move call at v3dv_bo_free,
replace call at bo_free for assert) (Iago)
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16678>
In f99ac7f2de ("v3dv: Don't use color aspects for depth/stencil
images"), we stopped using color aspects for depth/stencil images in a
bunch of cases. This causes us to trigger an assert in
copy_buffer_to_image_shader where it assumes 16-bit is always color but
now it can also be D16_UNORM. The assert isn't protecting us from
anything we weren't already doing before so we can just loosen it a bit.
Fixes: f99ac7f2de ("v3dv: Don't use color aspects for depth/stencil images")
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16592>
We already had a little workaround for v3dv where, for some if its meta
ops, it had to bind a depth/stenicil image as color. Instead of
special-casing binding depth/stencil as color, let's flip on the
drier_internal flag and get rid of most of the checks in that case.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16376>
Typically we free them when we upload the QPU code from the variant
to the assembly BO in the pipeline, however, if there is an error
during pipeline compilation that may not happen and we would leak
the QPU code from the variants.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16370>
We can output the final NIR form (which we store in the pipeline
stage) and the final QPU (which we can retrive from the assembly BO).
We should be careful not to fetch the shaders from the cache when
VK_PIPELINE_CREATE_CAPTURE_INTERNAL_REPRESENTATIONS_BIT_KHR is present,
since we don't store NIR shader in the pipeline shader data that is
cached, so a cache hit would leave us without the NIR shader. The spec
already contemplates this scenario:
"Enabling this flag must not affect the final compiled pipeline but
may disable pipeline caching or otherwise affect pipeline creation
time."
We also prevent disposing of the pipeline stages the variants when this
flag is requested to ensure this information is available later when
calling vkGetPipelineExecutableInternalRepresentationsKHR.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16370>
