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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 12 Aug 2026, Weifeng Liu <weifeng.liu.z@gmail.com> wrote:
> Gentle ping. I would like to provide some additional context about the
> issue this patch is trying to fix.

Have you filed a bug over at [1]? If not, please do, and add the
information there, with logs, and point at this thread too.

Thanks,
Jani.


[1] https://drm.pages.freedesktop.org/intel-docs/how-to-file-i915-bugs.html




>
> 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) \

-- 
Jani Nikula, Intel