Pre-patch, anv_descriptor_pool used a free list for host allocations
that never merged adjacent free blocks. If the pool only allocated
fixed-sized blocks, then this would not be a problem. But the pool
allocations are variable-sized, and this caused over half of the pool's
memory to be consumed by unusable free blocks in some workloads, causing
unnecessary memory footprint.
Replacing the free list with util_vma_heap, which does merge adjacent
free blocks, fixes the memory explosion in the target workload.
Disdavantges of util_vma_heap compared to the free list:
- The heap calls malloc() when a new hole is created.
- The heap calls free() when a hole disappears or is merged with an
adjacent hole.
- The Vulkan spec expects descriptor set creation/destruction to be
thread-local lockless in the common case. For workloads that
create/destroy with high frequency, malloc/free may cause overhead.
Profiling is needed.
Tested with a ChromeOS internal TensorFlow benchmark, provided by
package 'tensorflow', running with its OpenCL backend on clvk.
cmdline: benchmark_model --graph=mn2.tflite --use_gpu=true --min_secs=60
gpu: adl
memory footprint from start of benchmark:
before: init=132.691MB max=227.684MB
after: init=134.988MB max=134.988MB
Reported-by: Romaric Jodin <rjodin@google.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20289>
This reverts commit b1126abb38.
This breaks all hell at least on DG2, as there are several cases left
where current_pipeline gets checked against GPGPU to decide what to do,
and the value doesn't match that of ANV_HW_PIPELINE_STATE_COMPUTE.
On top of that, it also misses checking for
ANV_HW_PIPELINE_STATE_RAYTRACING.
Then there's the fact that in some cases, current_pipeline will be
UINT32_MAX, because it's the original undefined state and also used
after executing a secondary command buffer because we are not tracking
on which pipeline did the secondary left us.
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7910
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20349>
Based on Jianxun's ("iris: don't get format bits in AUX tables").
With gfx12.5+, the compression format is once again coming from the
surface state programming. MTL once again uses an aux-map, but it
ignores the format bits within the the aux-map metadata.
Ref: Bspec 44930: "Compression format from AUX page walk is ignored.
Instead compression format from Surface State is used."
gfx12.5+ also uses tile-4 rather than y-tiling, so if we don't see
y-tiling, we can return 0 from intel_aux_map_format_bits() for the
ignored format bits.
Rework:
* Just return 0 if not using y-tiling as suggested by Nanley.
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Nanley Chery <nanley.g.chery@intel.com>
Reviewed-by: Jianxun Zhang <jianxun.zhang@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20322>
This replaces brw_fs_get_dispatch_enables(), which was added in
b9403b1c47 ("intel: factor out dispatch PS enabling logic"), but this
function will not work well for future changes to 3DSTATE_PS.
So, instead, this moves the related code into a "genX" file which can
directly update 3DSTATE_PS for the given platform.
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20329>
This is another field that, after the recent commits, became unused.
It's either zero-initialized (by the memset) or copy-initialized
(which means it's also zero). And it never even gets used anywhere
anyway, so even if the value was non-zero it wouldn't matter.
Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20309>
As a consequence of the last two commits, reloc_bos is always NULL and
never used anywhere, so remove it.
Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20309>
The last commit made it clear that anv_reloc_list_grow() only ever
gets called with zero as num_additional_relocs, which means it will
always immediately return VK_SUCCESS without doing anything. That
means we can remove it.
Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20309>
There are only a few places in the code where num_relocs gets set:
- During anv_reloc_list_init() where it gets memset() to 0.
- At anv_reloc_list_init_clone() where it gets set with the value of
another anv_reloc_list->num_relocs.
- During anv_reloc_list_clear(), where it gets set to 0.
- During anv_reloc_list_append(), where it gets added with the value
of another anv_reloc_list->num_relocs.
As you can see, either we explicitly set the value to 0 or we copy the
value that's present in another anv_reloc_list, which should be 0. The
one place where we used to increment num_relocs was in
anv_reloc_list_add(), but that was deleted by:
7b7381e8d7 ("anv: Delete anv_reloc_list_add()")
So in this commit we delete the num_relocs field from struct
anv_reloc_list and we also delete some lines where, if the value is 0,
nothing will happen.
There's more we could be deleting here, but I wanted this commit to be
minimal so it's very clear that num_relocs can't be non-zero. We were
having some speculation that anv_reloc_list may still be important for
actually adding BOs to the batch and building the validation list, so
let's go slowly with the removal to make everything more easily
reviewable.
The one possibility I could be missing here is another situation like
the memset() we have at anv_reloc_list_init() or some other crazy
indirect overwrite, but as far as I have checked, that is not the
case.
Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20309>
Now that we removed relocations, this is not being used anywhere.
Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20309>
The hardcoded is either replaced with new interfaces or relocated
to C file if it is private.
Signed-off-by: Jianxun Zhang <jianxun.zhang@intel.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20259>
There are a lot of optimizations in opt_algebraic that match ('ine', a,
0), but there are almost none that match i2b. Instead of adding a huge
pile of additional patterns (including variations that include both ine
and i2b), always lower i2b to a != 0.
At this point in the series, it should be impossible for anything to
generate i2b, so there /should not/ be any changes.
The failing test on d3d12 is a pre-existing bug that is triggered by
this change. I talked to Jesse about it, and, after some analysis, he
suggested just adding it to the list of known failures.
v2: Don't rematerialize i2b instructions in dxil_nir_lower_x2b.
v3: Don't rematerialize i2b instructions in zink_nir_algebraic.py.
v4: Fix zink-on-TGL CI failures by calling nir_opt_algebraic after
nir_lower_doubles makes progress. The latter can generate b2i
instructions, but nir_lower_int64 can't handle them (anymore).
v5: Add back most of the hunk at line 2125 of nir_opt_algebraic.py. I
had accidentally removed the f2b(bf2(x)) optimization.
v6: Just eliminate the i2b instruction.
v7: Remove missed i2b32 in midgard_compile.c. Remove (now unused)
emit_alu_i2orf2_b1 function from sfn_instr_alu.cpp. Previously this
function was still used. 🤷
No shader-db changes on any Intel platform.
All Intel platforms had similar results. (Ice Lake shown)
Instructions in all programs: 141165875 -> 141165873 (-0.0%)
Instructions helped: 2
Cycles in all programs: 9098956382 -> 9098956350 (-0.0%)
Cycles helped: 2
The two Vulkan shaders are helped because of the "new" (('b2i32',
('ine', ('ubfe', a, b, 1), 0)), ('ubfe', a, b, 1)) algebraic pattern.
Acked-by: Jesse Natalie <jenatali@microsoft.com> [earlier version]
Acked-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Tested-by: Daniel Schürmann <daniel@schuermann.dev> [earlier version]
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15121>
In a future commit, nit_type_conversion_op won't be able to handle i2b
(and in a much later commit f2b), so switch many users to the fully
featured function.
No shader-db or fossil-db changes on any Intel platform.
Acked-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15121>
The new format support is only tested on Ice Lake and onward. Makes the
next patch clearer.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Jianxun Zhang <jianxun.zhang@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19937>
Tiger Lake and onward allow drivers to specify a compression format
independently from the surface format. So, even if the surface format
changes, hardware is still able to determine how to access the CCS.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Jianxun Zhang <jianxun.zhang@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19937>
The sampler's decompressor seems to lack support for some types of
format re-interpretation. Use the more capable decompressor for these
cases. This will be needed to avoid regressing piglit's
arb_texture_view-rendering-formats in later commits.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Jianxun Zhang <jianxun.zhang@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19937>
We'll want 2 batches :
* the main one
* another to contain dispatch commands to generate stuff in the
main batch
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20295>
And allow the function to get the very first address in the batch.
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20295>
Gen11 added a nifty feature where we have three custom system-generated
values called extended parameters that we can set to any 32-bit values
we want. These work just like vertex and instance ID and are controlled
in the pipeline by the 3DSTATE_SGVS_2 packet. They are provided to the
draw call either by extra DWORDs on the end of 3DSTATE_PRIMITIVE or by
storing values to more state registers.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20295>
On Gen11 and above, the 3DPRIMITIVE command takes an optional additional
three DWORDs of data as "extended parameters". These extended
parameters only exist in the packet if "Extended Parameters Present" is
set. Because our packing code doesn't handle variable-length commands
well, this commit adds a second version of the command which isn't real
but is just a copy of 3DPRIMITIVE with the additional dwords where the
"Extended Parameters Present" defaults to true and "DWord Length" is
adjusted by 3 as needed. The 3DPRIMITIVE command is then the gen4-9
version which still works fine but doesn't have the new parameters.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20295>
I did not properly understood that we cannot access the views written
to the descriptor sets because they might have been destroyed after
the write operation and the copy operation is allowed to copy what is
invalid data. The shader just can't access it.
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: 03e1e19246 ("anv: Refactor descriptor copy")
Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20222>
