Skip to content

Commit 907ef03

Browse files
unerligerodrigovivi
authored andcommitted
i915/guc: Get runtime pm in busyness worker only if already active
Ideally the busyness worker should take a gt pm wakeref because the worker only needs to be active while gt is awake. However, the gt_park path cancels the worker synchronously and this complicates the flow if the worker is also running at the same time. The cancel waits for the worker and when the worker releases the wakeref, that would call gt_park and would lead to a deadlock. The resolution is to take the global pm wakeref if runtime pm is already active. If not, we don't need to update the busyness stats as the stats would already be updated when the gt was parked. Note: - We do not requeue the worker if we cannot take a reference to runtime pm since intel_guc_busyness_unpark would requeue the worker in the resume path. - If the gt was parked longer than time taken for GT timestamp to roll over, we ignore those rollovers since we don't care about tracking the exact GT time. We only care about roll overs when the gt is active and running workloads. - There is a window of time between gt_park and runtime suspend, where the worker may run. This is acceptable since the worker will not find any new data to update busyness. v2: (Daniele) - Edit commit message and code comment - Use runtime pm in the worker - Put runtime pm after enabling the worker - Use Link tag and add Fixes tag v3: (Daniele) - Reword commit and comments and add details Link: https://gitlab.freedesktop.org/drm/intel/-/issues/7077 Fixes: 77cdd05 ("drm/i915/pmu: Connect engine busyness stats from GuC to pmu") Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230925192117.2497058-1-umesh.nerlige.ramappa@intel.com (cherry picked from commit e2f99b7) Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
1 parent b7599d2 commit 907ef03

File tree

1 file changed

+35
-3
lines changed

1 file changed

+35
-3
lines changed

drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1432,6 +1432,36 @@ static void guc_timestamp_ping(struct work_struct *wrk)
14321432
unsigned long index;
14331433
int srcu, ret;
14341434

1435+
/*
1436+
* Ideally the busyness worker should take a gt pm wakeref because the
1437+
* worker only needs to be active while gt is awake. However, the
1438+
* gt_park path cancels the worker synchronously and this complicates
1439+
* the flow if the worker is also running at the same time. The cancel
1440+
* waits for the worker and when the worker releases the wakeref, that
1441+
* would call gt_park and would lead to a deadlock.
1442+
*
1443+
* The resolution is to take the global pm wakeref if runtime pm is
1444+
* already active. If not, we don't need to update the busyness stats as
1445+
* the stats would already be updated when the gt was parked.
1446+
*
1447+
* Note:
1448+
* - We do not requeue the worker if we cannot take a reference to runtime
1449+
* pm since intel_guc_busyness_unpark would requeue the worker in the
1450+
* resume path.
1451+
*
1452+
* - If the gt was parked longer than time taken for GT timestamp to roll
1453+
* over, we ignore those rollovers since we don't care about tracking
1454+
* the exact GT time. We only care about roll overs when the gt is
1455+
* active and running workloads.
1456+
*
1457+
* - There is a window of time between gt_park and runtime suspend,
1458+
* where the worker may run. This is acceptable since the worker will
1459+
* not find any new data to update busyness.
1460+
*/
1461+
wakeref = intel_runtime_pm_get_if_active(&gt->i915->runtime_pm);
1462+
if (!wakeref)
1463+
return;
1464+
14351465
/*
14361466
* Synchronize with gt reset to make sure the worker does not
14371467
* corrupt the engine/guc stats. NB: can't actually block waiting
@@ -1440,10 +1470,9 @@ static void guc_timestamp_ping(struct work_struct *wrk)
14401470
*/
14411471
ret = intel_gt_reset_trylock(gt, &srcu);
14421472
if (ret)
1443-
return;
1473+
goto err_trylock;
14441474

1445-
with_intel_runtime_pm(&gt->i915->runtime_pm, wakeref)
1446-
__update_guc_busyness_stats(guc);
1475+
__update_guc_busyness_stats(guc);
14471476

14481477
/* adjust context stats for overflow */
14491478
xa_for_each(&guc->context_lookup, index, ce)
@@ -1452,6 +1481,9 @@ static void guc_timestamp_ping(struct work_struct *wrk)
14521481
intel_gt_reset_unlock(gt, srcu);
14531482

14541483
guc_enable_busyness_worker(guc);
1484+
1485+
err_trylock:
1486+
intel_runtime_pm_put(&gt->i915->runtime_pm, wakeref);
14551487
}
14561488

14571489
static int guc_action_enable_usage_stats(struct intel_guc *guc)

0 commit comments

Comments
 (0)