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>
Those crashes are gone if VK_EXT_extended_dynamic_state is supported.
It is worth to note that the previous entry mentioned Ricardo's patch
as tentative. That is already on vk-gl-cts main (not still on the tag
used by the CI), and the code at this point has been tested with and
without that patch.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27609>
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>
broadcom-rpi4-fails
The arb_texture_view@rendering-formats test crash is caused by V3D not
supporting PIPE_FORMAT_{R16,R16G16,R16G16B16A16}_UNORM for rendering
so move the rendering-formats test to the appropriate section
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28862>
This removes the need for drivers to handle both versions. The base will
get added once in nir_lower_system_values when converting from deref to
intrinsic and will be replaced by a zero for users not supporting it.
Reviewed-by: Daniel Schürmann <daniel@schuermann.dev>
Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Signed-off-by: Karol Herbst <kherbst@redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26800>
sim_state.perfcnt_total provides the total number of counters
supported by the underlying simulated platform and is what we
use when we create a perform to validate that the counters
requested are valid, so we should use this.
V3D_PERFCNT_NUM is a fixed enum value that is only valid for
V3D 4.2 at present and is not sufficiently large for all the
counters available in V3D 7.x.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28870>
This fixes a number of regressions in Vulkan subgroups tests in CTS.
Fixes: 97f5721bfc ('broadcom/compiler: needs_quad_helper_invocation enable PER_QUAD TMU access')
cc: mesa-stable
Reviewed-by: Jose Maria Casanova Crespo <jmcasanova@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28797>
We take advantage of the needs_quad_helper_invocation information to
only enable the PER_QUAD TMU access on Fragment Shaders when it is needed.
PER_QUAD access is also disabled on stages different to fragment shader.
Being enabled was causing MMU errors when TMU was doing indexed by vertexid
reads on disabled lanes on vertex stage. This problem was exercised by some
shaders from the GTK new GSK_RENDERER=ngl that were accessing a constant buffer
offset[6], but having PER_QUAD enabled on the TMU access by VertexID was
doing hidden incorrect access to not existing vertex 6 and 7 as TMU was
accessing the full quad.
cc: mesa-stable
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28740>
The hardware do not support setting different polygon modes for front
and back faces at the same time. In this case, unless we are culling one
of the faces, we show a warning to the user.
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/28675>
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>
