Re: [PATCH v3 2/5] drm/xe/guc: Don't ban LR VM exec queues on PM suspend

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Tue, 2026-05-26 at 16:38 +0100, Matthew Auld wrote:
> On 25/05/2026 14:30, Thomas Hellström wrote:
> > When xe_guc_submit_stop() is called during an S3/S4 suspend or GT
> > reset, guc_exec_queue_stop() bans any user exec queue that has a
> > job
> > which has started but not yet completed.  For normal (non-LR) exec
> > queues this is the correct behaviour: a started-but-incomplete job
> > at
> > reset time may indicate a hung workload.
> 
> Is it not too harsh to ban the user job for that? Say you are a well 
> behaved 3D workload, and forced suspend is triggered by the user, if
> you 
> are very unlucky you can get banned, if you hit the queue_stop flow
> with 
> a WIP job?

Actually (and this has bearing also on patch 1, I think) suspend /
resume must not sit in the critical path of any dma-fence job. Meaning
we explicitly have to wait for all outstanding dma-fences before
suspending, and add that if that's not already done.

/Thomas


> 
> > 
> > For exec queues attached to Long Running (LR) VMs the same
> > condition
> > is always true during normal operation: LR jobs are designed to run
> > indefinitely and are never "completed" in the DRM scheduler sense —
> > they are preempted and resumed via the preempt-fence mechanism.
> > Banning such an exec queue on PM suspend permanently prevents the
> > job
> > from restarting after resume, causing the userspace compute
> > workload to
> > fail silently.
> > 
> > Fix this by not banning LR VM exec queues when a system suspend or
> > hibernation is in progress, while preserving the ban for GT reset
> > where
> > a started-but-incomplete job is a legitimate indicator of a hang.
> > 
> > Fixes: f6375fb3aa94 ("drm/xe: Track LR jobs in DRM scheduler
> > pending list")
> > Cc: Matthew Brost <matthew.brost@intel.com>
> > Cc: Tomasz Lis <tomasz.lis@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: <stable@vger.kernel.org> # v6.19+
> > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Assisted-by: GitHub_Copilot:claude-sonnet-4.6
> > ---
> >   drivers/gpu/drm/xe/xe_device_types.h |  8 ++++++++
> >   drivers/gpu/drm/xe/xe_guc_submit.c   | 10 +++++++++-
> >   drivers/gpu/drm/xe/xe_pm.c           |  5 ++++-
> >   3 files changed, 21 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_device_types.h
> > b/drivers/gpu/drm/xe/xe_device_types.h
> > index 32dd2ffbc796..9dbf7b3a0c49 100644
> > --- a/drivers/gpu/drm/xe/xe_device_types.h
> > +++ b/drivers/gpu/drm/xe/xe_device_types.h
> > @@ -433,6 +433,14 @@ struct xe_device {
> >   	struct notifier_block pm_notifier;
> >   	/** @pm_block: Completion to block validating tasks on
> > suspend / hibernate prepare */
> >   	struct completion pm_block;
> > +	/**
> > +	 * @pm_suspend_in_progress: True while the device is going
> > through
> > +	 * system suspend or hibernation (set at xe_pm_suspend()
> > entry, cleared
> > +	 * at xe_pm_resume() entry or on suspend error). Used to
> > suppress exec
> > +	 * queue bans that should only apply during GT reset, not
> > PM suspend.
> > +	 * Serialised by the PM suspend sequence; no lock
> > required.
> > +	 */
> > +	bool pm_suspend_in_progress;
> >   	/** @rebind_resume_list: List of wq items to kick on
> > resume. */
> >   	struct list_head rebind_resume_list;
> >   	/** @rebind_resume_lock: Lock to protect the
> > rebind_resume_list */
> > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c
> > b/drivers/gpu/drm/xe/xe_guc_submit.c
> > index 2b8b316c0ca3..f1a6f13011b5 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> > @@ -2268,8 +2268,16 @@ static void guc_exec_queue_stop(struct
> > xe_guc *guc, struct xe_exec_queue *q)
> >   	 * Ban any engine (aside from kernel and engines used for
> > VM ops) with a
> >   	 * started but not complete job or if a job has gone
> > through a GT reset
> >   	 * more than twice.
> > +	 *
> > +	 * LR VM exec queues are excluded from this ban during PM
> > suspend: their
> > +	 * jobs are intentionally long-running and are preempted
> > and resumed via
> > +	 * the preempt-fence mechanism. Banning them on PM suspend
> > would
> > +	 * permanently prevent the job from restarting after
> > resume.
> > +	 * On GT reset however we do want to ban them, as that may
> > indicate a
> > +	 * genuinely hung workload.
> >   	 */
> > -	if (!(q->flags & (EXEC_QUEUE_FLAG_KERNEL |
> > EXEC_QUEUE_FLAG_VM))) {
> > +	if (!(q->flags & (EXEC_QUEUE_FLAG_KERNEL |
> > EXEC_QUEUE_FLAG_VM)) &&
> > +	    !(q->vm && xe_vm_in_lr_mode(q->vm) && guc_to_xe(guc)-
> > >pm_suspend_in_progress)) {
> >   		struct xe_sched_job *job =
> > xe_sched_first_pending_job(sched);
> >   		bool ban = false;
> >   
> > diff --git a/drivers/gpu/drm/xe/xe_pm.c
> > b/drivers/gpu/drm/xe/xe_pm.c
> > index c203a59d7000..76d211986822 100644
> > --- a/drivers/gpu/drm/xe/xe_pm.c
> > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > @@ -176,6 +176,7 @@ int xe_pm_suspend(struct xe_device *xe)
> >   	int err;
> >   
> >   	drm_dbg(&xe->drm, "Suspending device\n");
> > +	xe->pm_suspend_in_progress = true;
> >   	xe_pm_block_begin_signalling();
> >   	trace_xe_pm_suspend(xe, __builtin_return_address(0));
> >   
> > @@ -217,6 +218,7 @@ int xe_pm_suspend(struct xe_device *xe)
> >   	xe_pxp_pm_resume(xe->pxp);
> >   err:
> >   	drm_dbg(&xe->drm, "Device suspend failed %d\n", err);
> > +	xe->pm_suspend_in_progress = false;
> >   	xe_pm_block_end_signalling();
> >   	return err;
> >   }
> > @@ -234,8 +236,9 @@ int xe_pm_resume(struct xe_device *xe)
> >   	u8 id;
> >   	int err;
> >   
> > -	xe_pm_block_begin_signalling();
> > +	xe->pm_suspend_in_progress = false;
> >   	drm_dbg(&xe->drm, "Resuming device\n");
> > +	xe_pm_block_begin_signalling();
> >   	trace_xe_pm_resume(xe, __builtin_return_address(0));
> >   
> >   	for_each_gt(gt, xe, id)