So far we only write a maximum of 4 dwords further into the batch and
it seems just going over the CS prefetch was enough.
Turns out writing more dwords can delay the writes and we start
prefetching stuff that hasn't landed in memory yet.
This fixes the issue by stalling the CS to ensure the writes have
landed before we go over the prefetch.
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: 796fccce63 ("intel/mi-builder: add framework for self modifying batches")
Reviewed-by: Marcin Ślusarz <marcin.slusarz@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8525>
We sometimes use anv_layout_to_aux_state() to compute the aux state of
an image during the resolve operations at the end of a render
(sub)pass.
If we're dealing with a multisampled image that is created without a
transfer usage, our internal code might trigger a resolve using the
transfer layout (see genX_cmd_buffer.c:cmd_buffer_end_subpass), for
which the image doesn't the usage bit. The current code tries to AND
the 2 usages which won't have any bit in common, thus skipping all
checks below.
v2: Add the transfer usages depending on attachment usage (Lionel)
v3: Limit to samples > 1 (Jason) && DEPTH_STENCIL_ATTACHMENT_BIT (Lionel)
v4: Add transfer usage at image creation (Jason)
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: 54b525caf0 ("anv: Rework anv_layout_to_aux_state")
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4037
Reviewed-by: Reviewed-by: Tapani Pälli <tapani.palli@intel.com> (v1)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8307>
Fix defects reported by Coverity Scan.
Identical code for different branches (IDENTICAL_BRANCHES)
identical_branches: The same code is executed regardless of
whether 0 is true, because the 'then' and 'else' branches are
identical. Should one of the branches be modified, or the entire
'if' statement replaced?
Signed-off-by: Vinson Lee <vlee@freedesktop.org>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8001>
Should help some compilers/static analyzers understand this code and avoid
things like this:
../src/intel/tools/aubinator_error_decode.c:850:19: warning: "path" may be used uninitialized in this function [-Wmaybe-uninitialized]
850 | ret = asprintf(&filename, "%s/%d/i915_error_state", path, minor);
Signed-off-by: Marcin Ślusarz <marcin.slusarz@intel.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7994>
When DEBUG is not defined, no error reporting is done, the error is
just returned back. The current definition a couple of warnings in
anv_formats.c. First when the return value is intentionally ignored
../src/intel/vulkan/anv_formats.c:989:48: warning: statement with no effect [-Wunused-value]
989 | vk_errorfi(instance, physical_device, VK_ERROR_FORMAT_NOT_SUPPORTED,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/intel/vulkan/anv_private.h:486:55: note: in definition of macro ‘vk_errorfi’
486 | #define vk_errorfi(instance, obj, error, format, ...) error
| ^~~~~
and also when an argument is used only
../src/intel/vulkan/anv_formats.c:908:25: warning: unused variable ‘instance’ [-Wunused-variable]
908 | struct anv_instance *instance = physical_device->instance;
| ^~~~~~~~
../src/intel/vulkan/anv_formats.c: In function ‘anv_GetPhysicalDeviceImageFormatProperties2’:
../src/intel/vulkan/anv_formats.c:1231:25: warning: unused variable ‘instance’ [-Wunused-variable]
1231 | struct anv_instance *instance = physical_device->instance;
| ^~~~~~~~
to avoid both issues, use a static inline function that just returns
it's argument but can consume other input. Ignoring the return value
of a function is OK, and the extra input can be tagged as UNUSED
getting rid of both warnings.
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7860>
This means the pass has to walk all the instructions but it was doing
that in a bunch of cases anyway when it didn't have a HALT_TARGET.
However, removing HALT_TARGET frees up the scheduler a bit because
HALT_TARGET is considered a scheduling barrier. The shader-db results
are kind-of a wash but we're about to add HALT_TARGET unconditionally so
we want to be able to get rid of it.
Shader-db results on Ice Lake:
total instructions in shared programs: 19935623 -> 19935623 (0.00%)
instructions in affected programs: 0 -> 0
helped: 0
HURT: 0
total cycles in shared programs: 976758472 -> 976766135 (<.01%)
cycles in affected programs: 11097707 -> 11105370 (0.07%)
helped: 1750
HURT: 875
helped stats (abs) min: 1 max: 866 x̄: 26.39 x̃: 4
helped stats (rel) min: <.01% max: 39.24% x̄: 1.25% x̃: 0.46%
HURT stats (abs) min: 1 max: 1678 x̄: 61.54 x̃: 10
HURT stats (rel) min: <.01% max: 65.69% x̄: 1.86% x̃: 0.42%
95% mean confidence interval for cycles value: -2.48 8.32
95% mean confidence interval for cycles %-change: -0.40% -0.03%
Inconclusive result (value mean confidence interval includes 0).
LOST: 62
GAINED: 46
All of the lost/gained programs are SIMD32 fragment shaders.
Reviewed-by: Francisco Jerez <currojerez@riseup.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5071>
Caio Marcelo de Oliveira Filho