| Message ID | 20250326-fix-link-training-v2-3-756c8306f500@ite.com.tw (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/bridge: it6505: fix DP link traning and improve compatibility | expand |
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 > >
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; }