[PATCH] drm/radeon: restore hardware polling in fence_is_signaled to fix performance regression

[PATCH] drm/radeon: restore hardware polling in fence_is_signaled to fix performance regression

[2564278112]

From: Wang Jiang <jiangwang@kylinos.cn>

Commit 9eb00b5f5697b ("drm/radeon: delete radeon_fence_process in
is_signaled, no deadlock") removed the hardware polling from
radeon_fence_is_signaled() to fix a self-deadlock caused by
wake_up_all(&rdev->fence_queue) being called with the fence queue
lock held.

However, removing the polling entirely causes significant performance
regression (e.g. glxgears FPS drop) because the fence signaled check
becomes purely passive — it only reads the cached last_seq without
probing the GPU, so completed GPU work is not detected in time,
causing unnecessary CPU stalls in sync-heavy workloads.

Fix this by calling radeon_fence_activity() directly instead of
radeon_fence_process(). radeon_fence_activity() reads the hardware
fence counter and updates last_seq via atomic ops without calling
wake_up_all(), thus avoiding the deadlock while restoring timely
fence detection.

Fixes: 9eb00b5f5697b ("drm/radeon: delete radeon_fence_process in is_signaled, no deadlock")
Signed-off-by: Wang Jiang <jiangwang@kylinos.cn>
---
 drivers/gpu/drm/radeon/radeon_fence.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 02a40e4750c7..299865569252 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -360,6 +360,13 @@ static bool radeon_fence_is_signaled(struct dma_fence *f)
 	if (atomic64_read(&rdev->fence_drv[ring].last_seq) >= seq)
 		return true;
 
+	if (down_read_trylock(&rdev->exclusive_lock)) {
+		radeon_fence_activity(rdev, ring);
+		up_read(&rdev->exclusive_lock);
+
+		if (atomic64_read(&rdev->fence_drv[ring].last_seq) >= seq)
+			return true;
+	}
 	return false;
 }
 
-- 
2.25.1

Re: [PATCH] drm/radeon: restore hardware polling in fence_is_signaled to fix performance regression

[Christian König]

On 7/29/26 12:26, 2564278112@qq.com wrote:
> From: Wang Jiang <jiangwang@kylinos.cn>
> 
> Commit 9eb00b5f5697b ("drm/radeon: delete radeon_fence_process in
> is_signaled, no deadlock") removed the hardware polling from
> radeon_fence_is_signaled() to fix a self-deadlock caused by
> wake_up_all(&rdev->fence_queue) being called with the fence queue
> lock held.
> 
> However, removing the polling entirely causes significant performance
> regression (e.g. glxgears FPS drop) because the fence signaled check
> becomes purely passive — it only reads the cached last_seq without
> probing the GPU, so completed GPU work is not detected in time,
> causing unnecessary CPU stalls in sync-heavy workloads.
> 
> Fix this by calling radeon_fence_activity() directly instead of
> radeon_fence_process(). radeon_fence_activity() reads the hardware
> fence counter and updates last_seq via atomic ops without calling
> wake_up_all(), thus avoiding the deadlock while restoring timely
> fence detection.

Yeah I already feared that removing this in commit 9eb00b5f5697b could cause issues.

Adding Philip and Danilo since we recently had a discussion about the necessity of this.
> Fixes: 9eb00b5f5697b ("drm/radeon: delete radeon_fence_process in is_signaled, no deadlock")
> Signed-off-by: Wang Jiang <jiangwang@kylinos.cn>

As far as I remember the radeon code the solution should work, but I don't have time to double check.

Acked-by: Christian König <christian.koenig@amd.com>

Thanks,
Christian.

> ---
>  drivers/gpu/drm/radeon/radeon_fence.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
> index 02a40e4750c7..299865569252 100644
> --- a/drivers/gpu/drm/radeon/radeon_fence.c
> +++ b/drivers/gpu/drm/radeon/radeon_fence.c
> @@ -360,6 +360,13 @@ static bool radeon_fence_is_signaled(struct dma_fence *f)
>         if (atomic64_read(&rdev->fence_drv[ring].last_seq) >= seq)
>                 return true;
> 
> +       if (down_read_trylock(&rdev->exclusive_lock)) {
> +               radeon_fence_activity(rdev, ring);
> +               up_read(&rdev->exclusive_lock);
> +
> +               if (atomic64_read(&rdev->fence_drv[ring].last_seq) >= seq)
> +                       return true;
> +       }
>         return false;
>  }
> 
> --
> 2.25.1
> 

Re: [PATCH] drm/radeon: restore hardware polling in fence_is_signaled to fix performance regression

[Philipp Stanner]

On Wed, 2026-08-05 at 10:34 +0200, Christian König wrote:
> On 7/29/26 12:26, 2564278112@qq.com wrote:
> > From: Wang Jiang <jiangwang@kylinos.cn>
> > 
> > Commit 9eb00b5f5697b ("drm/radeon: delete radeon_fence_process in
> > is_signaled, no deadlock") removed the hardware polling from
> > radeon_fence_is_signaled() to fix a self-deadlock caused by
> > wake_up_all(&rdev->fence_queue) being called with the fence queue
> > lock held.
> > 
> > However, removing the polling entirely causes significant performance
> > regression (e.g. glxgears FPS drop) because the fence signaled check
> > becomes purely passive — it only reads the cached last_seq without
> > probing the GPU, so completed GPU work is not detected in time,
> > causing unnecessary CPU stalls in sync-heavy workloads.
> > 
> > Fix this by calling radeon_fence_activity() directly instead of
> > radeon_fence_process(). radeon_fence_activity() reads the hardware
> > fence counter and updates last_seq via atomic ops without calling
> > wake_up_all(), thus avoiding the deadlock while restoring timely
> > fence detection.
> 
> Yeah I already feared that removing this in commit 9eb00b5f5697b
> could cause issues.
> 
> Adding Philip and Danilo since we recently had a discussion about the
> necessity of this.

necessity of avoiding deadlocks with the callbacks somehow, or the
necessity for opportunistic signaling?


What we were wondering about was

a) who can even reach the hardware_fence when the gpu scheduler's fence
is in between and

b) why it might be a problem if we lock in dma_fence_is_signaled(),
because it would mean that someone (e.g. in userspace) is busy-looping
on the fence.


Since radeon does not use drm_sched, it seems to be one example for a),
I can see that.


Regards
P.

Re: [PATCH] drm/radeon: restore hardware polling in fence_is_signaled to fix performance regression

[Alex Deucher]

Applied.  Thanks!

On Wed, Aug 5, 2026 at 4:53 AM Christian König <christian.koenig@amd.com> wrote:
>
> On 7/29/26 12:26, 2564278112@qq.com wrote:
> > From: Wang Jiang <jiangwang@kylinos.cn>
> >
> > Commit 9eb00b5f5697b ("drm/radeon: delete radeon_fence_process in
> > is_signaled, no deadlock") removed the hardware polling from
> > radeon_fence_is_signaled() to fix a self-deadlock caused by
> > wake_up_all(&rdev->fence_queue) being called with the fence queue
> > lock held.
> >
> > However, removing the polling entirely causes significant performance
> > regression (e.g. glxgears FPS drop) because the fence signaled check
> > becomes purely passive — it only reads the cached last_seq without
> > probing the GPU, so completed GPU work is not detected in time,
> > causing unnecessary CPU stalls in sync-heavy workloads.
> >
> > Fix this by calling radeon_fence_activity() directly instead of
> > radeon_fence_process(). radeon_fence_activity() reads the hardware
> > fence counter and updates last_seq via atomic ops without calling
> > wake_up_all(), thus avoiding the deadlock while restoring timely
> > fence detection.
>
> Yeah I already feared that removing this in commit 9eb00b5f5697b could cause issues.
>
> Adding Philip and Danilo since we recently had a discussion about the necessity of this.
> > Fixes: 9eb00b5f5697b ("drm/radeon: delete radeon_fence_process in is_signaled, no deadlock")
> > Signed-off-by: Wang Jiang <jiangwang@kylinos.cn>
>
> As far as I remember the radeon code the solution should work, but I don't have time to double check.
>
> Acked-by: Christian König <christian.koenig@amd.com>
>
> Thanks,
> Christian.
>
> > ---
> >  drivers/gpu/drm/radeon/radeon_fence.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
> > index 02a40e4750c7..299865569252 100644
> > --- a/drivers/gpu/drm/radeon/radeon_fence.c
> > +++ b/drivers/gpu/drm/radeon/radeon_fence.c
> > @@ -360,6 +360,13 @@ static bool radeon_fence_is_signaled(struct dma_fence *f)
> >         if (atomic64_read(&rdev->fence_drv[ring].last_seq) >= seq)
> >                 return true;
> >
> > +       if (down_read_trylock(&rdev->exclusive_lock)) {
> > +               radeon_fence_activity(rdev, ring);
> > +               up_read(&rdev->exclusive_lock);
> > +
> > +               if (atomic64_read(&rdev->fence_drv[ring].last_seq) >= seq)
> > +                       return true;
> > +       }
> >         return false;
> >  }
> >
> > --
> > 2.25.1
> >
>