[PATCH 04/19] drm/amd/display: Fix handling of scaling and underscan.

Harry Wentland harry.wentland at amd.com
Wed May 31 15:52:03 UTC 2017


From: Andrey Grodzovsky <Andrey.Grodzovsky at amd.com>

Summury of changes:

1: Both in check and commit Connector properties were handled as
   part of for_each(crtc) loop while they shoud have been handled
   in a dedicated for_each(connector)
   loop since they are connector properties. Moved.

2: Removed hacky plane add in amdgpu_dm_connector_atomic_set_property
   to force iteration on plane forconnector property. This was
   causing double call to commit_surface_for_stream both in crtc loop
   and plane loop.
3: Remove middleman DC interface and  call dc_commit_surfaces_to_stream
   directly to increase code clarity.

Remove it from atomic_commit.

Change-Id: I1337b7abe4a2c6812c7500a5bf22f6c6f01890ca
Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky at amd.com>
Reviewed-by: Harry Wentland <Harry.Wentland at amd.com>
---
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c    | 207 ++++++++++-----------
 1 file changed, 99 insertions(+), 108 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
index a929df2e690d..15204bf7d9a8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
@@ -683,9 +683,8 @@ struct amdgpu_connector *aconnector_from_drm_crtc_id(
 static void update_stream_scaling_settings(
 		const struct drm_display_mode *mode,
 		const struct dm_connector_state *dm_state,
-		const struct dc_stream *stream)
+		struct dc_stream *stream)
 {
-	struct amdgpu_device *adev = dm_state->base.crtc->dev->dev_private;
 	enum amdgpu_rmx_type rmx_type;
 
 	struct rect src = { 0 }; /* viewport in composition space*/
@@ -727,7 +726,8 @@ static void update_stream_scaling_settings(
 		dst.height -= dm_state->underscan_vborder;
 	}
 
-	adev->dm.dc->stream_funcs.stream_update_scaling(adev->dm.dc, stream, &src, &dst);
+	stream->src = src;
+	stream->dst = dst;
 
 	DRM_DEBUG_KMS("Destination Rectangle x:%d  y:%d  width:%d  height:%d\n",
 			dst.x, dst.y, dst.width, dst.height);
@@ -1287,9 +1287,6 @@ int amdgpu_dm_connector_atomic_set_property(
 	struct dm_connector_state *dm_new_state =
 		to_dm_connector_state(connector_state);
 
-	struct drm_crtc_state *new_crtc_state;
-	struct drm_crtc *crtc;
-	int i;
 	int ret = -EINVAL;
 
 	if (property == dev->mode_config.scaling_mode_property) {
@@ -1335,34 +1332,6 @@ int amdgpu_dm_connector_atomic_set_property(
 		return ret;
 	}
 
-
-
-	for_each_crtc_in_state(
-		connector_state->state,
-		crtc,
-		new_crtc_state,
-		i) {
-
-		if (crtc == connector_state->crtc) {
-			struct drm_plane_state *plane_state;
-
-			/*
-			 * Bit of magic done here. We need to ensure
-			 * that planes get update after mode is set.
-			 * So, we need to add primary plane to state,
-			 * and this way atomic_update would be called
-			 * for it
-			 */
-			plane_state =
-				drm_atomic_get_plane_state(
-					connector_state->state,
-					crtc->primary);
-
-			if (!plane_state)
-				return -EINVAL;
-		}
-	}
-
 	return ret;
 }
 
@@ -2582,28 +2551,19 @@ static void amdgpu_dm_commit_surfaces(struct drm_atomic_state *state,
 		struct amdgpu_crtc *acrtc_attach = to_amdgpu_crtc(crtc);
 		struct drm_framebuffer *fb = plane_state->fb;
 		struct drm_connector *connector;
-		struct dm_connector_state *dm_state = NULL;
-
-		enum dm_commit_action action;
+		struct dm_connector_state *con_state = NULL;
 		bool pflip_needed;
 
 		if (!fb || !crtc || !crtc->state->active)
 			continue;
 
-		action = get_dm_commit_action(crtc->state);
-
-		/*
-		 * TODO - TO decide if it's a flip or surface update
-		 * stop relying on allow_modeset flag and query DC
-		 * using dc_check_update_surfaces_for_stream.
-		 */
 		pflip_needed = !state->allow_modeset;
 		if (!pflip_needed) {
 			list_for_each_entry(connector,
 					    &dev->mode_config.connector_list,
 					    head) {
 				if (connector->state->crtc == crtc) {
-					dm_state = to_dm_connector_state(
+					con_state = to_dm_connector_state(
 							connector->state);
 					break;
 				}
@@ -2623,8 +2583,10 @@ static void amdgpu_dm_commit_surfaces(struct drm_atomic_state *state,
 			 * Also it should be needed when used with actual
 			 * drm_atomic_commit ioctl in future
 			 */
-			if (!dm_state)
+			if (!con_state)
 				continue;
+
+
 			if (crtc == pcrtc) {
 				add_surface(dm->dc, crtc, plane,
 					    &dc_surfaces_constructed[planes_count]);
@@ -2676,7 +2638,8 @@ void amdgpu_dm_atomic_commit_tail(
 	const struct dc_stream *new_stream;
 	unsigned long flags;
 	bool wait_for_vblank = true;
-
+	struct drm_connector *connector;
+	struct drm_connector_state *old_conn_state;
 
 	drm_atomic_helper_update_legacy_modeset_state(dev, state);
 
@@ -2756,21 +2719,6 @@ void amdgpu_dm_atomic_commit_tail(
 
 			break;
 		}
-
-		case DM_COMMIT_ACTION_NOTHING: {
-			struct dm_connector_state *dm_state = NULL;
-
-			if (!aconnector)
-				break;
-
-			dm_state = to_dm_connector_state(aconnector->base.state);
-
-			/* Scaling update */
-			update_stream_scaling_settings(&crtc->state->mode,
-					dm_state, acrtc->stream);
-
-			break;
-		}
 		case DM_COMMIT_ACTION_DPMS_OFF:
 		case DM_COMMIT_ACTION_RESET:
 			DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc);
@@ -2778,9 +2726,48 @@ void amdgpu_dm_atomic_commit_tail(
 			if (acrtc->stream)
 				remove_stream(adev, acrtc);
 			break;
+
+		/*TODO retire */
+		case DM_COMMIT_ACTION_NOTHING:
+			continue;
 		} /* switch() */
 	} /* for_each_crtc_in_state() */
 
+	/* Handle scaling and undersacn changes*/
+	for_each_connector_in_state(state, connector, old_conn_state, i) {
+		struct amdgpu_connector *aconnector = to_amdgpu_connector(connector);
+		struct dm_connector_state *con_new_state =
+				to_dm_connector_state(aconnector->base.state);
+		struct dm_connector_state *con_old_state =
+				to_dm_connector_state(old_conn_state);
+		struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc);
+		const struct dc_stream_status *status = NULL;
+
+		/* Skip any modesets/resets */
+		if (!acrtc ||
+			get_dm_commit_action(acrtc->base.state) != DM_COMMIT_ACTION_NOTHING)
+			continue;
+
+		/* Skip any thing not scale or underscan chnages */
+		if (!is_scaling_state_different(con_new_state, con_old_state))
+			continue;
+
+		update_stream_scaling_settings(&con_new_state->base.crtc->mode,
+				con_new_state, (struct dc_stream *)acrtc->stream);
+
+		status = dc_stream_get_status(acrtc->stream);
+		WARN_ON(!status);
+		WARN_ON(!status->surface_count);
+
+		/*TODO How it works with MPO ?*/
+		if (!dc_commit_surfaces_to_stream(
+				dm->dc,
+				(const struct dc_surface **)status->surfaces,
+				status->surface_count,
+				acrtc->stream))
+			dm_error("%s: Failed to update stream scaling!\n", __func__);
+	}
+
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 
 		struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
@@ -3118,6 +3105,8 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
 	struct dc *dc = adev->dm.dc;
 	bool need_to_validate = false;
 	struct validate_context *context;
+	struct drm_connector *connector;
+	struct drm_connector_state *conn_state;
 	/*
 	 * This bool will be set for true for any modeset/reset
 	 * or surface update which implies non fast surfae update.
@@ -3203,52 +3192,6 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
 			break;
 		}
 
-		case DM_COMMIT_ACTION_NOTHING: {
-			const struct drm_connector *drm_connector = NULL;
-			struct drm_connector_state *conn_state = NULL;
-			struct dm_connector_state *dm_state = NULL;
-			struct dm_connector_state *old_dm_state = NULL;
-			struct dc_stream *new_stream;
-
-			if (!aconnector)
-				break;
-
-			for_each_connector_in_state(
-				state, drm_connector, conn_state, j) {
-				if (&aconnector->base == drm_connector)
-					break;
-			}
-
-			old_dm_state = to_dm_connector_state(drm_connector->state);
-			dm_state = to_dm_connector_state(conn_state);
-
-			/* Support underscan adjustment*/
-			if (!is_scaling_state_different(dm_state, old_dm_state))
-				break;
-
-			new_stream = create_stream_for_sink(aconnector, &crtc_state->mode, dm_state);
-
-			if (!new_stream) {
-				DRM_ERROR("%s: Failed to create new stream for crtc %d\n",
-						__func__, acrtc->base.base.id);
-				break;
-			}
-
-			new_streams[new_stream_count] = new_stream;
-			set_count = update_in_val_sets_stream(
-					set,
-					crtc_set,
-					set_count,
-					acrtc->stream,
-					new_stream,
-					crtc);
-
-			new_stream_count++;
-			need_to_validate = true;
-			wait_for_prev_commits = true;
-
-			break;
-		}
 		case DM_COMMIT_ACTION_DPMS_OFF:
 		case DM_COMMIT_ACTION_RESET:
 			/* i.e. reset mode */
@@ -3260,6 +3203,10 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
 				wait_for_prev_commits = true;
 			}
 			break;
+
+		/*TODO retire */
+		case DM_COMMIT_ACTION_NOTHING:
+			continue;
 		}
 
 		/*
@@ -3276,6 +3223,50 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
 		ret = -EINVAL;
 	}
 
+	/* Check scaling and undersacn changes*/
+	for_each_connector_in_state(state, connector, conn_state, i) {
+		struct amdgpu_connector *aconnector = to_amdgpu_connector(connector);
+		struct dm_connector_state *con_old_state =
+				to_dm_connector_state(aconnector->base.state);
+		struct dm_connector_state *con_new_state =
+						to_dm_connector_state(conn_state);
+		struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc);
+		struct dc_stream *new_stream;
+
+		/* Skip any modesets/resets */
+		if (!acrtc ||
+			get_dm_commit_action(acrtc->base.state) != DM_COMMIT_ACTION_NOTHING)
+			continue;
+
+		/* Skip any thing not scale or underscan chnages */
+		if (!is_scaling_state_different(con_new_state, con_old_state))
+			continue;
+
+		new_stream = create_stream_for_sink(
+				aconnector,
+				&acrtc->base.state->mode,
+				con_new_state);
+
+		if (!new_stream) {
+			DRM_ERROR("%s: Failed to create new stream for crtc %d\n",
+					__func__, acrtc->base.base.id);
+			continue;
+		}
+
+		new_streams[new_stream_count] = new_stream;
+		set_count = update_in_val_sets_stream(
+				set,
+				crtc_set,
+				set_count,
+				acrtc->stream,
+				new_stream,
+				&acrtc->base);
+
+		new_stream_count++;
+		need_to_validate = true;
+		wait_for_prev_commits = true;
+	}
+
 	for (i = 0; i < set_count; i++) {
 		for_each_plane_in_state(state, plane, plane_state, j) {
 			struct drm_crtc *crtc = plane_state->crtc;
-- 
2.11.0



More information about the amd-gfx mailing list