* [PATCH 1/2] drm/amd/display: Move iteration out of dm_update_planes
@ 2019-01-07 15:53 sunpeng.li-5C7GfCeVMHo
[not found] ` <1546876402-12812-1-git-send-email-sunpeng.li-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 3+ messages in thread
From: sunpeng.li-5C7GfCeVMHo @ 2019-01-07 15:53 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Leo Li
From: Leo Li <sunpeng.li@amd.com>
[Why]
To reduce indentation of dm_update_planes, and to make it operate on
single plane instances.
[How]
Move iteration of plane states into atomic_check.
No functional change is intended.
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 220 +++++++++++-----------
1 file changed, 114 insertions(+), 106 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 4877773..0b6bce5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5769,145 +5769,141 @@ static int dm_update_crtcs_state(struct amdgpu_display_manager *dm,
return ret;
}
-static int dm_update_planes_state(struct dc *dc,
- struct drm_atomic_state *state,
- bool enable,
- bool *lock_and_validation_needed)
+static int dm_update_plane_state(struct dc *dc,
+ struct drm_atomic_state *state,
+ struct drm_plane *plane,
+ struct drm_plane_state *old_plane_state,
+ struct drm_plane_state *new_plane_state,
+ bool enable,
+ bool *lock_and_validation_needed)
{
struct dm_atomic_state *dm_state = NULL;
struct drm_crtc *new_plane_crtc, *old_plane_crtc;
struct drm_crtc_state *old_crtc_state, *new_crtc_state;
- struct drm_plane *plane;
- struct drm_plane_state *old_plane_state, *new_plane_state;
struct dm_crtc_state *dm_new_crtc_state, *dm_old_crtc_state;
struct dm_plane_state *dm_new_plane_state, *dm_old_plane_state;
- int i ;
/* TODO return page_flip_needed() function */
bool pflip_needed = !state->allow_modeset;
int ret = 0;
- /* Add new planes, in reverse order as DC expectation */
- for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, new_plane_state, i) {
- new_plane_crtc = new_plane_state->crtc;
- old_plane_crtc = old_plane_state->crtc;
- dm_new_plane_state = to_dm_plane_state(new_plane_state);
- dm_old_plane_state = to_dm_plane_state(old_plane_state);
+ new_plane_crtc = new_plane_state->crtc;
+ old_plane_crtc = old_plane_state->crtc;
+ dm_new_plane_state = to_dm_plane_state(new_plane_state);
+ dm_old_plane_state = to_dm_plane_state(old_plane_state);
- /*TODO Implement atomic check for cursor plane */
- if (plane->type == DRM_PLANE_TYPE_CURSOR)
- continue;
+ /*TODO Implement atomic check for cursor plane */
+ if (plane->type == DRM_PLANE_TYPE_CURSOR)
+ return 0;
- /* Remove any changed/removed planes */
- if (!enable) {
- if (pflip_needed &&
- plane->type != DRM_PLANE_TYPE_OVERLAY)
- continue;
+ /* Remove any changed/removed planes */
+ if (!enable) {
+ if (pflip_needed &&
+ plane->type != DRM_PLANE_TYPE_OVERLAY)
+ return 0;
- if (!old_plane_crtc)
- continue;
+ if (!old_plane_crtc)
+ return 0;
- old_crtc_state = drm_atomic_get_old_crtc_state(
- state, old_plane_crtc);
- dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
+ old_crtc_state = drm_atomic_get_old_crtc_state(
+ state, old_plane_crtc);
+ dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
- if (!dm_old_crtc_state->stream)
- continue;
+ if (!dm_old_crtc_state->stream)
+ return 0;
- DRM_DEBUG_ATOMIC("Disabling DRM plane: %d on DRM crtc %d\n",
- plane->base.id, old_plane_crtc->base.id);
+ DRM_DEBUG_ATOMIC("Disabling DRM plane: %d on DRM crtc %d\n",
+ plane->base.id, old_plane_crtc->base.id);
- ret = dm_atomic_get_state(state, &dm_state);
- if (ret)
- return ret;
+ ret = dm_atomic_get_state(state, &dm_state);
+ if (ret)
+ return ret;
- if (!dc_remove_plane_from_context(
- dc,
- dm_old_crtc_state->stream,
- dm_old_plane_state->dc_state,
- dm_state->context)) {
+ if (!dc_remove_plane_from_context(
+ dc,
+ dm_old_crtc_state->stream,
+ dm_old_plane_state->dc_state,
+ dm_state->context)) {
- ret = EINVAL;
- return ret;
- }
+ ret = EINVAL;
+ return ret;
+ }
- dc_plane_state_release(dm_old_plane_state->dc_state);
- dm_new_plane_state->dc_state = NULL;
+ dc_plane_state_release(dm_old_plane_state->dc_state);
+ dm_new_plane_state->dc_state = NULL;
- *lock_and_validation_needed = true;
+ *lock_and_validation_needed = true;
- } else { /* Add new planes */
- struct dc_plane_state *dc_new_plane_state;
+ } else { /* Add new planes */
+ struct dc_plane_state *dc_new_plane_state;
- if (drm_atomic_plane_disabling(plane->state, new_plane_state))
- continue;
+ if (drm_atomic_plane_disabling(plane->state, new_plane_state))
+ return 0;
- if (!new_plane_crtc)
- continue;
+ if (!new_plane_crtc)
+ return 0;
- new_crtc_state = drm_atomic_get_new_crtc_state(state, new_plane_crtc);
- dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+ new_crtc_state = drm_atomic_get_new_crtc_state(state, new_plane_crtc);
+ dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
- if (!dm_new_crtc_state->stream)
- continue;
+ if (!dm_new_crtc_state->stream)
+ return 0;
- if (pflip_needed &&
- plane->type != DRM_PLANE_TYPE_OVERLAY)
- continue;
+ if (pflip_needed && plane->type != DRM_PLANE_TYPE_OVERLAY)
+ return 0;
- WARN_ON(dm_new_plane_state->dc_state);
+ WARN_ON(dm_new_plane_state->dc_state);
- dc_new_plane_state = dc_create_plane_state(dc);
- if (!dc_new_plane_state)
- return -ENOMEM;
+ dc_new_plane_state = dc_create_plane_state(dc);
+ if (!dc_new_plane_state)
+ return -ENOMEM;
- DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc %d\n",
- plane->base.id, new_plane_crtc->base.id);
+ DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc %d\n",
+ plane->base.id, new_plane_crtc->base.id);
- ret = fill_plane_attributes(
- new_plane_crtc->dev->dev_private,
- dc_new_plane_state,
- new_plane_state,
- new_crtc_state);
- if (ret) {
- dc_plane_state_release(dc_new_plane_state);
- return ret;
- }
+ ret = fill_plane_attributes(
+ new_plane_crtc->dev->dev_private,
+ dc_new_plane_state,
+ new_plane_state,
+ new_crtc_state);
+ if (ret) {
+ dc_plane_state_release(dc_new_plane_state);
+ return ret;
+ }
- ret = dm_atomic_get_state(state, &dm_state);
- if (ret) {
- dc_plane_state_release(dc_new_plane_state);
- return ret;
- }
+ ret = dm_atomic_get_state(state, &dm_state);
+ if (ret) {
+ dc_plane_state_release(dc_new_plane_state);
+ return ret;
+ }
- /*
- * Any atomic check errors that occur after this will
- * not need a release. The plane state will be attached
- * to the stream, and therefore part of the atomic
- * state. It'll be released when the atomic state is
- * cleaned.
- */
- if (!dc_add_plane_to_context(
- dc,
- dm_new_crtc_state->stream,
- dc_new_plane_state,
- dm_state->context)) {
-
- dc_plane_state_release(dc_new_plane_state);
- return -EINVAL;
- }
+ /*
+ * Any atomic check errors that occur after this will
+ * not need a release. The plane state will be attached
+ * to the stream, and therefore part of the atomic
+ * state. It'll be released when the atomic state is
+ * cleaned.
+ */
+ if (!dc_add_plane_to_context(
+ dc,
+ dm_new_crtc_state->stream,
+ dc_new_plane_state,
+ dm_state->context)) {
- dm_new_plane_state->dc_state = dc_new_plane_state;
+ dc_plane_state_release(dc_new_plane_state);
+ return -EINVAL;
+ }
- /* Tell DC to do a full surface update every time there
- * is a plane change. Inefficient, but works for now.
- */
- dm_new_plane_state->dc_state->update_flags.bits.full_update = 1;
+ dm_new_plane_state->dc_state = dc_new_plane_state;
- *lock_and_validation_needed = true;
- }
+ /* Tell DC to do a full surface update every time there
+ * is a plane change. Inefficient, but works for now.
+ */
+ dm_new_plane_state->dc_state->update_flags.bits.full_update = 1;
+
+ *lock_and_validation_needed = true;
}
@@ -6065,6 +6061,8 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
struct drm_connector_state *old_con_state, *new_con_state;
struct drm_crtc *crtc;
struct drm_crtc_state *old_crtc_state, *new_crtc_state;
+ struct drm_plane *plane;
+ struct drm_plane_state *old_plane_state, *new_plane_state;
enum surface_update_type update_type = UPDATE_TYPE_FAST;
enum surface_update_type overall_update_type = UPDATE_TYPE_FAST;
@@ -6099,9 +6097,14 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
}
/* Remove exiting planes if they are modified */
- ret = dm_update_planes_state(dc, state, false, &lock_and_validation_needed);
- if (ret) {
- goto fail;
+ for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, new_plane_state, i) {
+ ret = dm_update_plane_state(dc, state, plane,
+ old_plane_state,
+ new_plane_state,
+ false,
+ &lock_and_validation_needed);
+ if (ret)
+ goto fail;
}
/* Disable all crtcs which require disable */
@@ -6117,9 +6120,14 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
}
/* Add new/modified planes */
- ret = dm_update_planes_state(dc, state, true, &lock_and_validation_needed);
- if (ret) {
- goto fail;
+ for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, new_plane_state, i) {
+ ret = dm_update_plane_state(dc, state, plane,
+ old_plane_state,
+ new_plane_state,
+ true,
+ &lock_and_validation_needed);
+ if (ret)
+ goto fail;
}
/* Run this here since we want to validate the streams we created */
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] drm/amd/display: Move iteration out of dm_update_crtcs
[not found] ` <1546876402-12812-1-git-send-email-sunpeng.li-5C7GfCeVMHo@public.gmane.org>
@ 2019-01-07 15:53 ` sunpeng.li-5C7GfCeVMHo
[not found] ` <1546876402-12812-2-git-send-email-sunpeng.li-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 3+ messages in thread
From: sunpeng.li-5C7GfCeVMHo @ 2019-01-07 15:53 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Leo Li
From: Leo Li <sunpeng.li@amd.com>
[Why]
To reduce indent in dm_update_crtcs, and to make it operate on single
instances.
[How]
Move iteration of plane states into atomic_check.
No functional change is intended.
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 341 +++++++++++-----------
1 file changed, 175 insertions(+), 166 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 0b6bce5..70cd8bc 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5550,15 +5550,15 @@ static void reset_freesync_config_for_crtc(
sizeof(new_crtc_state->vrr_infopacket));
}
-static int dm_update_crtcs_state(struct amdgpu_display_manager *dm,
- struct drm_atomic_state *state,
- bool enable,
- bool *lock_and_validation_needed)
+static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
+ struct drm_atomic_state *state,
+ struct drm_crtc *crtc,
+ struct drm_crtc_state *old_crtc_state,
+ struct drm_crtc_state *new_crtc_state,
+ bool enable,
+ bool *lock_and_validation_needed)
{
struct dm_atomic_state *dm_state = NULL;
- struct drm_crtc *crtc;
- struct drm_crtc_state *old_crtc_state, *new_crtc_state;
- int i;
struct dm_crtc_state *dm_old_crtc_state, *dm_new_crtc_state;
struct dc_stream_state *new_stream;
int ret = 0;
@@ -5567,200 +5567,199 @@ static int dm_update_crtcs_state(struct amdgpu_display_manager *dm,
* TODO Move this code into dm_crtc_atomic_check once we get rid of dc_validation_set
* update changed items
*/
- for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
- struct amdgpu_crtc *acrtc = NULL;
- struct amdgpu_dm_connector *aconnector = NULL;
- struct drm_connector_state *drm_new_conn_state = NULL, *drm_old_conn_state = NULL;
- struct dm_connector_state *dm_new_conn_state = NULL, *dm_old_conn_state = NULL;
- struct drm_plane_state *new_plane_state = NULL;
+ struct amdgpu_crtc *acrtc = NULL;
+ struct amdgpu_dm_connector *aconnector = NULL;
+ struct drm_connector_state *drm_new_conn_state = NULL, *drm_old_conn_state = NULL;
+ struct dm_connector_state *dm_new_conn_state = NULL, *dm_old_conn_state = NULL;
+ struct drm_plane_state *new_plane_state = NULL;
- new_stream = NULL;
+ new_stream = NULL;
- dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
- dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
- acrtc = to_amdgpu_crtc(crtc);
+ dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
+ dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+ acrtc = to_amdgpu_crtc(crtc);
- new_plane_state = drm_atomic_get_new_plane_state(state, new_crtc_state->crtc->primary);
+ new_plane_state = drm_atomic_get_new_plane_state(state, new_crtc_state->crtc->primary);
- if (new_crtc_state->enable && new_plane_state && !new_plane_state->fb) {
- ret = -EINVAL;
- goto fail;
- }
+ if (new_crtc_state->enable && new_plane_state && !new_plane_state->fb) {
+ ret = -EINVAL;
+ goto fail;
+ }
- aconnector = amdgpu_dm_find_first_crtc_matching_connector(state, crtc);
+ aconnector = amdgpu_dm_find_first_crtc_matching_connector(state, crtc);
- /* TODO This hack should go away */
- if (aconnector && enable) {
- /* Make sure fake sink is created in plug-in scenario */
- drm_new_conn_state = drm_atomic_get_new_connector_state(state,
- &aconnector->base);
- drm_old_conn_state = drm_atomic_get_old_connector_state(state,
- &aconnector->base);
+ /* TODO This hack should go away */
+ if (aconnector && enable) {
+ /* Make sure fake sink is created in plug-in scenario */
+ drm_new_conn_state = drm_atomic_get_new_connector_state(state,
+ &aconnector->base);
+ drm_old_conn_state = drm_atomic_get_old_connector_state(state,
+ &aconnector->base);
- if (IS_ERR(drm_new_conn_state)) {
- ret = PTR_ERR_OR_ZERO(drm_new_conn_state);
- break;
- }
+ if (IS_ERR(drm_new_conn_state)) {
+ ret = PTR_ERR_OR_ZERO(drm_new_conn_state);
+ goto fail;
+ }
- dm_new_conn_state = to_dm_connector_state(drm_new_conn_state);
- dm_old_conn_state = to_dm_connector_state(drm_old_conn_state);
+ dm_new_conn_state = to_dm_connector_state(drm_new_conn_state);
+ dm_old_conn_state = to_dm_connector_state(drm_old_conn_state);
- new_stream = create_stream_for_sink(aconnector,
- &new_crtc_state->mode,
- dm_new_conn_state,
- dm_old_crtc_state->stream);
+ new_stream = create_stream_for_sink(aconnector,
+ &new_crtc_state->mode,
+ dm_new_conn_state,
+ dm_old_crtc_state->stream);
- /*
- * we can have no stream on ACTION_SET if a display
- * was disconnected during S3, in this case it is not an
- * error, the OS will be updated after detection, and
- * will do the right thing on next atomic commit
- */
+ /*
+ * we can have no stream on ACTION_SET if a display
+ * was disconnected during S3, in this case it is not an
+ * error, the OS will be updated after detection, and
+ * will do the right thing on next atomic commit
+ */
- if (!new_stream) {
- DRM_DEBUG_DRIVER("%s: Failed to create new stream for crtc %d\n",
- __func__, acrtc->base.base.id);
- break;
- }
+ if (!new_stream) {
+ DRM_DEBUG_DRIVER("%s: Failed to create new stream for crtc %d\n",
+ __func__, acrtc->base.base.id);
+ ret = -ENOMEM;
+ goto fail;
+ }
- dm_new_crtc_state->abm_level = dm_new_conn_state->abm_level;
+ dm_new_crtc_state->abm_level = dm_new_conn_state->abm_level;
- if (dc_is_stream_unchanged(new_stream, dm_old_crtc_state->stream) &&
- dc_is_stream_scaling_unchanged(new_stream, dm_old_crtc_state->stream)) {
- new_crtc_state->mode_changed = false;
- DRM_DEBUG_DRIVER("Mode change not required, setting mode_changed to %d",
- new_crtc_state->mode_changed);
- }
+ if (dc_is_stream_unchanged(new_stream, dm_old_crtc_state->stream) &&
+ dc_is_stream_scaling_unchanged(new_stream, dm_old_crtc_state->stream)) {
+ new_crtc_state->mode_changed = false;
+ DRM_DEBUG_DRIVER("Mode change not required, setting mode_changed to %d",
+ new_crtc_state->mode_changed);
}
+ }
- if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
- goto next_crtc;
-
- DRM_DEBUG_DRIVER(
- "amdgpu_crtc id:%d crtc_state_flags: enable:%d, active:%d, "
- "planes_changed:%d, mode_changed:%d,active_changed:%d,"
- "connectors_changed:%d\n",
- acrtc->crtc_id,
- new_crtc_state->enable,
- new_crtc_state->active,
- new_crtc_state->planes_changed,
- new_crtc_state->mode_changed,
- new_crtc_state->active_changed,
- new_crtc_state->connectors_changed);
+ if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
+ goto skip_modeset;
- /* Remove stream for any changed/disabled CRTC */
- if (!enable) {
+ DRM_DEBUG_DRIVER(
+ "amdgpu_crtc id:%d crtc_state_flags: enable:%d, active:%d, "
+ "planes_changed:%d, mode_changed:%d,active_changed:%d,"
+ "connectors_changed:%d\n",
+ acrtc->crtc_id,
+ new_crtc_state->enable,
+ new_crtc_state->active,
+ new_crtc_state->planes_changed,
+ new_crtc_state->mode_changed,
+ new_crtc_state->active_changed,
+ new_crtc_state->connectors_changed);
- if (!dm_old_crtc_state->stream)
- goto next_crtc;
+ /* Remove stream for any changed/disabled CRTC */
+ if (!enable) {
- ret = dm_atomic_get_state(state, &dm_state);
- if (ret)
- goto fail;
+ if (!dm_old_crtc_state->stream)
+ goto skip_modeset;
- DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
- crtc->base.id);
+ ret = dm_atomic_get_state(state, &dm_state);
+ if (ret)
+ goto fail;
- /* i.e. reset mode */
- if (dc_remove_stream_from_ctx(
- dm->dc,
- dm_state->context,
- dm_old_crtc_state->stream) != DC_OK) {
- ret = -EINVAL;
- goto fail;
- }
+ DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
+ crtc->base.id);
- dc_stream_release(dm_old_crtc_state->stream);
- dm_new_crtc_state->stream = NULL;
+ /* i.e. reset mode */
+ if (dc_remove_stream_from_ctx(
+ dm->dc,
+ dm_state->context,
+ dm_old_crtc_state->stream) != DC_OK) {
+ ret = -EINVAL;
+ goto fail;
+ }
- reset_freesync_config_for_crtc(dm_new_crtc_state);
+ dc_stream_release(dm_old_crtc_state->stream);
+ dm_new_crtc_state->stream = NULL;
- *lock_and_validation_needed = true;
+ reset_freesync_config_for_crtc(dm_new_crtc_state);
- } else {/* Add stream for any updated/enabled CRTC */
- /*
- * Quick fix to prevent NULL pointer on new_stream when
- * added MST connectors not found in existing crtc_state in the chained mode
- * TODO: need to dig out the root cause of that
- */
- if (!aconnector || (!aconnector->dc_sink && aconnector->mst_port))
- goto next_crtc;
+ *lock_and_validation_needed = true;
- if (modereset_required(new_crtc_state))
- goto next_crtc;
+ } else {/* Add stream for any updated/enabled CRTC */
+ /*
+ * Quick fix to prevent NULL pointer on new_stream when
+ * added MST connectors not found in existing crtc_state in the chained mode
+ * TODO: need to dig out the root cause of that
+ */
+ if (!aconnector || (!aconnector->dc_sink && aconnector->mst_port))
+ goto skip_modeset;
- if (modeset_required(new_crtc_state, new_stream,
- dm_old_crtc_state->stream)) {
+ if (modereset_required(new_crtc_state))
+ goto skip_modeset;
- WARN_ON(dm_new_crtc_state->stream);
+ if (modeset_required(new_crtc_state, new_stream,
+ dm_old_crtc_state->stream)) {
- ret = dm_atomic_get_state(state, &dm_state);
- if (ret)
- goto fail;
+ WARN_ON(dm_new_crtc_state->stream);
- dm_new_crtc_state->stream = new_stream;
+ ret = dm_atomic_get_state(state, &dm_state);
+ if (ret)
+ goto fail;
- dc_stream_retain(new_stream);
+ dm_new_crtc_state->stream = new_stream;
- DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
- crtc->base.id);
+ dc_stream_retain(new_stream);
- if (dc_add_stream_to_ctx(
- dm->dc,
- dm_state->context,
- dm_new_crtc_state->stream) != DC_OK) {
- ret = -EINVAL;
- goto fail;
- }
+ DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
+ crtc->base.id);
- *lock_and_validation_needed = true;
+ if (dc_add_stream_to_ctx(
+ dm->dc,
+ dm_state->context,
+ dm_new_crtc_state->stream) != DC_OK) {
+ ret = -EINVAL;
+ goto fail;
}
- }
-next_crtc:
- /* Release extra reference */
- if (new_stream)
- dc_stream_release(new_stream);
+ *lock_and_validation_needed = true;
+ }
+ }
- /*
- * We want to do dc stream updates that do not require a
- * full modeset below.
- */
- if (!(enable && aconnector && new_crtc_state->enable &&
- new_crtc_state->active))
- continue;
- /*
- * Given above conditions, the dc state cannot be NULL because:
- * 1. We're in the process of enabling CRTCs (just been added
- * to the dc context, or already is on the context)
- * 2. Has a valid connector attached, and
- * 3. Is currently active and enabled.
- * => The dc stream state currently exists.
- */
- BUG_ON(dm_new_crtc_state->stream == NULL);
+skip_modeset:
+ /* Release extra reference */
+ if (new_stream)
+ dc_stream_release(new_stream);
- /* Scaling or underscan settings */
- if (is_scaling_state_different(dm_old_conn_state, dm_new_conn_state))
- update_stream_scaling_settings(
- &new_crtc_state->mode, dm_new_conn_state, dm_new_crtc_state->stream);
+ /*
+ * We want to do dc stream updates that do not require a
+ * full modeset below.
+ */
+ if (!(enable && aconnector && new_crtc_state->enable &&
+ new_crtc_state->active))
+ return 0;
+ /*
+ * Given above conditions, the dc state cannot be NULL because:
+ * 1. We're in the process of enabling CRTCs (just been added
+ * to the dc context, or already is on the context)
+ * 2. Has a valid connector attached, and
+ * 3. Is currently active and enabled.
+ * => The dc stream state currently exists.
+ */
+ BUG_ON(dm_new_crtc_state->stream == NULL);
- /*
- * Color management settings. We also update color properties
- * when a modeset is needed, to ensure it gets reprogrammed.
- */
- if (dm_new_crtc_state->base.color_mgmt_changed ||
- drm_atomic_crtc_needs_modeset(new_crtc_state)) {
- ret = amdgpu_dm_set_regamma_lut(dm_new_crtc_state);
- if (ret)
- goto fail;
- amdgpu_dm_set_ctm(dm_new_crtc_state);
- }
+ /* Scaling or underscan settings */
+ if (is_scaling_state_different(dm_old_conn_state, dm_new_conn_state))
+ update_stream_scaling_settings(
+ &new_crtc_state->mode, dm_new_conn_state, dm_new_crtc_state->stream);
- /* Update Freesync settings. */
- get_freesync_config_for_crtc(dm_new_crtc_state,
- dm_new_conn_state);
+ /*
+ * Color management settings. We also update color properties
+ * when a modeset is needed, to ensure it gets reprogrammed.
+ */
+ if (dm_new_crtc_state->base.color_mgmt_changed ||
+ drm_atomic_crtc_needs_modeset(new_crtc_state)) {
+ ret = amdgpu_dm_set_regamma_lut(dm_new_crtc_state);
+ if (ret)
+ goto fail;
+ amdgpu_dm_set_ctm(dm_new_crtc_state);
}
+ /* Update Freesync settings. */
+ get_freesync_config_for_crtc(dm_new_crtc_state,
+ dm_new_conn_state);
+
return ret;
fail:
@@ -6108,15 +6107,25 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
}
/* Disable all crtcs which require disable */
- ret = dm_update_crtcs_state(&adev->dm, state, false, &lock_and_validation_needed);
- if (ret) {
- goto fail;
+ for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+ ret = dm_update_crtc_state(&adev->dm, state, crtc,
+ old_crtc_state,
+ new_crtc_state,
+ false,
+ &lock_and_validation_needed);
+ if (ret)
+ goto fail;
}
/* Enable all crtcs which require enable */
- ret = dm_update_crtcs_state(&adev->dm, state, true, &lock_and_validation_needed);
- if (ret) {
- goto fail;
+ for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+ ret = dm_update_crtc_state(&adev->dm, state, crtc,
+ old_crtc_state,
+ new_crtc_state,
+ true,
+ &lock_and_validation_needed);
+ if (ret)
+ goto fail;
}
/* Add new/modified planes */
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] drm/amd/display: Move iteration out of dm_update_crtcs
[not found] ` <1546876402-12812-2-git-send-email-sunpeng.li-5C7GfCeVMHo@public.gmane.org>
@ 2019-01-07 18:07 ` Kazlauskas, Nicholas
0 siblings, 0 replies; 3+ messages in thread
From: Kazlauskas, Nicholas @ 2019-01-07 18:07 UTC (permalink / raw)
To: Li, Sun peng (Leo), amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On 1/7/19 10:53 AM, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@amd.com>
>
> [Why]
> To reduce indent in dm_update_crtcs, and to make it operate on single
> instances.
>
> [How]
> Move iteration of plane states into atomic_check.
> No functional change is intended.
>
> Signed-off-by: Leo Li <sunpeng.li@amd.com>
Series is:
Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Looks good to me. I guess we don't need the other two patches from the
last series if we're not going to be making use of the helper.
Nicholas Kazlauskas
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 341 +++++++++++-----------
> 1 file changed, 175 insertions(+), 166 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 0b6bce5..70cd8bc 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5550,15 +5550,15 @@ static void reset_freesync_config_for_crtc(
> sizeof(new_crtc_state->vrr_infopacket));
> }
>
> -static int dm_update_crtcs_state(struct amdgpu_display_manager *dm,
> - struct drm_atomic_state *state,
> - bool enable,
> - bool *lock_and_validation_needed)
> +static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
> + struct drm_atomic_state *state,
> + struct drm_crtc *crtc,
> + struct drm_crtc_state *old_crtc_state,
> + struct drm_crtc_state *new_crtc_state,
> + bool enable,
> + bool *lock_and_validation_needed)
> {
> struct dm_atomic_state *dm_state = NULL;
> - struct drm_crtc *crtc;
> - struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> - int i;
> struct dm_crtc_state *dm_old_crtc_state, *dm_new_crtc_state;
> struct dc_stream_state *new_stream;
> int ret = 0;
> @@ -5567,200 +5567,199 @@ static int dm_update_crtcs_state(struct amdgpu_display_manager *dm,
> * TODO Move this code into dm_crtc_atomic_check once we get rid of dc_validation_set
> * update changed items
> */
> - for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> - struct amdgpu_crtc *acrtc = NULL;
> - struct amdgpu_dm_connector *aconnector = NULL;
> - struct drm_connector_state *drm_new_conn_state = NULL, *drm_old_conn_state = NULL;
> - struct dm_connector_state *dm_new_conn_state = NULL, *dm_old_conn_state = NULL;
> - struct drm_plane_state *new_plane_state = NULL;
> + struct amdgpu_crtc *acrtc = NULL;
> + struct amdgpu_dm_connector *aconnector = NULL;
> + struct drm_connector_state *drm_new_conn_state = NULL, *drm_old_conn_state = NULL;
> + struct dm_connector_state *dm_new_conn_state = NULL, *dm_old_conn_state = NULL;
> + struct drm_plane_state *new_plane_state = NULL;
>
> - new_stream = NULL;
> + new_stream = NULL;
>
> - dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
> - dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
> - acrtc = to_amdgpu_crtc(crtc);
> + dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
> + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
> + acrtc = to_amdgpu_crtc(crtc);
>
> - new_plane_state = drm_atomic_get_new_plane_state(state, new_crtc_state->crtc->primary);
> + new_plane_state = drm_atomic_get_new_plane_state(state, new_crtc_state->crtc->primary);
>
> - if (new_crtc_state->enable && new_plane_state && !new_plane_state->fb) {
> - ret = -EINVAL;
> - goto fail;
> - }
> + if (new_crtc_state->enable && new_plane_state && !new_plane_state->fb) {
> + ret = -EINVAL;
> + goto fail;
> + }
>
> - aconnector = amdgpu_dm_find_first_crtc_matching_connector(state, crtc);
> + aconnector = amdgpu_dm_find_first_crtc_matching_connector(state, crtc);
>
> - /* TODO This hack should go away */
> - if (aconnector && enable) {
> - /* Make sure fake sink is created in plug-in scenario */
> - drm_new_conn_state = drm_atomic_get_new_connector_state(state,
> - &aconnector->base);
> - drm_old_conn_state = drm_atomic_get_old_connector_state(state,
> - &aconnector->base);
> + /* TODO This hack should go away */
> + if (aconnector && enable) {
> + /* Make sure fake sink is created in plug-in scenario */
> + drm_new_conn_state = drm_atomic_get_new_connector_state(state,
> + &aconnector->base);
> + drm_old_conn_state = drm_atomic_get_old_connector_state(state,
> + &aconnector->base);
>
> - if (IS_ERR(drm_new_conn_state)) {
> - ret = PTR_ERR_OR_ZERO(drm_new_conn_state);
> - break;
> - }
> + if (IS_ERR(drm_new_conn_state)) {
> + ret = PTR_ERR_OR_ZERO(drm_new_conn_state);
> + goto fail;
> + }
>
> - dm_new_conn_state = to_dm_connector_state(drm_new_conn_state);
> - dm_old_conn_state = to_dm_connector_state(drm_old_conn_state);
> + dm_new_conn_state = to_dm_connector_state(drm_new_conn_state);
> + dm_old_conn_state = to_dm_connector_state(drm_old_conn_state);
>
> - new_stream = create_stream_for_sink(aconnector,
> - &new_crtc_state->mode,
> - dm_new_conn_state,
> - dm_old_crtc_state->stream);
> + new_stream = create_stream_for_sink(aconnector,
> + &new_crtc_state->mode,
> + dm_new_conn_state,
> + dm_old_crtc_state->stream);
>
> - /*
> - * we can have no stream on ACTION_SET if a display
> - * was disconnected during S3, in this case it is not an
> - * error, the OS will be updated after detection, and
> - * will do the right thing on next atomic commit
> - */
> + /*
> + * we can have no stream on ACTION_SET if a display
> + * was disconnected during S3, in this case it is not an
> + * error, the OS will be updated after detection, and
> + * will do the right thing on next atomic commit
> + */
>
> - if (!new_stream) {
> - DRM_DEBUG_DRIVER("%s: Failed to create new stream for crtc %d\n",
> - __func__, acrtc->base.base.id);
> - break;
> - }
> + if (!new_stream) {
> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for crtc %d\n",
> + __func__, acrtc->base.base.id);
> + ret = -ENOMEM;
> + goto fail;
> + }
>
> - dm_new_crtc_state->abm_level = dm_new_conn_state->abm_level;
> + dm_new_crtc_state->abm_level = dm_new_conn_state->abm_level;
>
> - if (dc_is_stream_unchanged(new_stream, dm_old_crtc_state->stream) &&
> - dc_is_stream_scaling_unchanged(new_stream, dm_old_crtc_state->stream)) {
> - new_crtc_state->mode_changed = false;
> - DRM_DEBUG_DRIVER("Mode change not required, setting mode_changed to %d",
> - new_crtc_state->mode_changed);
> - }
> + if (dc_is_stream_unchanged(new_stream, dm_old_crtc_state->stream) &&
> + dc_is_stream_scaling_unchanged(new_stream, dm_old_crtc_state->stream)) {
> + new_crtc_state->mode_changed = false;
> + DRM_DEBUG_DRIVER("Mode change not required, setting mode_changed to %d",
> + new_crtc_state->mode_changed);
> }
> + }
>
> - if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
> - goto next_crtc;
> -
> - DRM_DEBUG_DRIVER(
> - "amdgpu_crtc id:%d crtc_state_flags: enable:%d, active:%d, "
> - "planes_changed:%d, mode_changed:%d,active_changed:%d,"
> - "connectors_changed:%d\n",
> - acrtc->crtc_id,
> - new_crtc_state->enable,
> - new_crtc_state->active,
> - new_crtc_state->planes_changed,
> - new_crtc_state->mode_changed,
> - new_crtc_state->active_changed,
> - new_crtc_state->connectors_changed);
> + if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
> + goto skip_modeset;
>
> - /* Remove stream for any changed/disabled CRTC */
> - if (!enable) {
> + DRM_DEBUG_DRIVER(
> + "amdgpu_crtc id:%d crtc_state_flags: enable:%d, active:%d, "
> + "planes_changed:%d, mode_changed:%d,active_changed:%d,"
> + "connectors_changed:%d\n",
> + acrtc->crtc_id,
> + new_crtc_state->enable,
> + new_crtc_state->active,
> + new_crtc_state->planes_changed,
> + new_crtc_state->mode_changed,
> + new_crtc_state->active_changed,
> + new_crtc_state->connectors_changed);
>
> - if (!dm_old_crtc_state->stream)
> - goto next_crtc;
> + /* Remove stream for any changed/disabled CRTC */
> + if (!enable) {
>
> - ret = dm_atomic_get_state(state, &dm_state);
> - if (ret)
> - goto fail;
> + if (!dm_old_crtc_state->stream)
> + goto skip_modeset;
>
> - DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
> - crtc->base.id);
> + ret = dm_atomic_get_state(state, &dm_state);
> + if (ret)
> + goto fail;
>
> - /* i.e. reset mode */
> - if (dc_remove_stream_from_ctx(
> - dm->dc,
> - dm_state->context,
> - dm_old_crtc_state->stream) != DC_OK) {
> - ret = -EINVAL;
> - goto fail;
> - }
> + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
> + crtc->base.id);
>
> - dc_stream_release(dm_old_crtc_state->stream);
> - dm_new_crtc_state->stream = NULL;
> + /* i.e. reset mode */
> + if (dc_remove_stream_from_ctx(
> + dm->dc,
> + dm_state->context,
> + dm_old_crtc_state->stream) != DC_OK) {
> + ret = -EINVAL;
> + goto fail;
> + }
>
> - reset_freesync_config_for_crtc(dm_new_crtc_state);
> + dc_stream_release(dm_old_crtc_state->stream);
> + dm_new_crtc_state->stream = NULL;
>
> - *lock_and_validation_needed = true;
> + reset_freesync_config_for_crtc(dm_new_crtc_state);
>
> - } else {/* Add stream for any updated/enabled CRTC */
> - /*
> - * Quick fix to prevent NULL pointer on new_stream when
> - * added MST connectors not found in existing crtc_state in the chained mode
> - * TODO: need to dig out the root cause of that
> - */
> - if (!aconnector || (!aconnector->dc_sink && aconnector->mst_port))
> - goto next_crtc;
> + *lock_and_validation_needed = true;
>
> - if (modereset_required(new_crtc_state))
> - goto next_crtc;
> + } else {/* Add stream for any updated/enabled CRTC */
> + /*
> + * Quick fix to prevent NULL pointer on new_stream when
> + * added MST connectors not found in existing crtc_state in the chained mode
> + * TODO: need to dig out the root cause of that
> + */
> + if (!aconnector || (!aconnector->dc_sink && aconnector->mst_port))
> + goto skip_modeset;
>
> - if (modeset_required(new_crtc_state, new_stream,
> - dm_old_crtc_state->stream)) {
> + if (modereset_required(new_crtc_state))
> + goto skip_modeset;
>
> - WARN_ON(dm_new_crtc_state->stream);
> + if (modeset_required(new_crtc_state, new_stream,
> + dm_old_crtc_state->stream)) {
>
> - ret = dm_atomic_get_state(state, &dm_state);
> - if (ret)
> - goto fail;
> + WARN_ON(dm_new_crtc_state->stream);
>
> - dm_new_crtc_state->stream = new_stream;
> + ret = dm_atomic_get_state(state, &dm_state);
> + if (ret)
> + goto fail;
>
> - dc_stream_retain(new_stream);
> + dm_new_crtc_state->stream = new_stream;
>
> - DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
> - crtc->base.id);
> + dc_stream_retain(new_stream);
>
> - if (dc_add_stream_to_ctx(
> - dm->dc,
> - dm_state->context,
> - dm_new_crtc_state->stream) != DC_OK) {
> - ret = -EINVAL;
> - goto fail;
> - }
> + DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
> + crtc->base.id);
>
> - *lock_and_validation_needed = true;
> + if (dc_add_stream_to_ctx(
> + dm->dc,
> + dm_state->context,
> + dm_new_crtc_state->stream) != DC_OK) {
> + ret = -EINVAL;
> + goto fail;
> }
> - }
>
> -next_crtc:
> - /* Release extra reference */
> - if (new_stream)
> - dc_stream_release(new_stream);
> + *lock_and_validation_needed = true;
> + }
> + }
>
> - /*
> - * We want to do dc stream updates that do not require a
> - * full modeset below.
> - */
> - if (!(enable && aconnector && new_crtc_state->enable &&
> - new_crtc_state->active))
> - continue;
> - /*
> - * Given above conditions, the dc state cannot be NULL because:
> - * 1. We're in the process of enabling CRTCs (just been added
> - * to the dc context, or already is on the context)
> - * 2. Has a valid connector attached, and
> - * 3. Is currently active and enabled.
> - * => The dc stream state currently exists.
> - */
> - BUG_ON(dm_new_crtc_state->stream == NULL);
> +skip_modeset:
> + /* Release extra reference */
> + if (new_stream)
> + dc_stream_release(new_stream);
>
> - /* Scaling or underscan settings */
> - if (is_scaling_state_different(dm_old_conn_state, dm_new_conn_state))
> - update_stream_scaling_settings(
> - &new_crtc_state->mode, dm_new_conn_state, dm_new_crtc_state->stream);
> + /*
> + * We want to do dc stream updates that do not require a
> + * full modeset below.
> + */
> + if (!(enable && aconnector && new_crtc_state->enable &&
> + new_crtc_state->active))
> + return 0;
> + /*
> + * Given above conditions, the dc state cannot be NULL because:
> + * 1. We're in the process of enabling CRTCs (just been added
> + * to the dc context, or already is on the context)
> + * 2. Has a valid connector attached, and
> + * 3. Is currently active and enabled.
> + * => The dc stream state currently exists.
> + */
> + BUG_ON(dm_new_crtc_state->stream == NULL);
>
> - /*
> - * Color management settings. We also update color properties
> - * when a modeset is needed, to ensure it gets reprogrammed.
> - */
> - if (dm_new_crtc_state->base.color_mgmt_changed ||
> - drm_atomic_crtc_needs_modeset(new_crtc_state)) {
> - ret = amdgpu_dm_set_regamma_lut(dm_new_crtc_state);
> - if (ret)
> - goto fail;
> - amdgpu_dm_set_ctm(dm_new_crtc_state);
> - }
> + /* Scaling or underscan settings */
> + if (is_scaling_state_different(dm_old_conn_state, dm_new_conn_state))
> + update_stream_scaling_settings(
> + &new_crtc_state->mode, dm_new_conn_state, dm_new_crtc_state->stream);
>
> - /* Update Freesync settings. */
> - get_freesync_config_for_crtc(dm_new_crtc_state,
> - dm_new_conn_state);
> + /*
> + * Color management settings. We also update color properties
> + * when a modeset is needed, to ensure it gets reprogrammed.
> + */
> + if (dm_new_crtc_state->base.color_mgmt_changed ||
> + drm_atomic_crtc_needs_modeset(new_crtc_state)) {
> + ret = amdgpu_dm_set_regamma_lut(dm_new_crtc_state);
> + if (ret)
> + goto fail;
> + amdgpu_dm_set_ctm(dm_new_crtc_state);
> }
>
> + /* Update Freesync settings. */
> + get_freesync_config_for_crtc(dm_new_crtc_state,
> + dm_new_conn_state);
> +
> return ret;
>
> fail:
> @@ -6108,15 +6107,25 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> }
>
> /* Disable all crtcs which require disable */
> - ret = dm_update_crtcs_state(&adev->dm, state, false, &lock_and_validation_needed);
> - if (ret) {
> - goto fail;
> + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> + ret = dm_update_crtc_state(&adev->dm, state, crtc,
> + old_crtc_state,
> + new_crtc_state,
> + false,
> + &lock_and_validation_needed);
> + if (ret)
> + goto fail;
> }
>
> /* Enable all crtcs which require enable */
> - ret = dm_update_crtcs_state(&adev->dm, state, true, &lock_and_validation_needed);
> - if (ret) {
> - goto fail;
> + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> + ret = dm_update_crtc_state(&adev->dm, state, crtc,
> + old_crtc_state,
> + new_crtc_state,
> + true,
> + &lock_and_validation_needed);
> + if (ret)
> + goto fail;
> }
>
> /* Add new/modified planes */
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-01-07 18:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-07 15:53 [PATCH 1/2] drm/amd/display: Move iteration out of dm_update_planes sunpeng.li-5C7GfCeVMHo
[not found] ` <1546876402-12812-1-git-send-email-sunpeng.li-5C7GfCeVMHo@public.gmane.org>
2019-01-07 15:53 ` [PATCH 2/2] drm/amd/display: Move iteration out of dm_update_crtcs sunpeng.li-5C7GfCeVMHo
[not found] ` <1546876402-12812-2-git-send-email-sunpeng.li-5C7GfCeVMHo@public.gmane.org>
2019-01-07 18:07 ` Kazlauskas, Nicholas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.