On V3D 4.2 (Raspberry Pi 4), there is a hardware bug where the binner
can trigger a GPU reset in some situations where primitives are
discarded, such as due to primitive restarts.
The way to avoid this is to force the binner to do always something, by
emitting the proper CL. In this case we decided to always set point
size, as it is a very simple and fast operation.
This fixes resets caused by
dEQP-VK.pipeline.monolithic.input_assembly.primitive_restart.*.
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/39826>
Use v3dv_job_apply_barrier_state to consume pending barriers when
executing secondary command buffers. This ensures we only serialize
against relevant stages, addressing FIXME.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39278>
Move tfu_supports_tex_format() and
get_internal_type_bpp_for_output_format() from v3dvx_private.h
to v3dvx_format_table.h.
Move v3dv_format_plane and v3dv_format struct from v3dv_private.h
to v3dv_format_table.h.
Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38732>
Certain subgroup operations don’t impose constraints on
CSD supergroup packing. Mark these as supported
and account for them in v3d_csd_choose_workgroups_per_supergroup()
so packing remains unchanged when they are present.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/37836>
The attachment loadOp for stencil buffers is not stored at
attachment->desc.loadOp. It is stored at attachment->desc.stencilLoadOp
so v3dv was ignoring the clear values for stencil if the depth buffer
wasn't setup with the VK_ATTACHMENT_LOAD_OP_CLEAR.
Fixes: 4d0e497a3e ("v3dv: implement support for depth testing")
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/37228>
In the C23 standard unreachable() is now a predefined function-like
macro in <stddef.h>
See https://android.googlesource.com/platform/bionic/+/HEAD/docs/c23.md#is-now-a-predefined-function_like-macro-in
And this causes build errors when building for C23:
-----------------------------------------------------------------------
In file included from ../src/util/log.h:30,
from ../src/util/log.c:30:
../src/util/macros.h:123:9: warning: "unreachable" redefined
123 | #define unreachable(str) \
| ^~~~~~~~~~~
In file included from ../src/util/macros.h:31:
/usr/lib/gcc/x86_64-linux-gnu/14/include/stddef.h:456:9: note: this is the location of the previous definition
456 | #define unreachable() (__builtin_unreachable ())
| ^~~~~~~~~~~
-----------------------------------------------------------------------
So don't redefine it with the same name, but use the name UNREACHABLE()
to also signify it's a macro.
Using a different name also makes sense because the behavior of the
macro was extending the one of __builtin_unreachable() anyway, and it
also had a different signature, accepting one argument, compared to the
standard unreachable() with no arguments.
This change improves the chances of building mesa with the C23 standard,
which for instance is the default in recent AOSP versions.
All the instances of the macro, including the definition, were updated
with the following command line:
git grep -l '[^_]unreachable(' -- "src/**" | sort | uniq | \
while read file; \
do \
sed -e 's/\([^_]\)unreachable(/\1UNREACHABLE(/g' -i "$file"; \
done && \
sed -e 's/#undef unreachable/#undef UNREACHABLE/g' -i src/intel/isl/isl_aux_info.c
Reviewed-by: Erik Faye-Lund <erik.faye-lund@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/36437>
If the render area is restricted to a section of the framebuffer, there
is no need to consider all the framebuffer size when configuring the
supertiles, as only the supertiles coordinates of the affected area will
be submitted.
This allow to create supertiles smaller than the ones in case
considering the full screen, reducing the tiles that need to be
processed.
This also fixes https://gitlab.freedesktop.org/mesa/mesa/-/issues/13218.
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/35257>
So far the driver was configuring the supertiles to be less than 256.
But actually, there can be up to 256, not strictly less than 256.
There is one restriction though: the frame width or height in supertiles
must be less than 256.
It also moves this limit to the limits file, which is shared by v3d and
v3dv.
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/35257>
The indirect CSD job is added to the list of jobs when the device do not
support CPU queues. Otherwise it is not added, which means the job is
not free when the command buffer frees all the resources, generating a
leak.
This fixes this leak by identifying the job that contains the indirect
CSD job, and freeing it before freeing the job itself.
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34892>
When pushing constants.
This fixes an issue detected by static analyzer: null pointer passed to
1st parameter expecting 'nonnull'.
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Reviewed-by: Jose Maria Casanova Crespo <jmcasanova@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34050>
Instead of the cmd_buffer. Also, rename it to drop the cmd_buffer
reference, make it a public helper, make it accumulate the
barrier state instead of overwriting it and make it return whether
it actually applies a barrier into the job.
We will use this new public helper in a follow-up change from the
queue to better handle barriers at the end of a command buffer.
Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33507>
This will ensure we always generate a new job after a barrier and
that the new job is setup to be serialized against all previous jobs.
Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33350>
The Mesa log infrastructure is really useful as it allows us to get
debug and error information in Android systems. Apart from that, it also
allows us to forward diagnostics into the right logs and files.
Therefore, instead of using stderr for all messages, use Mesa log and
separate the messages into debugging and error messages.
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/32304>
memcpy() pointers arguments are declared to be non NULL.
This has been detected by Undefined Behaviour Sanitizer (UBSan).
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/29911>
We can use this to specify the maximum vertex index that can
be accessed, which the hardware will use to detect and prevent
out-of-bounds accesses to vertex buffers.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29425>
Before VK_KHR_maintenance5 point size is undefined unless the
shader explicitly writes it, but this extension changes this and
expects a default point size of 1.0 if none has been written.
We accomplish this by emitting a POINT_SIZE packet with the
default point size the first time we draw with a POINT primitive
in the job. If the shaders used in the draw call doesn't write
point size then the hardware will take the point size from the
state set by the packet. If the shader does write to point size
then the value written in the shader will be used instead.
Passes all tests we support in:
dEQP-VK.rasterization.primitive_size.default_size.points.*
when forcing maintenance5 enabled.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29413>
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>
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>
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>
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>
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>
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>
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>
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>
