amdgpu_dm_commit_streams() takes a runtime PM reference for every
active CRTC that needs a modeset and has a new stream, but
amdgpu_dm_atomic_commit_tail() drops one only for each CRTC that goes
from active to inactive. A modeset of a CRTC that is already active (a
mode change, or anything else that needs a full modeset) therefore
leaks one reference. Once that has happened, the usage count no longer
returns to zero with every CRTC off, and the device cannot runtime
suspend again until reboot. The two also disagree the other way when a
CRTC turns on without a stream ("Failed to create new stream"): no
reference is taken, but one is dropped when it turns off.
Take the reference in the loop that counts the disabled CRTCs, on the
mirror condition: when the CRTC goes from inactive to active, which is
what the commit that added it intended ("every time we enable a
previously disabled CRTC"). Each active CRTC then holds exactly one
reference.
Tested on an RX 9070 XT (Navi 48) over OCuLink, with the change
backported to 7.2.9: an unplugged connector forced on, then mode A on a
CRTC, mode B on the same active CRTC, and the CRTC off, three rounds in
a row. The usage count rose by 3 with the stock module, by 0 with this
patch, and by 3 again after reloading the stock module.
Fixes: 97028037a38a ("drm/amdgpu: Grab/put runtime PM references in
atomic_commit_tail()")
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5951
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Luigi Lauro <[email protected]>
---
Alex asked for this on the issue. It differs from the change tested
there, which only skipped the get in amdgpu_dm_commit_streams() for a
CRTC that was already active: with that form, an off-to-on modeset with
no stream (the "Failed to create new stream" continue) takes no
reference, and a later modeset of the same CRTC with a stream and then
an off drops one more than was taken. Pairing the get with the put
closes both.
Who changes: any system where a CRTC that is already active gets a full
modeset (a mode change, an HDR or colorspace change). Today each such
modeset leaks one reference; with this patch it takes none. A CRTC
turned on without a stream now takes the reference its later turn-off
drops; I could not exercise that path.
Also tested at rest, on a boot with no leaked references: with nothing
using the card and its connectors never probed (a TV is attached), it
runtime suspends to D3hot with this patch after a forced wake, and the
usage count drops to 0, as it does with the narrower form loaded before
and after. That test runs no modeset; the rounds above do.
The change applies unchanged to 7.2.y.
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c4fee73a0..605f289ac 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4707,8 +4707,6 @@ static void amdgpu_dm_commit_streams(struct
drm_atomic_commit *state,
if (dm_old_crtc_state->stream)
remove_stream(adev, acrtc,
dm_old_crtc_state->stream);
- pm_runtime_get_noresume(dev->dev);
-
acrtc->enabled = true;
acrtc->hw_mode = new_crtc_state->mode;
crtc->hwmode = new_crtc_state->mode;
@@ -5292,8 +5290,13 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_commit *state)
#ifdef CONFIG_DEBUG_FS
enum amdgpu_dm_pipe_crc_source cur_crc_src;
#endif
- /* Count number of newly disabled CRTCs for dropping PM refs
later. */
- if (old_crtc_state->active && !new_crtc_state->active)
+ /*
+ * One runtime PM reference per active CRTC: take it when the
+ * CRTC turns on, count it when it turns off for dropping later.
+ */
+ if (!old_crtc_state->active && new_crtc_state->active)
+ pm_runtime_get_noresume(dev->dev);
+ else if (old_crtc_state->active && !new_crtc_state->active)
crtc_disable_count++;
dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
--
2.56.0
