[PATCH] drm/amdgpu/sdma5.2: Update wptr registers as well as doorbell

[Friedrich Vock]

On 16.07.24 17:54, Alex Deucher wrote:
> We seem to have a case where SDMA will sometimes miss a doorbell
> if GFX is entering the powergating state when the doorbell comes in.
> To workaround this, we can update the wptr via MMIO, however,
> this is only safe because we disallow gfxoff in begin_ring() for
> SDMA 5.2 and then allow it again in end_ring().
>
> Enable this workaround while we are root causing the issue with
> the HW team.
>
> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/3440
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>

Looks like it works for me.
Tested-by: Friedrich Vock <friedrich.vock at gmx.de>

Is there a particular reason you chose to still go with the doorbell
path plus updating the wptr via MMIO instead of setting
ring->use_doorbell to false? The workaround shipping in SteamOS does
that - if that has some adverse effects or something like that we should
probably stop :)

Thanks,
Friedrich

> ---
>   drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> index 7e475d9b554e..3c37e3cd3cbf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> @@ -225,6 +225,14 @@ static void sdma_v5_2_ring_set_wptr(struct amdgpu_ring *ring)
>   		DRM_DEBUG("calling WDOORBELL64(0x%08x, 0x%016llx)\n",
>   				ring->doorbell_index, ring->wptr << 2);
>   		WDOORBELL64(ring->doorbell_index, ring->wptr << 2);
> +		/* SDMA seems to miss doorbells sometimes when powergating kicks in.
> +		 * Updating the wptr directly will wake it. This is only safe because
> +		 * we disallow gfxoff in begin_use() and then allow it again in end_use().
> +		 */
> +		WREG32(sdma_v5_2_get_reg_offset(adev, ring->me, mmSDMA0_GFX_RB_WPTR),
> +		       lower_32_bits(ring->wptr << 2));
> +		WREG32(sdma_v5_2_get_reg_offset(adev, ring->me, mmSDMA0_GFX_RB_WPTR_HI),
> +		       upper_32_bits(ring->wptr << 2));
>   	} else {
>   		DRM_DEBUG("Not using doorbell -- "
>   				"mmSDMA%i_GFX_RB_WPTR == 0x%08x "
> @@ -1707,6 +1715,10 @@ static void sdma_v5_2_ring_begin_use(struct amdgpu_ring *ring)
>   	 * but it shouldn't hurt for other parts since
>   	 * this GFXOFF will be disallowed anyway when SDMA is
>   	 * active, this just makes it explicit.
> +	 * sdma_v5_2_ring_set_wptr() takes advantage of this
> +	 * to update the wptr because sometimes SDMA seems to miss
> +	 * doorbells when entering PG.  If you remove this, update
> +	 * sdma_v5_2_ring_set_wptr() as well!
>   	 */
>   	amdgpu_gfx_off_ctrl(adev, false);
>   }