Re: [Intel-gfx] [PATCH v1 1/3] drm/i915/guc: Flush context destruction worker at suspend

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Aug 02, 2023 at 04:34:59PM -0700, Alan Previn wrote:
> Suspend is not like reset, it can unroll, so we have to properly
> flush pending context-guc-id deregistrations to complete before
> we return from suspend calls.

But if is 'unrolls' the execution should just continue, no?!
In other words, why is this flush needed? What happens if we
don't flush, but resume doesn't proceed? in in which case
of resume you are thinking that it returns and not having flushed?

> 
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_pm.c             | 6 +++++-
>  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 5 +++++
>  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h | 2 ++
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c             | 7 +++++++
>  drivers/gpu/drm/i915/gt/uc/intel_uc.h             | 1 +
>  5 files changed, 20 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 5a942af0a14e..3162d859ed68 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -289,8 +289,10 @@ int intel_gt_resume(struct intel_gt *gt)
>  
>  static void wait_for_suspend(struct intel_gt *gt)
>  {
> -	if (!intel_gt_pm_is_awake(gt))
> +	if (!intel_gt_pm_is_awake(gt)) {
> +		intel_uc_suspend_prepare(&gt->uc);

why only on idle?

Well, I know, if we are in idle it is because all the requests had
already ended and gt will be wedged, but why do we need to do anything
if we are in idle?

And why here and not some upper layer? like in prepare....

>  		return;
> +	}
>  
>  	if (intel_gt_wait_for_idle(gt, I915_GT_SUSPEND_IDLE_TIMEOUT) == -ETIME) {
>  		/*
> @@ -299,6 +301,8 @@ static void wait_for_suspend(struct intel_gt *gt)
>  		 */
>  		intel_gt_set_wedged(gt);
>  		intel_gt_retire_requests(gt);
> +	} else {
> +		intel_uc_suspend_prepare(&gt->uc);
>  	}
>  
>  	intel_gt_pm_wait_for_idle(gt);
> 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 a0e3ef1c65d2..dc7735a19a5a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1578,6 +1578,11 @@ static void guc_flush_submissions(struct intel_guc *guc)
>  	spin_unlock_irqrestore(&sched_engine->lock, flags);
>  }
>  
> +void intel_guc_submission_suspend_prepare(struct intel_guc *guc)
> +{
> +	flush_work(&guc->submission_state.destroyed_worker);
> +}
> +
>  static void guc_flush_destroyed_contexts(struct intel_guc *guc);
>  
>  void intel_guc_submission_reset_prepare(struct intel_guc *guc)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> index c57b29cdb1a6..7f0705ece74b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> @@ -38,6 +38,8 @@ int intel_guc_wait_for_pending_msg(struct intel_guc *guc,
>  				   bool interruptible,
>  				   long timeout);
>  
> +void intel_guc_submission_suspend_prepare(struct intel_guc *guc);
> +
>  static inline bool intel_guc_submission_is_supported(struct intel_guc *guc)
>  {
>  	return guc->submission_supported;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 18250fb64bd8..468d7b397927 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -679,6 +679,13 @@ void intel_uc_runtime_suspend(struct intel_uc *uc)
>  	guc_disable_communication(guc);
>  }
>  
> +void intel_uc_suspend_prepare(struct intel_uc *uc)
> +{
> +	struct intel_guc *guc = &uc->guc;
> +
> +	intel_guc_submission_suspend_prepare(guc);
> +}
> +
>  void intel_uc_suspend(struct intel_uc *uc)
>  {
>  	struct intel_guc *guc = &uc->guc;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> index 014bb7d83689..036877a07261 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> @@ -49,6 +49,7 @@ void intel_uc_reset_prepare(struct intel_uc *uc);
>  void intel_uc_reset(struct intel_uc *uc, intel_engine_mask_t stalled);
>  void intel_uc_reset_finish(struct intel_uc *uc);
>  void intel_uc_cancel_requests(struct intel_uc *uc);
> +void intel_uc_suspend_prepare(struct intel_uc *uc);
>  void intel_uc_suspend(struct intel_uc *uc);
>  void intel_uc_runtime_suspend(struct intel_uc *uc);
>  int intel_uc_resume(struct intel_uc *uc);
> -- 
> 2.39.0
>