[PATCH] drm/amd/display: Only allow idle optimizations when all CRTCs are idle

[Claudio Forain <claudioforain@gmail.com>]

[Why]
Idle optimizations are HW-wide, but the Idle State Manager (ISM) runs per
CRTC. When a CRTC's ISM reaches OPTIMIZED_IDLE it enables idle
optimizations globally as long as no CRTC holds a vblank reference
(dm->active_vblank_irq_count == 0). It does not check whether the ISMs
of the other CRTCs are idle.

With two displays on X11, one static and one receiving cursor updates,
the static CRTC's ISM enables idle optimizations while the cursor CRTC is
busy. Legacy cursor updates do not take a vblank reference, so nothing
vetoes it. On ASICs that set debug.exit_idle_opt_for_cursor_updates
(DCN30x, DCN32x, DCN401, ...), every call to
dc_stream_program_cursor_attributes() and
dc_stream_program_cursor_position() then exits and re-enters idle
optimizations itself. That is up to four synchronous DMUB commands per
cursor update, each taking up to ~1.6 ms on DCN401, all under dc_lock in
the X server's input thread. The cursor visibly speeds up and slows down
while the desktop is otherwise idle.

Measured on RX 9070 XT (DCN401), 240 Hz + 60 Hz displays, Xorg, cursor
moving continuously for 15 s (ftrace, cursor handler entry to
dc_stream_program_cursor_position entry):

                                 before      after
  cursor updates/s                1686        2572
  latency median / P95 / P99    144/858/927  17/19/87 us
  samples > 500 us                5460           0
  idle opt calls from cursor   58093           0
    programming
  dcn401_apply_idle_power_
    optimizations calls          58198           6

With a single display, or with any vblank requestor active, the issue
does not occur. Commit a1fc7bf6677e ("drm/amd/display: Restore 5s vbl
offdelay for NV3x+ DGPUs") hid the issue by keeping vblank enabled on the
cursor CRTC; it reappeared with commit f39283eab44f ("Revert
"drm/amd/display: Restore 5s vbl offdelay for NV3x+ DGPUs"").

[How]
Before allowing idle optimizations, also require every CRTC with an
active stream to be in OPTIMIZED_IDLE or OPTIMIZED_IDLE_SSO. The ISM
updates current_state before dispatching, so the last CRTC to go idle
still allows idle optimizations and no idle entry is lost. Idle
optimizations are still entered on a static desktop (173 entries in a
15 s static capture with the patch applied), idle GPU power is
unchanged (25.9 W before, 25.8 W after), and vblank enable/disable is
unchanged.

The problem was found and the fix written with an AI coding assistant,
working from ftrace captures taken on the affected system: function
tracer counts and caller attribution, and function_graph durations of
the cursor, ISM and DMUB paths. The assistant proposed the change and
drafted this changelog; the submitter reproduced the issue, built and
ran each test kernel, and confirmed the results.

Fixes: 754003486c3c ("drm/amd/display: Add Idle state manager(ISM)")
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/6115
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/4906
Assisted-by: claude-opus-5-5
Signed-off-by: Claudio Forain <claudioforain@gmail.com>
---
Testing and limitations:
- Reproduced and tested on v7.3.3 (Gentoo) with RX 9070 XT (DCN401),
  Xorg, two DP displays (240 Hz + 60 Hz). The issue reproduces only with
  both displays active; disabling one makes stock 7.3.3 behave.
- Built and booted on v7.3.3. This posting is rebased onto
  amd-staging-drm-next, where it applies cleanly, but it has not been
  built or run on that tree.
- Not tested: other DCN generations, APUs, PSR/Replay panels (the
  OPTIMIZED_IDLE_SSO path), Wayland compositors, hotplug and
  suspend/resume with the patch, and the ISM KUnit tests.
- Idle GPU power on a static desktop (hwmon power1_average, 40 x 1 s):
  25.9 W before, 25.8 W after. Only checked on this setup.
- crtc->state is read under dc_lock only, like the existing
  to_dm_crtc_state(acrtc->base.state) in amdgpu_dm_ism_commit_event().

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_ism.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_ism.c
index 127eaba..79546ad 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_ism.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_ism.c
@@ -276,6 +276,32 @@ bool dm_ism_trigger_event(struct amdgpu_dm_ism *ism,
 EXPORT_IF_KUNIT(dm_ism_trigger_event);
 
 
+/*
+ * Idle optimizations are HW-wide, but each CRTC runs its own ISM. Only allow
+ * them once every active CRTC's ISM has reached an optimized idle state, so a
+ * static CRTC cannot enable idle optimizations while another CRTC is still busy
+ * (e.g. receiving cursor updates without a vblank reference). Otherwise every
+ * cursor update on the busy CRTC must exit and re-enter idle through DMUB.
+ */
+static bool dm_ism_all_crtcs_idle(struct amdgpu_display_manager *dm)
+{
+	struct drm_crtc *crtc;
+
+	drm_for_each_crtc(crtc, adev_to_drm(dm->adev)) {
+		struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
+		struct dm_crtc_state *state = to_dm_crtc_state(crtc->state);
+
+		if (!state || !state->stream)
+			continue;
+
+		if (acrtc->ism.current_state != DM_ISM_STATE_OPTIMIZED_IDLE &&
+		    acrtc->ism.current_state != DM_ISM_STATE_OPTIMIZED_IDLE_SSO)
+			return false;
+	}
+
+	return true;
+}
+
 static void dm_ism_commit_idle_optimization_state(struct amdgpu_dm_ism *ism,
 					     struct dc_stream_state *stream,
 					     bool vblank_enabled,
@@ -326,7 +352,8 @@ static void dm_ism_commit_idle_optimization_state(struct amdgpu_dm_ism *ism,
 	 * optimizations at the start of this func -- we should disallow
 	 * whenever there's *an* active CRTC.
 	 */
-	if (!vblank_enabled && dm->active_vblank_irq_count == 0) {
+	if (!vblank_enabled && dm->active_vblank_irq_count == 0 &&
+	    dm_ism_all_crtcs_idle(dm)) {
 		dc_post_update_surfaces_to_stream(dm->dc);
 		dc_allow_idle_optimizations(dm->dc, true);
 	}
-- 
2.x