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

Tvrtko Ursulin
2021-09-23T10:13:18Z

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.
>