[PATCH] drm/i915/display: Expand runtime_pm protection to atomic commit work

Rodrigo Vivi rodrigo.vivi at intel.com
Thu May 23 22:03:18 UTC 2024


Xe memory management relies on outer bound callers of runtime PM
protection and it will warn us when some is missing:

<4> [274.904535] xe 0000:00:02.0: Missing outer runtime PM protection
<4> [274.905051]  ? xe_pm_runtime_get_noresume+0x48/0x60 [xe]
<4> [274.905118]  xe_ggtt_remove_node+0x28/0x90 [xe]
<4> [274.905164]  __xe_unpin_fb_vma+0x91/0x120 [xe]
<4> [274.905234]  intel_plane_unpin_fb+0x19/0x30 [xe]
<4> [274.905306]  intel_cleanup_plane_fb+0x3d/0x50 [xe]
<4> [274.905391]  drm_atomic_helper_cleanup_planes+0x49/0x70 [drm_kms_helper]
<4> [274.905407]  intel_atomic_cleanup_work+0x69/0xd0 [xe]

The atomic commit helpers in i915 display are already protected.
However, they return the wakeref right before scheduling the thread
work items, what can lead to unprotected memory accesses.

Hence, expand the protections to the work items.

An alternative way would be to keep the state->wakeref, returning
them only at the workers. But this could lead in unbalanced scenarios
if workers gets canceled. So, the preference was to keep it simple
and get a new reference inside the thread.

Cc: Matthew Auld <matthew.auld at intel.com>
Cc: Francois Dugast <francois.dugast at intel.com>
Cc: Imre Deak <imre.deak at intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 1e8e2fd52cf6..03a0abc589fd 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7172,14 +7172,19 @@ static void intel_atomic_cleanup_work(struct work_struct *work)
 	struct drm_i915_private *i915 = to_i915(state->base.dev);
 	struct intel_crtc_state *old_crtc_state;
 	struct intel_crtc *crtc;
+	intel_wakeref_t wakeref;
 	int i;
 
+	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
+
 	for_each_old_intel_crtc_in_state(state, crtc, old_crtc_state, i)
 		intel_color_cleanup_commit(old_crtc_state);
 
 	drm_atomic_helper_cleanup_planes(&i915->drm, &state->base);
 	drm_atomic_helper_commit_cleanup_done(&state->base);
 	drm_atomic_state_put(&state->base);
+
+	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 }
 
 static void intel_atomic_prepare_plane_clear_colors(struct intel_atomic_state *state)
@@ -7453,8 +7458,12 @@ static void intel_atomic_commit_work(struct work_struct *work)
 {
 	struct intel_atomic_state *state =
 		container_of(work, struct intel_atomic_state, base.commit_work);
+	struct drm_i915_private *i915 = to_i915(state->base.dev);
+	intel_wakeref_t wakeref;
 
+	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
 	intel_atomic_commit_tail(state);
+	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 }
 
 static void intel_atomic_track_fbs(struct intel_atomic_state *state)
-- 
2.45.1



More information about the Intel-gfx mailing list