Message-ID: <nh3mxefaqznbirhez4c5xtkvs756cjasmb5bdsxol7cai7u4da@3nmtq6figidw>
Date: Wed, 17 Sep 2025 16:47:02 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Hermes.Wu@....com.tw
Cc: andrzej.hajda@...el.com, neil.armstrong@...aro.org, rfoss@...nel.org,
        Laurent.pinchart@...asonboard.com, jonas@...boo.se,
        jernej.skrabec@...il.com, maarten.lankhorst@...ux.intel.com,
        mripard@...nel.org, tzimmermann@...e.de, airlied@...il.com,
        simona@...ll.ch, Pet.Weng@....com.tw, Kenneth.Hung@....com.tw,
        treapking@...omium.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.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@....com.tw wrote:
> 
> >-----Original Message-----
> >From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com> 
> >Sent: Tuesday, September 16, 2025 6:49 PM
> >To: Hermes Wu (吳佳宏) <Hermes.Wu@....com.tw>
> >Cc: Andrzej Hajda <andrzej.hajda@...el.com>; Neil Armstrong <neil.armstrong@...aro.org>; Robert Foss <rfoss@...nel.org>; Laurent Pinchart <Laurent.pinchart@...asonboard.com>; Jonas Karlman <jonas@...boo.se>; Jernej Skrabec <jernej.skrabec@...il.com>; Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>; Maxime Ripard <mripard@...nel.org>; Thomas Zimmermann <tzimmermann@...e.de>; David Airlie <airlied@...il.com>; Simona Vetter <simona@...ll.ch>; Pet Weng (翁玉芬) <Pet.Weng@....com.tw>; Kenneth Hung (洪家倫) <Kenneth.Hung@....com.tw>; treapking@...omium.org; dri-devel@...ts.freedesktop.org; linux-kernel@...r.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@....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@....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

Powered by blists - more mailing lists