On Fri, Oct 16, 2026 at 5:11 AM Luigi Lauro <[email protected]> wrote: > > 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]>
Reviewed-by: Alex Deucher <[email protected]> and applied. Thanks! Alex > --- > 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 >
