drm/i915/guc: Improve clean up of busyness stats worker

The stats worker thread management was mis-matched between
enable/disable call sites. Fix those up. Also, abstract the
cancel/enable code into a helper function rather than replicating in
multiple places.

v2: Rename the helpers and wrap the enable as well as the cancel
(review feedback from Daniele).

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230217223308.3449737-2-John.C.Harrison@Intel.com

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 be495e6..a04d704 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1352,6 +1352,16 @@ static ktime_t guc_engine_busyness(struct intel_engine_cs *engine, ktime_t *now)
        return ns_to_ktime(total);
 }
 
+static void guc_enable_busyness_worker(struct intel_guc *guc)
+{
+       mod_delayed_work(system_highpri_wq, &guc->timestamp.work, guc->timestamp.ping_delay);
+}
+
+static void guc_cancel_busyness_worker(struct intel_guc *guc)
+{
+       cancel_delayed_work_sync(&guc->timestamp.work);
+}
+
 static void __reset_guc_busyness_stats(struct intel_guc *guc)
 {
        struct intel_gt *gt = guc_to_gt(guc);
@@ -1360,7 +1370,7 @@ static void __reset_guc_busyness_stats(struct intel_guc *guc)
        unsigned long flags;
        ktime_t unused;
 
-       cancel_delayed_work_sync(&guc->timestamp.work);
+       guc_cancel_busyness_worker(guc);
 
        spin_lock_irqsave(&guc->timestamp.lock, flags);
 
@@ -1416,8 +1426,7 @@ static void guc_timestamp_ping(struct work_struct *wrk)
 
        intel_gt_reset_unlock(gt, srcu);
 
-       mod_delayed_work(system_highpri_wq, &guc->timestamp.work,
-                        guc->timestamp.ping_delay);
+       guc_enable_busyness_worker(guc);
 }
 
 static int guc_action_enable_usage_stats(struct intel_guc *guc)
@@ -1436,16 +1445,15 @@ static void guc_init_engine_stats(struct intel_guc *guc)
 {
        struct intel_gt *gt = guc_to_gt(guc);
        intel_wakeref_t wakeref;
+       int ret;
 
-       mod_delayed_work(system_highpri_wq, &guc->timestamp.work,
-                        guc->timestamp.ping_delay);
-
-       with_intel_runtime_pm(&gt->i915->runtime_pm, wakeref) {
-               int ret = guc_action_enable_usage_stats(guc);
+       with_intel_runtime_pm(&gt->i915->runtime_pm, wakeref)
+               ret = guc_action_enable_usage_stats(guc);
 
-               if (ret)
-                       guc_err(guc, "Failed to enable usage stats: %pe\n", ERR_PTR(ret));
-       }
+       if (ret)
+               guc_err(guc, "Failed to enable usage stats: %pe\n", ERR_PTR(ret));
+       else
+               guc_enable_busyness_worker(guc);
 }
 
 void intel_guc_busyness_park(struct intel_gt *gt)
@@ -1460,7 +1468,7 @@ void intel_guc_busyness_park(struct intel_gt *gt)
         * and causes an unclaimed register access warning. Cancel the worker
         * synchronously here.
         */
-       cancel_delayed_work_sync(&guc->timestamp.work);
+       guc_cancel_busyness_worker(guc);
 
        /*
         * Before parking, we should sample engine busyness stats if we need to.
@@ -1487,8 +1495,7 @@ void intel_guc_busyness_unpark(struct intel_gt *gt)
        spin_lock_irqsave(&guc->timestamp.lock, flags);
        guc_update_pm_timestamp(guc, &unused);
        spin_unlock_irqrestore(&guc->timestamp.lock, flags);
-       mod_delayed_work(system_highpri_wq, &guc->timestamp.work,
-                        guc->timestamp.ping_delay);
+       guc_enable_busyness_worker(guc);
 }
 
 static inline bool
@@ -4408,11 +4415,12 @@ void intel_guc_submission_enable(struct intel_guc *guc)
        guc_init_global_schedule_policy(guc);
 }
 
+/* Note: By the time we're here, GuC may have already been reset */
 void intel_guc_submission_disable(struct intel_guc *guc)
 {
        struct intel_gt *gt = guc_to_gt(guc);
 
-       /* Note: By the time we're here, GuC may have already been reset */
+       guc_cancel_busyness_worker(guc);
 
        /* Disable and route to host */
        if (GRAPHICS_VER(gt->i915) >= 12)