Re: [PATCH v12 11/13] drm/xe: Force flush system memory AuxCCS framebuffers before scan out

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Sep 24, 2025 at 02:09:42PM +0100, Tvrtko Ursulin wrote:
> 
> On 23/09/2025 15:52, Ville Syrjälä wrote:
> > On Tue, Sep 23, 2025 at 03:40:55PM +0100, Tvrtko Ursulin wrote:
> >>
> >> On 23/09/2025 14:20, Ville Syrjälä wrote:
> >>> On Tue, Sep 23, 2025 at 01:25:58PM +0100, Tvrtko Ursulin wrote:
> >>>>
> >>>> On 23/09/2025 13:01, Ville Syrjälä wrote:
> >>>>> On Tue, Sep 23, 2025 at 11:48:59AM +0100, Tvrtko Ursulin wrote:
> >>>>>>
> >>>>>> On 23/09/2025 11:19, Ville Syrjälä wrote:
> >>>>>>> On Tue, Sep 23, 2025 at 11:08:04AM +0100, Tvrtko Ursulin wrote:
> >>>>>>>> Even though frame buffer objects are created as write-combined, in
> >>>>>>>> practice, on top of all the ring buffer flushing, an additional clflush
> >>>>>>>> seems to be needed before display engine can coherently scan out the
> >>>>>>>> AuxCCS compressed data without transient artifacts.
> >>>>>>>>
> >>>>>>>> If for comparison we look at how i915 handles things (where AuxCCS works
> >>>>>>>> fine), as it happens it has this same clflush before a frame buffer is
> >>>>>>>> pinned for display for the first time, courtesy the dynamic tracking of
> >>>>>>>> the buffer cache mode and setting the latter to uncached before handing
> >>>>>>>> to display.
> >>>>>>>>
> >>>>>>>> Since xe considers the buffer object caching mode as static we can
> >>>>>>>> implement the same approach by adding a flag telling us if the buffer
> >>>>>>>> was ever pinned for display and flush on the first pin. Subsequent re-pins
> >>>>>>>> will not repeat the clflush but so far I have not observed any glitching
> >>>>>>>> after the first pin.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> >>>>>>>> ---
> >>>>>>>>      drivers/gpu/drm/xe/display/xe_fb_pin.c | 14 +++++++++++++-
> >>>>>>>>      drivers/gpu/drm/xe/xe_bo_types.h       | 14 +++++++++-----
> >>>>>>>>      2 files changed, 22 insertions(+), 6 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> >>>>>>>> index d8aa23b8cf14..f247c0da6b9e 100644
> >>>>>>>> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
> >>>>>>>> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> >>>>>>>> @@ -382,6 +382,7 @@ static struct i915_vma *__xe_pin_fb_vma(const struct intel_framebuffer *fb,
> >>>>>>>>      	struct xe_bo *bo = gem_to_xe_bo(obj);
> >>>>>>>>      	struct xe_validation_ctx ctx;
> >>>>>>>>      	struct drm_exec exec;
> >>>>>>>> +	bool first_pin;
> >>>>>>>>      	int ret = 0;
> >>>>>>>>      
> >>>>>>>>      	if (!vma)
> >>>>>>>> @@ -422,8 +423,11 @@ static struct i915_vma *__xe_pin_fb_vma(const struct intel_framebuffer *fb,
> >>>>>>>>      			ret = xe_bo_validate(bo, NULL, true, &exec);
> >>>>>>>>      		drm_exec_retry_on_contention(&exec);
> >>>>>>>>      		xe_validation_retry_on_oom(&ctx, &ret);
> >>>>>>>> -		if (!ret)
> >>>>>>>> +		if (!ret) {
> >>>>>>>>      			ttm_bo_pin(&bo->ttm);
> >>>>>>>> +			first_pin = !bo->display_pin;
> >>>>>>>> +			bo->display_pin = true;
> >>>>>>>> +		}
> >>>>>>>>      	}
> >>>>>>>>      	if (ret)
> >>>>>>>>      		goto err;
> >>>>>>>> @@ -436,6 +440,14 @@ static struct i915_vma *__xe_pin_fb_vma(const struct intel_framebuffer *fb,
> >>>>>>>>      	if (ret)
> >>>>>>>>      		goto err_unpin;
> >>>>>>>>      
> >>>>>>>> +	/*
> >>>>>>>> +	 * Force flush frame buffer data for non-coherent display access when
> >>>>>>>> +	 * AuxCCS formats are used.
> >>>>>>>> +	 */
> >>>>>>>> +	if (first_pin && !xe_bo_is_vram(bo) && !xe_bo_is_stolen(bo) &&
> >>>>>>>> +	    intel_fb_is_ccs_modifier(fb->base.modifier))
> >>>>>>>> +		drm_clflush_sg(xe_bo_sg(bo));
> >>>>>>>
> >>>>>>> You still haven't found the actual bug that causes the dirty cache?
> >>>>>>
> >>>>>> Sadly no. I cross referenced everything numerous times, including
> >>>>>> workarounds, tried pretty much 1:1 i915 vs xe ring buffer programming,
> >>>>>> but this extra flush always remains required. I also heard that how
> >>>>>> flushing of the aux metadata works isn't documented anywhere and i915
> >>>>>> does have this flush, by design or accident I don't know.
> >>>>>
> >>>>> i915 has the flush for the *whole* bo because it started out as
> >>>>> cached. As soon as it undergoes a cached->uncached change (either
> >>>>> due to set_caching ioctl or becoming a display scanout buffer) we
> >>>>> clflush it and switch the GPU page tables from WB to UC. After
> >>>>> that the bo stays uncached, and will need no further clflushes,
> >>>>> assuming that everyone follows the rules:
> >>>>>
> >>>>> - CPU accesses via a WB mapping to an uncached bo will explicitly
> >>>>>      clflush before (invalidate) and after (write-back) the access
> >>>>> - userspace always sets the GPU to use use "consult the PTE" MOCS
> >>>>>      setting for any potential scanout buffer. That way the GPU will
> >>>>>      use WB as long as the bo is cached, and once it becomes
> >>>>>      uncached the GPU also switches to UC accesses
> >>>>>
> >>>>> With xe I presume the BO should already start out as UC/WC with
> >>>>> clean caches, and nothing should be dirtying the caches unless
> >>>>> there is a real bug somewhere.
> >>>>
> >>>> Correct, it is all WC and MOCS are correctly set as WC.
> >>>>
> >>>> For example for the media compression flavour that was broken and needed
> >>>> fixing in b412b144685f ("lib/intel/veboxcopy: Respect buffer MOCS
> >>>> index"). (For the render compression flavour MOCS was already correct.)
> >>>> But MOCS is not enough on its own for some reason.
> >>>>
> >>>> I also looked into the rendercopy surface state programming and that too
> >>>> looks okay. In fact it is possible to see how using the wrong MOCS makes
> >>>> things worse. But it also appears the surface state MOCS only applies to
> >>>> the main surface, while the aux state is the one which appears to
> >>>> contain cached/unflushed data.
> >>>
> >>> Are you saying that if you start scanning out a compressed buffer,
> >>> then clean the caches, and then do frontbuffer rendering you get
> >>> more cache dirt?
> >>
> >> Render with the GPU or CPU? Do you know of any tests or userspaces which
> >> do that?
> > 
> > On the GPU.
> > 
> > Can't think of anything nice off the top of my head.
> > 
> > I'd probably just write a quick igt:
> > 1. create a compressed fb
> > 2. flip to it
> > 3. manually clflush the whole thing to be sure it's clean
> > 4. rendercopy some junk around, and keep an eye out for cache dirt
> 
> Looks the same dirt on i915 and xe, which would support the theory that 
> MOCS settings do not apply to the aux surface.
> 
> If you look at these videos frame by frame you can see it:
> 
> https://people.igalia.com/tursulin/i915-ccs-cache-dirt.MOV
> https://people.igalia.com/tursulin/xe-ccs-cache-dirt.MOV
> 

That doesn't really look like cache dirt since it doesn't linger.
So I'd say that is more tearing than anything. Tearing can look
rather weird with tiled/compressed buffers.

To raelly see cache dirt you should tweak the test to
select a wb mocs.

Oh, and disable CPU c-states. Turns out deep c-states cause extra
cache flushes all the time which will hide the dirt in short order.
Without those extra flushes cache dirt will linger for a long time
on screen.

And also enable_psr=0 + enable_fbc=0 to make sure the display
engine is really fetching from the memory we just wrote.

-- 
Ville Syrjälä
Intel