[PATCH 2/2] drm/amd/display: Move iteration out of dm_update_crtcs

sunpeng.li at amd.com sunpeng.li at amd.com
Mon Jan 7 15:53:22 UTC 2019


From: Leo Li <sunpeng.li at 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 at 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



More information about the amd-gfx mailing list