Commit 9b690ef3 authored by Bhawanpreet Lakha's avatar Bhawanpreet Lakha Committed by Alex Deucher

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: default avatarBhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
Reviewed-by: default avatarAndrey Grodzovsky <Andrey.Grodzovsky@amd.com>
Acked-by: default avatarHarry Wentland <Harry.Wentland@amd.com>
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent d54d29db
...@@ -1608,8 +1608,16 @@ struct dm_connector_state { ...@@ -1608,8 +1608,16 @@ struct dm_connector_state {
#define to_dm_connector_state(x)\ #define to_dm_connector_state(x)\
container_of((x), struct dm_connector_state, base) 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)) if (!drm_atomic_crtc_needs_modeset(crtc_state))
return false; return false;
...@@ -2904,7 +2912,8 @@ static int dm_crtc_helper_atomic_check( ...@@ -2904,7 +2912,8 @@ static int dm_crtc_helper_atomic_check(
struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(state); struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(state);
int ret = -EINVAL; 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); WARN_ON(1);
return ret; return ret;
} }
...@@ -4087,7 +4096,7 @@ void amdgpu_dm_atomic_commit_tail( ...@@ -4087,7 +4096,7 @@ void amdgpu_dm_atomic_commit_tail(
* aconnector as needed * 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); 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_device *dev, ...@@ -4536,6 +4545,9 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
for_each_crtc_in_state(state, crtc, crtc_state, i) { for_each_crtc_in_state(state, crtc, crtc_state, i) {
struct amdgpu_crtc *acrtc = NULL; struct amdgpu_crtc *acrtc = NULL;
struct amdgpu_connector *aconnector = 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); old_acrtc_state = to_dm_crtc_state(crtc->state);
new_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_device *dev, ...@@ -4555,23 +4567,50 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
crtc_state->active_changed, crtc_state->active_changed,
crtc_state->connectors_changed); crtc_state->connectors_changed);
if (modeset_required(crtc_state)) { if (modereset_required(crtc_state)) {
struct dc_stream *new_stream = NULL; /* i.e. reset mode */
struct drm_connector_state *conn_state = NULL; if (new_acrtc_state->stream) {
struct dm_connector_state *dm_conn_state = NULL; 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) { 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)) { if (IS_ERR(conn_state)) {
ret = PTR_ERR_OR_ZERO(conn_state); ret = PTR_ERR_OR_ZERO(conn_state);
goto fail; goto fail;
} }
dm_conn_state = to_dm_connector_state(conn_state); 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 * we can have no stream on ACTION_SET if a display
...@@ -4579,39 +4618,27 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, ...@@ -4579,39 +4618,27 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
* error, the OS will be updated after detection, and * error, the OS will be updated after detection, and
* do the right thing on next atomic commit * 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 */ set_count = update_in_val_sets_stream(
if (new_acrtc_state->stream) {
set_count = remove_from_val_sets(
set, set,
set_count, set_count,
new_acrtc_state->stream); old_acrtc_state->stream,
new_acrtc_state->stream,
dc_stream_release(new_acrtc_state->stream); crtc);
new_acrtc_state->stream = NULL;
lock_and_validation_needed = true; lock_and_validation_needed = true;
} else {
/*
* The new stream is unused, so we release it
*/
if (new_stream)
dc_stream_release(new_stream);
} }
} }
......
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