[PATCH v2 0/5] drm/bridge: it6505: fix DP link traning and improve compatibility

[PATCH v2 0/5] drm/bridge: it6505: fix DP link traning and improve compatibility

[Hermes Wu via B4 Relay]

IT6505 supports HW auto link training which will write DPCD and check
training status automatically. Some DP device can not pass
HW auto link training and must set link training step by step.

when HW auto link training fail, it may trigger video FIFO error,
and link training process will reset to beginning, and never try
step training method.

Modify training method improve compatibility to these DP devices.

v1 -> v2:
	1. Split [PATCH 1/3] into 3 commits
	2. Drop non necessary variable auto_ttrain_retry
	v1 link: https://lore.kernel.org/all/20250318-fix-link-training-v1-0-19266711142c@ite.com.tw/

Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
---
Hermes Wu (5):
      drm/bridge: it6505: fix link training state HW register reset
      drm/bridge: it6505: check INT_LINK_TRAIN_FAIL while link auto training
      drm/bridge: it6505: modify DP link auto training
      drm/bridge: it6505: modify DP link training work
      drm/bridge: it6505: skip auto training when previous try fail

 drivers/gpu/drm/bridge/ite-it6505.c | 112 +++++++++++++++++++++---------------
 1 file changed, 65 insertions(+), 47 deletions(-)
---
base-commit: 938fbb16aba8f7b88e0fdcf56f315a5bbad41aad
change-id: 20250121-fix-link-training-461495494655

Best regards,
-- 
Hermes Wu <Hermes.wu@ite.com.tw>

[PATCH v2 1/5] drm/bridge: it6505: fix link training state HW register reset

[Hermes Wu via B4 Relay]

From: Hermes Wu <Hermes.wu@ite.com.tw>

In order to reset HW link auto training state,
bits FORCE_RETRAIN and MANUAL_TRAIN at REG_TRAIN_CTRL1 must be set
at the same time.

Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
---
 drivers/gpu/drm/bridge/ite-it6505.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index 88ef76a37fe6accacdd343839ff2569b31b18ceb..b47042dc06bb1e1dfd92bc0dc6d69dc918a140ba 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -1806,7 +1806,9 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
 	mutex_lock(&it6505->aux_lock);
 	it6505_set_bits(it6505, REG_TRAIN_CTRL0,
 			FORCE_CR_DONE | FORCE_EQ_DONE, 0x00);
-	it6505_write(it6505, REG_TRAIN_CTRL1, FORCE_RETRAIN);
+	/* reset link state machine and re start training*/
+	it6505_write(it6505, REG_TRAIN_CTRL1,
+		     FORCE_RETRAIN | MANUAL_TRAIN);
 	it6505_write(it6505, REG_TRAIN_CTRL1, AUTO_TRAIN);
 
 	while (timeout > 0) {
@@ -2377,7 +2379,7 @@ static void it6505_stop_link_train(struct it6505 *it6505)
 {
 	it6505->link_state = LINK_IDLE;
 	cancel_work_sync(&it6505->link_works);
-	it6505_write(it6505, REG_TRAIN_CTRL1, FORCE_RETRAIN);
+	it6505_write(it6505, REG_TRAIN_CTRL1, FORCE_RETRAIN | MANUAL_TRAIN);
 }
 
 static void it6505_link_train_ok(struct it6505 *it6505)

-- 
2.34.1

[PATCH v2 2/5] drm/bridge: it6505: check INT_LINK_TRAIN_FAIL while link auto training

[Hermes Wu via B4 Relay]

From: Hermes Wu <Hermes.wu@ite.com.tw>

When start link training, interrupt status INT_LINK_TRAIN_FAIL can
use to check link training fail and no need to wait until timeout.

it6505_irq_link_train_fail() remove from interrupt and no longer used.

Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
---
 drivers/gpu/drm/bridge/ite-it6505.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index b47042dc06bb1e1dfd92bc0dc6d69dc918a140ba..0607f99446b37c82b41a376c2f4e10c7565d1b61 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -821,8 +821,8 @@ static void it6505_int_mask_enable(struct it6505 *it6505)
 	it6505_write(it6505, INT_MASK_02, BIT(INT_AUX_CMD_FAIL) |
 		     BIT(INT_HDCP_KSV_CHECK) | BIT(INT_AUDIO_FIFO_ERROR));
 
-	it6505_write(it6505, INT_MASK_03, BIT(INT_LINK_TRAIN_FAIL) |
-		     BIT(INT_VID_FIFO_ERROR) | BIT(INT_IO_LATCH_FIFO_OVERFLOW));
+	it6505_write(it6505, INT_MASK_03, BIT(INT_VID_FIFO_ERROR) |
+		     BIT(INT_IO_LATCH_FIFO_OVERFLOW));
 }
 
 static void it6505_int_mask_disable(struct it6505 *it6505)
@@ -1802,6 +1802,7 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
 {
 	int timeout = 500, link_training_state;
 	bool state = false;
+	int int03;
 
 	mutex_lock(&it6505->aux_lock);
 	it6505_set_bits(it6505, REG_TRAIN_CTRL0,
@@ -1814,6 +1815,17 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
 	while (timeout > 0) {
 		usleep_range(1000, 2000);
 		link_training_state = it6505_read(it6505, REG_LINK_TRAIN_STS);
+		int03 = it6505_read(it6505, INT_STATUS_03);
+		if (int03 & BIT(INT_LINK_TRAIN_FAIL)) {
+			it6505_write(it6505, INT_STATUS_03,
+				     BIT(INT_LINK_TRAIN_FAIL));
+
+			DRM_DEV_DEBUG_DRIVER(dev,
+					     "INT_LINK_TRAIN_FAIL(%x)!",
+					      int03);
+
+			break;
+		}
 
 		if (link_training_state > 0 &&
 		    (link_training_state & LINK_STATE_NORP)) {
@@ -2693,14 +2705,6 @@ static void it6505_irq_audio_fifo_error(struct it6505 *it6505)
 		it6505_enable_audio(it6505);
 }
 
-static void it6505_irq_link_train_fail(struct it6505 *it6505)
-{
-	struct device *dev = it6505->dev;
-
-	DRM_DEV_DEBUG_DRIVER(dev, "link training fail interrupt");
-	schedule_work(&it6505->link_works);
-}
-
 static bool it6505_test_bit(unsigned int bit, const unsigned int *addr)
 {
 	return 1 & (addr[bit / BITS_PER_BYTE] >> (bit % BITS_PER_BYTE));
@@ -2765,7 +2769,6 @@ static irqreturn_t it6505_int_threaded_handler(int unused, void *data)
 		{ BIT_INT_AUX_CMD_FAIL, it6505_irq_aux_cmd_fail },
 		{ BIT_INT_HDCP_KSV_CHECK, it6505_irq_hdcp_ksv_check },
 		{ BIT_INT_AUDIO_FIFO_ERROR, it6505_irq_audio_fifo_error },
-		{ BIT_INT_LINK_TRAIN_FAIL, it6505_irq_link_train_fail },
 	};
 	int int_status[3], i;
 

-- 
2.34.1

[PATCH v2 3/5] drm/bridge: it6505: modify DP link auto training

[Hermes Wu via B4 Relay]

From: Hermes Wu <Hermes.wu@ite.com.tw>

IT6505 supports HW link training which will write DPCD and check
training status automatically.

In the case that driver set link rate at 2.7G and HW fail to training,
it will change link configuration and try 1.65G. And this will cause
INT_VID_FIFO_ERROR triggered when link clock is changed.

When video error occurs, video logic is reset and link training restart,
this will cause endless auto link training.

Modify link auto training with disable INT_VID_FIFO_ERROR to avoid loop
and check INT_LINK_TRAIN_FAIL event to abort wait training done.

Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
---
 drivers/gpu/drm/bridge/ite-it6505.c | 38 +++++++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index 0607f99446b37c82b41a376c2f4e10c7565d1b61..e0e13e737763fb801fd1cd803734a0d6ae1dd812 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -1800,11 +1800,20 @@ static void it6505_link_training_setup(struct it6505 *it6505)
 
 static bool it6505_link_start_auto_train(struct it6505 *it6505)
 {
-	int timeout = 500, link_training_state;
+	int link_training_state;
 	bool state = false;
 	int int03;
+	struct device *dev = it6505->dev;
+	unsigned long timeout;
+
+	guard(mutex)(&it6505->aux_lock);
+	/* Disable FIFO error interrupt trigger  */
+	/* to prevent training fail loop issue   */
+	it6505_set_bits(it6505, INT_MASK_03, BIT(INT_VID_FIFO_ERROR), 0);
+
+	it6505_write(it6505, INT_STATUS_03,
+		     BIT(INT_LINK_TRAIN_FAIL) | BIT(INT_VID_FIFO_ERROR));
 
-	mutex_lock(&it6505->aux_lock);
 	it6505_set_bits(it6505, REG_TRAIN_CTRL0,
 			FORCE_CR_DONE | FORCE_EQ_DONE, 0x00);
 	/* reset link state machine and re start training*/
@@ -1812,32 +1821,41 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
 		     FORCE_RETRAIN | MANUAL_TRAIN);
 	it6505_write(it6505, REG_TRAIN_CTRL1, AUTO_TRAIN);
 
-	while (timeout > 0) {
+	timeout = jiffies + msecs_to_jiffies(100) + 1;
+	for (;;) {
 		usleep_range(1000, 2000);
 		link_training_state = it6505_read(it6505, REG_LINK_TRAIN_STS);
 		int03 = it6505_read(it6505, INT_STATUS_03);
 		if (int03 & BIT(INT_LINK_TRAIN_FAIL)) {
-			it6505_write(it6505, INT_STATUS_03,
-				     BIT(INT_LINK_TRAIN_FAIL));
-
 			DRM_DEV_DEBUG_DRIVER(dev,
 					     "INT_LINK_TRAIN_FAIL(%x)!",
 					      int03);
 
+			/* Ignore INT_VID_FIFO_ERROR when auto training fail*/
+			it6505_write(it6505, INT_STATUS_03,
+				     BIT(INT_LINK_TRAIN_FAIL) |
+				     BIT(INT_VID_FIFO_ERROR));
+
+			if (int03 & BIT(INT_VID_FIFO_ERROR))
+				DRM_DEV_DEBUG_DRIVER(dev,
+						     "video fifo error when training fail");
+
 			break;
 		}
 
 		if (link_training_state > 0 &&
 		    (link_training_state & LINK_STATE_NORP)) {
 			state = true;
-			goto unlock;
+			break;
 		}
 
-		timeout--;
+		if (time_after(jiffies, timeout))
+			break;
 	}
-unlock:
-	mutex_unlock(&it6505->aux_lock);
 
+	/* recover interrupt trigger*/
+	it6505_set_bits(it6505, INT_MASK_03,
+			BIT(INT_VID_FIFO_ERROR), BIT(INT_VID_FIFO_ERROR));
 	return state;
 }
 

-- 
2.34.1

[PATCH v2 4/5] drm/bridge: it6505: modify DP link training work

[Hermes Wu via B4 Relay]

From: Hermes Wu <Hermes.wu@ite.com.tw>

The DP link training work include auto training and after
auto training failed "AUTO_TRAIN_RETRY" times, it switch to
step training mode.

It will more efficiency that finish link auto training,
include retry, and step training in a work, rather than
re-schedule train work when each training fail.

Drop auto_train_retry from it6505 structure,
and it6505_dump() is remove from link trainig work,
it takes too much time to read all register area,
and is not necessary.

Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
---
 drivers/gpu/drm/bridge/ite-it6505.c | 40 ++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index e0e13e737763fb801fd1cd803734a0d6ae1dd812..2bbe787ccb1b57906f2a31a011dd92f327019b08 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -467,7 +467,6 @@ struct it6505 {
 	struct delayed_work hdcp_work;
 	struct work_struct hdcp_wait_ksv_list;
 	struct completion extcon_completion;
-	u8 auto_train_retry;
 	bool hdcp_desired;
 	bool is_repeater;
 	u8 hdcp_down_stream_count;
@@ -666,7 +665,7 @@ static int it6505_get_dpcd(struct it6505 *it6505, int offset, u8 *dpcd, int num)
 	return 0;
 }
 
-static void it6505_dump(struct it6505 *it6505)
+static void __maybe_unused it6505_dump(struct it6505 *it6505)
 {
 	unsigned int i, j;
 	u8 regs[16];
@@ -1399,7 +1398,6 @@ static void it6505_variable_config(struct it6505 *it6505)
 	it6505->lane_count = MAX_LANE_COUNT;
 	it6505->link_state = LINK_IDLE;
 	it6505->hdcp_desired = HDCP_DESIRED;
-	it6505->auto_train_retry = AUTO_TRAIN_RETRY;
 	it6505->audio.select = AUDIO_SELECT;
 	it6505->audio.sample_rate = AUDIO_SAMPLE_RATE;
 	it6505->audio.channel_count = AUDIO_CHANNEL_COUNT;
@@ -2472,7 +2470,7 @@ static void it6505_link_training_work(struct work_struct *work)
 {
 	struct it6505 *it6505 = container_of(work, struct it6505, link_works);
 	struct device *dev = it6505->dev;
-	int ret;
+	int ret, retry;
 
 	DRM_DEV_DEBUG_DRIVER(dev, "it6505->sink_count: %d",
 			     it6505->sink_count);
@@ -2480,27 +2478,22 @@ static void it6505_link_training_work(struct work_struct *work)
 	if (!it6505_get_sink_hpd_status(it6505))
 		return;
 
-	it6505_link_training_setup(it6505);
-	it6505_reset_hdcp(it6505);
-	it6505_aux_reset(it6505);
-
-	if (it6505->auto_train_retry < 1) {
-		it6505_link_step_train_process(it6505);
-		return;
-	}
-
-	ret = it6505_link_start_auto_train(it6505);
-	DRM_DEV_DEBUG_DRIVER(dev, "auto train %s, auto_train_retry: %d",
-			     ret ? "pass" : "failed", it6505->auto_train_retry);
+	for (retry = AUTO_TRAIN_RETRY; retry > 0; retry--) {
+		it6505_link_training_setup(it6505);
+		it6505_reset_hdcp(it6505);
+		it6505_aux_reset(it6505);
 
-	if (ret) {
-		it6505->auto_train_retry = AUTO_TRAIN_RETRY;
-		it6505_link_train_ok(it6505);
-	} else {
-		it6505->auto_train_retry--;
-		it6505_dump(it6505);
+		ret = it6505_link_start_auto_train(it6505);
+		DRM_DEV_DEBUG_DRIVER(dev, "auto train %s,",
+				     ret ? "pass" : "failed");
+		if (ret) {
+			it6505_link_train_ok(it6505);
+			return;
+		}
 	}
 
+	/*After HW auto training fail, try link training step by step*/
+	it6505_link_step_train_process(it6505);
 }
 
 static void it6505_plugged_status_to_codec(struct it6505 *it6505)
@@ -2581,7 +2574,6 @@ static int it6505_process_hpd_irq(struct it6505 *it6505)
 			     (int)ARRAY_SIZE(link_status), link_status);
 
 	if (!drm_dp_channel_eq_ok(link_status, it6505->lane_count)) {
-		it6505->auto_train_retry = AUTO_TRAIN_RETRY;
 		it6505_video_reset(it6505);
 	}
 
@@ -2607,7 +2599,6 @@ static void it6505_irq_hpd(struct it6505 *it6505)
 			it6505_variable_config(it6505);
 			it6505_parse_link_capabilities(it6505);
 		}
-		it6505->auto_train_retry = AUTO_TRAIN_RETRY;
 
 		it6505_drm_dp_link_set_power(&it6505->aux, &it6505->link,
 					     DP_SET_POWER_D0);
@@ -2744,7 +2735,6 @@ static void it6505_irq_video_handler(struct it6505 *it6505, const int *int_statu
 			     (unsigned int *)int_status)) ||
 	    (it6505_test_bit(BIT_INT_VID_FIFO_ERROR,
 			     (unsigned int *)int_status))) {
-		it6505->auto_train_retry = AUTO_TRAIN_RETRY;
 		flush_work(&it6505->link_works);
 		it6505_stop_hdcp(it6505);
 		it6505_video_reset(it6505);

-- 
2.34.1

[PATCH v2 5/5] drm/bridge: it6505: skip auto training when previous try fail

[Hermes Wu via B4 Relay]

From: Hermes Wu <Hermes.wu@ite.com.tw>

When connect to device which can only training done by
step training, skip auto training when link training restart,
usually happen when display resolution is changed.

Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
---
 drivers/gpu/drm/bridge/ite-it6505.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index 2bbe787ccb1b57906f2a31a011dd92f327019b08..47f8134df8b6f5c2172c6cfdbc91da8063993a05 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -467,6 +467,7 @@ struct it6505 {
 	struct delayed_work hdcp_work;
 	struct work_struct hdcp_wait_ksv_list;
 	struct completion extcon_completion;
+	u8 step_train_only;
 	bool hdcp_desired;
 	bool is_repeater;
 	u8 hdcp_down_stream_count;
@@ -2457,11 +2458,13 @@ static void it6505_link_step_train_process(struct it6505 *it6505)
 				     ret ? "pass" : "failed", i + 1);
 		if (ret) {
 			it6505_link_train_ok(it6505);
+			it6505->step_train_only = true;
 			return;
 		}
 	}
 
 	DRM_DEV_DEBUG_DRIVER(dev, "training fail");
+	it6505->step_train_only = false;
 	it6505->link_state = LINK_IDLE;
 	it6505_video_reset(it6505);
 }
@@ -2477,18 +2480,19 @@ static void it6505_link_training_work(struct work_struct *work)
 
 	if (!it6505_get_sink_hpd_status(it6505))
 		return;
-
-	for (retry = AUTO_TRAIN_RETRY; retry > 0; retry--) {
-		it6505_link_training_setup(it6505);
-		it6505_reset_hdcp(it6505);
-		it6505_aux_reset(it6505);
-
-		ret = it6505_link_start_auto_train(it6505);
-		DRM_DEV_DEBUG_DRIVER(dev, "auto train %s,",
-				     ret ? "pass" : "failed");
-		if (ret) {
-			it6505_link_train_ok(it6505);
-			return;
+	if (!it6505->step_train_only) {
+		for (retry = AUTO_TRAIN_RETRY; retry > 0; retry--) {
+			it6505_link_training_setup(it6505);
+			it6505_reset_hdcp(it6505);
+			it6505_aux_reset(it6505);
+
+			ret = it6505_link_start_auto_train(it6505);
+			DRM_DEV_DEBUG_DRIVER(dev, "auto train %s,",
+					     ret ? "pass" : "failed");
+			if (ret) {
+				it6505_link_train_ok(it6505);
+				return;
+			}
 		}
 	}
 
@@ -2599,6 +2603,7 @@ static void it6505_irq_hpd(struct it6505 *it6505)
 			it6505_variable_config(it6505);
 			it6505_parse_link_capabilities(it6505);
 		}
+		it6505->step_train_only = false;
 
 		it6505_drm_dp_link_set_power(&it6505->aux, &it6505->link,
 					     DP_SET_POWER_D0);

-- 
2.34.1

Re: [PATCH v2 1/5] drm/bridge: it6505: fix link training state HW register reset

[Dmitry Baryshkov]

On Wed, Mar 26, 2025 at 01:34:13PM +0800, Hermes Wu via B4 Relay wrote:
> From: Hermes Wu <Hermes.wu@ite.com.tw>
> 
> In order to reset HW link auto training state,
> bits FORCE_RETRAIN and MANUAL_TRAIN at REG_TRAIN_CTRL1 must be set
> at the same time.

Nit: please start by describing the problem that the patch attempts to
solve.

> 
> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> ---
>  drivers/gpu/drm/bridge/ite-it6505.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index 88ef76a37fe6accacdd343839ff2569b31b18ceb..b47042dc06bb1e1dfd92bc0dc6d69dc918a140ba 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -1806,7 +1806,9 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
>  	mutex_lock(&it6505->aux_lock);
>  	it6505_set_bits(it6505, REG_TRAIN_CTRL0,
>  			FORCE_CR_DONE | FORCE_EQ_DONE, 0x00);
> -	it6505_write(it6505, REG_TRAIN_CTRL1, FORCE_RETRAIN);
> +	/* reset link state machine and re start training*/
> +	it6505_write(it6505, REG_TRAIN_CTRL1,
> +		     FORCE_RETRAIN | MANUAL_TRAIN);
>  	it6505_write(it6505, REG_TRAIN_CTRL1, AUTO_TRAIN);
>  
>  	while (timeout > 0) {
> @@ -2377,7 +2379,7 @@ static void it6505_stop_link_train(struct it6505 *it6505)
>  {
>  	it6505->link_state = LINK_IDLE;
>  	cancel_work_sync(&it6505->link_works);
> -	it6505_write(it6505, REG_TRAIN_CTRL1, FORCE_RETRAIN);
> +	it6505_write(it6505, REG_TRAIN_CTRL1, FORCE_RETRAIN | MANUAL_TRAIN);
>  }
>  
>  static void it6505_link_train_ok(struct it6505 *it6505)
> 
> -- 
> 2.34.1
> 
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/5] drm/bridge: it6505: check INT_LINK_TRAIN_FAIL while link auto training

[Dmitry Baryshkov]

On Wed, Mar 26, 2025 at 01:34:14PM +0800, Hermes Wu via B4 Relay wrote:
> From: Hermes Wu <Hermes.wu@ite.com.tw>
> 
> When start link training, interrupt status INT_LINK_TRAIN_FAIL can
> use to check link training fail and no need to wait until timeout.
> 
> it6505_irq_link_train_fail() remove from interrupt and no longer used.
> 
> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> ---
>  drivers/gpu/drm/bridge/ite-it6505.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

Re: [PATCH v2 3/5] drm/bridge: it6505: modify DP link auto training

[Dmitry Baryshkov]

On Wed, Mar 26, 2025 at 01:34:15PM +0800, Hermes Wu via B4 Relay wrote:
> From: Hermes Wu <Hermes.wu@ite.com.tw>
> 
> IT6505 supports HW link training which will write DPCD and check
> training status automatically.
> 
> In the case that driver set link rate at 2.7G and HW fail to training,
> it will change link configuration and try 1.65G. And this will cause
> INT_VID_FIFO_ERROR triggered when link clock is changed.
> 
> When video error occurs, video logic is reset and link training restart,
> this will cause endless auto link training.
> 
> Modify link auto training with disable INT_VID_FIFO_ERROR to avoid loop
> and check INT_LINK_TRAIN_FAIL event to abort wait training done.
> 
> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> ---
>  drivers/gpu/drm/bridge/ite-it6505.c | 38 +++++++++++++++++++++++++++----------
>  1 file changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index 0607f99446b37c82b41a376c2f4e10c7565d1b61..e0e13e737763fb801fd1cd803734a0d6ae1dd812 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -1800,11 +1800,20 @@ static void it6505_link_training_setup(struct it6505 *it6505)
>  
>  static bool it6505_link_start_auto_train(struct it6505 *it6505)
>  {
> -	int timeout = 500, link_training_state;
> +	int link_training_state;
>  	bool state = false;
>  	int int03;
> +	struct device *dev = it6505->dev;
> +	unsigned long timeout;
> +
> +	guard(mutex)(&it6505->aux_lock);

Please don't mix functional changes and refactoring.

> +	/* Disable FIFO error interrupt trigger  */
> +	/* to prevent training fail loop issue   */
> +	it6505_set_bits(it6505, INT_MASK_03, BIT(INT_VID_FIFO_ERROR), 0);
> +
> +	it6505_write(it6505, INT_STATUS_03,
> +		     BIT(INT_LINK_TRAIN_FAIL) | BIT(INT_VID_FIFO_ERROR));
>  
> -	mutex_lock(&it6505->aux_lock);
>  	it6505_set_bits(it6505, REG_TRAIN_CTRL0,
>  			FORCE_CR_DONE | FORCE_EQ_DONE, 0x00);
>  	/* reset link state machine and re start training*/
> @@ -1812,32 +1821,41 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
>  		     FORCE_RETRAIN | MANUAL_TRAIN);
>  	it6505_write(it6505, REG_TRAIN_CTRL1, AUTO_TRAIN);
>  
> -	while (timeout > 0) {
> +	timeout = jiffies + msecs_to_jiffies(100) + 1;
> +	for (;;) {
>  		usleep_range(1000, 2000);
>  		link_training_state = it6505_read(it6505, REG_LINK_TRAIN_STS);
>  		int03 = it6505_read(it6505, INT_STATUS_03);
>  		if (int03 & BIT(INT_LINK_TRAIN_FAIL)) {
> -			it6505_write(it6505, INT_STATUS_03,
> -				     BIT(INT_LINK_TRAIN_FAIL));
> -
>  			DRM_DEV_DEBUG_DRIVER(dev,
>  					     "INT_LINK_TRAIN_FAIL(%x)!",
>  					      int03);
>  
> +			/* Ignore INT_VID_FIFO_ERROR when auto training fail*/
> +			it6505_write(it6505, INT_STATUS_03,
> +				     BIT(INT_LINK_TRAIN_FAIL) |
> +				     BIT(INT_VID_FIFO_ERROR));

Why can't it stay where it was?

> +
> +			if (int03 & BIT(INT_VID_FIFO_ERROR))
> +				DRM_DEV_DEBUG_DRIVER(dev,
> +						     "video fifo error when training fail");

Isn't the first message enough?

> +
>  			break;
>  		}
>  
>  		if (link_training_state > 0 &&
>  		    (link_training_state & LINK_STATE_NORP)) {
>  			state = true;
> -			goto unlock;
> +			break;
>  		}
>  
> -		timeout--;
> +		if (time_after(jiffies, timeout))
> +			break;
>  	}
> -unlock:
> -	mutex_unlock(&it6505->aux_lock);
>  
> +	/* recover interrupt trigger*/
> +	it6505_set_bits(it6505, INT_MASK_03,
> +			BIT(INT_VID_FIFO_ERROR), BIT(INT_VID_FIFO_ERROR));
>  	return state;
>  }
>  
> 
> -- 
> 2.34.1
> 
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 4/5] drm/bridge: it6505: modify DP link training work

[Dmitry Baryshkov]

On Wed, Mar 26, 2025 at 01:34:16PM +0800, Hermes Wu via B4 Relay wrote:
> From: Hermes Wu <Hermes.wu@ite.com.tw>
> 
> The DP link training work include auto training and after
> auto training failed "AUTO_TRAIN_RETRY" times, it switch to
> step training mode.
> 
> It will more efficiency that finish link auto training,
> include retry, and step training in a work, rather than
> re-schedule train work when each training fail.
> 
> Drop auto_train_retry from it6505 structure,
> and it6505_dump() is remove from link trainig work,
> it takes too much time to read all register area,
> and is not necessary.
> 
> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> ---
>  drivers/gpu/drm/bridge/ite-it6505.c | 40 ++++++++++++++-----------------------
>  1 file changed, 15 insertions(+), 25 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

Re: [PATCH v2 5/5] drm/bridge: it6505: skip auto training when previous try fail

[Dmitry Baryshkov]

On Wed, Mar 26, 2025 at 01:34:17PM +0800, Hermes Wu via B4 Relay wrote:
> From: Hermes Wu <Hermes.wu@ite.com.tw>
> 
> When connect to device which can only training done by
> step training, skip auto training when link training restart,
> usually happen when display resolution is changed.
> 
> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> ---
>  drivers/gpu/drm/bridge/ite-it6505.c | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index 2bbe787ccb1b57906f2a31a011dd92f327019b08..47f8134df8b6f5c2172c6cfdbc91da8063993a05 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -467,6 +467,7 @@ struct it6505 {
>  	struct delayed_work hdcp_work;
>  	struct work_struct hdcp_wait_ksv_list;
>  	struct completion extcon_completion;
> +	u8 step_train_only;
>  	bool hdcp_desired;
>  	bool is_repeater;
>  	u8 hdcp_down_stream_count;
> @@ -2457,11 +2458,13 @@ static void it6505_link_step_train_process(struct it6505 *it6505)
>  				     ret ? "pass" : "failed", i + 1);
>  		if (ret) {
>  			it6505_link_train_ok(it6505);
> +			it6505->step_train_only = true;
>  			return;
>  		}
>  	}
>  
>  	DRM_DEV_DEBUG_DRIVER(dev, "training fail");
> +	it6505->step_train_only = false;
>  	it6505->link_state = LINK_IDLE;
>  	it6505_video_reset(it6505);
>  }
> @@ -2477,18 +2480,19 @@ static void it6505_link_training_work(struct work_struct *work)
>  
>  	if (!it6505_get_sink_hpd_status(it6505))
>  		return;
> -
> -	for (retry = AUTO_TRAIN_RETRY; retry > 0; retry--) {
> -		it6505_link_training_setup(it6505);
> -		it6505_reset_hdcp(it6505);
> -		it6505_aux_reset(it6505);
> -
> -		ret = it6505_link_start_auto_train(it6505);
> -		DRM_DEV_DEBUG_DRIVER(dev, "auto train %s,",
> -				     ret ? "pass" : "failed");
> -		if (ret) {
> -			it6505_link_train_ok(it6505);
> -			return;
> +	if (!it6505->step_train_only) {

No need to. Just

		for (retry = AUTO_TRAIN_RETRY; retry > 0 && !it6505->step_train_only; retry--) {

> +		for (retry = AUTO_TRAIN_RETRY; retry > 0; retry--) {
> +			it6505_link_training_setup(it6505);
> +			it6505_reset_hdcp(it6505);
> +			it6505_aux_reset(it6505);
> +
> +			ret = it6505_link_start_auto_train(it6505);
> +			DRM_DEV_DEBUG_DRIVER(dev, "auto train %s,",
> +					     ret ? "pass" : "failed");
> +			if (ret) {
> +				it6505_link_train_ok(it6505);
> +				return;
> +			}
>  		}
>  	}
>  
> @@ -2599,6 +2603,7 @@ static void it6505_irq_hpd(struct it6505 *it6505)
>  			it6505_variable_config(it6505);
>  			it6505_parse_link_capabilities(it6505);
>  		}
> +		it6505->step_train_only = false;
>  
>  		it6505_drm_dp_link_set_power(&it6505->aux, &it6505->link,
>  					     DP_SET_POWER_D0);
> 
> -- 
> 2.34.1
> 
> 

-- 
With best wishes
Dmitry

RE: [PATCH v2 3/5] drm/bridge: it6505: modify DP link auto training

[Hermes.Wu]

>-----Original Message-----
>From: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> 
>Sent: Monday, March 31, 2025 2:44 AM
>To: Hermes Wu (吳佳宏) <Hermes.Wu@ite.com.tw>
>Cc: Andrzej Hajda <andrzej.hajda@intel.com>; Neil Armstrong <neil.armstrong@linaro.org>; Robert Foss <rfoss@kernel.org>; Laurent Pinchart <Laurent.pinchart@ideasonboard.com>; Jonas Karlman <jonas@kwiboo.se>; Jernej Skrabec <jernej.skrabec@gmail.com>; Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David Airlie <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Pet Weng (翁玉芬) <Pet.Weng@ite.com.tw>; Kenneth Hung (洪家倫) <Kenneth.Hung@ite.com.tw>; treapking@chromium.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH v2 3/5] drm/bridge: it6505: modify DP link auto training
>
>On Wed, Mar 26, 2025 at 01:34:15PM +0800, Hermes Wu via B4 Relay wrote:
>> From: Hermes Wu <Hermes.wu@ite.com.tw>
>> 
>> IT6505 supports HW link training which will write DPCD and check 
>> training status automatically.
>> 
>> In the case that driver set link rate at 2.7G and HW fail to training, 
>> it will change link configuration and try 1.65G. And this will cause 
>> INT_VID_FIFO_ERROR triggered when link clock is changed.
>> 
>> When video error occurs, video logic is reset and link training 
>> restart, this will cause endless auto link training.
>> 
>> Modify link auto training with disable INT_VID_FIFO_ERROR to avoid 
>> loop and check INT_LINK_TRAIN_FAIL event to abort wait training done.
>> 
>> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
>> ---
>>  drivers/gpu/drm/bridge/ite-it6505.c | 38 
>> +++++++++++++++++++++++++++----------
>>  1 file changed, 28 insertions(+), 10 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c 
>> b/drivers/gpu/drm/bridge/ite-it6505.c
>> index 
>> 0607f99446b37c82b41a376c2f4e10c7565d1b61..e0e13e737763fb801fd1cd803734
>> a0d6ae1dd812 100644
>> --- a/drivers/gpu/drm/bridge/ite-it6505.c
>> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
>> @@ -1800,11 +1800,20 @@ static void it6505_link_training_setup(struct 
>> it6505 *it6505)
>>  
>>  static bool it6505_link_start_auto_train(struct it6505 *it6505)  {
>> -	int timeout = 500, link_training_state;
>> +	int link_training_state;
>>  	bool state = false;
>>  	int int03;
>> +	struct device *dev = it6505->dev;
>> +	unsigned long timeout;
>> +
>> +	guard(mutex)(&it6505->aux_lock);
>
>Please don't mix functional changes and refactoring.
>
>> +	/* Disable FIFO error interrupt trigger  */
>> +	/* to prevent training fail loop issue   */
>> +	it6505_set_bits(it6505, INT_MASK_03, BIT(INT_VID_FIFO_ERROR), 0);
>> +
>> +	it6505_write(it6505, INT_STATUS_03,
>> +		     BIT(INT_LINK_TRAIN_FAIL) | BIT(INT_VID_FIFO_ERROR));
>>  
>> -	mutex_lock(&it6505->aux_lock);
>>  	it6505_set_bits(it6505, REG_TRAIN_CTRL0,
>>  			FORCE_CR_DONE | FORCE_EQ_DONE, 0x00);
>>  	/* reset link state machine and re start training*/ @@ -1812,32 
>> +1821,41 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
>>  		     FORCE_RETRAIN | MANUAL_TRAIN);
>>  	it6505_write(it6505, REG_TRAIN_CTRL1, AUTO_TRAIN);
>>  
>> -	while (timeout > 0) {
>> +	timeout = jiffies + msecs_to_jiffies(100) + 1;
>> +	for (;;) {
>>  		usleep_range(1000, 2000);
>>  		link_training_state = it6505_read(it6505, REG_LINK_TRAIN_STS);
>>  		int03 = it6505_read(it6505, INT_STATUS_03);
>>  		if (int03 & BIT(INT_LINK_TRAIN_FAIL)) {
>> -			it6505_write(it6505, INT_STATUS_03,
>> -				     BIT(INT_LINK_TRAIN_FAIL));
>> -
>>  			DRM_DEV_DEBUG_DRIVER(dev,
>>  					     "INT_LINK_TRAIN_FAIL(%x)!",
>>  					      int03);
>>  
>> +			/* Ignore INT_VID_FIFO_ERROR when auto training fail*/
>> +			it6505_write(it6505, INT_STATUS_03,
>> +				     BIT(INT_LINK_TRAIN_FAIL) |
>> +				     BIT(INT_VID_FIFO_ERROR));
>
>Why can't it stay where it was?

Do you mean stay at original INT_LINK_TRAIN_FAIL clear and just add INT_VID_FIFO_ERROR?


>> +
>> +			if (int03 & BIT(INT_VID_FIFO_ERROR))
>> +				DRM_DEV_DEBUG_DRIVER(dev,
>> +						     "video fifo error when training fail");
>
>Isn't the first message enough?

It just leave for checking if INT_VID_FIFO_ERROR happen,
The first message has shown int03 value, and this message will remove next patch.
>> +
>>  			break;
>>  		}
>>  
>>  		if (link_training_state > 0 &&
>>  		    (link_training_state & LINK_STATE_NORP)) {
>>  			state = true;
>> -			goto unlock;
>> +			break;
>>  		}
>>  
>> -		timeout--;
>> +		if (time_after(jiffies, timeout))
>> +			break;
>>  	}
>> -unlock:
>> -	mutex_unlock(&it6505->aux_lock);
>>  
>> +	/* recover interrupt trigger*/
>> +	it6505_set_bits(it6505, INT_MASK_03,
>> +			BIT(INT_VID_FIFO_ERROR), BIT(INT_VID_FIFO_ERROR));
>>  	return state;
>>  }
>>  
>> 
>> --
>> 2.34.1
>> 
>> 
>
>--
>With best wishes
>Dmitry
>

BR,
Hermes

Re: [PATCH v2 3/5] drm/bridge: it6505: modify DP link auto training

[Dmitry Baryshkov]

On Mon, Mar 31, 2025 at 02:31:30AM +0000, Hermes.Wu@ite.com.tw wrote:
> >-----Original Message-----
> >From: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> 
> >Sent: Monday, March 31, 2025 2:44 AM
> >To: Hermes Wu (吳佳宏) <Hermes.Wu@ite.com.tw>
> >Cc: Andrzej Hajda <andrzej.hajda@intel.com>; Neil Armstrong <neil.armstrong@linaro.org>; Robert Foss <rfoss@kernel.org>; Laurent Pinchart <Laurent.pinchart@ideasonboard.com>; Jonas Karlman <jonas@kwiboo.se>; Jernej Skrabec <jernej.skrabec@gmail.com>; Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David Airlie <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Pet Weng (翁玉芬) <Pet.Weng@ite.com.tw>; Kenneth Hung (洪家倫) <Kenneth.Hung@ite.com.tw>; treapking@chromium.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
> >Subject: Re: [PATCH v2 3/5] drm/bridge: it6505: modify DP link auto training
> >
> >On Wed, Mar 26, 2025 at 01:34:15PM +0800, Hermes Wu via B4 Relay wrote:
> >> From: Hermes Wu <Hermes.wu@ite.com.tw>
> >> 
> >> IT6505 supports HW link training which will write DPCD and check 
> >> training status automatically.
> >> 
> >> In the case that driver set link rate at 2.7G and HW fail to training, 
> >> it will change link configuration and try 1.65G. And this will cause 
> >> INT_VID_FIFO_ERROR triggered when link clock is changed.
> >> 
> >> When video error occurs, video logic is reset and link training 
> >> restart, this will cause endless auto link training.
> >> 
> >> Modify link auto training with disable INT_VID_FIFO_ERROR to avoid 
> >> loop and check INT_LINK_TRAIN_FAIL event to abort wait training done.
> >> 
> >> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> >> ---
> >>  drivers/gpu/drm/bridge/ite-it6505.c | 38 
> >> +++++++++++++++++++++++++++----------
> >>  1 file changed, 28 insertions(+), 10 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c 
> >> b/drivers/gpu/drm/bridge/ite-it6505.c
> >> index 
> >> 0607f99446b37c82b41a376c2f4e10c7565d1b61..e0e13e737763fb801fd1cd803734
> >> a0d6ae1dd812 100644
> >> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> >> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> >> @@ -1800,11 +1800,20 @@ static void it6505_link_training_setup(struct 
> >> it6505 *it6505)
> >>  
> >>  static bool it6505_link_start_auto_train(struct it6505 *it6505)  {
> >> -	int timeout = 500, link_training_state;
> >> +	int link_training_state;
> >>  	bool state = false;
> >>  	int int03;
> >> +	struct device *dev = it6505->dev;
> >> +	unsigned long timeout;
> >> +
> >> +	guard(mutex)(&it6505->aux_lock);
> >
> >Please don't mix functional changes and refactoring.
> >
> >> +	/* Disable FIFO error interrupt trigger  */
> >> +	/* to prevent training fail loop issue   */
> >> +	it6505_set_bits(it6505, INT_MASK_03, BIT(INT_VID_FIFO_ERROR), 0);
> >> +
> >> +	it6505_write(it6505, INT_STATUS_03,
> >> +		     BIT(INT_LINK_TRAIN_FAIL) | BIT(INT_VID_FIFO_ERROR));
> >>  
> >> -	mutex_lock(&it6505->aux_lock);
> >>  	it6505_set_bits(it6505, REG_TRAIN_CTRL0,
> >>  			FORCE_CR_DONE | FORCE_EQ_DONE, 0x00);
> >>  	/* reset link state machine and re start training*/ @@ -1812,32 
> >> +1821,41 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
> >>  		     FORCE_RETRAIN | MANUAL_TRAIN);
> >>  	it6505_write(it6505, REG_TRAIN_CTRL1, AUTO_TRAIN);
> >>  
> >> -	while (timeout > 0) {
> >> +	timeout = jiffies + msecs_to_jiffies(100) + 1;
> >> +	for (;;) {
> >>  		usleep_range(1000, 2000);
> >>  		link_training_state = it6505_read(it6505, REG_LINK_TRAIN_STS);
> >>  		int03 = it6505_read(it6505, INT_STATUS_03);
> >>  		if (int03 & BIT(INT_LINK_TRAIN_FAIL)) {
> >> -			it6505_write(it6505, INT_STATUS_03,
> >> -				     BIT(INT_LINK_TRAIN_FAIL));
> >> -
> >>  			DRM_DEV_DEBUG_DRIVER(dev,
> >>  					     "INT_LINK_TRAIN_FAIL(%x)!",
> >>  					      int03);
> >>  
> >> +			/* Ignore INT_VID_FIFO_ERROR when auto training fail*/
> >> +			it6505_write(it6505, INT_STATUS_03,
> >> +				     BIT(INT_LINK_TRAIN_FAIL) |
> >> +				     BIT(INT_VID_FIFO_ERROR));
> >
> >Why can't it stay where it was?
> 
> Do you mean stay at original INT_LINK_TRAIN_FAIL clear and just add INT_VID_FIFO_ERROR?

Yes.

> 
> 

-- 
With best wishes
Dmitry