From: Ivan Lipski <[email protected]> [Why] Multiple paths issue DMUB commands without managing IPS state, causing dc_wake_and_execute_gpint/dmub_cmd to internally wake from IPS and reallow idle. This flips idle_allowed back to true while idle_optimizations_allowed remains false during in-flight commits, desynchronizing the two flags.
Affected paths: - amdgpu_dm_psr_set_event() and amdgpu_dm_replay_set_event() calls from amdgpu_dm_handle_vrr_transition(), amdgpu_dm_commit_planes() and amdgpu_dm_mod_power_update_streams(), that are invoked on atomic commits. - debugfs psr_get(), psr_read_residency(), replay_get_state(), replay_set_residency() access hardware without holding dc_lock or disabling IPS. [How] - Explicitly exit IPS before PSR/Replay set_event w/ hw_programming, called within atomic commit. - Wrap debugfs PSR/Replay state getters and setters with IPS exit/entry + dc_lock. Reviewed-by: Sunpeng Li <[email protected]> Signed-off-by: Ivan Lipski <[email protected]> Signed-off-by: James Lin <[email protected]> --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 + .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 77 +++++++++++++++++++ 2 files changed, 81 insertions(+) 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 7ff1af3528dd..4e9b4fd505c2 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -9937,6 +9937,7 @@ static void amdgpu_dm_handle_vrr_transition(struct amdgpu_display_manager *dm, __func__, new_state->base.crtc->base.id); scoped_guard(mutex, &dm->dc_lock) { + dc_exit_ips_for_hw_access(dm->dc); amdgpu_dm_psr_set_event(dm, new_state->stream, true, psr_event_vrr_transition, true); amdgpu_dm_replay_set_event(dm, new_state->stream, true, @@ -9952,6 +9953,7 @@ static void amdgpu_dm_handle_vrr_transition(struct amdgpu_display_manager *dm, __func__, new_state->base.crtc->base.id); scoped_guard(mutex, &dm->dc_lock) { + dc_exit_ips_for_hw_access(dm->dc); amdgpu_dm_psr_set_event(dm, new_state->stream, false, psr_event_vrr_transition, false); amdgpu_dm_replay_set_event(dm, new_state->stream, false, @@ -10253,6 +10255,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, mutex_lock(&dm->dc_lock); acrtc_state->stream->link->psr_settings.psr_dirty_rects_change_timestamp_ns = timestamp_ns; + dc_exit_ips_for_hw_access(dm->dc); amdgpu_dm_psr_set_event(dm, acrtc_state->stream, true, psr_event_hw_programming, true); mutex_unlock(&dm->dc_lock); @@ -10610,6 +10613,7 @@ static void amdgpu_dm_mod_power_update_streams(struct drm_atomic_state *state, */ if (old_crtc_state->active) { scoped_guard(mutex, &dm->dc_lock) { + dc_exit_ips_for_hw_access(dm->dc); amdgpu_dm_psr_set_event(dm, dm_old_crtc_state->stream, true, psr_event_hw_programming, true); amdgpu_dm_replay_set_event(dm, dm_old_crtc_state->stream, true, diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c index 49226d6d0311..4e68a3541639 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c @@ -3163,10 +3163,25 @@ static int replay_get_state(void *data, u64 *val) { struct amdgpu_dm_connector *connector = data; struct dc_link *link = connector->dc_link; + struct amdgpu_device *adev = drm_to_adev(connector->base.dev); + struct dc *dc = adev->dm.dc; uint64_t state = REPLAY_STATE_INVALID; + bool reallow_idle = false; + + mutex_lock(&adev->dm.dc_lock); + + if (dc->idle_optimizations_allowed) { + dc_allow_idle_optimizations(dc, false); + reallow_idle = true; + } dc_link_get_replay_state(link, &state); + if (reallow_idle) + dc_allow_idle_optimizations(dc, true); + + mutex_unlock(&adev->dm.dc_lock); + *val = state; return 0; @@ -3179,10 +3194,26 @@ static int replay_set_residency(void *data, u64 val) { struct amdgpu_dm_connector *connector = data; struct dc_link *link = connector->dc_link; + struct amdgpu_device *adev = drm_to_adev(connector->base.dev); + struct dc *dc = adev->dm.dc; bool is_start = (val != 0); u32 residency = 0; + bool reallow_idle = false; + + mutex_lock(&adev->dm.dc_lock); + + if (dc->idle_optimizations_allowed) { + dc_allow_idle_optimizations(dc, false); + reallow_idle = true; + } link->dc->link_srv->edp_replay_residency(link, &residency, is_start, PR_RESIDENCY_MODE_PHY); + + if (reallow_idle) + dc_allow_idle_optimizations(dc, true); + + mutex_unlock(&adev->dm.dc_lock); + return 0; } @@ -3193,9 +3224,25 @@ static int replay_get_residency(void *data, u64 *val) { struct amdgpu_dm_connector *connector = data; struct dc_link *link = connector->dc_link; + struct amdgpu_device *adev = drm_to_adev(connector->base.dev); + struct dc *dc = adev->dm.dc; u32 residency = 0; + bool reallow_idle = false; + + mutex_lock(&adev->dm.dc_lock); + + if (dc->idle_optimizations_allowed) { + dc_allow_idle_optimizations(dc, false); + reallow_idle = true; + } link->dc->link_srv->edp_replay_residency(link, &residency, false, PR_RESIDENCY_MODE_PHY); + + if (reallow_idle) + dc_allow_idle_optimizations(dc, true); + + mutex_unlock(&adev->dm.dc_lock); + *val = (u64)residency; return 0; @@ -3208,10 +3255,25 @@ static int psr_get(void *data, u64 *val) { struct amdgpu_dm_connector *connector = data; struct dc_link *link = connector->dc_link; + struct amdgpu_device *adev = drm_to_adev(connector->base.dev); + struct dc *dc = adev->dm.dc; enum dc_psr_state state = PSR_STATE0; + bool reallow_idle = false; + + mutex_lock(&adev->dm.dc_lock); + + if (dc->idle_optimizations_allowed) { + dc_allow_idle_optimizations(dc, false); + reallow_idle = true; + } dc_link_get_psr_state(link, &state); + if (reallow_idle) + dc_allow_idle_optimizations(dc, true); + + mutex_unlock(&adev->dm.dc_lock); + *val = state; return 0; @@ -3224,10 +3286,25 @@ static int psr_read_residency(void *data, u64 *val) { struct amdgpu_dm_connector *connector = data; struct dc_link *link = connector->dc_link; + struct amdgpu_device *adev = drm_to_adev(connector->base.dev); + struct dc *dc = adev->dm.dc; u32 residency = 0; + bool reallow_idle = false; + + mutex_lock(&adev->dm.dc_lock); + + if (dc->idle_optimizations_allowed) { + dc_allow_idle_optimizations(dc, false); + reallow_idle = true; + } link->dc->link_srv->edp_get_psr_residency(link, &residency, PSR_RESIDENCY_MODE_PHY); + if (reallow_idle) + dc_allow_idle_optimizations(dc, true); + + mutex_unlock(&adev->dm.dc_lock); + *val = (u64)residency; return 0; -- 2.43.0
