On Fri, Jun 29, 2018 at 7:30 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> The buffer object backing the user fence is reserved using the non-user
> fence, i.e., as soon as the non-user fence is signaled, the user fence
> buffer object can be moved or even destroyed.
>
> Therefore, emit the user fence first.
>
> Both fences have the same cache invalidation behavior, so this should
> have no user-visible effect.
>
> Signed-off-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
> ---
> There is one aspect to this change that I'm a bit unsure about: what does
> insert_end do? It's only used by UVD & friends, and since those rings
> don't use user fences I guess this patch doesn't really change anything
> for them. And having the insert_end between those fences always looked
> a bit suspicious...
It should be fine I think. some MM engines just need start and end
markers around a work batch. They don't support user fences so it
shouldn't be an issue. Patch looks good to me.
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 829c4d2a33b9..8117b8c2113e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -227,38 +227,38 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
> if (ring->funcs->emit_tmz)
> amdgpu_ring_emit_tmz(ring, false);
>
> if (ring->funcs->emit_hdp_invalidate
> #ifdef CONFIG_X86_64
> && !(adev->flags & AMD_IS_APU)
> #endif
> )
> amdgpu_ring_emit_hdp_invalidate(ring);
>
> + /* wrap the last IB with fence */
> + if (job && job->uf_addr) {
> + amdgpu_ring_emit_fence(ring, job->uf_addr, job->uf_sequence,
> + AMDGPU_FENCE_FLAG_64BIT);
> + }
> +
> r = amdgpu_fence_emit(ring, f);
> if (r) {
> dev_err(adev->dev, "failed to emit fence (%d)\n", r);
> if (job && job->vmid)
> amdgpu_vmid_reset(adev, ring->funcs->vmhub, job->vmid);
> amdgpu_ring_undo(ring);
> return r;
> }
>
> if (ring->funcs->insert_end)
> ring->funcs->insert_end(ring);
>
> - /* wrap the last IB with fence */
> - if (job && job->uf_addr) {
> - amdgpu_ring_emit_fence(ring, job->uf_addr, job->uf_sequence,
> - AMDGPU_FENCE_FLAG_64BIT);
> - }
> -
> if (patch_offset != ~0 && ring->funcs->patch_cond_exec)
> amdgpu_ring_patch_cond_exec(ring, patch_offset);
>
> ring->current_ctx = fence_ctx;
> if (vm && ring->funcs->emit_switch_buffer)
> amdgpu_ring_emit_switch_buffer(ring);
> amdgpu_ring_commit(ring);
> return 0;
> }
>
> --
> 2.14.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
