Re: [Intel-gfx] [PATCH v6 3/9] drm/i915/gt: Increase suspend timeout

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 22/09/2021 07:25, Thomas Hellström wrote:
> With GuC submission on DG1, the execution of the requests times out
> for the gem_exec_suspend igt test case after executing around 800-900
> of 1000 submitted requests.
> 
> Given the time we allow elsewhere for fences to signal (in the order of
> seconds), increase the timeout before we mark the gt wedged and proceed.

I suspect it is not about requests not retiring in time but about the 
intel_guc_wait_for_idle part of intel_gt_wait_for_idle. Although I don't 
know which G2H message is the code waiting for at suspend time so 
perhaps something to run past the GuC experts.

Anyway, if that turns out to be correct then perhaps it would be better 
to split the two timeouts (like if required GuC timeout is perhaps 
fundamentally independent) so it's clear who needs how much time. Adding 
Matt and John to comment.

To be clear, as timeout is AFAIK an arbitrary value, I don't have 
fundamental objections here. Just think it would be good to have 
accurate story in the commit message.

Regards,

Tvrtko

> 
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_gt_pm.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> index dea8e2479897..f84f2bfe2de0 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -19,6 +19,8 @@
>   #include "intel_rps.h"
>   #include "intel_wakeref.h"
>   
> +#define I915_GT_SUSPEND_IDLE_TIMEOUT (HZ / 2)
> +
>   static void user_forcewake(struct intel_gt *gt, bool suspend)
>   {
>   	int count = atomic_read(&gt->user_wakeref);
> @@ -279,7 +281,7 @@ 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_GEM_IDLE_TIMEOUT) == -ETIME) {
> +	if (intel_gt_wait_for_idle(gt, I915_GT_SUSPEND_IDLE_TIMEOUT) == -ETIME) {
>   		/*
>   		 * Forcibly cancel outstanding work and leave
>   		 * the gpu quiet.
>