While working on VK_EXT_extended_dynamic_state2 we found two issues
the stencil emission code, after the update for StencilTestEnable
being dynamic.
Specifically:
* pack_stencil_cfg: if we don't have a ds_info, we need to return,
as pack_single_stencil_cfg uses it to fill it up. Also the check
for MESA_VK_DYNAMIC_DS_STENCIL_TEST_ENABLE was not needed. That
state doesn't affect the content of the STENCIL_CFG
packet. Stencil is enabled/disabled at the CFG_BITS packet.
* cmd_buffer_emit_stencil: we can't use pipeline->emit_stencil_cfg
to filter if it is needed to emit that as since
stencil_test_enable and stencil_op become dynamic.
We also update which states we check that are dynamic. As
mentioned STENCIL_TEST_ENABLE doesn't affect here.
Fixes: 60e9237e81 ("v3dv: StencilOp and StencilTestEnable are now dynamic")
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28980>
There were some pending places to update after PrimitiveTopology
become dynamic. FWIW, this was not catched by any CTS test.
As we are here we add a comment to explain why we still use the
topology on the pipeline.
Fixes: 2526f74ade ("v3dv: PrimitiveTopology is now dynamic")
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28980>
When logging a failed IOCTL, an errno is more useful than the output of
`drmIoctl()`. When the IOCTL fails, the return is usually -1 and this
value isn't very useful. On the other hand, the errno can help us to
debug the reason why the IOCTL failed.
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29067>
Remove the v3dv_GetPhysicalDeviceProperties and the
v3dv_GetPhysicalDeviceProperties2 functions, replace them
by a private get_device_properties() called at device initialization
time.
(given the diff, the change is best viewed with --diff-algorithm=histogram)
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Tested-by: Roman Stratiienko <r.stratiienko@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26386>
When this was promoted to EXT it expanded its properties struct to add a new
supportsNonZeroFirstInstance field.
Fixes: d38ff02c03 ("v3dv: mark some promoted extensions as supported")
Fixes: dEQP-VK.api.info.vulkan1p2_limits_validation.khr_vertex_attribute_divisor
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28964>
This commit introduces a significant change when we emit STENCIL_CFG,
with any dynamic state: we stop to use cl_emit_with_prepacked, and use
directly cl_emit. The reason is that now most of the STENCIL_CFG
parameters are dynamic, any improvement of using
cl_emit_with_prepacked is minimized. Also gets the code simpler, and
avoid the need to be extra careful with the fact that
cl_emit_with_prepaked doesn't override values.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27609>
Note that although the topology affects the final shader, and it is
part of the v3d_fs_key (through is_points and is_lines), changing
dynamically the topology would not trigger a shader recompilation as
that would only needed if there was a topology class change. From
spec:
"VUID-vkCmdDraw-dynamicPrimitiveTopologyUnrestricted-07500
If the bound graphics pipeline state was created with the
VK_DYNAMIC_STATE_PRIMITIVE_TOPOLOGY dynamic state enabled and the
dynamicPrimitiveTopologyUnrestricted is VK_FALSE, then the
primitiveTopology parameter of vkCmdSetPrimitiveTopology must be of
the same topology class as the pipeline
VkPipelineInputAssemblyStateCreateInfo::topology state"
dynamicPrimitiveTopologyUnrestricted is defined at
VK_EXT_extended_dynamic_state3, so for now it is false. And even if in
the future we support that extension, it is really likely that we
would return False there.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27609>
As the values depends on several values that can be dynamic too.
Note that the current approach of this commit is keeping this info
duplicated on the pipeline and the cmd_buffer. The alternative would
be to just track it at the cmd_buffer, like we did recently with
z_updates_enable, but getting the values for ez_state/incompatible_ez
were more complex, so this commit still computes it when the pipeline
is created, and uses it as default value.
This is debatable though, and the alternative would be to just keep
ez_state/incompatible_ez_state at the command buffer.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27609>
As it depends on values that could be dynamic now. Technically we
could try to keep pre-emitting, just in case that info is provided
statically.
But for the dynamic case, we would still need to compute that bits,
and we would need to discard all the pre-emitted CFG set, and
recompute it completely (as right now cl_emit_with_prepacked doesn't
allow to override values).
It is also gets a simpler code by setting those flags in only one
codepath.
As we are here, we also move z_updates_enable from the pipeline to the
cmd_buffer. This values doesn't require a complex compute, so it is
easier to just keep it on one place.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27609>
As with CmdSetViewport, we need to provide a custom implementation
because we want to call and save the outcome of viewport_compute_xform
when the viewport is set, not during emission.
We can just call v3dv_CmdSetViewport, as that one is already calling
vk_common_SetViewportWithCount.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27609>
Mostly equal to vkCmdBindVertexBuffers, but adding strides, that with
VK_EXT_extended_dynamic_state become dynamic, and setting pSizes.
It is worth to note that at this moment we don't use
CmdBindVertexBuffers2 pSizes.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27609>
Specifically to use the common vk_dynamic_graphics_state.
The advantage of using the common struct is not only reducing the size
of our custom one, but also using common helpers (like all those cmd
buffer setters), and a lot of the logic that in the future will be
used for other extensions.
Some notes:
* We still keep dirty flags, for things like PIPELINE,
DESCRIPTOR_SETS, etc. Other driver do the same. FWIW, this is also
an improvement, as before we were mixing those with the per-spec
Vulkan dynamic info.
* For the port viewport/scissor we still keep some data on a custom
structure, as we cache the translate/scale info that is derived
from scissor/viewport, but used in three different places.
For that we also maintain a custom implementation of
CmdSetViewport, that computes translate/scale, and call the common
implementation.
* We make the same for color_write_enables. The vulkan runtime saves
it as a 8-bit bitset, with a bit per attachment. But when combining
with color_write_mask you need a 32bit with 4 bits set per
attachment. To avoid recompute it during emission, we also cache
the color_write_enables, using the runtime just to track the dirty
status.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27609>
Even if the pipeline is the same.
The followin sequence, used on
dEQP-VK.dynamic_state.*.double_static_bind tests, is valid:
1. Bind pipeline with some static state.
2. Set state command for that static state (to a bad value).
3. Bind the same pipeline again.
4. Draw.
So on 3 we need to ensure to load again the pipeline static state.
Fixes: dEQP-VK.dynamic_state.*.double_static_bind
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28897>
With the simultaneous use flag we can reuse the same command
buffer multiple times. That means, for example, that we can
have an instance of a job running in the GPU while we are
submitting another one for execution to a queue.
This scenario is problematic with dynamic rendering and job
suspension because suspended jobs need to be patched with the
resume address at queue submit time, and thus, if we have another
instance of the same job currently executing in the GPU we could
stomp its resume address, which could be different.
To fix this, at queue submission time, when we detect a suspending
job in a command buffer with the simultaneous use flag, we clone the
job and create its own copy of the BCL so we can patch the resume
address into it safely without conflicting with any other instance
of the job that may be running.
We need to flag these clones as having their own BCL since
we would have to free it when the job is destroyed, unlike other
clones that don't own any resources of their own. Also, because
this job is created at queue submit time, it won't be in the
execution list of the command buffer, so it won't be automatically
destroyed with it, so we need to add it to the command buffer
as a private object.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28521>
This will be useful to know which is the actual executable
size of a BO in a CL that branches into a another BO. We
will need this soon to implement deep clones of the BCL
for suspending jobs with the command buffer simultaneous
use flag.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28521>
We had these pointing to the original job instead of pointing
to the cloned job. This can be confusing, particularly, if we
then emit commands that include references to new BOs into the
cloned jobs, since we would then try to insert these BOs in the
original jobs instead of the clones, which was the situation
we had when we implemented resume address patching with dynamic
rendering.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28521>
This will clone the job but it won't automatically put it in the
job list of a command buffer. This will come in handy to handle
the required job cloning for suspending jobs with the command buffer
reuse flag in a follow-up patch.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28521>
Multisync has been available in kernel releases for a long time
now and Raspberry Pi OS kernels have been supporting it for a
while too.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28541>
Fixes crashes in tests like
dEQP-VK.pipeline.monolithic.render_to_image.core.2d_array.huge.width_height_layers.r8g8b8a8_unorm
with CTS main.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28364>
This is achieved by the following steps:
#ifndef DEBUG => #if !MESA_DEBUG
defined(DEBUG) => MESA_DEBUG
#ifdef DEBUG => #if MESA_DEBUG
This is done by replace in vscode
excludes
docs,*.rs,addrlib,src/imgui,*.sh,src/intel/vulkan/grl/gpu
These are safe because those files should keep DEBUG macro is already excluded;
and not directly replace DEBUG, as we have some symbols around it.
Use debug or NDEBUG instead of DEBUG in comments when proper
This for reduce the usage of DEBUG,
so it's easier migrating to MESA_DEBUG
These are found when migrating DEBUG to MESA_DEBUG,
these are all comment update, so it's safe
Replace comment /* DEBUG */ and /* !DEBUG */ with proper /* MESA_DEBUG */ or /* !MESA_DEBUG */ manually
DEBUG || !NDEBUG -> MESA_DEBUG || !NDEBUG
!DEBUG && NDEBUG -> !(MESA_DEBUG || !NDEBUG)
Replace the DEBUG present in comment with proper new MESA_DEBUG manually
Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Acked-by: David Heidelberg <david.heidelberg@collabora.com>
Reviewed-by: Eric Engestrom <eric@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28092>
This was missing, this is implemented in common code.
Signed-off-by: Joshua Ashton <joshua@froggi.es>
Acked-by: Faith Ekstrand <faith.ekstrand@collabora.com>
Acked-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28275>
With dynamic rendering secondary command buffers can start subpasses
so we need this. Outside dynamic rendering secondary command buffers
won't be calling here since they are restricted to record commands
within a subpass.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27978>
If a secondary command buffer recording a dynamic pass has the
VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT flag
then the rendering information for it should come from a
VkCommandBufferInheritanceRenderingInfo struct in the pNext
chain instead of the usual render pass information in the
VkCommandBufferInheritanceInfo struct. We take the information
from the new struct and build a render pass description from it
assuming a setup without a framebuffer (which is optional for
regular render passes too).
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27978>
Because we are cloning these into primaries but the cloning is
superficial the command lists in them still point to the original
jobs and therefore paching new addresses would make the packing
code add the BO of the resume address to the original job. This
has two problems:
1. This is probably not what we want since the patching should only
be affecting the clone.
2. The bo_count of the clone job will not be updated accordingly and
we end up with a mismatch that will blow up when we submit.
The solution used here is a big hack, but works for now: we just
specify the address by its full offset rather than a relative
offset from a BO. We already have to add all the BOS in the resume
job manually which will include this the BO for the branch address
too, so this is fine.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27978>
This was used only in secondary CL command buffers so it made
sense but with dynamic rendering we are going to also have
regular CLs also in secondaries (since secondaries can now
record full dynamic rendering passes), so renaming this to
INCOMPLETE makes more sense, since this is really what they
refer to: parts of CLs that are intended to be merged into
other primaries through branching.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27978>
Dynamic rendering allows the client to suspend recording of a
render pass and have it continued in a different command buffer.
When a suspended command buffer is submitted to a queue, the
resuming command buffer must be te next one in submission order.
This means we need to be able to "merge" or "stitch" together
these command buffers at submit time.
To accomplish this, when we suspend a command buffer we emit
a BRANCH instruction to finish it. Then at submit time, when
we know the resuming job, we patch the BRANCH address with the
address of the resuming binning list (bcl). This is very similar
to how we execute secondary command buffers inside a render pass.
Also, only the last resuming job should flush the binning lists
in the bcl since we won't have processed the full binning command
list until we have execute the last linked job in the resume
list.
Since all jobs and command buffers in the suspend/resume chain
must be part of the same dynamic render pass, we only need to
produce and emit the render command list (rcl) once.
Since the way we implement stitching is that we branch from the
suspending job into the resuming one, the first job suspending
will link into all the resuming jobs necessary to complete the
chain, therefore, after the stitching is complete, we only want
to submit the first job in the suspend/resume chain, and thus,
we only produce and emit the rcl for this one job.
Notice as well that suspending only affects the last job
recording a dynamic rendering pass (the one that needs the branch
so we can resume execution with another job in another command
buffer).
Resuming affects all jobs in the dynamic render pass, since
we won't produce RCLs for them (as only the originating job
on the suspend/resume chain will emit the RCL).
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27978>
With this we are able to run basic dynamic render passes, however,
we are still missing a few things like support for secondary
render passes, suspend/resume, etc that will be adding in follow-up
patches.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27978>
This builds up on the previous patch and rewrites all the pipeline
code that fetched information from the pipeline's render pass (which
will be NULL for dynamic rendering) to instead fetch it through the
new rendering_info field, which will be valid for both regular and
dynamic render passes.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27978>
With dynamic rendering the API formally eliminates render passes,
so the pipeline create info can now have a NULL render pass, in
which case rendering info must be provided via pNext struct
VkPipelineRenderingCreateInfo, or if this is missing too then
defaults to no multiview and no attachments.
Since we don't want to have separate paths all over the place
whenever we need to access render pass / rendering info for the
pipeline, we will always produce a valid vk_render_pass_state
struct with the relevant information even when we have a render
pass, so we can rely on that always being available.
A follow-up patch will rewrite all the places where we assumed
the existence of a render pass in the pipeline to instead fetch
the info it needs from this new field instead.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27978>
Since the plan is to leverage our render pass infrastructure, we
also need to setup a framebuffer from the rendering info provided
with dynamic rendering.
We allocate the framebuffer lazily, only once, if a dynamic render
pass is used. To do this, we make it so it can hold the maximum
number of attachments possible with our hardware.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27978>
The idea is to build a regular render pass from the rendering info
provided with dynamic rendering. We will use this when recording
dynamic render passes to leverage our existing implementation
for render passes with dynamic rendering.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27978>
It is allowed for a shader to enable the multiview extension
even if the draw call in which it is used doesn't use multidraw.
This allows the shader to still use gl_ViewIndex, which will
always be 0 in that scenario.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27978>
This is based on a lowering that we are already using in the OpenGL
driver.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28171>
So far to check if rasterization discard is enabled or not we assumed
that rasterization state struct was never NULL.
However, as this will change with VK_EXT_extended_dynamic_state3, it can
be a good idea just to assume it can be NULL, so adding the check too.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28171>
