Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

drm/amd/display: Do not skip unrelated mode changes in DSC validation

Starting with commit 17ce8a6907f7 ("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: 17ce8a6907f7 ("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 cc7c7121ae082b7b82891baa7280f1ff2608f22b)

authored by

Yussuf Khalil and committed by
Alex Deucher
aed3d041 c3692998

+9 -1
+5
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
··· 12524 12524 12525 12525 if (dc_resource_is_dsc_encoding_supported(dc)) { 12526 12526 for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { 12527 + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state); 12528 + dm_new_crtc_state->mode_changed_independent_from_dsc = new_crtc_state->mode_changed; 12529 + } 12530 + 12531 + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { 12527 12532 if (drm_atomic_crtc_needs_modeset(new_crtc_state)) { 12528 12533 ret = add_affected_mst_dsc_crtcs(state, crtc); 12529 12534 if (ret) {
+1
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
··· 984 984 985 985 bool freesync_vrr_info_changed; 986 986 987 + bool mode_changed_independent_from_dsc; 987 988 bool dsc_force_changed; 988 989 bool vrr_supported; 989 990 struct mod_freesync_config freesync_config;
+3 -1
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
··· 1744 1744 int ind = find_crtc_index_in_state_by_stream(state, stream); 1745 1745 1746 1746 if (ind >= 0) { 1747 + struct dm_crtc_state *dm_new_crtc_state = to_dm_crtc_state(state->crtcs[ind].new_state); 1748 + 1747 1749 DRM_INFO_ONCE("%s:%d MST_DSC no mode changed for stream 0x%p\n", 1748 1750 __func__, __LINE__, stream); 1749 - state->crtcs[ind].new_state->mode_changed = 0; 1751 + dm_new_crtc_state->base.mode_changed = dm_new_crtc_state->mode_changed_independent_from_dsc; 1750 1752 } 1751 1753 } 1752 1754 }