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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a6dcfa0c0a1e4f799dacda908888729a@ite.com.tw>
Date: Mon, 31 Mar 2025 02:31:30 +0000
From: <Hermes.Wu@....com.tw>
To: <dmitry.baryshkov@....qualcomm.com>
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 v2 3/5] drm/bridge: it6505: modify DP link auto training

>-----Original Message-----
>From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com> 
>Sent: Monday, March 31, 2025 2:44 AM
>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 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@....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 | 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