On 18.05.2016 13:23, Marek Olšák wrote:
,On Wed, May 18, 2016 at 7:48 PM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
On 18.05.2016 11:58, Marek Olšák wrote:
On Sat, May 7, 2016 at 5:12 PM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
Looks good to me, just two remarks below...
On 06.05.2016 13:31, Marek Olšák wrote:
From: Marek Olšák <marek.ol...@amd.com>
Ported from the initial amdgpu winsys from the private AMD branch.
The thread creates the buffer list, submits IBs, and cleans up
the submission context, which can also destroy buffers.
3-5% reduction in CPU overhead is expected for apps submitting a lot
of IBs per frame. This is most visible with DMA IBs.
---
src/gallium/winsys/amdgpu/drm/amdgpu_bo.c | 26 ++-
src/gallium/winsys/amdgpu/drm/amdgpu_bo.h | 4 +
src/gallium/winsys/amdgpu/drm/amdgpu_cs.c | 311
+++++++++++++++++---------
src/gallium/winsys/amdgpu/drm/amdgpu_cs.h | 52 +++--
src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c | 61 +++++
src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h | 9 +
6 files changed, 333 insertions(+), 130 deletions(-)
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
index 37a41c0..ec5fa6a 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
@@ -43,8 +43,21 @@ static bool amdgpu_bo_wait(struct pb_buffer *_buf,
uint64_t timeout,
{
struct amdgpu_winsys_bo *bo = amdgpu_winsys_bo(_buf);
struct amdgpu_winsys *ws = bo->ws;
+ int64_t abs_timeout;
int i;
+ if (timeout == 0) {
+ if (p_atomic_read(&bo->num_active_ioctls))
+ return false;
+
+ } else {
+ abs_timeout = os_time_get_absolute_timeout(timeout);
+
+ /* Wait if any ioctl is being submitted with this buffer. */
+ if (!os_wait_until_zero_abs_timeout(&bo->num_active_ioctls,
abs_timeout))
+ return false;
+ }
I'd suggest to do the cs_sync_flush here instead of below - there is less
action at a distance, and some additional code paths end up covered by a
flush as well.
Unfortunately, amdgpu_bo_wait is exposed via the winsys interface and
doesn't accept a CS. We could extend it to accept a CS or two, but
that would mean adding most of what r600_buffer_map_sync_with_rings is
doing. It would be a bigger cleanup and I think it should be done as a
separate patch if we wanted to go down that road.
Okay, fair enough. Let's keep an eye out for whether some use cases get into
busy waits, just in case.
The amdgpu_cs_sync_flush should be added to the other branch of the
PIPE_TRANSFER_WRITE check in amdgpu_bo_map though, right?
Yes, good catch.
Did you mean this with the semaphore?
https://cgit.freedesktop.org/~mareko/mesa/commit/?h=tmp&id=401abd01d5f44430df71de9999b74b6e78a0eac2
Yes, that looks good to me. With that change, the patch is
Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com>
Marek
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
