diff mbox series

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

Message ID 20240523220318.25446-1-rodrigo.vivi@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/display: Expand runtime_pm protection to atomic commit work | expand

Commit Message

Rodrigo Vivi May 23, 2024, 10:03 p.m. UTC 
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@intel.com>
Cc: Francois Dugast <francois.dugast@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 9 +++++++++
 1 file changed, 9 insertions(+)
diff mbox series

Patch

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)