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

Hermes Wu via B4 Relay posted 5 patches 2 weeks, 2 days ago
There is a newer version of this series
Re: [PATCH RESEND v3 3/5] drm/bridge: it6505: modify DP link auto training
Posted by Dmitry Baryshkov 2 weeks, 2 days ago
On Tue, Sep 16, 2025 at 12:47:43PM +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 | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index 7f6227c278a51358c70a3de93454aafeef64f2bb..f9b99c70789eea6beb3c6513155c9a4ca103d219 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -1806,6 +1806,13 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
>  	struct device *dev = it6505->dev;
>  
>  	mutex_lock(&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));
>  	it6505_set_bits(it6505, REG_TRAIN_CTRL0,
>  			FORCE_CR_DONE | FORCE_EQ_DONE, 0x00);
>  	/* reset link state machine and re start training*/
> @@ -1818,8 +1825,10 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
>  		link_training_state = it6505_read(it6505, REG_LINK_TRAIN_STS);
>  		int03 = it6505_read(it6505, INT_STATUS_03);
>  		if (int03 & BIT(INT_LINK_TRAIN_FAIL)) {
> +			/* Ignore INT_VID_FIFO_ERROR when auto training fail*/
>  			it6505_write(it6505, INT_STATUS_03,
> -				     BIT(INT_LINK_TRAIN_FAIL));
> +				     BIT(INT_LINK_TRAIN_FAIL) |
> +				     BIT(INT_VID_FIFO_ERROR));

I'm really unusure about this change. Judging by the description of the
problem, it's fix for the issue, but the issue gets introduced in the
previous patch.

>  
>  			DRM_DEV_DEBUG_DRIVER(dev,
>  					     "INT_LINK_TRAIN_FAIL(%x)!",
> @@ -1837,6 +1846,9 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
>  		timeout--;
>  	}
>  unlock:
> +	/* recover interrupt trigger*/
> +	it6505_set_bits(it6505, INT_MASK_03,
> +			BIT(INT_VID_FIFO_ERROR), BIT(INT_VID_FIFO_ERROR));
>  	mutex_unlock(&it6505->aux_lock);
>  
>  	return state;
> 
> -- 
> 2.34.1
> 
> 

-- 
With best wishes
Dmitry
RE: [PATCH RESEND v3 3/5] drm/bridge: it6505: modify DP link auto training
Posted by Hermes.Wu@ite.com.tw 2 weeks, 1 day ago
>-----Original Message-----
>From: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> 
>Sent: Tuesday, September 16, 2025 6:49 PM
>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 RESEND v3 3/5] drm/bridge: it6505: modify DP link auto training
>
>On Tue, Sep 16, 2025 at 12:47:43PM +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 | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
>> index 7f6227c278a51358c70a3de93454aafeef64f2bb..f9b99c70789eea6beb3c6513155c9a4ca103d219 100644
>> --- a/drivers/gpu/drm/bridge/ite-it6505.c
>> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
>> @@ -1806,6 +1806,13 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
>>  	struct device *dev = it6505->dev;
>>  
>>  	mutex_lock(&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));
>>  	it6505_set_bits(it6505, REG_TRAIN_CTRL0,
>>  			FORCE_CR_DONE | FORCE_EQ_DONE, 0x00);
>>  	/* reset link state machine and re start training*/
>> @@ -1818,8 +1825,10 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
>>  		link_training_state = it6505_read(it6505, REG_LINK_TRAIN_STS);
>>  		int03 = it6505_read(it6505, INT_STATUS_03);
>>  		if (int03 & BIT(INT_LINK_TRAIN_FAIL)) {
>> +			/* Ignore INT_VID_FIFO_ERROR when auto training fail*/
>>  			it6505_write(it6505, INT_STATUS_03,
>> -				     BIT(INT_LINK_TRAIN_FAIL));
>> +				     BIT(INT_LINK_TRAIN_FAIL) |
>> +				     BIT(INT_VID_FIFO_ERROR));
>
>I'm really unusure about this change. Judging by the description of the
>problem, it's fix for the issue, but the issue gets introduced in the
>previous patch.
In this patch serious?

This patch serious fix this FIFO error issue, it change link training algorithm first then fix wrong FIFO error status.

The link training process start after video status is stable, and when video FIFO error occurs,
video stable status will also lost, link training will reset to idle and wait until video stable again.

IT6505 HW auto training will process link training automatically, which include CR/EQ DPCD setting, link status check,
and try lower link rate is the 2.7G cannot pass training.

In some case, DP connect to a DP sink device which cannot pass IT6505 HW auto training. 
when link auto training fail on 2.7G and IT6505 HW change link rate to 1.65G and retry training automatically,
at this time video FIFO error will occur because of the link rate change(chip issue), the video signal from SOC is not lost actually.


>>  
>>  			DRM_DEV_DEBUG_DRIVER(dev,
>>  					     "INT_LINK_TRAIN_FAIL(%x)!",
>> @@ -1837,6 +1846,9 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
>>  		timeout--;
>>  	}
>>  unlock:
>> +	/* recover interrupt trigger*/
>> +	it6505_set_bits(it6505, INT_MASK_03,
>> +			BIT(INT_VID_FIFO_ERROR), BIT(INT_VID_FIFO_ERROR));
>>  	mutex_unlock(&it6505->aux_lock);
>>  
>>  	return state;
>> 
>> -- 
>> 2.34.1
>> 
>> 
>
>-- 
>With best wishes
>Dmitry
>
BR.
Hermes Wu
Re: [PATCH RESEND v3 3/5] drm/bridge: it6505: modify DP link auto training
Posted by Dmitry Baryshkov 2 weeks, 1 day ago
On Wed, Sep 17, 2025 at 08:37:10AM +0000, Hermes.Wu@ite.com.tw wrote:
> 
> >-----Original Message-----
> >From: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> 
> >Sent: Tuesday, September 16, 2025 6:49 PM
> >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 RESEND v3 3/5] drm/bridge: it6505: modify DP link auto training
> >
> >On Tue, Sep 16, 2025 at 12:47:43PM +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 | 14 +++++++++++++-
> >>  1 file changed, 13 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> >> index 7f6227c278a51358c70a3de93454aafeef64f2bb..f9b99c70789eea6beb3c6513155c9a4ca103d219 100644
> >> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> >> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> >> @@ -1806,6 +1806,13 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
> >>  	struct device *dev = it6505->dev;
> >>  
> >>  	mutex_lock(&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));
> >>  	it6505_set_bits(it6505, REG_TRAIN_CTRL0,
> >>  			FORCE_CR_DONE | FORCE_EQ_DONE, 0x00);
> >>  	/* reset link state machine and re start training*/
> >> @@ -1818,8 +1825,10 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
> >>  		link_training_state = it6505_read(it6505, REG_LINK_TRAIN_STS);
> >>  		int03 = it6505_read(it6505, INT_STATUS_03);
> >>  		if (int03 & BIT(INT_LINK_TRAIN_FAIL)) {
> >> +			/* Ignore INT_VID_FIFO_ERROR when auto training fail*/
> >>  			it6505_write(it6505, INT_STATUS_03,
> >> -				     BIT(INT_LINK_TRAIN_FAIL));
> >> +				     BIT(INT_LINK_TRAIN_FAIL) |
> >> +				     BIT(INT_VID_FIFO_ERROR));
> >
> >I'm really unusure about this change. Judging by the description of the
> >problem, it's fix for the issue, but the issue gets introduced in the
> >previous patch.
> In this patch serious?
> 
> This patch serious fix this FIFO error issue, it change link training algorithm first then fix wrong FIFO error status.
> 
> The link training process start after video status is stable, and when video FIFO error occurs,
> video stable status will also lost, link training will reset to idle and wait until video stable again.
> 
> IT6505 HW auto training will process link training automatically, which include CR/EQ DPCD setting, link status check,
> and try lower link rate is the 2.7G cannot pass training.
> 
> In some case, DP connect to a DP sink device which cannot pass IT6505 HW auto training. 
> when link auto training fail on 2.7G and IT6505 HW change link rate to 1.65G and retry training automatically,
> at this time video FIFO error will occur because of the link rate change(chip issue), the video signal from SOC is not lost actually.

We seems to be misunderstanding each other. I pointed out that your are
fixing the code that was introduced in the previous patch. Would it make
more sense to reoder or to squash the patches?

> 
> 
> >>  
> >>  			DRM_DEV_DEBUG_DRIVER(dev,
> >>  					     "INT_LINK_TRAIN_FAIL(%x)!",
> >> @@ -1837,6 +1846,9 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
> >>  		timeout--;
> >>  	}
> >>  unlock:
> >> +	/* recover interrupt trigger*/
> >> +	it6505_set_bits(it6505, INT_MASK_03,
> >> +			BIT(INT_VID_FIFO_ERROR), BIT(INT_VID_FIFO_ERROR));
> >>  	mutex_unlock(&it6505->aux_lock);
> >>  
> >>  	return state;
> >> 
> >> -- 
> >> 2.34.1
> >> 
> >> 
> >
> >-- 
> >With best wishes
> >Dmitry
> >
> BR.
> Hermes Wu

-- 
With best wishes
Dmitry
RE: [PATCH RESEND v3 3/5] drm/bridge: it6505: modify DP link auto training
Posted by Hermes.Wu@ite.com.tw 2 weeks ago
>
>
>-----Original Message-----
>From: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> 
>Sent: Wednesday, September 17, 2025 9:47 PM
>To: Hermes Wu (吳佳宏) <Hermes.Wu@ite.com.tw>
>Cc: andrzej.hajda@intel.com; neil.armstrong@linaro.org; rfoss@kernel.org; Laurent.pinchart@ideasonboard.com; jonas@kwiboo.se; jernej.skrabec@gmail.com; maarten.lankhorst@linux.intel.com; mripard@kernel.org; tzimmermann@suse.de; airlied@gmail.com; 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 RESEND v3 3/5] drm/bridge: it6505: modify DP link auto training
>
>On Wed, Sep 17, 2025 at 08:37:10AM +0000, Hermes.Wu@ite.com.tw wrote:
>> 
>> >-----Original Message-----
>> >From: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> 
>> >Sent: Tuesday, September 16, 2025 6:49 PM
>> >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 RESEND v3 3/5] drm/bridge: it6505: modify DP link auto training
>> >
>> >On Tue, Sep 16, 2025 at 12:47:43PM +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 | 14 +++++++++++++-
>> >>  1 file changed, 13 insertions(+), 1 deletion(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
>> >> index 7f6227c278a51358c70a3de93454aafeef64f2bb..f9b99c70789eea6beb3c6513155c9a4ca103d219 100644
>> >> --- a/drivers/gpu/drm/bridge/ite-it6505.c
>> >> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
>> >> @@ -1806,6 +1806,13 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
>> >>  	struct device *dev = it6505->dev;
>> >>  
>> >>  	mutex_lock(&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));
>> >>  	it6505_set_bits(it6505, REG_TRAIN_CTRL0,
>> >>  			FORCE_CR_DONE | FORCE_EQ_DONE, 0x00);
>> >>  	/* reset link state machine and re start training*/
>> >> @@ -1818,8 +1825,10 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
>> >>  		link_training_state = it6505_read(it6505, REG_LINK_TRAIN_STS);
>> >>  		int03 = it6505_read(it6505, INT_STATUS_03);
>> >>  		if (int03 & BIT(INT_LINK_TRAIN_FAIL)) {
>> >> +			/* Ignore INT_VID_FIFO_ERROR when auto training fail*/
>> >>  			it6505_write(it6505, INT_STATUS_03,
>> >> -				     BIT(INT_LINK_TRAIN_FAIL));
>> >> +				     BIT(INT_LINK_TRAIN_FAIL) |
>> >> +				     BIT(INT_VID_FIFO_ERROR));
>> >
>> >I'm really unusure about this change. Judging by the description of the
>> >problem, it's fix for the issue, but the issue gets introduced in the
>> >previous patch.
>> In this patch serious?
>> 
>> This patch serious fix this FIFO error issue, it change link training algorithm first then fix wrong FIFO error status.
>> 
>> The link training process start after video status is stable, and when video FIFO error occurs,
>> video stable status will also lost, link training will reset to idle and wait until video stable again.
>> 
>> IT6505 HW auto training will process link training automatically, which include CR/EQ DPCD setting, link status check,
>> and try lower link rate is the 2.7G cannot pass training.
>> 
>> In some case, DP connect to a DP sink device which cannot pass IT6505 HW auto training. 
>> when link auto training fail on 2.7G and IT6505 HW change link rate to 1.65G and retry training automatically,
>> at this time video FIFO error will occur because of the link rate change(chip issue), the video signal from SOC is not lost actually.
>
>We seems to be misunderstanding each other. I pointed out that your are
>fixing the code that was introduced in the previous patch. Would it make
>more sense to reoder or to squash the patches?

this patch can squash to previous one. they are part of auto link training fail check.

I will update in next patch serious

>> 
>> >>  
>> >>  			DRM_DEV_DEBUG_DRIVER(dev,
>> >>  					     "INT_LINK_TRAIN_FAIL(%x)!",
>> >> @@ -1837,6 +1846,9 @@ static bool it6505_link_start_auto_train(struct it6505 *it6505)
>> >>  		timeout--;
>> >>  	}
>> >>  unlock:
>> >> +	/* recover interrupt trigger*/
>> >> +	it6505_set_bits(it6505, INT_MASK_03,
>> >> +			BIT(INT_VID_FIFO_ERROR), BIT(INT_VID_FIFO_ERROR));
>> >>  	mutex_unlock(&it6505->aux_lock);
>> >>  
>> >>  	return state;
>> >> 
>> >> -- 
>> >> 2.34.1
>> >> 
>> >> 
>> >
>> >-- 
>> >With best wishes
>> >Dmitry
>> >
>> BR.
>> Hermes Wu
>
>-- 
>With best wishes
>Dmitry
>