Hi Iago,
On 15/07/26 03:16, Iago Toral wrote:
Hi Maíra,
has this change fixed anything?
Not real bug, just the theoretical race-condition between
v3d_clean_caches() and v3d_flush_l2t().
Also, I believe that GFXH-1897 is exclusive to Pi4. Have you tested if
this change has any visible performance impact on Pi5?
I'll test it, however I don't imagine it would have a impact, as the
race condition should be rare enough. We would need a CACHE_CLEAN job
running simultaneously to v3d_invalidate_caches().
One thing I believe it might be possible is to skip a flush if a flush
is already happening, but it would be a micro-optimization.
Best regards,
- Maíra
Iago
El mar, 07-07-2026 a las 17:05 -0300, Maíra Canal escribió:
v3d_clean_caches() and v3d_flush_l2t() both write the single L2TCACTL
register and poll its status bits. The mutex cache_clean_lock exists
to
serialize them, but v3d_clean_caches() only took the lock around its
final
FLM_CLEAN write.
These functions run concurrently: v3d_flush_l2t() is issued from the
BIN/RENDER/CSD invalidate path while v3d_clean_caches() runs from the
CACHE_CLEAN queue, and each queue's scheduler uses its own ordered
workqueue, so their run_job callbacks execute in parallel.
Because clean locked only its final write, a concurrent flush can
write
L2TCACTL during clean's unlocked phase. Both use non read-modify-
write
writes to the one register, so whichever lands last wins: clean's
TMUWCF
write can land on the flush's in-flight L2TFLS invalidate, triggering
the
GFXH-1897 hazard of writing L2TCACTL while a flush is pending.
Hold cache_clean_lock across the entire L2TCACTL access sequence so
it
is fully mutually exclusive with v3d_flush_l2t(), which already takes
the
lock around its own write.
Cc: [email protected]
Fixes: abf888b03a98 ("drm/v3d: Wait for pending L2T flush before
cleaning caches")
Signed-off-by: Maíra Canal <[email protected]>
---
drivers/gpu/drm/v3d/v3d_gem.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c
b/drivers/gpu/drm/v3d/v3d_gem.c
index c43d9af41374..e597b6fd47c4 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -204,6 +204,8 @@ v3d_clean_caches(struct v3d_dev *v3d)
struct drm_device *dev = &v3d->drm;
int core = 0;
+ guard(mutex)(&v3d->cache_clean_lock);
+
trace_v3d_cache_clean_begin(dev);
/* GFXH-1897: Ensure pending flushes complete before writing
L2TCACTL */
@@ -220,7 +222,6 @@ v3d_clean_caches(struct v3d_dev *v3d)
drm_err(dev, "Timeout waiting for TMU write combiner
flush\n");
}
- mutex_lock(&v3d->cache_clean_lock);
V3D_CORE_WRITE(core, V3D_CTL_L2TCACTL,
V3D_L2TCACTL_L2TFLS |
V3D_SET_FIELD(V3D_L2TCACTL_FLM_CLEAN,
V3D_L2TCACTL_FLM));
@@ -230,8 +231,6 @@ v3d_clean_caches(struct v3d_dev *v3d)
drm_err(dev, "Timeout waiting for L2T clean\n");
}
- mutex_unlock(&v3d->cache_clean_lock);
-
trace_v3d_cache_clean_end(dev);
}
