linux-kernel - Re: [PATCH 2/6] drm/bridge: synopsys: Add DW DPTX Controller support library

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <65706d63-18aa-41e0-81c4-4eab3d186250@rock-chips.com>
Date: Mon, 3 Mar 2025 16:59:50 +0800
From: Yubing Zhang <yubing.zhang@...k-chips.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
 Andy Yan <andyshrk@....com>
Cc: neil.armstrong@...aro.org, sebastian.reichel@...labora.com,
 heiko@...ech.de, devicetree@...r.kernel.org, hjc@...k-chips.com,
 mripard@...nel.org, linux-kernel@...r.kernel.org,
 linux-rockchip@...ts.infradead.org, dri-devel@...ts.freedesktop.org,
 Andy Yan <andy.yan@...k-chips.com>, krzk+dt@...nel.org, robh@...nel.org,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/6] drm/bridge: synopsys: Add DW DPTX Controller support
 library

Hi Dmitry,

On 2025/3/2 2:14, Dmitry Baryshkov wrote:
> On Sun, Feb 23, 2025 at 07:30:25PM +0800, Andy Yan wrote:
>> From: Andy Yan <andy.yan@...k-chips.com>
>>
>> The DW DP TX Controller is compliant with the DisplayPort Specification
>> Version 1.4 with the following features:
>>
>> * DisplayPort 1.4a
>> * Main Link: 1/2/4 lanes
>> * Main Link Support 1.62Gbps, 2.7Gbps, 5.4Gbps and 8.1Gbps
>> * AUX channel 1Mbps
>> * Single Stream Transport(SST)
>> * Multistream Transport (MST)
>> *Type-C support (alternate mode)
>> * HDCP 2.2, HDCP 1.3
>> * Supports up to 8/10 bits per color component
>> * Supports RBG, YCbCr4:4:4, YCbCr4:2:2, YCbCr4:2:0
>> * Pixel clock up to 594MHz
>> * I2S, SPDIF audio interface
>>
>> Add library with common helpers to make it can be shared with
>> other SoC.
>>
>> Signed-off-by: Andy Yan <andy.yan@...k-chips.com>
>>
>> drm/bridge: cleanup
> 
> Stray line?
> 
>>
>> ---
>>
>>   drivers/gpu/drm/bridge/synopsys/Kconfig  |    7 +
>>   drivers/gpu/drm/bridge/synopsys/Makefile |    1 +
>>   drivers/gpu/drm/bridge/synopsys/dw-dp.c  | 2155 ++++++++++++++++++++++
>>   include/drm/bridge/dw_dp.h               |   19 +
>>   4 files changed, 2182 insertions(+)
>>   create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-dp.c

......

>> +
>> +static u8 dw_dp_voltage_max(u8 preemph)
>> +{
>> +	switch (preemph & DP_TRAIN_PRE_EMPHASIS_MASK) {
>> +	case DP_TRAIN_PRE_EMPH_LEVEL_0:
>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> +	case DP_TRAIN_PRE_EMPH_LEVEL_1:
>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>> +	case DP_TRAIN_PRE_EMPH_LEVEL_2:
>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_1;
>> +	case DP_TRAIN_PRE_EMPH_LEVEL_3:
>> +	default:
>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
>> +	}
>> +}
>> +
>> +static void dw_dp_link_get_adjustments(struct dw_dp_link *link,
>> +				       u8 status[DP_LINK_STATUS_SIZE])
>> +{
>> +	struct dw_dp_link_train_set *adjust = &link->train.adjust;
>> +	u8 v = 0;
>> +	u8 p = 0;
>> +	unsigned int i;
>> +
>> +	for (i = 0; i < link->lanes; i++) {
>> +		v = drm_dp_get_adjust_request_voltage(status, i);
>> +		p = drm_dp_get_adjust_request_pre_emphasis(status, i);
>> +		if (p >=  DP_TRAIN_PRE_EMPH_LEVEL_3) {
>> +			adjust->pre_emphasis[i] = DP_TRAIN_PRE_EMPH_LEVEL_3 >>
>> +						  DP_TRAIN_PRE_EMPHASIS_SHIFT;
>> +			adjust->pre_max_reached[i] = true;
>> +		} else {
>> +			adjust->pre_emphasis[i] = p >> DP_TRAIN_PRE_EMPHASIS_SHIFT;
>> +			adjust->pre_max_reached[i] = false;
>> +		}
>> +		v = min(v, dw_dp_voltage_max(p));
>> +		if (v >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3) {
>> +			adjust->voltage_swing[i] = DP_TRAIN_VOLTAGE_SWING_LEVEL_3 >>
>> +						   DP_TRAIN_VOLTAGE_SWING_SHIFT;
>> +			adjust->voltage_max_reached[i] = true;
>> +		} else {
>> +			adjust->voltage_swing[i] = v >> DP_TRAIN_VOLTAGE_SWING_SHIFT;
>> +			adjust->voltage_max_reached[i] = false;
>> +		}
>> +	}
>> +}
>> +
>> +static void dw_dp_link_train_adjust(struct dw_dp_link_train *train)
>> +{
>> +	struct dw_dp_link_train_set *request = &train->request;
>> +	struct dw_dp_link_train_set *adjust = &train->adjust;
>> +	unsigned int i;
>> +
>> +	for (i = 0; i < 4; i++) {
> 
> Shouldn't it be a loop up to link->lanes?
> 
>> +		if (request->voltage_swing[i] != adjust->voltage_swing[i])
>> +			request->voltage_swing[i] = adjust->voltage_swing[i];
>> +		if (request->voltage_max_reached[i] != adjust->voltage_max_reached[i])
>> +			request->voltage_max_reached[i] = adjust->voltage_max_reached[i];
>> +	}
>> +
>> +	for (i = 0; i < 4; i++) {
>> +		if (request->pre_emphasis[i] != adjust->pre_emphasis[i])
>> +			request->pre_emphasis[i] = adjust->pre_emphasis[i];
>> +		if (request->pre_max_reached[i] != adjust->pre_max_reached[i])
>> +			request->pre_max_reached[i] = adjust->pre_max_reached[i];
> 
> Why do you need separate request and adjustment structs?
During link training cr sequence, if dprx keep the LANEx_CR_DONE bit(s) 
cleared, the request and adjustment structs are used to check the
old and new valud of ADJUST_REQUEST_LANEx_y register(s) is changed or not