From: Bhawanpreet Lakha Date: Fri, 28 Jul 2017 13:11:00 -0400 Subject: drm/amd/display: Avoid full modeset when not required Git-commit: 9b690ef3c70422cdcd0cf912db33f2c92ef4a53f Patch-mainline: v4.15-rc1 References: FATE#326289 FATE#326079 FATE#326049 FATE#322398 FATE#326166 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 Acked-by: Petr Tesarik --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 99 ++++++++++++++-------- 1 file changed, 63 insertions(+), 36 deletions(-) --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -1608,8 +1608,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; @@ -2904,7 +2912,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; } @@ -4087,7 +4096,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); @@ -4536,6 +4545,9 @@ int amdgpu_dm_atomic_check(struct drm_de 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); @@ -4555,23 +4567,50 @@ int amdgpu_dm_atomic_check(struct drm_de crtc_state->active_changed, crtc_state->connectors_changed); - if (modeset_required(crtc_state)) { + if (modereset_required(crtc_state)) { + + /* 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; + } - struct dc_stream *new_stream = NULL; - struct drm_connector_state *conn_state = NULL; - struct dm_connector_state *dm_conn_state = NULL; + } 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 @@ -4579,39 +4618,27 @@ int amdgpu_dm_atomic_check(struct drm_de * 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); + } }