The 'struct drm_amdgpu_cs_chunk_fence' is processed as
'struct drm_amdgpu_cs_chunk_data' which is a union.
This change ensures the proper alignment for this structure
to be processed as 'struct drm_amdgpu_cs_chunk_data'.
The presence of __u64 as one member of
'struct drm_amdgpu_cs_chunk_data' makes the
whole structure expected to be 64-bit aligned.
This is a minor issue detected by the gcc sanitizer (ubsan), for instance at the libdrm library:
../amdgpu/amdgpu_cs.c:937:26: runtime error: member access within misaligned address 0x63100001484c for type 'struct drm_amdgpu_cs_chunk_data', which requires 8 byte alignment
0x63100001484c: note: pointer points here
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
^
Fixes: ae7e4d7619 ("amd: rename ring_type --> amd_ip_type and match the kernel enum values")
Signed-off-by: Patrick Lerda <patrick9876@free.fr>
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22920>
Some chips support firmware based mcbp. If supported this means
radeonsi needs to allocate 3 buffers and pass them to the firmware.
From there, the firmware will handle mcbp and register shadowing
on its own so we don't need to insert LOAD packet in the preamble.
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21986>
and don't increment the total sum after the first rejection
Reviewed-by: Mihai Preda <mhpreda@gmail.com>
Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17968>
buffer_indices_hashlist is only used by the current
amdgpu_cs_context (= amdgpu_cs.csc).
So store a single 16k int array instead of 2, and switch
the owner when flushing the cs.
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11010>
This decreases the CPU time percentage of amdgpu_cs_add_buffer by 50%
on Ryzen 3900X.
We don't need to call amdgpu_bo_is_referenced_by_any_cs
in amdgpu_bo_can_reclaim. The reclaim function is only called for buffers
that have 0 references.
The only downside is that amdgpu_bo_is_referenced_by_cs might be slower
in some very rare cases. Overall the driver overhead is better.
Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Reviewed-by: Zoltán Böszörményi <zboszor@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8849>
It's straightforward except that the amdgpu winsys had to be cleaned up
to allow this.
radeon_cmdbuf is inlined and optionally the winsys can save the pointer
to it. radeon_cmdbuf::priv points to the winsys cs structure.
Acked-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7907>
Instead of exposing a cs_set_secure() callback that always needs a call
to si_flush_gfx_cs before a switch, this commit introduces a new
flag to switch between secure and non-secure on submissions.
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6049>
- Execute cs_preamble_state as a separate IB with different flags.
- Set the PREEMPT flag for the main IB.
Acked-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5798>
Use it instead of the libdrm provided amdgpu_drm.h header. I used
the kernel revision from the README to get the header so the
header versions should be consistent.
Tested by removing /usr/include/libdrm/amdgpu_drm.h from my dev-machine.
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4749>
It extends pipe_screen / radeon_winsys and references amdgpu_winsys.
Multiple amdgpu_screen_winsys instances may reference the same
amdgpu_winsys instance, which corresponds to an amdgpu_device_handle.
The purpose of amdgpu_screen_winsys is to keep a duplicate of the DRM
file descriptor passed to amdgpu_winsys_create, which will be needed
in the next change.
v2:
* Add comment in amdgpu_winsys_unref explaining why it always returns
true (Marek Olšák)
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Tested-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Add the ability to signal a syncobj when a cs completes execution.
v2: corresponding changes for gallium fence->semaphore rename
v3: s/semaphore/fence for pipe objects
Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
The idea is to fix the following interleaving of operations
that can arise from deferred fences:
Thread 1 / Context 1 Thread 2 / Context 2
-------------------- --------------------
f = deferred flush
<------- application-side synchronization ------->
fence_server_sync(f)
...
flush()
flush()
We will now stall in fence_server_sync until the flush of context 1
has completed.
This scenario was unlikely to occur previously, because applications
seem to be doing
Thread 1 / Context 1 Thread 2 / Context 2
-------------------- --------------------
f = glFenceSync()
glFlush()
<------- application-side synchronization ------->
glWaitSync(f)
... and indeed they probably *have* to use this ordering to avoid
deadlocks in the GLX model, where all GL operations conceptually
go through a single connection to the X server. However, it's less
clear whether applications have to do this with other WSI (i.e. EGL).
Besides, even this sequence of GL commands can be translated into
the Gallium-level sequence outlined above when Gallium threading
and asynchronous flushes are used. So it makes sense to be more
robust.
As a side effect, we no longer busy-wait on submission_in_progress.
We won't enable asynchronous flushes on radeon, but add a
cs_add_fence_dependency stub anyway to document the potential
issue.
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
It's inaccurate. Instead, see the copyright and use "git log" and
"git blame" to know the authorship.
Acked-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
We have come to the conclusion that it doesn't improve performance.
Tested-by: Dieter Nützel <Dieter@nuetzel-hh.de>
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
First this happens:
1) amdgpu_cs_flush (lock bo_fence_lock)
-> amdgpu_add_fence_dependency
-> os_wait_until_zero (wait for submission_in_progress) - WAITING
2) amdgpu_bo_create
-> pb_cache_reclaim_buffer (lock pb_cache::mutex)
-> pb_cache_is_buffer_compat
-> amdgpu_bo_wait (lock bo_fence_lock) - WAITING
So both bo_fence_lock and pb_cache::mutex are held. amdgpu_bo_create can't
continue. amdgpu_cs_flush is waiting for the CS ioctl to finish the job,
but the CS ioctl is trying to release a buffer:
3) amdgpu_cs_submit_ib (CS thread - job entrypoint)
-> amdgpu_cs_context_cleanup
-> pb_reference
-> pb_destroy
-> amdgpu_bo_destroy_or_cache
-> pb_cache_add_buffer (lock pb_cache::mutex) - DEADLOCK
The simple solution is not to wait for submission_in_progress, which we
need in order to create the list of dependencies for the CS ioctl. Instead
of building the list of dependencies as a direct input to the CS ioctl,
build the list of dependencies as a list of fences, and make the final list
of dependencies in the CS thread itself.
Therefore, amdgpu_cs_flush doesn't have to wait and can continue.
Then, amdgpu_bo_create can continue and return. And then amdgpu_cs_submit_ib
can continue.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101294
Cc: 17.1 <mesa-stable@lists.freedesktop.org>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
... and implement the corresponding fence handling.
v2:
- add missing bit in amdgpu_bo_is_referenced_by_cs_with_usage
- remove pipe_mutex_*
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
This probably has only minor performance effects, but it simplifies some
subsequent code slightly.
Ideally, it could also be used to simplify the handling of slab buffers
in the same way, but unfortunately that's not possible as long as we need
indices for relocations.
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
