Commit eb3dc897 authored by Nicholas Kazlauskas's avatar Nicholas Kazlauskas Committed by Alex Deucher

drm/amd/display: Use private obj helpers for dm_atomic_state

[Why]
Two non-blocking commits in succession can result in a sequence where
the same dc->current_state is queried for both commits.

1. 1st commit -> check -> commit -> swaps atomic state -> queues work
2. 2nd commit -> check -> commit -> swaps atomic state -> queues work
3. 1st commit work finishes

The issue with this sequence is that the same dc->current_state is
read in both atomic checks. If the first commit modifies streams or
planes those will be missing from the dc->current_state for the
second atomic check. This result in many stream and plane errors in
atomic commit tail.

[How]
The driver still needs to track old to new state to determine if the
commit in its current implementation. Updating the dc_state in
atomic tail is wrong since the dc_state swap should be happening as
part of drm_atomic_helper_swap_state *before* the worker queue kicks
its work off.

The simplest replacement for the subclassing (which doesn't properly
manage the old to new atomic state swap) is to use the drm private
object helpers. While some of the dc_state members could be merged
into dm_crtc_state or dm_plane_state and copied over that way it is
easier for now to just treat the whole dc_state structure as a single
private object.

This allows amdgpu_dm to drop the dc->current_state copy from within
atomic check. It's replaced by a copy from the current atomic state
which is propagated correctly for the sequence described above.

Since access to the dm_state private object is now locked this should
also fix issues that could arise if submitting non-blocking commits
from different threads.

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Signed-off-by: default avatarNicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Reviewed-by: default avatarLeo Li <sunpeng.li@amd.com>
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent 68c12d24
......@@ -123,6 +123,17 @@ struct amdgpu_display_manager {
struct drm_device *ddev;
u16 display_indexes_num;
/**
* @atomic_obj
*
* In combination with &dm_atomic_state it helps manage
* global atomic state that doesn't map cleanly into existing
* drm resources, like &dc_context.
*/
struct drm_private_obj atomic_obj;
struct drm_modeset_lock atomic_obj_lock;
/**
* @irq_handler_list_low_tab:
*
......@@ -254,7 +265,7 @@ struct dm_crtc_state {
#define to_dm_crtc_state(x) container_of(x, struct dm_crtc_state, base)
struct dm_atomic_state {
struct drm_atomic_state base;
struct drm_private_state base;
struct dc_state *context;
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment