disp: msm: dp: add private state to dp_mst_bridge

Current dp_mst_bridge has variables that are accessed by both
check phase and commit phase, which causes racing issues. This
change will add private state to dp_mst_bridge to separate check
phase and commit phase.

Furthermore, this change is a partial rollback of commit 2446602565ec
("drm/msm/dp: add private state to dp_mst_bridge") where active_enc_cnt
is removed. In this change we retain the encoder availability check in
mode_valid.

Change-Id: I8ac05cf5f1755375e4e9f34e42dbaea1d23bac64
Signed-off-by: Xiaowen Wu <[email protected]>
Signed-off-by: Sankeerth Billakanti <[email protected]>

msm/dp/dp_mst_drm.c

@@ -110,20 +110,18 @@ struct dp_mst_sim_mode {
 
 struct dp_mst_bridge {
 	struct drm_bridge base;
+	struct drm_private_obj obj;
 	u32 id;
 
 	bool in_use;
 
 	struct dp_display *display;
 	struct drm_encoder *encoder;
-	bool encoder_active_sts;
 
 	struct drm_display_mode drm_mode;
 	struct dp_display_mode dp_mode;
 	struct drm_connector *connector;
-	struct drm_connector *old_connector;
 	void *dp_panel;
-	void *old_dp_panel;
 
 	int vcpi;
 	int pbn;
@@ -135,6 +133,13 @@ struct dp_mst_bridge {
 	struct drm_connector *fixed_connector;
 };
 
+struct dp_mst_bridge_state {
+	struct drm_private_state base;
+	struct drm_connector *connector;
+	void *dp_panel;
+	int num_slots;
+};
+
 struct dp_mst_private {
 	bool mst_initialized;
 	struct dp_mst_caps caps;
@@ -154,6 +159,12 @@ struct dp_mst_encoder_info_cache {
 };
 
 #define to_dp_mst_bridge(x)     container_of((x), struct dp_mst_bridge, base)
+#define to_dp_mst_bridge_priv(x) \
+		container_of((x), struct dp_mst_bridge, obj)
+#define to_dp_mst_bridge_priv_state(x) \
+		container_of((x), struct dp_mst_bridge_state, base)
+#define to_dp_mst_bridge_state(x) \
+		to_dp_mst_bridge_priv_state((x)->obj.state)
 
 struct dp_mst_private dp_mst;
 struct dp_mst_encoder_info_cache dp_mst_enc_cache;
@@ -174,6 +185,45 @@ static void dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
 	DP_MST_INFO_LOG("mst hot plug event\n");
 }
 
+static struct drm_private_state *dp_mst_duplicate_bridge_state(
+		struct drm_private_obj *obj)
+{
+	struct dp_mst_bridge_state *state;
+
+	state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return NULL;
+
+	__drm_atomic_helper_private_obj_duplicate_state(obj, &state->base);
+
+	return &state->base;
+}
+
+static void dp_mst_destroy_bridge_state(struct drm_private_obj *obj,
+		struct drm_private_state *state)
+{
+	struct dp_mst_bridge_state *priv_state =
+		to_dp_mst_bridge_priv_state(state);
+
+	kfree(priv_state);
+}
+
+static const struct drm_private_state_funcs dp_mst_bridge_state_funcs = {
+	.atomic_duplicate_state = dp_mst_duplicate_bridge_state,
+	.atomic_destroy_state = dp_mst_destroy_bridge_state,
+};
+
+static struct dp_mst_bridge_state *dp_mst_get_bridge_atomic_state(
+		struct drm_atomic_state *state, struct dp_mst_bridge *bridge)
+{
+	struct drm_device *dev = bridge->base.dev;
+
+	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
+
+	return to_dp_mst_bridge_priv_state(
+			drm_atomic_get_private_obj_state(state, &bridge->obj));
+}
+
 static void dp_mst_sim_destroy_port(struct kref *ref)
 {
 	struct drm_dp_mst_port *port = container_of(ref,
@@ -626,6 +676,8 @@ static bool dp_mst_bridge_mode_fixup(struct drm_bridge *drm_bridge,
 	struct dp_display_mode dp_mode;
 	struct dp_mst_bridge *bridge;
 	struct dp_display *dp;
+	struct drm_crtc_state *crtc_state;
+	struct dp_mst_bridge_state *bridge_state;
 
 	DP_MST_DEBUG("enter\n");
 
@@ -636,13 +688,17 @@ static bool dp_mst_bridge_mode_fixup(struct drm_bridge *drm_bridge,
 	}
 
 	bridge = to_dp_mst_bridge(drm_bridge);
-	if (!bridge->connector) {
-		DP_ERR("Invalid connector\n");
+
+	crtc_state = container_of(mode, struct drm_crtc_state, mode);
+	bridge_state = dp_mst_get_bridge_atomic_state(crtc_state->state,
+			bridge);
+	if (IS_ERR(bridge_state)) {
+		DP_ERR("invalid bridge state\n");
 		ret = false;
 		goto end;
 	}
 
-	if (!bridge->dp_panel) {
+	if (!bridge_state->dp_panel) {
 		DP_ERR("Invalid dp_panel\n");
 		ret = false;
 		goto end;
@@ -650,7 +706,7 @@ static bool dp_mst_bridge_mode_fixup(struct drm_bridge *drm_bridge,
 
 	dp = bridge->display;
 
-	dp->convert_to_dp_mode(dp, bridge->dp_panel, mode, &dp_mode);
+	dp->convert_to_dp_mode(dp, bridge_state->dp_panel, mode, &dp_mode);
 	convert_to_drm_mode(&dp_mode, adjusted_mode);
 
 	DP_MST_DEBUG("mst bridge [%d] mode:%s fixup\n", bridge->id, mode->name);
@@ -664,7 +720,6 @@ static int _dp_mst_compute_config(struct drm_atomic_state *state,
 {
 	int slots = 0, pbn;
 	struct sde_connector *c_conn = to_sde_connector(connector);
-	int rc = 0;
 
 	DP_MST_DEBUG("enter\n");
 
@@ -680,7 +735,7 @@ static int _dp_mst_compute_config(struct drm_atomic_state *state,
 
 	DP_MST_DEBUG("exit\n");
 
-	return rc;
+	return slots;
 }
 
 static void _dp_mst_update_timeslots(struct dp_mst_private *mst,
@@ -873,9 +928,6 @@ static void dp_mst_bridge_pre_enable(struct drm_bridge *drm_bridge)
 	bridge = to_dp_mst_bridge(drm_bridge);
 	dp = bridge->display;
 
-	bridge->old_connector = NULL;
-	bridge->old_dp_panel = NULL;
-
 	if (!bridge->connector) {
 		DP_ERR("Invalid connector\n");
 		return;
@@ -1025,17 +1077,8 @@ static void dp_mst_bridge_post_disable(struct drm_bridge *drm_bridge)
 		DP_INFO("[%d] DP display unprepare failed, rc=%d\n",
 		       bridge->id, rc);
 
-	/* maintain the connector to encoder link during suspend/resume */
-	if (mst->state != PM_SUSPEND) {
-		/* Disconnect the connector and panel info from bridge */
-		mst->mst_bridge[bridge->id].old_connector =
-				mst->mst_bridge[bridge->id].connector;
-		mst->mst_bridge[bridge->id].old_dp_panel =
-				mst->mst_bridge[bridge->id].dp_panel;
-		mst->mst_bridge[bridge->id].connector = NULL;
-		mst->mst_bridge[bridge->id].dp_panel = NULL;
-		mst->mst_bridge[bridge->id].encoder_active_sts = false;
-	}
+	bridge->connector = NULL;
+	bridge->dp_panel =  NULL;
 
 	DP_MST_INFO_LOG("mst bridge [%d] post disable complete\n",
 			bridge->id);
@@ -1046,6 +1089,7 @@ static void dp_mst_bridge_mode_set(struct drm_bridge *drm_bridge,
 				const struct drm_display_mode *adjusted_mode)
 {
 	struct dp_mst_bridge *bridge;
+	struct dp_mst_bridge_state *dp_bridge_state;
 	struct dp_display *dp;
 
 	DP_MST_DEBUG("enter\n");
@@ -1056,23 +1100,10 @@ static void dp_mst_bridge_mode_set(struct drm_bridge *drm_bridge,
 	}
 
 	bridge = to_dp_mst_bridge(drm_bridge);
-	if (!bridge->connector) {
-		if (!bridge->old_connector) {
-			DP_ERR("Invalid connector\n");
-			return;
-		}
-		bridge->connector = bridge->old_connector;
-		bridge->old_connector = NULL;
-	}
 
-	if (!bridge->dp_panel) {
-		if (!bridge->old_dp_panel) {
-			DP_ERR("Invalid dp_panel\n");
-			return;
-		}
-		bridge->dp_panel = bridge->old_dp_panel;
-		bridge->old_dp_panel = NULL;
-	}
+	dp_bridge_state = to_dp_mst_bridge_state(bridge);
+	bridge->connector = dp_bridge_state->connector;
+	bridge->dp_panel = dp_bridge_state->dp_panel;
 
 	dp = bridge->display;
 
@@ -1104,6 +1135,7 @@ int dp_mst_drm_bridge_init(void *data, struct drm_encoder *encoder)
 {
 	int rc = 0;
 	struct dp_mst_bridge *bridge = NULL;
+	struct dp_mst_bridge_state *state;
 	struct drm_device *dev;
 	struct dp_display *display = data;
 	struct msm_drm_private *priv = NULL;
@@ -1155,6 +1187,16 @@ int dp_mst_drm_bridge_init(void *data, struct drm_encoder *encoder)
 	encoder->bridge = &bridge->base;
 	priv->bridges[priv->num_bridges++] = &bridge->base;
 
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (state == NULL) {
+		rc = -ENOMEM;
+		goto end;
+	}
+
+	drm_atomic_private_obj_init(dev, &bridge->obj,
+				    &state->base,
+				    &dp_mst_bridge_state_funcs);
+
 	DP_MST_DEBUG("mst drm bridge init. bridge id:%d\n", i);
 
 	/*
@@ -1169,6 +1211,7 @@ int dp_mst_drm_bridge_init(void *data, struct drm_encoder *encoder)
 				bridge->encoder);
 		if (bridge->fixed_connector == NULL) {
 			DP_ERR("failed to create fixed connector\n");
+			kfree(state);
 			rc = -ENOMEM;
 			goto end;
 		}
@@ -1254,8 +1297,9 @@ enum drm_mode_status dp_mst_connector_mode_valid(
 	struct drm_dp_mst_port *mst_port;
 	struct dp_display_mode dp_mode;
 	uint16_t available_pbn, required_pbn;
-	int i, slots_in_use = 0, active_enc_cnt = 0;
 	int available_slots, required_slots;
+	struct dp_mst_bridge_state *dp_bridge_state;
+	int i, slots_in_use = 0, active_enc_cnt = 0;
 	const u32 tot_slots = 63;
 
 	if (!connector || !mode || !display) {
@@ -1267,20 +1311,20 @@ enum drm_mode_status dp_mst_connector_mode_valid(
 	c_conn = to_sde_connector(connector);
 	mst_port = c_conn->mst_port;
 
-	mutex_lock(&mst->mst_lock);
-	available_pbn = mst_port->available_pbn;
+	/* dp bridge state is protected by drm_mode_config.connection_mutex */
 	for (i = 0; i < MAX_DP_MST_DRM_BRIDGES; i++) {
-		if (mst->mst_bridge[i].encoder_active_sts &&
-			(mst->mst_bridge[i].connector != connector)) {
+		dp_bridge_state = to_dp_mst_bridge_state(&mst->mst_bridge[i]);
+		if (dp_bridge_state->connector &&
+				dp_bridge_state->connector != connector) {
 			active_enc_cnt++;
-			slots_in_use += mst->mst_bridge[i].num_slots;
+			slots_in_use += dp_bridge_state->num_slots;
 		}
 	}
-	mutex_unlock(&mst->mst_lock);
 
-	if (active_enc_cnt < DP_STREAM_MAX)
+	if (active_enc_cnt < DP_STREAM_MAX) {
+		available_pbn = mst_port->available_pbn;
 		available_slots = tot_slots - slots_in_use;
-	else {
+	} else {
 		DP_DEBUG("all mst streams are active\n");
 		return MODE_BAD;
 	}
@@ -1357,24 +1401,38 @@ dp_mst_atomic_best_encoder(struct drm_connector *connector,
 	struct dp_mst_private *mst = dp_display->dp_mst_prv_info;
 	struct sde_connector *conn = to_sde_connector(connector);
 	struct drm_encoder *enc = NULL;
+	struct dp_mst_bridge_state *bridge_state;
 	u32 i;
 
+	if (state->best_encoder)
+		return state->best_encoder;
+
 	for (i = 0; i < MAX_DP_MST_DRM_BRIDGES; i++) {
-		if (mst->mst_bridge[i].connector == connector) {
+		bridge_state = dp_mst_get_bridge_atomic_state(
+				state->state, &mst->mst_bridge[i]);
+		if (IS_ERR(bridge_state))
+			goto end;
+
+		if (bridge_state->connector == connector) {
 			enc = mst->mst_bridge[i].encoder;
 			goto end;
 		}
 	}
 
 	for (i = 0; i < MAX_DP_MST_DRM_BRIDGES; i++) {
-		if (!mst->mst_bridge[i].encoder_active_sts &&
-			!mst->mst_bridge[i].fixed_connector) {
-			mst->mst_bridge[i].encoder_active_sts = true;
-			mst->mst_bridge[i].connector = connector;
-			mst->mst_bridge[i].dp_panel = conn->drv_panel;
+		if (mst->mst_bridge[i].fixed_connector)
+			continue;
+
+		bridge_state = dp_mst_get_bridge_atomic_state(
+				state->state, &mst->mst_bridge[i]);
+
+		if (!bridge_state->connector) {
+			bridge_state->connector = connector;
+			bridge_state->dp_panel = conn->drv_panel;
 			enc = mst->mst_bridge[i].encoder;
 			break;
 		}
+
 	}
 
 end:
@@ -1388,23 +1446,6 @@ end:
 	return enc;
 }
 
-static struct dp_mst_bridge *_dp_mst_get_bridge_from_encoder(
-		struct dp_display *dp_display,
-		struct drm_encoder *encoder)
-{
-	struct dp_mst_private *mst = dp_display->dp_mst_prv_info;
-	int i;
-
-	for (i = 0; i < MAX_DP_MST_DRM_BRIDGES; i++) {
-		if (mst->mst_bridge[i].encoder == encoder)
-			return &mst->mst_bridge[i];
-	}
-
-	DP_MST_DEBUG("mst bridge detect for encoder failed\n");
-
-	return NULL;
-}
-
 static int dp_mst_connector_atomic_check(struct drm_connector *connector,
 		void *display, struct drm_atomic_state *state)
 {
@@ -1413,7 +1454,8 @@ static int dp_mst_connector_atomic_check(struct drm_connector *connector,
 	struct drm_connector_state *new_conn_state;
 	struct drm_crtc *old_crtc;
 	struct drm_crtc_state *crtc_state;
-	struct dp_mst_bridge *bridge = NULL;
+	struct dp_mst_bridge *bridge;
+	struct dp_mst_bridge_state *bridge_state;
 	struct dp_display *dp_display = display;
 	struct dp_mst_private *mst = dp_display->dp_mst_prv_info;
 	struct sde_connector *c_conn = to_sde_connector(connector);
@@ -1421,13 +1463,6 @@ static int dp_mst_connector_atomic_check(struct drm_connector *connector,
 
 	DP_MST_DEBUG("enter:\n");
 
-	/*
-	 * Skip atomic check during mst suspend, to avoid mismanagement of
-	 * available vcpi slots.
-	 */
-	if (mst->state == PM_SUSPEND)
-		return rc;
-
 	if (!state)
 		return rc;
 
@@ -1435,8 +1470,6 @@ static int dp_mst_connector_atomic_check(struct drm_connector *connector,
 	if (!new_conn_state)
 		return rc;
 
-	mutex_lock(&mst->mst_lock);
-
 	old_conn_state = drm_atomic_get_old_connector_state(state, connector);
 	if (!old_conn_state)
 		goto mode_set;
@@ -1454,20 +1487,45 @@ static int dp_mst_connector_atomic_check(struct drm_connector *connector,
 				bridge->num_slots);
 	}
 
-	bridge = _dp_mst_get_bridge_from_encoder(dp_display,
-			old_conn_state->best_encoder);
-	if (!bridge)
-		goto end;
+	if (drm_atomic_crtc_needs_modeset(crtc_state)) {
+		if (WARN_ON(!old_conn_state->best_encoder)) {
+			rc = -EINVAL;
+			goto end;
+		}
+
+		bridge = to_dp_mst_bridge(
+				old_conn_state->best_encoder->bridge);
+
+		bridge_state = dp_mst_get_bridge_atomic_state(state, bridge);
+		if (IS_ERR(bridge_state)) {
+			rc = PTR_ERR(bridge_state);
+			goto end;
+		}
 
-	slots = bridge->num_slots;
-	if (drm_atomic_crtc_needs_modeset(crtc_state) && slots > 0) {
-		rc = mst->mst_fw_cbs->atomic_release_vcpi_slots(state,
-				&mst->mst_mgr, c_conn->mst_port);
-		if (rc) {
-			DP_ERR("failed releasing %d vcpi slots rc:%d\n",
-					slots, rc);
+		if (WARN_ON(bridge_state->connector != connector)) {
+			rc = -EINVAL;
 			goto end;
 		}
+
+		slots = bridge_state->num_slots;
+		if (slots > 0) {
+			rc = mst->mst_fw_cbs->atomic_release_vcpi_slots(state,
+					&mst->mst_mgr, c_conn->mst_port);
+			if (rc) {
+				DP_ERR("failed releasing %d vcpi slots %d\n",
+						slots, rc);
+				goto end;
+			}
+		}
+
+		bridge_state->num_slots = 0;
+
+		if (!new_conn_state->crtc && mst->state != PM_SUSPEND) {
+			bridge_state->connector = NULL;
+			bridge_state->dp_panel = NULL;
+
+			DP_MST_DEBUG("clear best encoder: %d\n", bridge->id);
+		}
 	}
 
 mode_set:
@@ -1477,28 +1535,47 @@ mode_set:
 	crtc_state = drm_atomic_get_new_crtc_state(state, new_conn_state->crtc);
 
 	if (drm_atomic_crtc_needs_modeset(crtc_state) && crtc_state->active) {
+		c_conn = to_sde_connector(connector);
+
+		if (WARN_ON(!new_conn_state->best_encoder)) {
+			rc = -EINVAL;
+			goto end;
+		}
+
+		bridge = to_dp_mst_bridge(
+				new_conn_state->best_encoder->bridge);
+
+		bridge_state = dp_mst_get_bridge_atomic_state(state, bridge);
+		if (IS_ERR(bridge_state)) {
+			rc = PTR_ERR(bridge_state);
+			goto end;
+		}
+
+		if (WARN_ON(bridge_state->connector != connector)) {
+			rc = -EINVAL;
+			goto end;
+		}
+
+		if (WARN_ON(bridge_state->num_slots)) {
+			rc = -EINVAL;
+			goto end;
+		}
+
 		dp_display->convert_to_dp_mode(dp_display, c_conn->drv_panel,
 				&crtc_state->mode, &dp_mode);
 
 		slots = _dp_mst_compute_config(state, mst, connector, &dp_mode);
 		if (slots < 0) {
 			rc = slots;
-
-			/* Disconnect the conn and panel info from bridge */
-			bridge = _dp_mst_get_bridge_from_encoder(dp_display,
-						new_conn_state->best_encoder);
-			if (!bridge)
-				goto end;
-
-			bridge->connector = NULL;
-			bridge->dp_panel = NULL;
-			bridge->encoder_active_sts = false;
+			goto end;
 		}
+
+		bridge_state->num_slots = slots;
 	}
 
 end:
-	mutex_unlock(&mst->mst_lock);
-	DP_MST_DEBUG("mst connector:%d atomic check\n", connector->base.id);
+	DP_MST_DEBUG("mst connector:%d atomic check ret %d\n",
+			connector->base.id, rc);
 	return rc;
 }
 
@@ -1662,20 +1739,21 @@ dp_mst_fixed_atomic_best_encoder(struct drm_connector *connector,
 	struct dp_mst_private *mst = dp_display->dp_mst_prv_info;
 	struct sde_connector *conn = to_sde_connector(connector);
 	struct drm_encoder *enc = NULL;
+	struct dp_mst_bridge_state *bridge_state;
 	u32 i;
 
-	for (i = 0; i < MAX_DP_MST_DRM_BRIDGES; i++) {
-		if (mst->mst_bridge[i].connector == connector) {
-			enc = mst->mst_bridge[i].encoder;
-			goto end;
-		}
-	}
+	if (state->best_encoder)
+		return state->best_encoder;
 
 	for (i = 0; i < MAX_DP_MST_DRM_BRIDGES; i++) {
 		if (mst->mst_bridge[i].fixed_connector == connector) {
-			mst->mst_bridge[i].encoder_active_sts = true;
-			mst->mst_bridge[i].connector = connector;
-			mst->mst_bridge[i].dp_panel = conn->drv_panel;
+			bridge_state = dp_mst_get_bridge_atomic_state(
+					state->state, &mst->mst_bridge[i]);
+			if (IS_ERR(bridge_state))
+				goto end;
+
+			bridge_state->connector = connector;
+			bridge_state->dp_panel = conn->drv_panel;
 			enc = mst->mst_bridge[i].encoder;
 			break;
 		}