Re: [PATCH] drm/msm/a5xx: Write CP_RB_WPTR under the ring's preempt_lock

[Alexey Minnekhanov <alexeymin@minlexx.ru>]

On 01.11.2026 05:30, Fabricio Akio via B4 Relay wrote:
> From: Fabricio Akio <fabricioakio@gmail.com>
> 
> CP_RB_WPTR tells the CP how far the ring has been filled. Two functions
> write it: a5xx_flush(), after a submit is added to the ring, and
> update_wptr(), which a5xx_preempt_trigger() calls from the IRQ handler
> each time a submit completes, whenever there is more than one ring,
> even when no ring switch follows. Both read the ring's wptr holding
> ring->preempt_lock, but write the register after releasing it, so the
> two writes can land in the wrong order:
> 
>    CPU A, IRQ:   update_wptr() reads wptr N
>    CPU B, flush: a5xx_flush() reads wptr N+1, writes N+1
>    CPU A, IRQ:   update_wptr() writes N
> 
> CP_RB_WPTR then points behind the last submit. If the CP has already
> read past N, it treats the ring as nearly full and executes the
> commands left over from its previous pass through the ring. This shows
> up as CP opcode errors, and the old submits write old fences, so the
> fence in memptrs goes backwards. Recovery then finds no submit to
> blame and does not reset the GPU, which stays hung until reboot.
> 
> Write the register before releasing the lock in both places, so the
> last value written is always the newest, as a6xx_flush() does.
> 
> Fixes: b1fc2839d2f9 ("drm/msm: Implement preemption for A5XX targets")
> Signed-off-by: Fabricio Akio <fabricioakio@gmail.com>
> ---
> Tested on A508 (SDM630), with piglit's texelfetch tests run in
> parallel: CP opcode errors in each of seven one-minute runs, the
> seventh ending in a hang that never recovers; with this, none in 25.
> 
> The downstream driver writes the register holding the ring's
> preempt_lock in both of its places:
> https://github.com/LineageOS/android_kernel_motorola_msm8998/blob/ce9ea8f125c7302ae9b2fec84f23a69271e957c3/drivers/gpu/msm/adreno_ringbuffer.c#L104
> https://github.com/LineageOS/android_kernel_motorola_msm8998/blob/ce9ea8f125c7302ae9b2fec84f23a69271e957c3/drivers/gpu/msm/adreno_a5xx_preempt.c#L25
> ---
>   drivers/gpu/drm/msm/adreno/a5xx_gpu.c     | 10 +++++++---
>   drivers/gpu/drm/msm/adreno/a5xx_preempt.c |  3 +--
>   2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 16c7335c4b1f..63b313328979 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -53,14 +53,18 @@ void a5xx_flush(struct msm_gpu *gpu, struct msm_ringbuffer *ring,
>   	/* Make sure to wrap wptr if we need to */
>   	wptr = get_wptr(ring);
>   
> -	spin_unlock_irqrestore(&ring->preempt_lock, flags);
> -
>   	/* Make sure everything is posted before making a decision */
>   	mb();
>   
> -	/* Update HW if this is the current ring and we are not in preempt */
> +	/*
> +	 * Update HW if this is the current ring and we are not in preempt.
> +	 * Under the lock, so that update_wptr() cannot write an older wptr
> +	 * after this one.
> +	 */
>   	if (a5xx_gpu->cur_ring == ring && !a5xx_in_preempt(a5xx_gpu))
>   		gpu_write(gpu, REG_A5XX_CP_RB_WPTR, wptr);
> +
> +	spin_unlock_irqrestore(&ring->preempt_lock, flags);
>   }
>   
>   static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct msm_gem_submit *submit)
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> index e4924b5e1c48..0deac58bc22e 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> @@ -47,9 +47,8 @@ static inline void update_wptr(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
>   
>   	spin_lock_irqsave(&ring->preempt_lock, flags);
>   	wptr = get_wptr(ring);
> -	spin_unlock_irqrestore(&ring->preempt_lock, flags);
> -
>   	gpu_write(gpu, REG_A5XX_CP_RB_WPTR, wptr);
> +	spin_unlock_irqrestore(&ring->preempt_lock, flags);
>   }
>   
>   /* Return the highest priority ringbuffer with something in it */
> 
> ---
> base-commit: a39c950806213f6861d00f90729acaaa21f2d95a
> change-id: 20261031-a5xx-wptr-lock-55b151332fcf
> 
> Best regards,
> --
> Fabricio Akio <fabricioakio@gmail.com>

I've been using this patch for a few days on various sdm630/636/660
devices with a508/509/512 and so far haven't seen any GPU hangs.

It's hard to prove definitively that hangs are gone, due to the
random nature of the hang, but at least it didn't make anything
worse.

Tested-by: Alexey Minnekhanov <alexeymin@minlexx.ru>

--
Regards,
Alexey Minnekhanov