On Mon, Mar 31, 2025 at 02:31:30AM +0000, hermes...@ite.com.tw wrote:
> >-----Original Message-----
> >From: Dmitry Baryshkov <dmitry.barysh...@oss.qualcomm.com>
> >Sent: Monday, March 31, 2025 2:44 AM
> >To: Hermes Wu (吳佳宏) <hermes...@ite.com.tw>
> >Cc: Andrzej Hajda <andrzej.ha...@intel.com>; Neil Armstrong
> ><neil.armstr...@linaro.org>; Robert Foss <rf...@kernel.org>; Laurent
> >Pinchart <laurent.pinch...@ideasonboard.com>; Jonas Karlman
> ><jo...@kwiboo.se>; Jernej Skrabec <jernej.skra...@gmail.com>; Maarten
> >Lankhorst <maarten.lankho...@linux.intel.com>; Maxime Ripard
> ><mrip...@kernel.org>; Thomas Zimmermann <tzimmerm...@suse.de>; David Airlie
> ><airl...@gmail.com>; Simona Vetter <sim...@ffwll.ch>; Pet Weng (翁玉芬)
> ><pet.w...@ite.com.tw>; Kenneth Hung (洪家倫) <kenneth.h...@ite.com.tw>;
> >treapk...@chromium.org; dri-devel@lists.freedesktop.org;
> >linux-ker...@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...@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...@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
