Re: [PATCH 2/5] drm/amdgpu/sdma4: set wptr shadow atomically

[Felix Kuehling <felix.kuehling-5C7GfCeVMHo@public.gmane.org>]

On 17-07-27 03:46 PM, Alex Deucher wrote:
> No functional change until wptr polling uses this
> location (future patch).
>
> Cc: Frank Min <Frank.Min@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 7cb5320..9392799 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -301,8 +301,8 @@ static void sdma_v4_0_ring_set_wptr(struct amdgpu_ring *ring)
>  				lower_32_bits(ring->wptr << 2),
>  				upper_32_bits(ring->wptr << 2));
>  		/* XXX check if swapping is necessary on BE */
> -		adev->wb.wb[ring->wptr_offs] = lower_32_bits(ring->wptr << 2);
> -		adev->wb.wb[ring->wptr_offs + 1] = upper_32_bits(ring->wptr << 2);
> +		atomic64_set((atomic64_t *)&adev->wb.wb[ring->wptr_offs],
> +			     (ring->wptr << 2));

This looks a bit awkward. Would "writeq" do the job? That's what we use
for updating kernel queue doorbells in KFD.

The generic implementation of atomic64_set uses a spinlock. But that
doesn't do anything for protecting against concurrent access by the GPU
hardware.

Regards,
  Felix

>  		DRM_DEBUG("calling WDOORBELL64(0x%08x, 0x%016llx)\n",
>  				ring->doorbell_index, ring->wptr << 2);
>  

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx