emit_subpass_color_clear_rects and emit_subpass_ds_clear_rects is
receiving a v3dv_render_pass parameter pass, but then using
cmd_buffer->state.pass to access the current pass. All calls to those
methods are already initializing that parameter to that value.
This commit just uses the parameter. An alternative would be to remove
one of the parameters of the function call. I find this option
slightly more readable.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29218>
The resulting pipeline/shaders are different when we are using
multiview (for example, a geometry shader is injected in order to
support multiview).
Doesn't fix any CTS test run individually, but fixes some
dEQP-VK.draw.dynamic_rendering.primary_cmd_buff.multi_draw.overlapping*
CTS tests when run in a batch (using deqp-vk --deqp-caselist-file),
like:
dEQP-VK.draw.dynamic_rendering.primary_cmd_buff.multi_draw.mosaic.indexed_mixed.16_draws.stride_zero.10_instances.vert_only.single_view.offset_6_no_draw_id
dEQP-VK.draw.dynamic_rendering.primary_cmd_buff.multi_draw.overlapping.normal.one_draw.stride_zero.1_instance.vert_only.multiview.no_offset_no_draw_id
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29218>
2712D0 has V3D 7.1.10 which included draw index and
base vertex in the shader state record packet, shuffling
the locations of most of its fields. Handle this at run
time by emitting the appropriate packet based on the
V3D version since our current versioning framework doesn't
support changes based on revision number alone.
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29189>
2710D0 has V3D 7.1.10 which included draw index and
base vertex in the shader state record packet, shuffling
the locations of most of its fields. Handle this at run
time by emitting the appropriate packet based on the
V3D version since our current versoning framework doesn't
support changes based on revision number alone.
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29189>
As this is intended to be used also by VC4, change the suffix to
something more convenient, like tlb_color_brcm.
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/29119>
This lowering is applied only for vertex and geometry shaders. So detect
earlier this situation and do not go ahead with other shader.
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/29102>
In the past where backends had to deal with nir_register we needed
a specific path for them here because nir_def_components_read only
worked on ssa defs, but now that we got rid of nir_register we
only have nir_def and we don't need to go out of our way to do this
and we can just always use nir_def_components_read.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28978>
This is similar to what we do for textures where we program a number
of channels matching the number of componentes actually read by the
shader.
Makes tests like dEQP-VK.image.load_store.with_format.2d.r32_uint
drop from 18 instructions to 14 by emitting a single ldtmu instead
of 4.
From dEQP-VK.image.load_store.*:
total instructions in shared programs: 12681 -> 12093 (-4.64%)
instructions in affected programs: 4866 -> 4278 (-12.08%)
helped: 256
HURT: 0
helped stats (abs) min: 1 max: 4 x̄: 2.30 x̃: 2
helped stats (rel) min: 4.76% max: 25.00% x̄: 12.35% x̃: 11.11%
95% mean confidence interval for instructions value: -2.40 -2.19
95% mean confidence interval for instructions %-change: -12.99% -11.71%
Instructions are helped.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28978>
If a shader has too may outputs overflowing the array may
overwrite other pieces of state, which can then be tricky to
debug.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28978>
Any drivers which use vk_spirv_to_nir() now no longer need to build a
caps table manually.
Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Reviewed-by: Iván Briano <ivan.briano@intel.com>
Acked-By: Mike Blumenkrantz <michael.blumenkrantz@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28905>
A few of the parameters are not actually used at all. So let's clean
them.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29101>
Note that we don't support (and clarify on code why) two of the
features of this extension:
* extendedDynamicState2PatchControlPoints: as we don't support
Tessellation Shaders
* extendedDynamicState2LogicOp: as supporting it would need to allow
compile shader variants after pipeline creation, that we try to
avoid as much as possible (and it is not supported right now)
Note that those two features are not mandatory for Vulkan 1.3. From
spec:
"Promotion to Vulkan 1.3
This extension has been partially promoted. The dynamic state
enumerants VK_DYNAMIC_STATE_DEPTH_BIAS_ENABLE_EXT,
VK_DYNAMIC_STATE_PRIMITIVE_RESTART_ENABLE_EXT, and
VK_DYNAMIC_STATE_RASTERIZER_DISCARD_ENABLE_EXT; and the
corresponding entry points in this extension are included in core
Vulkan 1.3, with the EXT suffix omitted. The enumerants and entry
points for dynamic logic operation and patch control points are not
promoted, nor is the feature structure. Extension interfaces that
were promoted remain available as aliases of the core functionality."
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28980>
Note that when it is dynamic, it goes to the codepath of having
enabled raster_enabled at the pipeline, even if at the end it will be
disabled. The fragment shader compilation, and the stage keys, depends
on rasterization being enabled or not. As mentioned, if the state is
dynamic, it assumes that the rasterization is enabled.
That would work, as then the rasterization could be discarded at the
CFG_BITS package, by the command buffer at draw time. We just have a
(discarded) shader slightly more complex that it would have been with
rasterization enabled.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28980>
Since VK_EXT_extended_dynamic_state2
We just move all related with depth bias to the command buffer. There
is not good reason to compute and save it at the pipeline.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28980>
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>
