From ac6ec881757478abf15cd357f32514a56a59263e Mon Sep 17 00:00:00 2001 From: Bhawanpreet Lakha Date: Fri, 28 Jul 2017 13:11:00 -0400 Subject: [PATCH 0706/4131] drm/amd/display: Avoid full modeset when not required Fix IGT test case (kms_atomic_transition) -DRM sets the mode_changed flag, while we don't need to do a full modeset. -We want to override the mode_changed flag in this case If we don't do this, we will still bypass the modeset in DC. This will fail to update the new stream_status, causing nullptr at a later stage when trying to access stream_status" We now avoid this by discarding the new_stream instead of partially filling it. Signed-off-by: Bhawanpreet Lakha Reviewed-by: Andrey Grodzovsky Acked-by: Harry Wentland Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 97 +++++++++++++++-------- 1 file changed, 62 insertions(+), 35 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 8f3038f..271786d 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -1613,8 +1613,16 @@ struct dm_connector_state { #define to_dm_connector_state(x)\ container_of((x), struct dm_connector_state, base) -static bool modeset_required(struct drm_crtc_state *crtc_state) +static bool modeset_required(struct drm_crtc_state *crtc_state, + struct dc_stream *new_stream, + struct dc_stream *old_stream) { + if (dc_is_stream_unchanged(new_stream, old_stream)) { + crtc_state->mode_changed = false; + DRM_DEBUG_KMS("Mode change not required, setting mode_changed to %d", + crtc_state->mode_changed); + } + if (!drm_atomic_crtc_needs_modeset(crtc_state)) return false; @@ -3053,7 +3061,8 @@ static int dm_crtc_helper_atomic_check( struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(state); int ret = -EINVAL; - if (unlikely(!dm_crtc_state->stream && modeset_required(state))) { + if (unlikely(!dm_crtc_state->stream && + modeset_required(state, NULL, dm_crtc_state->stream))) { WARN_ON(1); return ret; } @@ -4245,7 +4254,7 @@ void amdgpu_dm_atomic_commit_tail( * aconnector as needed */ - if (modeset_required(new_state)) { + if (modeset_required(new_state, new_acrtc_state->stream, old_acrtc_state->stream)) { DRM_INFO("Atomic commit: SET crtc id %d: [%p]\n", acrtc->crtc_id, acrtc); @@ -4694,6 +4703,9 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, for_each_crtc_in_state(state, crtc, crtc_state, i) { struct amdgpu_crtc *acrtc = NULL; struct amdgpu_connector *aconnector = NULL; + struct dc_stream *new_stream = NULL; + struct drm_connector_state *conn_state = NULL; + struct dm_connector_state *dm_conn_state = NULL; old_acrtc_state = to_dm_crtc_state(crtc->state); new_acrtc_state = to_dm_crtc_state(crtc_state); @@ -4713,23 +4725,50 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, crtc_state->active_changed, crtc_state->connectors_changed); - if (modeset_required(crtc_state)) { + if (modereset_required(crtc_state)) { - struct dc_stream *new_stream = NULL; - struct drm_connector_state *conn_state = NULL; - struct dm_connector_state *dm_conn_state = NULL; + /* i.e. reset mode */ + if (new_acrtc_state->stream) { + set_count = remove_from_val_sets( + set, + set_count, + new_acrtc_state->stream); + + dc_stream_release(new_acrtc_state->stream); + new_acrtc_state->stream = NULL; + + lock_and_validation_needed = true; + } + + } else { if (aconnector) { - conn_state = drm_atomic_get_connector_state(state, &aconnector->base); + conn_state = drm_atomic_get_connector_state(state, + &aconnector->base); + if (IS_ERR(conn_state)) { ret = PTR_ERR_OR_ZERO(conn_state); goto fail; } dm_conn_state = to_dm_connector_state(conn_state); + + new_stream = create_stream_for_sink(aconnector, + &crtc_state->mode, + dm_conn_state); + + if (!new_stream) { + DRM_DEBUG_KMS("%s: Failed to create new stream for crtc %d\n", + __func__, acrtc->base.base.id); + break; + } + + } - new_stream = create_stream_for_sink(aconnector, &crtc_state->mode, dm_conn_state); + if (modeset_required(crtc_state, new_stream, + old_acrtc_state->stream)) { + /* * we can have no stream on ACTION_SET if a display @@ -4737,39 +4776,27 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, * error, the OS will be updated after detection, and * do the right thing on next atomic commit */ - if (!new_stream) { - DRM_DEBUG_KMS("%s: Failed to create new stream for crtc %d\n", - __func__, acrtc->base.base.id); - break; - } - - if (new_acrtc_state->stream) - dc_stream_release(new_acrtc_state->stream); - - new_acrtc_state->stream = new_stream; - - set_count = update_in_val_sets_stream( - set, - set_count, - old_acrtc_state->stream, - new_acrtc_state->stream, - crtc); - lock_and_validation_needed = true; + if (new_acrtc_state->stream) + dc_stream_release(new_acrtc_state->stream); - } else if (modereset_required(crtc_state)) { + new_acrtc_state->stream = new_stream; - /* i.e. reset mode */ - if (new_acrtc_state->stream) { - set_count = remove_from_val_sets( + set_count = update_in_val_sets_stream( set, set_count, - new_acrtc_state->stream); - - dc_stream_release(new_acrtc_state->stream); - new_acrtc_state->stream = NULL; + old_acrtc_state->stream, + new_acrtc_state->stream, + crtc); lock_and_validation_needed = true; + } else { + /* + * The new stream is unused, so we release it + */ + if (new_stream) + dc_stream_release(new_stream); + } } -- 2.7.4