diff mbox series

drm/i915/guc: Cancel GuC engine busyness worker synchronously

Message ID 20220827002135.139349-1-umesh.nerlige.ramappa@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/guc: Cancel GuC engine busyness worker synchronously | expand

Commit Message

Umesh Nerlige Ramappa Aug. 27, 2022, 12:21 a.m. UTC
The worker is canceled in gt_park path, but earlier it was assumed that
gt_park path cannot sleep and the cancel is asynchronous. This caused a
race with suspend flow where the worker runs after suspend and causes an
unclaimed register access warning. Cancel the worker synchronously since
the gt_park is indeed allowed to sleep.

v2: Fix author name and sign-off mismatch

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4419
Fixes: 77cdd054dd2c ("drm/i915/pmu: Connect engine busyness stats from GuC to pmu")
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Dixit, Ashutosh Sept. 5, 2022, 9:30 p.m. UTC | #1
On Fri, 26 Aug 2022 17:21:35 -0700, Umesh Nerlige Ramappa wrote:
>
> The worker is canceled in gt_park path, but earlier it was assumed that
> gt_park path cannot sleep and the cancel is asynchronous. This caused a
> race with suspend flow where the worker runs after suspend and causes an
> unclaimed register access warning. Cancel the worker synchronously since
> the gt_park is indeed allowed to sleep.

Indeed, __gt_park already calls cancel_work_sync and synchronize_irq which
can sleep:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

> v2: Fix author name and sign-off mismatch
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4419
> Fixes: 77cdd054dd2c ("drm/i915/pmu: Connect engine busyness stats from GuC to pmu")
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> 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 0d56b615bf78..e6275380b253 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1438,7 +1438,12 @@ void intel_guc_busyness_park(struct intel_gt *gt)
>	if (!guc_submission_initialized(guc))
>		return;
>
> -	cancel_delayed_work(&guc->timestamp.work);
> +	/*
> +	 * There is a race with suspend flow where the worker runs after suspend
> +	 * and causes an unclaimed register access warning. Cancel the worker
> +	 * synchronously here.
> +	 */
> +	cancel_delayed_work_sync(&guc->timestamp.work);
>
>	/*
>	 * Before parking, we should sample engine busyness stats if we need to.
> --
> 2.25.1
>
diff mbox series

Patch

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 0d56b615bf78..e6275380b253 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1438,7 +1438,12 @@  void intel_guc_busyness_park(struct intel_gt *gt)
 	if (!guc_submission_initialized(guc))
 		return;
 
-	cancel_delayed_work(&guc->timestamp.work);
+	/*
+	 * There is a race with suspend flow where the worker runs after suspend
+	 * and causes an unclaimed register access warning. Cancel the worker
+	 * synchronously here.
+	 */
+	cancel_delayed_work_sync(&guc->timestamp.work);
 
 	/*
 	 * Before parking, we should sample engine busyness stats if we need to.