Thanks, if we don't see any impact on performance on Pi5, this is: Reviewed-by: Iago Toral Quiroga <[email protected]>
El mié, 15-07-2026 a las 10:46 -0300, Maíra Canal escribió: > 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); > > > } > > > > > > >
