Skip to content

Commit 21159d8

Browse files
pp3345gregkh
authored andcommitted
drm/amd/display: Do not skip unrelated mode changes in DSC validation
[ Upstream commit aed3d04 ] Starting with commit 17ce8a6 ("drm/amd/display: Add dsc pre-validation in atomic check"), amdgpu resets the CRTC state mode_changed flag to false when recomputing the DSC configuration results in no timing change for a particular stream. However, this is incorrect in scenarios where a change in MST/DSC configuration happens in the same KMS commit as another (unrelated) mode change. For example, the integrated panel of a laptop may be configured differently (e.g., HDR enabled/disabled) depending on whether external screens are attached. In this case, plugging in external DP-MST screens may result in the mode_changed flag being dropped incorrectly for the integrated panel if its DSC configuration did not change during precomputation in pre_validate_dsc(). At this point, however, dm_update_crtc_state() has already created new streams for CRTCs with DSC-independent mode changes. In turn, amdgpu_dm_commit_streams() will never release the old stream, resulting in a memory leak. amdgpu_dm_atomic_commit_tail() will never acquire a reference to the new stream either, which manifests as a use-after-free when the stream gets disabled later on: BUG: KASAN: use-after-free in dc_stream_release+0x25/0x90 [amdgpu] Write of size 4 at addr ffff88813d836524 by task kworker/9:9/29977 Workqueue: events drm_mode_rmfb_work_fn Call Trace: <TASK> dump_stack_lvl+0x6e/0xa0 print_address_description.constprop.0+0x88/0x320 ? dc_stream_release+0x25/0x90 [amdgpu] print_report+0xfc/0x1ff ? srso_alias_return_thunk+0x5/0xfbef5 ? __virt_addr_valid+0x225/0x4e0 ? dc_stream_release+0x25/0x90 [amdgpu] kasan_report+0xe1/0x180 ? dc_stream_release+0x25/0x90 [amdgpu] kasan_check_range+0x125/0x200 dc_stream_release+0x25/0x90 [amdgpu] dc_state_destruct+0x14d/0x5c0 [amdgpu] dc_state_release.part.0+0x4e/0x130 [amdgpu] dm_atomic_destroy_state+0x3f/0x70 [amdgpu] drm_atomic_state_default_clear+0x8ee/0xf30 ? drm_mode_object_put.part.0+0xb1/0x130 __drm_atomic_state_free+0x15c/0x2d0 atomic_remove_fb+0x67e/0x980 Since there is no reliable way of figuring out whether a CRTC has unrelated mode changes pending at the time of DSC validation, remember the value of the mode_changed flag from before the point where a CRTC was marked as potentially affected by a change in DSC configuration. Reset the mode_changed flag to this earlier value instead in pre_validate_dsc(). Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5004 Fixes: 17ce8a6 ("drm/amd/display: Add dsc pre-validation in atomic check") Signed-off-by: Yussuf Khalil <dev@pp3345.net> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> (cherry picked from commit cc7c712) Signed-off-by: Fang Wang <32840572@qq.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent c79cf42 commit 21159d8

3 files changed

Lines changed: 11 additions & 2 deletions

File tree

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10152,6 +10152,11 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
1015210152
}
1015310153

1015410154
if (dc_resource_is_dsc_encoding_supported(dc)) {
10155+
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
10156+
dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
10157+
dm_new_crtc_state->mode_changed_independent_from_dsc = new_crtc_state->mode_changed;
10158+
}
10159+
1015510160
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
1015610161
if (drm_atomic_crtc_needs_modeset(new_crtc_state)) {
1015710162
ret = add_affected_mst_dsc_crtcs(state, crtc);

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,7 @@ struct dm_crtc_state {
737737

738738
bool freesync_vrr_info_changed;
739739

740+
bool mode_changed_independent_from_dsc;
740741
bool dsc_force_changed;
741742
bool vrr_supported;
742743
struct mod_freesync_config freesync_config;

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1587,8 +1587,11 @@ int pre_validate_dsc(struct drm_atomic_state *state,
15871587
} else {
15881588
int ind = find_crtc_index_in_state_by_stream(state, stream);
15891589

1590-
if (ind >= 0)
1591-
state->crtcs[ind].new_state->mode_changed = 0;
1590+
if (ind >= 0) {
1591+
struct dm_crtc_state *dm_new_crtc_state = to_dm_crtc_state(state->crtcs[ind].new_state);
1592+
1593+
dm_new_crtc_state->base.mode_changed = dm_new_crtc_state->mode_changed_independent_from_dsc;
1594+
}
15921595
}
15931596
}
15941597
clean_exit:

0 commit comments

Comments
 (0)