Re: [PATCH] video: exynos_dp: adjust voltage swing and pre-emphasis during Link Training

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 07/30/2012 04:08 AM, Jingoo Han wrote:
> On Thursday, July 19, 2012 1:53 PM, Jingoo Han wrote:
>>
>> This patch adds adjustement for voltage swing and pre-emphasis during
>> Link Training procedure. According to the DP specification, unless all
>> the LANEx_CR_DONE bits are set, the transmitter must read
>> the ADJUST_REQUEST_LANEx_x, increase the voltage swing according to
>> the request, and update the TRAINING_LANEx_SET bytes to match the new
>> voltage swing setting.
>>
>> Refer to the DP specification v1.1a, Section 3.5.1.3 Link Training.
>>
>> Signed-off-by: Jingoo Han <jg1.han@xxxxxxxxxxx>
> 
> 
> Hi Florian,
> 
> Could you accept this patch for 3.6-rc1?

I rather consider it for -rc2 or -rc3. You could have made my life easier by

(1) sending it earlier, not just half a week before the merge window
opened. For some reason a lot of patches ended up hitting my Inbox in
that timeframe which is bad timing for things that should go in this
merge window as I usually wait a week before applying to give others the
chance to comment on it and I should have my final tree ready the day
the merge window opens.

(2) reducing it to the bare minimum changes required or splitting it up
and not doing a bunch of unrelated changes

> This patch was already verified and tested with different 2 kinds of eDP LCD panels.

I'm not saying that your patch is wrong. I think it is important, but
given its size I don't feel comfortable with just looking at the code
but feel that it should be longer in -next than 2 days.


Best regards,

Florian Tobias Schandinat

> Thank you.
> 
> Best regards,
> Jingoo Han
> 
> 
>> ---
>>  drivers/video/exynos/exynos_dp_core.c |  282 +++++++++++++++++----------------
>>  drivers/video/exynos/exynos_dp_core.h |    2 +-
>>  2 files changed, 144 insertions(+), 140 deletions(-)
>>
>> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
>> index c6c016a..9c0140f 100644
>> --- a/drivers/video/exynos/exynos_dp_core.c
>> +++ b/drivers/video/exynos/exynos_dp_core.c
>> @@ -260,7 +260,7 @@ static void exynos_dp_set_lane_lane_pre_emphasis(struct exynos_dp_device *dp,
>>
>>  static void exynos_dp_link_start(struct exynos_dp_device *dp)
>>  {
>> -	u8 buf[5];
>> +	u8 buf[4];
>>  	int lane;
>>  	int lane_count;
>>
>> @@ -295,10 +295,10 @@ static void exynos_dp_link_start(struct exynos_dp_device *dp)
>>  	exynos_dp_set_training_pattern(dp, TRAINING_PTN1);
>>
>>  	/* Set RX training pattern */
>> -	buf[0] = DPCD_SCRAMBLING_DISABLED |
>> -		 DPCD_TRAINING_PATTERN_1;
>>  	exynos_dp_write_byte_to_dpcd(dp,
>> -		DPCD_ADDR_TRAINING_PATTERN_SET, buf[0]);
>> +		DPCD_ADDR_TRAINING_PATTERN_SET,
>> +		DPCD_SCRAMBLING_DISABLED |
>> +		DPCD_TRAINING_PATTERN_1);
>>
>>  	for (lane = 0; lane < lane_count; lane++)
>>  		buf[lane] = DPCD_PRE_EMPHASIS_PATTERN2_LEVEL0 |
>> @@ -308,7 +308,7 @@ static void exynos_dp_link_start(struct exynos_dp_device *dp)
>>  		lane_count, buf);
>>  }
>>
>> -static unsigned char exynos_dp_get_lane_status(u8 link_status[6], int lane)
>> +static unsigned char exynos_dp_get_lane_status(u8 link_status[2], int lane)
>>  {
>>  	int shift = (lane & 1) * 4;
>>  	u8 link_value = link_status[lane>>1];
>> @@ -316,7 +316,7 @@ static unsigned char exynos_dp_get_lane_status(u8 link_status[6], int lane)
>>  	return (link_value >> shift) & 0xf;
>>  }
>>
>> -static int exynos_dp_clock_recovery_ok(u8 link_status[6], int lane_count)
>> +static int exynos_dp_clock_recovery_ok(u8 link_status[2], int lane_count)
>>  {
>>  	int lane;
>>  	u8 lane_status;
>> @@ -329,22 +329,23 @@ static int exynos_dp_clock_recovery_ok(u8 link_status[6], int lane_count)
>>  	return 0;
>>  }
>>
>> -static int exynos_dp_channel_eq_ok(u8 link_status[6], int lane_count)
>> +static int exynos_dp_channel_eq_ok(u8 link_align[3], int lane_count)
>>  {
>>  	int lane;
>>  	u8 lane_align;
>>  	u8 lane_status;
>>
>> -	lane_align = link_status[2];
>> +	lane_align = link_align[2];
>>  	if ((lane_align & DPCD_INTERLANE_ALIGN_DONE) == 0)
>>  		return -EINVAL;
>>
>>  	for (lane = 0; lane < lane_count; lane++) {
>> -		lane_status = exynos_dp_get_lane_status(link_status, lane);
>> +		lane_status = exynos_dp_get_lane_status(link_align, lane);
>>  		lane_status &= DPCD_CHANNEL_EQ_BITS;
>>  		if (lane_status != DPCD_CHANNEL_EQ_BITS)
>>  			return -EINVAL;
>>  	}
>> +
>>  	return 0;
>>  }
>>
>> @@ -417,69 +418,17 @@ static unsigned int exynos_dp_get_lane_link_training(
>>
>>  static void exynos_dp_reduce_link_rate(struct exynos_dp_device *dp)
>>  {
>> -	if (dp->link_train.link_rate == LINK_RATE_2_70GBPS) {
>> -		/* set to reduced bit rate */
>> -		dp->link_train.link_rate = LINK_RATE_1_62GBPS;
>> -		dev_err(dp->dev, "set to bandwidth %.2x\n",
>> -			dp->link_train.link_rate);
>> -		dp->link_train.lt_state = START;
>> -	} else {
>> -		exynos_dp_training_pattern_dis(dp);
>> -		/* set enhanced mode if available */
>> -		exynos_dp_set_enhanced_mode(dp);
>> -		dp->link_train.lt_state = FAILED;
>> -	}
>> -}
>> -
>> -static void exynos_dp_get_adjust_train(struct exynos_dp_device *dp,
>> -				u8 adjust_request[2])
>> -{
>> -	int lane;
>> -	int lane_count;
>> -	u8 voltage_swing;
>> -	u8 pre_emphasis;
>> -	u8 training_lane;
>> +	exynos_dp_training_pattern_dis(dp);
>> +	exynos_dp_set_enhanced_mode(dp);
>>
>> -	lane_count = dp->link_train.lane_count;
>> -	for (lane = 0; lane < lane_count; lane++) {
>> -		voltage_swing = exynos_dp_get_adjust_request_voltage(
>> -						adjust_request, lane);
>> -		pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
>> -						adjust_request, lane);
>> -		training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
>> -				DPCD_PRE_EMPHASIS_SET(pre_emphasis);
>> -
>> -		if (voltage_swing == VOLTAGE_LEVEL_3 ||
>> -		   pre_emphasis == PRE_EMPHASIS_LEVEL_3) {
>> -			training_lane |= DPCD_MAX_SWING_REACHED;
>> -			training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
>> -		}
>> -		dp->link_train.training_lane[lane] = training_lane;
>> -	}
>> -}
>> -
>> -static int exynos_dp_check_max_cr_loop(struct exynos_dp_device *dp,
>> -					u8 voltage_swing)
>> -{
>> -	int lane;
>> -	int lane_count;
>> -
>> -	lane_count = dp->link_train.lane_count;
>> -	for (lane = 0; lane < lane_count; lane++) {
>> -		if (voltage_swing == VOLTAGE_LEVEL_3 ||
>> -			dp->link_train.cr_loop[lane] == MAX_CR_LOOP)
>> -			return -EINVAL;
>> -	}
>> -	return 0;
>> +	dp->link_train.lt_state = FAILED;
>>  }
>>
>>  static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
>>  {
>> -	u8 data;
>> -	u8 link_status[6];
>> +	u8 link_status[2];
>>  	int lane;
>>  	int lane_count;
>> -	u8 buf[5];
>>
>>  	u8 adjust_request[2];
>>  	u8 voltage_swing;
>> @@ -488,98 +437,152 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
>>
>>  	usleep_range(100, 101);
>>
>> -	exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
>> -				6, link_status);
>>  	lane_count = dp->link_train.lane_count;
>>
>> +	exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
>> +				2, link_status);
>> +
>>  	if (exynos_dp_clock_recovery_ok(link_status, lane_count) == 0) {
>>  		/* set training pattern 2 for EQ */
>>  		exynos_dp_set_training_pattern(dp, TRAINING_PTN2);
>>
>> -		adjust_request[0] = link_status[4];
>> -		adjust_request[1] = link_status[5];
>> +		for (lane = 0; lane < lane_count; lane++) {
>> +			exynos_dp_read_bytes_from_dpcd(dp,
>> +					DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
>> +					2, adjust_request);
>> +			voltage_swing = exynos_dp_get_adjust_request_voltage(
>> +							adjust_request, lane);
>> +			pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
>> +							adjust_request, lane);
>> +			training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
>> +					DPCD_PRE_EMPHASIS_SET(pre_emphasis);
>>
>> -		exynos_dp_get_adjust_train(dp, adjust_request);
>> +			if (voltage_swing == VOLTAGE_LEVEL_3)
>> +				training_lane |= DPCD_MAX_SWING_REACHED;
>> +			if (pre_emphasis == PRE_EMPHASIS_LEVEL_3)
>> +				training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
>>
>> -		buf[0] = DPCD_SCRAMBLING_DISABLED |
>> -			 DPCD_TRAINING_PATTERN_2;
>> -		exynos_dp_write_byte_to_dpcd(dp,
>> -			DPCD_ADDR_TRAINING_PATTERN_SET,
>> -			buf[0]);
>> +			dp->link_train.training_lane[lane] = training_lane;
>>
>> -		for (lane = 0; lane < lane_count; lane++) {
>>  			exynos_dp_set_lane_link_training(dp,
>>  				dp->link_train.training_lane[lane],
>>  				lane);
>> -			buf[lane] = dp->link_train.training_lane[lane];
>> -			exynos_dp_write_byte_to_dpcd(dp,
>> -				DPCD_ADDR_TRAINING_LANE0_SET + lane,
>> -				buf[lane]);
>>  		}
>> -		dp->link_train.lt_state = EQUALIZER_TRAINING;
>> -	} else {
>> -		exynos_dp_read_byte_from_dpcd(dp,
>> -			DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
>> -			&data);
>> -		adjust_request[0] = data;
>>
>> -		exynos_dp_read_byte_from_dpcd(dp,
>> -			DPCD_ADDR_ADJUST_REQUEST_LANE2_3,
>> -			&data);
>> -		adjust_request[1] = data;
>> +		exynos_dp_write_byte_to_dpcd(dp,
>> +			DPCD_ADDR_TRAINING_PATTERN_SET,
>> +			DPCD_SCRAMBLING_DISABLED |
>> +			DPCD_TRAINING_PATTERN_2);
>> +
>> +		exynos_dp_write_bytes_to_dpcd(dp,
>> +			DPCD_ADDR_TRAINING_LANE0_SET,
>> +			lane_count,
>> +			dp->link_train.training_lane);
>>
>> +		dev_info(dp->dev, "Link Training Clock Recovery success\n");
>> +		dp->link_train.lt_state = EQUALIZER_TRAINING;
>> +	} else {
>>  		for (lane = 0; lane < lane_count; lane++) {
>>  			training_lane = exynos_dp_get_lane_link_training(
>>  							dp, lane);
>> +			exynos_dp_read_bytes_from_dpcd(dp,
>> +					DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
>> +					2, adjust_request);
>>  			voltage_swing = exynos_dp_get_adjust_request_voltage(
>>  							adjust_request, lane);
>>  			pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
>>  							adjust_request, lane);
>> -			if ((DPCD_VOLTAGE_SWING_GET(training_lane) == voltage_swing) &&
>> -			    (DPCD_PRE_EMPHASIS_GET(training_lane) == pre_emphasis))
>> -				dp->link_train.cr_loop[lane]++;
>> -			dp->link_train.training_lane[lane] = training_lane;
>> -		}
>>
>> -		if (exynos_dp_check_max_cr_loop(dp, voltage_swing) != 0) {
>> -			exynos_dp_reduce_link_rate(dp);
>> -		} else {
>> -			exynos_dp_get_adjust_train(dp, adjust_request);
>> +			if (voltage_swing == VOLTAGE_LEVEL_3 ||
>> +			    pre_emphasis == PRE_EMPHASIS_LEVEL_3) {
>> +				dev_err(dp->dev, "voltage or pre emphasis reached max level\n");
>> +				goto reduce_link_rate;
>> +			}
>>
>> -			for (lane = 0; lane < lane_count; lane++) {
>> -				exynos_dp_set_lane_link_training(dp,
>> -					dp->link_train.training_lane[lane],
>> -					lane);
>> -				buf[lane] = dp->link_train.training_lane[lane];
>> -				exynos_dp_write_byte_to_dpcd(dp,
>> -					DPCD_ADDR_TRAINING_LANE0_SET + lane,
>> -					buf[lane]);
>> +			if ((DPCD_VOLTAGE_SWING_GET(training_lane) ==
>> +					voltage_swing) &&
>> +			   (DPCD_PRE_EMPHASIS_GET(training_lane) ==
>> +					pre_emphasis)) {
>> +				dp->link_train.cr_loop[lane]++;
>> +				if (dp->link_train.cr_loop[lane] == MAX_CR_LOOP) {
>> +					dev_err(dp->dev, "CR Max loop\n");
>> +					goto reduce_link_rate;
>> +				}
>>  			}
>> +
>> +			training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
>> +					DPCD_PRE_EMPHASIS_SET(pre_emphasis);
>> +
>> +			if (voltage_swing == VOLTAGE_LEVEL_3)
>> +				training_lane |= DPCD_MAX_SWING_REACHED;
>> +			if (pre_emphasis == PRE_EMPHASIS_LEVEL_3)
>> +				training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
>> +
>> +			dp->link_train.training_lane[lane] = training_lane;
>> +
>> +			exynos_dp_set_lane_link_training(dp,
>> +				dp->link_train.training_lane[lane], lane);
>>  		}
>> +
>> +		exynos_dp_write_bytes_to_dpcd(dp,
>> +			DPCD_ADDR_TRAINING_LANE0_SET,
>> +			lane_count,
>> +			dp->link_train.training_lane);
>>  	}
>>
>>  	return 0;
>> +
>> +reduce_link_rate:
>> +	exynos_dp_reduce_link_rate(dp);
>> +	return -EIO;
>>  }
>>
>>  static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
>>  {
>> -	u8 link_status[6];
>> +	u8 link_status[2];
>> +	u8 link_align[3];
>>  	int lane;
>>  	int lane_count;
>> -	u8 buf[5];
>>  	u32 reg;
>>
>>  	u8 adjust_request[2];
>> +	u8 voltage_swing;
>> +	u8 pre_emphasis;
>> +	u8 training_lane;
>>
>>  	usleep_range(400, 401);
>>
>> -	exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
>> -				6, link_status);
>>  	lane_count = dp->link_train.lane_count;
>>
>> +	exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
>> +				2, link_status);
>> +
>>  	if (exynos_dp_clock_recovery_ok(link_status, lane_count) == 0) {
>> -		adjust_request[0] = link_status[4];
>> -		adjust_request[1] = link_status[5];
>> +		link_align[0] = link_status[0];
>> +		link_align[1] = link_status[1];
>> +
>> +		exynos_dp_read_byte_from_dpcd(dp,
>> +			DPCD_ADDR_LANE_ALIGN_STATUS_UPDATED,
>> +			&link_align[2]);
>> +
>> +		for (lane = 0; lane < lane_count; lane++) {
>> +			exynos_dp_read_bytes_from_dpcd(dp,
>> +					DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
>> +					2, adjust_request);
>> +			voltage_swing = exynos_dp_get_adjust_request_voltage(
>> +							adjust_request, lane);
>> +			pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
>> +							adjust_request, lane);
>> +			training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
>> +					DPCD_PRE_EMPHASIS_SET(pre_emphasis);
>> +
>> +			if (voltage_swing == VOLTAGE_LEVEL_3)
>> +				training_lane |= DPCD_MAX_SWING_REACHED;
>> +			if (pre_emphasis == PRE_EMPHASIS_LEVEL_3)
>> +				training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
>> +
>> +			dp->link_train.training_lane[lane] = training_lane;
>> +		}
>>
>>  		if (exynos_dp_channel_eq_ok(link_status, lane_count) == 0) {
>>  			/* traing pattern Set to Normal */
>> @@ -596,39 +599,42 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
>>  			dp->link_train.lane_count = reg;
>>  			dev_dbg(dp->dev, "final lane count = %.2x\n",
>>  				dp->link_train.lane_count);
>> +
>>  			/* set enhanced mode if available */
>>  			exynos_dp_set_enhanced_mode(dp);
>> -
>>  			dp->link_train.lt_state = FINISHED;
>>  		} else {
>>  			/* not all locked */
>>  			dp->link_train.eq_loop++;
>>
>>  			if (dp->link_train.eq_loop > MAX_EQ_LOOP) {
>> -				exynos_dp_reduce_link_rate(dp);
>> -			} else {
>> -				exynos_dp_get_adjust_train(dp, adjust_request);
>> -
>> -				for (lane = 0; lane < lane_count; lane++) {
>> -					exynos_dp_set_lane_link_training(dp,
>> -						dp->link_train.training_lane[lane],
>> -						lane);
>> -					buf[lane] = dp->link_train.training_lane[lane];
>> -					exynos_dp_write_byte_to_dpcd(dp,
>> -						DPCD_ADDR_TRAINING_LANE0_SET + lane,
>> -						buf[lane]);
>> -				}
>> +				dev_err(dp->dev, "EQ Max loop\n");
>> +				goto reduce_link_rate;
>>  			}
>> +
>> +			for (lane = 0; lane < lane_count; lane++)
>> +				exynos_dp_set_lane_link_training(dp,
>> +					dp->link_train.training_lane[lane],
>> +					lane);
>> +
>> +			exynos_dp_write_bytes_to_dpcd(dp,
>> +				DPCD_ADDR_TRAINING_LANE0_SET,
>> +				lane_count,
>> +				dp->link_train.training_lane);
>>  		}
>>  	} else {
>> -		exynos_dp_reduce_link_rate(dp);
>> +		goto reduce_link_rate;
>>  	}
>>
>>  	return 0;
>> +
>> +reduce_link_rate:
>> +	exynos_dp_reduce_link_rate(dp);
>> +	return -EIO;
>>  }
>>
>>  static void exynos_dp_get_max_rx_bandwidth(struct exynos_dp_device *dp,
>> -			u8 *bandwidth)
>> +					u8 *bandwidth)
>>  {
>>  	u8 data;
>>
>> @@ -641,7 +647,7 @@ static void exynos_dp_get_max_rx_bandwidth(struct exynos_dp_device *dp,
>>  }
>>
>>  static void exynos_dp_get_max_rx_lane_count(struct exynos_dp_device *dp,
>> -			u8 *lane_count)
>> +					u8 *lane_count)
>>  {
>>  	u8 data;
>>
>> @@ -693,13 +699,7 @@ static void exynos_dp_init_training(struct exynos_dp_device *dp,
>>  static int exynos_dp_sw_link_training(struct exynos_dp_device *dp)
>>  {
>>  	int retval = 0;
>> -	int training_finished;
>> -
>> -	/* Turn off unnecessary lane */
>> -	if (dp->link_train.lane_count == 1)
>> -		exynos_dp_set_analog_power_down(dp, CH1_BLOCK, 1);
>> -
>> -	training_finished = 0;
>> +	int training_finished = 0;
>>
>>  	dp->link_train.lt_state = START;
>>
>> @@ -710,10 +710,14 @@ static int exynos_dp_sw_link_training(struct exynos_dp_device *dp)
>>  			exynos_dp_link_start(dp);
>>  			break;
>>  		case CLOCK_RECOVERY:
>> -			exynos_dp_process_clock_recovery(dp);
>> +			retval = exynos_dp_process_clock_recovery(dp);
>> +			if (retval)
>> +				dev_err(dp->dev, "LT CR failed!\n");
>>  			break;
>>  		case EQUALIZER_TRAINING:
>> -			exynos_dp_process_equalizer_training(dp);
>> +			retval = exynos_dp_process_equalizer_training(dp);
>> +			if (retval)
>> +				dev_err(dp->dev, "LT EQ failed!\n");
>>  			break;
>>  		case FINISHED:
>>  			training_finished = 1;
>> diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
>> index 8526e54..44c11e1 100644
>> --- a/drivers/video/exynos/exynos_dp_core.h
>> +++ b/drivers/video/exynos/exynos_dp_core.h
>> @@ -144,7 +144,7 @@ void exynos_dp_disable_scrambling(struct exynos_dp_device *dp);
>>  #define DPCD_ADDR_TRAINING_PATTERN_SET		0x0102
>>  #define DPCD_ADDR_TRAINING_LANE0_SET		0x0103
>>  #define DPCD_ADDR_LANE0_1_STATUS		0x0202
>> -#define DPCD_ADDR_LANE_ALIGN__STATUS_UPDATED	0x0204
>> +#define DPCD_ADDR_LANE_ALIGN_STATUS_UPDATED	0x0204
>>  #define DPCD_ADDR_ADJUST_REQUEST_LANE0_1	0x0206
>>  #define DPCD_ADDR_ADJUST_REQUEST_LANE2_3	0x0207
>>  #define DPCD_ADDR_TEST_REQUEST			0x0218
>> --
>> 1.7.1
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Tourism]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux