Re: [PATCH] drm/xe: Do not preempt fence signaling CS instructions

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Jan 27, 2026 at 12:15:52PM -0800, Matthew Brost wrote:
> On Tue, Jan 27, 2026 at 08:20:36AM +0100, Zbigniew Kempczyński wrote:
> > On Thu, Jan 22, 2026 at 09:44:14AM +0100, Zbigniew Kempczyński wrote:
> > > On Tue, Jan 20, 2026 at 01:11:18PM -0800, Matthew Brost wrote:
> > > > On Mon, Jan 19, 2026 at 01:01:34PM +0100, Zbigniew Kempczyński wrote:
> > > > > On Fri, Jan 16, 2026 at 01:05:01PM -0800, Matthew Brost wrote:
> > > > > > On Fri, Jan 16, 2026 at 10:45:39AM +0100, Zbigniew Kempczyński wrote:
> > > > > > > On Wed, Jan 14, 2026 at 04:45:46PM -0800, Matthew Brost wrote:
> > > > > > > > If a batch buffer is complete, it makes little sense to preempt the
> > > > > > > > fence signaling instructions in the ring, as the largest portion of the
> > > > > > > > work (the batch buffer) is already done and fence signaling consists of
> > > > > > > > only a few instructions. If these instructions are preempted, the GuC
> > > > > > > > would need to perform a context switch just to signal the fence, which
> > > > > > > > is costly and delays fence signaling. Avoid this scenario by disabling
> > > > > > > > preemption immediately after the BB start instruction and re-enabling it
> > > > > > > > after executing the fence signaling instructions.
> > > > > > > > 
> > > > > > > > Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
> > > > > > > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > > > > > > Cc: Carlos Santa <carlos.santa@intel.com>
> > > > > > > > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > > > > > > > ---
> > > > > > > >  drivers/gpu/drm/xe/xe_ring_ops.c | 9 +++++++++
> > > > > > > >  1 file changed, 9 insertions(+)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/gpu/drm/xe/xe_ring_ops.c b/drivers/gpu/drm/xe/xe_ring_ops.c
> > > > > > > > index a1fd99f2d539..cd645ee400b9 100644
> > > > > > > > --- a/drivers/gpu/drm/xe/xe_ring_ops.c
> > > > > > > > +++ b/drivers/gpu/drm/xe/xe_ring_ops.c
> > > > > > > > @@ -282,6 +282,9 @@ static void __emit_job_gen12_simple(struct xe_sched_job *job, struct xe_lrc *lrc
> > > > > > > >  
> > > > > > > >  	i = emit_bb_start(batch_addr, ppgtt_flag, dw, i);
> > > > > > > >  
> > > > > > > > +	/* Don't preempt fence signaling */
> > > > > > > > +	dw[i++] = MI_ARB_ON_OFF | MI_ARB_DISABLE;
> > > > > > > > +
> > > > > > > >  	if (job->user_fence.used) {
> > > > > > > >  		i = emit_flush_dw(dw, i);
> > > > > > > >  		i = emit_store_imm_ppgtt_posted(job->user_fence.addr,
> > > > > > > > @@ -347,6 +350,9 @@ static void __emit_job_gen12_video(struct xe_sched_job *job, struct xe_lrc *lrc,
> > > > > > > >  
> > > > > > > >  	i = emit_bb_start(batch_addr, ppgtt_flag, dw, i);
> > > > > > > >  
> > > > > > > > +	/* Don't preempt fence signaling */
> > > > > > > > +	dw[i++] = MI_ARB_ON_OFF | MI_ARB_DISABLE;
> > > > > > > > +
> > > > > > > >  	if (job->user_fence.used) {
> > > > > > > >  		i = emit_flush_dw(dw, i);
> > > > > > > >  		i = emit_store_imm_ppgtt_posted(job->user_fence.addr,
> > > > > > > > @@ -399,6 +405,9 @@ static void __emit_job_gen12_render_compute(struct xe_sched_job *job,
> > > > > > > >  
> > > > > > > >  	i = emit_bb_start(batch_addr, ppgtt_flag, dw, i);
> > > > > > > >  
> > > > > > > > +	/* Don't preempt fence signaling */
> > > > > > > > +	dw[i++] = MI_ARB_ON_OFF | MI_ARB_DISABLE;
> > > > > > > > +
> > > > > > > 
> > > > > > > IGT tests which calls compute-walker, then bbe are asynchronous (don't
> > > > > > > wait for completion, pipe-control is necessary to wait on
> > > > > > > compute-walker).
> > > > > > > 
> > > > > > 
> > > > > > This asynchronous behavior may explain things. Is this a common use
> > > > > > case?
> > > > > 
> > > > > Compute runtime if I'm not wrong uses pipe-control explicitely. IGT are
> > > > > not doing this relying on kmd.
> > > > > 
> > > > 
> > > > It would be good confirm this.
> > > > 
> > > > > > 
> > > > > > Also do you know if render engines have similar asynchronous behaviors
> > > > > > or is this specific to compute engines?
> > > > > 
> > > > > I don't know, I think Mesa folks may know the answer.
> > > > >
> > > > 
> > > > Let me ask around about this.
> > > >  
> > > > > > 
> > > > > > Lastly, the i915 disables preemption on both render / compute engines
> > > > > > immediately after the BB before emitting the pipe control. Is this async
> > > > > > behavior a new few feature in Xe2 parts which only the Xe driver
> > > > > > supports? This might explain why the i915 works and Xe does not.
> > 
> > We don't have platforms which supports WMTP in i915 driver. This imo explains why
> > there're no issues observed there. I mean regardless preemption state
> > disabled/enabled all instructions soon or later completes. 
> > 
> > > > > 
> > > > > Test exercises WMTP and this is supported starting at Xe2+. Probably
> > > > > what test is doing has a meaning in this case. First compute-walker
> > > > > submits kernel which loops until it will observe some memory write.
> > > > > Second job executes compute-walker with kernel which does some quick job.
> > > > > But first occupies all EU's so second job can be preempted only when
> > > > > preemption occurs and SIP will be executed. So if we disable preemption
> > > > > immediately we submit compute-walker I think we have no change to enter
> > > > > SIP and switch. Even if I add pipe-control to batch level according
> > > > > to Daniele comment job it is still preemptable and we move pipe-control
> > > > > location from kmd -> batch level..
> > > > 
> > > > Does the test work with a pipe control in the batch? That would be a
> > > > good data point - if let me know how I can change the test I can test
> > > > out too.
> > > 
> > > I'm going to add appropriate pipe-control to pipeline and send the
> > > patch.
> > 
> > This change: https://patchwork.freedesktop.org/series/160484/
> > 
> > along with your patch passes the test.
> > 
> 
> Thanks! Great data point. Now just need to test out Mesa / Compute. I
> suspect they properly insert pipe controls in batches as preemption
> points but need to confirm.
> 

Both Mesa / Compute have completed CI runs with this patch and reported
no regressions.

Can I get an RB on this patch to merge?

Zbigniew - did you ever get the IGT change merged?

Matt

> Matt 
> 
> > --
> > Zbigniew
> > 
> > > 
> > > --
> > > Zbigniew
> > > 
> > > > 
> > > > It is fine the batch preempts, we actually want that. Mid-batch
> > > > preemption is not what we are trying to prevent - we are trying to
> > > > prevent preemption of the fence signaling after the batch is done. As
> > > > long as we don't regress any user applications we can safely make this
> > > > change - it is fine to break IGTs which are not doing the right thing
> > > > wrt to batch programming.
> > > > 
> > > > Matt
> > > > 
> > > > > 
> > > > > --
> > > > > Zbigniew
> > > > > 
> > > > > > 
> > > > > > Matt
> > > > > > 
> > > > > > > May you try to put arb disable after emit_render_cache_flush?
> > > > > > > 
> > > > > > > --
> > > > > > > Zbigniew
> > > > > > > 
> > > > > > > >  	i = emit_render_cache_flush(job, dw, i);
> > > > > > > >  
> > > > > > > >  	if (job->user_fence.used)
> > > > > > > > -- 
> > > > > > > > 2.34.1
> > > > > > > >