On Tue, 2022-06-07 at 14:44 +0800, CK Hu wrote:

Hi, Rex:

On Mon, 2022-05-23 at 12:47 +0200, Guillaume Ranquet wrote:

quoted

From: Markus Schneider-Pargmann <msp@baylibre.com>

This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.

It supports the mt8195, the embedded DisplayPort units. It offers
DisplayPort 1.4 with up to 4 lanes.

The driver creates a child device for the phy. The child device
will
never exist without the parent being active. As they are sharing a
register range, the parent passes a regmap pointer to the child so
that
both can work with the same register range. The phy driver sets
device
data that is read by the parent to get the phy device that can be
used
to control the phy properties.

This driver is based on an initial version by
Jason-JH.Lin [off-list ref].

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
Signed-off-by: Guillaume Ranquet <redacted>
---

[snip]

quoted

+
+static int mtk_dp_train_handler(struct mtk_dp *mtk_dp)
+{
+	bool training_done = false;
+	short max_retry = 50;
+	int ret = 0;
+
+	do {
+		switch (mtk_dp->train_state) {
+		case MTK_DP_TRAIN_STATE_STARTUP:

mtk_dp->train_state is initialized as MTK_DP_TRAIN_STATE_STARTUP even
though HPD ISR does not exist. Does this mean HPD ISR is redundant?
If
HPD ISR is not redundant, create a new state MTK_DP_TRAIN_STATE_NONE
for init state.

Hello CK,

I think we don't need MTK_DP_TRAIN_STATE_NONE.
Because it's "DP_TRAIN_STATE" not "DP_STATE", I think it's ok if we
start this state machine with "MTK_DP_TRAIN_STATE_STARTUP".

quoted

+			mtk_dp_state_handler(mtk_dp);
+			mtk_dp->train_state =
MTK_DP_TRAIN_STATE_CHECKCAP;
+			break;
+
+		case MTK_DP_TRAIN_STATE_CHECKCAP:
+			if (mtk_dp_parse_capabilities(mtk_dp)) {
+				mtk_dp->train_info.check_cap_count = 0;
+				mtk_dp->train_state =
MTK_DP_TRAIN_STATE_CHECKEDID;
+			} else {
+				mtk_dp->train_info.check_cap_count++;
+
+				if (mtk_dp->train_info.check_cap_count

quoted

+				    MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT
) {
+					mtk_dp-

quoted

train_info.check_cap_count = 0;

+					mtk_dp->train_state =
MTK_DP_TRAIN_STATE_DPIDLE;
+					ret = -ETIMEDOUT;
+				}
+			}
+			break;
+
+		case MTK_DP_TRAIN_STATE_CHECKEDID:
+			mtk_dp->train_state =
MTK_DP_TRAIN_STATE_TRAINING_PRE;

MTK_DP_TRAIN_STATE_CHECKEDID is a redundant state, drop it.

quoted

+			break;
+
+		case MTK_DP_TRAIN_STATE_TRAINING_PRE:
+			mtk_dp_state_handler(mtk_dp);
+			mtk_dp->train_state =
MTK_DP_TRAIN_STATE_TRAINING;
+			break;
+
+		case MTK_DP_TRAIN_STATE_TRAINING:
+			ret = mtk_dp_train_start(mtk_dp);
+			if (ret == 0) {
+				mtk_dp_video_mute(mtk_dp, true);
+				mtk_dp->train_state =
MTK_DP_TRAIN_STATE_NORMAL;
+				mtk_dp_fec_enable(mtk_dp, mtk_dp-

quoted

has_fec);

+			} else if (ret != -EAGAIN) {
+				mtk_dp->train_state =
MTK_DP_TRAIN_STATE_DPIDLE;
+			}
+			break;
+		case MTK_DP_TRAIN_STATE_NORMAL:
+			mtk_dp_state_handler(mtk_dp);
+			training_done = true;
+			break;
+		case MTK_DP_TRAIN_STATE_DPIDLE:

When would this case happen?

Regards,
CK

Yes, if it's disconnected if we are still training for dp.
or failed to training min spec RBR.

BRs,
Rex

quoted

+			break;
+		default:
+			break;
+		}
+
+		if (ret) {
+			if (ret == -EAGAIN)
+				continue;
+			/*
+			 * If we get any other error number, it doesn't
+			 * make any sense to keep iterating.
+			 */
+			break;
+		}
+	} while (!training_done || --max_retry);
+
+	return ret;
+}