Re: [PATCH] drm/i915: Make suspend idle wait uninterruptible

[Weifeng Liu <weifeng.liu.z@gmail.com>]

Gentle ping. I would like to provide some additional context about the
issue this patch is trying to fix.

A customer observed a shutdown hang in an automated S3 resume and
power-off test. This issue occurs after resuming from S3 and later
starting the power-off sequence. During process teardown, i915 reports
GuC CT errors such as:

  i915 0000:00:02.0: [drm] *ERROR* GT0: GUC: CT: Unsolicited response
message: len 1, data 0xe0000100 (fence 2941, last 2945)
  i915 0000:00:02.0: [drm] *ERROR* GT0: GUC: CT: Failed to handle HXG
message (-ENOKEY) 00 01 00 e0
  i915 0000:00:02.0: [drm] *ERROR* GT0: GUC: CT: Failed to process CT
message (-ENOKEY) 01 00 7d 0b 00 01 00 e0

No rendering or display failure was observed before shutdown. The GuC
error appears to be related to inconsistent context state after S3, but
this patch does not attempt to fix that underlying issue; I focused on
fixing the shutdown hang issue currently.

With initcall_debug enabled, the last device shutdown message is:

  i915 0000:00:02.0: shutdown

and device_shutdown() does not make further progress.

While investigating this hang, I found that wait_for_suspend() assumes
that intel_gt_wait_for_idle() either succeeds or returns -ETIME.
However, the request retirement and GuC pending-message waits are
interruptible and may return -EINTR or -ERESTARTSYS when the shutdown
task has a pending signal. These errors bypass the existing wedge and
cleanup path, after which wait_for_suspend() may block waiting for a GT
wakeref that cannot be released.

As a diagnostic experiment, adding a timeout to
intel_wakeref_wait_for_idle() allowed the power-off sequence to
complete consistently. I did not propose that approach upstream because
continuing suspend with an active GT would weaken the existing suspend
invariant.

Instead, this patch makes only GT idle wait used by wait_for_suspend()
uninterruptible. It retains the existing timeout and wedges the GT only
if that timeout expires. All other intel_gt_wait_for_idle() callers
remain interruptible.

Does this approach look reasonable, or would you prefer a different way
to prevent signals from bypassing the existing timeout recovery path?

And I'll be very grateful if you can provide some hints about the root
cause of the GuC error.

Best regards,
Weifeng

On Wed, 2026-08-05 at 14:45 +0800, Weifeng Liu wrote:
> wait_for_suspend() only handles -ETIME from the GT idle wait.
> However,
> the wait can also be interrupted by a pending signal while retiring
> requests or waiting for GuC messages, e.g., during the reboot process
> invoked by init. This skips the wedge and cleanup path and may leave
> suspend waiting indefinitely for a leaked wakeref.
> 
> Make only the suspend idle wait uninterruptible while preserving its
> existing timeout. Other callers remain interruptible, and the GT is
> still wedged only when the timeout expires.
> 
> Cc: Matthew Brost <matthew.brost@intel.com>
> Assisted-by: OpenCode:GPT-5.6-Sol
> Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt.c            | 24 +++++++++++++++--
> --
>  drivers/gpu/drm/i915/gt/intel_gt.h            |  1 +
>  drivers/gpu/drm/i915/gt/intel_gt_pm.c         |  3 ++-
>  drivers/gpu/drm/i915/gt/intel_gt_requests.c   |  8 ++++---
>  drivers/gpu/drm/i915/gt/intel_gt_requests.h   | 13 ++++++++--
>  drivers/gpu/drm/i915/gt/uc/intel_guc.h        |  3 ++-
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  5 ++--
>  drivers/gpu/drm/i915/gt/uc/intel_uc.h         |  6 +++--
>  8 files changed, 47 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 5c7f862f7100..5eaa52f535d4 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -659,7 +659,9 @@ static void __intel_gt_disable(struct intel_gt
> *gt)
>  	GEM_BUG_ON(intel_gt_pm_is_awake(gt));
>  }
>  
> -int intel_gt_wait_for_idle(struct intel_gt *gt, long timeout)
> +static int __intel_gt_wait_for_idle(struct intel_gt *gt,
> +				    bool interruptible,
> +				    long timeout)
>  {
>  	long remaining_timeout;
>  
> @@ -667,10 +669,11 @@ int intel_gt_wait_for_idle(struct intel_gt *gt,
> long timeout)
>  	if (!intel_gt_pm_is_awake(gt))
>  		return 0;
>  
> -	while ((timeout = intel_gt_retire_requests_timeout(gt,
> timeout,
> -							  
> &remaining_timeout)) > 0) {
> +	while ((timeout = __intel_gt_retire_requests_timeout(gt,
> interruptible,
> +							    
> timeout,
> +							    
> &remaining_timeout)) > 0) {
>  		cond_resched();
> -		if (signal_pending(current))
> +		if (interruptible && signal_pending(current))
>  			return -EINTR;
>  	}
>  
> @@ -680,7 +683,18 @@ int intel_gt_wait_for_idle(struct intel_gt *gt,
> long timeout)
>  	if (remaining_timeout < 0)
>  		remaining_timeout = 0;
>  
> -	return intel_uc_wait_for_idle(&gt->uc, remaining_timeout);
> +	return intel_uc_wait_for_idle(&gt->uc, interruptible,
> +				      remaining_timeout);
> +}
> +
> +int intel_gt_wait_for_idle(struct intel_gt *gt, long timeout)
> +{
> +	return __intel_gt_wait_for_idle(gt, true, timeout);
> +}
> +
> +int intel_gt_wait_for_idle_uninterruptible(struct intel_gt *gt, long
> timeout)
> +{
> +	return __intel_gt_wait_for_idle(gt, false, timeout);
>  }
>  
>  int intel_gt_init(struct intel_gt *gt)
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h
> b/drivers/gpu/drm/i915/gt/intel_gt.h
> index 998ca029b73a..6c7e5415f1dc 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.h
> @@ -143,6 +143,7 @@ void intel_gt_driver_release(struct intel_gt
> *gt);
>  void intel_gt_driver_late_release_all(struct drm_i915_private
> *i915);
>  
>  int intel_gt_wait_for_idle(struct intel_gt *gt, long timeout);
> +int intel_gt_wait_for_idle_uninterruptible(struct intel_gt *gt, long
> timeout);
>  
>  void intel_gt_check_and_clear_faults(struct intel_gt *gt);
>  i915_reg_t intel_gt_perf_limit_reasons_reg(struct intel_gt *gt);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> index c7f59d60fac6..9fe31aa07ae8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -315,7 +315,8 @@ static void wait_for_suspend(struct intel_gt *gt)
>  	if (!intel_gt_pm_is_awake(gt))
>  		return;
>  
> -	if (intel_gt_wait_for_idle(gt, I915_GT_SUSPEND_IDLE_TIMEOUT)
> == -ETIME) {
> +	if (intel_gt_wait_for_idle_uninterruptible(gt,
> +						  
> I915_GT_SUSPEND_IDLE_TIMEOUT) == -ETIME) {
>  		/*
>  		 * Forcibly cancel outstanding work and leave
>  		 * the gpu quiet.
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
> b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
> index 93298820bee2..b37e8f1d0b5c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
> @@ -130,8 +130,10 @@ void intel_engine_fini_retire(struct
> intel_engine_cs *engine)
>  	GEM_BUG_ON(engine->retire);
>  }
>  
> -long intel_gt_retire_requests_timeout(struct intel_gt *gt, long
> timeout,
> -				      long *remaining_timeout)
> +long __intel_gt_retire_requests_timeout(struct intel_gt *gt,
> +					bool interruptible,
> +					long timeout,
> +					long *remaining_timeout)
>  {
>  	struct intel_gt_timelines *timelines = &gt->timelines;
>  	struct intel_timeline *tl, *tn;
> @@ -159,7 +161,7 @@ long intel_gt_retire_requests_timeout(struct
> intel_gt *gt, long timeout,
>  				mutex_unlock(&tl->mutex);
>  
>  				timeout =
> dma_fence_wait_timeout(fence,
> -								
> true,
> +								
> interruptible,
>  								
> timeout);
>  				dma_fence_put(fence);
>  
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.h
> b/drivers/gpu/drm/i915/gt/intel_gt_requests.h
> index d2969f68dd64..9242f85f4040 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_requests.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.h
> @@ -12,8 +12,17 @@ struct intel_engine_cs;
>  struct intel_gt;
>  struct intel_timeline;
>  
> -long intel_gt_retire_requests_timeout(struct intel_gt *gt, long
> timeout,
> -				      long *remaining_timeout);
> +long __intel_gt_retire_requests_timeout(struct intel_gt *gt,
> +					bool interruptible,
> +					long timeout,
> +					long *remaining_timeout);
> +static inline long
> +intel_gt_retire_requests_timeout(struct intel_gt *gt, long timeout,
> +				 long *remaining_timeout)
> +{
> +	return __intel_gt_retire_requests_timeout(gt, true, timeout,
> +						   
> remaining_timeout);
> +}
>  static inline void intel_gt_retire_requests(struct intel_gt *gt)
>  {
>  	intel_gt_retire_requests_timeout(gt, 0, NULL);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index 053780f562c1..b5f4fddf5c1d 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -509,7 +509,8 @@ static inline void intel_guc_disable_msg(struct
> intel_guc *guc, u32 mask)
>  	spin_unlock_irq(&guc->irq_lock);
>  }
>  
> -int intel_guc_wait_for_idle(struct intel_guc *guc, long timeout);
> +int intel_guc_wait_for_idle(struct intel_guc *guc, bool
> interruptible,
> +			    long timeout);
>  
>  int intel_guc_deregister_done_process_msg(struct intel_guc *guc,
>  					  const u32 *msg, u32 len);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 788e59cdfac9..aa74208c2b4e 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -680,14 +680,15 @@ int intel_guc_wait_for_pending_msg(struct
> intel_guc *guc,
>  	return (timeout < 0) ? timeout : 0;
>  }
>  
> -int intel_guc_wait_for_idle(struct intel_guc *guc, long timeout)
> +int intel_guc_wait_for_idle(struct intel_guc *guc, bool
> interruptible,
> +			    long timeout)
>  {
>  	if (!intel_uc_uses_guc_submission(&guc_to_gt(guc)->uc))
>  		return 0;
>  
>  	return intel_guc_wait_for_pending_msg(guc,
>  					      &guc-
> >outstanding_submission_g2h,
> -					      true, timeout);
> +					      interruptible,
> timeout);
>  }
>  
>  static int guc_context_policy_init_v70(struct intel_context *ce,
> bool loop);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> index 014bb7d83689..b62fe3b96b64 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> @@ -96,9 +96,11 @@ uc_state_checkers(gsc, gsc_uc);
>  #undef uc_state_checkers
>  #undef __uc_state_checker
>  
> -static inline int intel_uc_wait_for_idle(struct intel_uc *uc, long
> timeout)
> +static inline int intel_uc_wait_for_idle(struct intel_uc *uc,
> +					 bool interruptible,
> +					 long timeout)
>  {
> -	return intel_guc_wait_for_idle(&uc->guc, timeout);
> +	return intel_guc_wait_for_idle(&uc->guc, interruptible,
> timeout);
>  }
>  
>  #define intel_uc_ops_function(_NAME, _OPS, _TYPE, _RET) \