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

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 1/22/2026 1:22 AM, Zbigniew Kempczyński wrote:
> On Tue, Jan 20, 2026 at 01:10:20PM -0800, Daniele Ceraolo Spurio wrote:
>>
>> On 1/19/2026 4:01 AM, 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.
>>>
>>>> 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.
>>>
>>>> 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.
>>> 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..
>> So basically the test requires a preemption but does not put any preemption
>> points within the batch? I'd argue that the fact that the test works at all
>> is by chance, because the kernel just happens to add a preemption point
>> after the BB and the batch doesn't wait for the results before completing.
>> IMO it should be ok to go ahead with this patch and rework the test, but we
>> probably need an ack from a maintainer because something that worked before
>> (even if just by chance) is not going to work anymore.
> Why user would need to explicitely add preemption points within batch?
> It's imo driver responsibility to enable/disable preemption and additional
> preemption points in the batch may change preemption distribution,
> not the ability to preempt.

If the batch logic requires a preemption (like in this case where the 
batch needs a different batch on the same engine to signal it), then IMO 
the user should make sure that there is a preemption point in the batch. 
The driver does keep preemption enabled while the batch is executing, 
but I don't think we want to guarantee that preemption will be enabled 
or that there will be a preemption point after the batch has completed. 
The fact that there is a preemption point after the batch is kind of by 
chance, just because the instruction we use to do a memory flush happens 
to be preemptable.

> Compute-walkers we already had in IGT tests (gpgpu-fill) were called
> in fire-and-forget mode leaving pipe-control to kmd. Code written
> before I joined the team were written with this assumption and I haven't
> seen any objection to this approach. So I assumed kmd will take care
> of job completion when batch returns to ring. If this won't apply
> anymore we just need to change IGT tests to add pipe-control in the
> batch. And ensure nothing else is broken (compute / mesa).

Agreed we need to make sure that no one apart from IGT is already 
relying on this, otherwise as Matt said we'll need to make this an 
opt-in behavior.

Daniele

>
> I'm going to send pipe-control patch to IGTs, imo it will fix the
> WMTP case. Sync wait at the batch level should keep execution with
> preemption enabled allowing to call SIP and switch the context.
>
> --
> Zbigniew
>
>> Daniele
>>
>>> --
>>> 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
>>>>>>