Hi, Rex:
On Tue, 2022-06-07 at 20:44 +0800, Rex-BC Chen wrote:
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:
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 jason-jh.lin@mediatek.com.
Signed-off-by: Markus Schneider-Pargmann msp@baylibre.com Signed-off-by: Guillaume Ranquet granquet@baylibre.com
[snip]
+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".
The initial state is MTK_DP_TRAIN_STATE_STARTUP, and HPD thread would change state from MTK_DP_TRAIN_STATE_STARTUP to MTK_DP_TRAIN_STATE_STARTUP, this is redundant. So drop the state change in HPD thread.
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
MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT
) {
mtk_dp-
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.
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-
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.
I mean, every time state change to MTK_DP_TRAIN_STATE_DPIDLE, it would jump out of this loop and would not get into this loop, so this case would never get in. This is redundant, so remove this.
Regards, CK
BRs, Rex
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;
+}