diff mbox

[7/8] drm/i915: Move max voltage and pre emphasis to intel_dp_link_training.c

Message ID 1441715280-11623-8-git-send-email-ander.conselvan.de.oliveira@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ander Conselvan de Oliveira Sept. 8, 2015, 12:27 p.m. UTC
So link training tests can use real hardware limits.
---
 drivers/gpu/drm/i915/intel_dp.c               | 99 ---------------------------
 drivers/gpu/drm/i915/intel_dp_link_training.c | 92 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h              | 11 +--
 3 files changed, 99 insertions(+), 103 deletions(-)

Comments

Ville Syrjala Sept. 8, 2015, 1:01 p.m. UTC | #1
On Tue, Sep 08, 2015 at 03:27:58PM +0300, Ander Conselvan de Oliveira wrote:
> So link training tests can use real hardware limits.

These need to be kept in sync with the _signal_levels() functions, so
moving them to a separate file is a bit questionable.

I suggest that we should attempt to restructure this information as
some kind of tables, from which we can look up the max values as
well as the hardware specific values for each setting.

That of course won't solve your problem. So far I don't know what your
test even does and why does it need this information, so I guess I'll
need to read on...

> ---
>  drivers/gpu/drm/i915/intel_dp.c               | 99 ---------------------------
>  drivers/gpu/drm/i915/intel_dp_link_training.c | 92 +++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h              | 11 +--
>  3 files changed, 99 insertions(+), 103 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5778059..da87aef 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -111,13 +111,6 @@ static bool is_edp(struct intel_dp *intel_dp)
>  	return intel_dig_port->base.type == INTEL_OUTPUT_EDP;
>  }
>  
> -static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
> -{
> -	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> -
> -	return intel_dig_port->base.base.dev;
> -}
> -
>  static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
>  {
>  	return enc_to_intel_dp(&intel_attached_encoder(connector)->base);
> @@ -3054,98 +3047,6 @@ intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_
>  				       DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
>  }
>  
> -/* These are source-specific values. */
> -uint8_t
> -intel_dp_voltage_max(struct intel_dp *intel_dp)
> -{
> -	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	enum port port = dp_to_dig_port(intel_dp)->port;
> -
> -	if (IS_BROXTON(dev))
> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> -	else if (INTEL_INFO(dev)->gen >= 9) {
> -		if (dev_priv->edp_low_vswing && port == PORT_A)
> -			return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> -	} else if (IS_VALLEYVIEW(dev))
> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> -	else if (IS_GEN7(dev) && port == PORT_A)
> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> -	else if (HAS_PCH_CPT(dev) && port != PORT_A)
> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> -	else
> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> -}
> -
> -uint8_t
> -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
> -{
> -	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> -	enum port port = dp_to_dig_port(intel_dp)->port;
> -
> -	if (INTEL_INFO(dev)->gen >= 9) {
> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_3;
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> -		default:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> -		}
> -	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_3;
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> -		default:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> -		}
> -	} else if (IS_VALLEYVIEW(dev)) {
> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_3;
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> -		default:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> -		}
> -	} else if (IS_GEN7(dev) && port == PORT_A) {
> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> -		default:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> -		}
> -	} else {
> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> -		default:
> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> -		}
> -	}
> -}
> -
>  static uint32_t vlv_signal_levels(struct intel_dp *intel_dp)
>  {
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index f33cbbb..8d27dce 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -23,6 +23,98 @@
>  
>  #include "intel_drv.h"
>  
> +/* These are source-specific values. */
> +static uint8_t
> +intel_dp_voltage_max(struct intel_dp *intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum port port = dp_to_dig_port(intel_dp)->port;
> +
> +	if (IS_BROXTON(dev))
> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> +	else if (INTEL_INFO(dev)->gen >= 9) {
> +		if (dev_priv->edp_low_vswing && port == PORT_A)
> +			return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> +	} else if (IS_VALLEYVIEW(dev))
> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> +	else if (IS_GEN7(dev) && port == PORT_A)
> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> +	else if (HAS_PCH_CPT(dev) && port != PORT_A)
> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> +	else
> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> +}
> +
> +static uint8_t
> +intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	enum port port = dp_to_dig_port(intel_dp)->port;
> +
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_3;
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> +		default:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> +		}
> +	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_3;
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> +		default:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> +		}
> +	} else if (IS_VALLEYVIEW(dev)) {
> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_3;
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> +		default:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> +		}
> +	} else if (IS_GEN7(dev) && port == PORT_A) {
> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> +		default:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> +		}
> +	} else {
> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> +		default:
> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> +		}
> +	}
> +}
> +
>  static void
>  intel_get_adjust_train(struct intel_dp *intel_dp,
>  		       const uint8_t link_status[DP_LINK_STATUS_SIZE])
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 29ae4bb..671a20f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1216,15 +1216,18 @@ intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
>  void
>  intel_dp_update_signal_levels(struct intel_dp *intel_dp);
>  void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
> -uint8_t
> -intel_dp_voltage_max(struct intel_dp *intel_dp);
> -uint8_t
> -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing);
>  void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
>  			   uint8_t *link_bw, uint8_t *rate_select);
>  bool intel_dp_source_supports_hbr2(struct drm_device *dev);
>  bool
>  intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]);
> +static inline struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +
> +	return intel_dig_port->base.base.dev;
> +}
> +
>  
>  /* intel_dp_mst.c */
>  int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
> -- 
> 2.4.3
Sivakumar Thulasimani Oct. 19, 2015, 5:14 a.m. UTC | #2
As an FYI, both of these functions need to be rewritten when we want the 
code to be
compliant to DP spec.
We should read the pre-emphasis given by the panel and if vwing exeeds 
max value
we should use the max vswing supported for that pre-emphasis but current 
code
is opposite of that.  you can read more detailed logic in section 
"3.5.1.2 Link Training"
in DP Spec.

by the way same is true for much of the link training logic and related 
functions
i don't know if such rewriting should be done after fixing those or 
before :(.

regards,
Sivakumar.

On 9/8/2015 6:31 PM, Ville Syrjälä wrote:
> On Tue, Sep 08, 2015 at 03:27:58PM +0300, Ander Conselvan de Oliveira wrote:
>> So link training tests can use real hardware limits.
> These need to be kept in sync with the _signal_levels() functions, so
> moving them to a separate file is a bit questionable.
>
> I suggest that we should attempt to restructure this information as
> some kind of tables, from which we can look up the max values as
> well as the hardware specific values for each setting.
>
> That of course won't solve your problem. So far I don't know what your
> test even does and why does it need this information, so I guess I'll
> need to read on...
>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c               | 99 ---------------------------
>>   drivers/gpu/drm/i915/intel_dp_link_training.c | 92 +++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_drv.h              | 11 +--
>>   3 files changed, 99 insertions(+), 103 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 5778059..da87aef 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -111,13 +111,6 @@ static bool is_edp(struct intel_dp *intel_dp)
>>   	return intel_dig_port->base.type == INTEL_OUTPUT_EDP;
>>   }
>>   
>> -static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
>> -{
>> -	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> -
>> -	return intel_dig_port->base.base.dev;
>> -}
>> -
>>   static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
>>   {
>>   	return enc_to_intel_dp(&intel_attached_encoder(connector)->base);
>> @@ -3054,98 +3047,6 @@ intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_
>>   				       DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
>>   }
>>   
>> -/* These are source-specific values. */
>> -uint8_t
>> -intel_dp_voltage_max(struct intel_dp *intel_dp)
>> -{
>> -	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>> -	enum port port = dp_to_dig_port(intel_dp)->port;
>> -
>> -	if (IS_BROXTON(dev))
>> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> -	else if (INTEL_INFO(dev)->gen >= 9) {
>> -		if (dev_priv->edp_low_vswing && port == PORT_A)
>> -			return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>> -	} else if (IS_VALLEYVIEW(dev))
>> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> -	else if (IS_GEN7(dev) && port == PORT_A)
>> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>> -	else if (HAS_PCH_CPT(dev) && port != PORT_A)
>> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> -	else
>> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>> -}
>> -
>> -uint8_t
>> -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
>> -{
>> -	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> -	enum port port = dp_to_dig_port(intel_dp)->port;
>> -
>> -	if (INTEL_INFO(dev)->gen >= 9) {
>> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_3;
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> -		default:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> -		}
>> -	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_3;
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> -		default:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> -		}
>> -	} else if (IS_VALLEYVIEW(dev)) {
>> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_3;
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> -		default:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> -		}
>> -	} else if (IS_GEN7(dev) && port == PORT_A) {
>> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> -		default:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> -		}
>> -	} else {
>> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> -		default:
>> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> -		}
>> -	}
>> -}
>> -
>>   static uint32_t vlv_signal_levels(struct intel_dp *intel_dp)
>>   {
>>   	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> index f33cbbb..8d27dce 100644
>> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
>> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> @@ -23,6 +23,98 @@
>>   
>>   #include "intel_drv.h"
>>   
>> +/* These are source-specific values. */
>> +static uint8_t
>> +intel_dp_voltage_max(struct intel_dp *intel_dp)
>> +{
>> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	enum port port = dp_to_dig_port(intel_dp)->port;
>> +
>> +	if (IS_BROXTON(dev))
>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> +	else if (INTEL_INFO(dev)->gen >= 9) {
>> +		if (dev_priv->edp_low_vswing && port == PORT_A)
>> +			return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>> +	} else if (IS_VALLEYVIEW(dev))
>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> +	else if (IS_GEN7(dev) && port == PORT_A)
>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>> +	else if (HAS_PCH_CPT(dev) && port != PORT_A)
>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> +	else
>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>> +}
>> +
>> +static uint8_t
>> +intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
>> +{
>> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> +	enum port port = dp_to_dig_port(intel_dp)->port;
>> +
>> +	if (INTEL_INFO(dev)->gen >= 9) {
>> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_3;
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> +		default:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> +		}
>> +	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_3;
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> +		default:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> +		}
>> +	} else if (IS_VALLEYVIEW(dev)) {
>> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_3;
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> +		default:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> +		}
>> +	} else if (IS_GEN7(dev) && port == PORT_A) {
>> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> +		default:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> +		}
>> +	} else {
>> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> +		default:
>> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> +		}
>> +	}
>> +}
>> +
>>   static void
>>   intel_get_adjust_train(struct intel_dp *intel_dp,
>>   		       const uint8_t link_status[DP_LINK_STATUS_SIZE])
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 29ae4bb..671a20f 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1216,15 +1216,18 @@ intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
>>   void
>>   intel_dp_update_signal_levels(struct intel_dp *intel_dp);
>>   void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
>> -uint8_t
>> -intel_dp_voltage_max(struct intel_dp *intel_dp);
>> -uint8_t
>> -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing);
>>   void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
>>   			   uint8_t *link_bw, uint8_t *rate_select);
>>   bool intel_dp_source_supports_hbr2(struct drm_device *dev);
>>   bool
>>   intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]);
>> +static inline struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
>> +{
>> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> +
>> +	return intel_dig_port->base.base.dev;
>> +}
>> +
>>   
>>   /* intel_dp_mst.c */
>>   int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
>> -- 
>> 2.4.3
Ander Conselvan de Oliveira Oct. 19, 2015, 5:59 a.m. UTC | #3
On Mon, 2015-10-19 at 10:44 +0530, Thulasimani, Sivakumar wrote:
> As an FYI, both of these functions need to be rewritten when we want the 
> code to be
> compliant to DP spec.
> We should read the pre-emphasis given by the panel and if vwing exeeds 
> max value
> we should use the max vswing supported for that pre-emphasis but current 
> code
> is opposite of that.  you can read more detailed logic in section 
> "3.5.1.2 Link Training"
> in DP Spec.

Yeah, the spec is pretty clear on that.


> by the way same is true for much of the link training logic and related 
> functions

Do you have a list of other spec violations? I'm aware that we don't fallback to
lower bandwidth when link training fails, but that problem can't be solved with
the driver alone. Since we already pick the lowest bandwidth that can support
the mode chosen by userspace, the fallback needs to allow it to choose a new
mode.

There's been some discussion on how to solve this, but I don't know if there was
any conclusions. One of the possibilities was to send an uevent to userspace to
tell it to to choose a smaller mode.

> i don't know if such rewriting should be done after fixing those or 
> before :(.

I have posted a new version of the refactoring patches which I believed are
ready to go. So I'd go for getting those changes in and fix the max voltage vs.
pre-emphasis problem on top of that.

Thanks,
Ander

> 
> regards,
> Sivakumar.
> 
> On 9/8/2015 6:31 PM, Ville Syrjälä wrote:
> > On Tue, Sep 08, 2015 at 03:27:58PM +0300, Ander Conselvan de Oliveira wrote:
> > > So link training tests can use real hardware limits.
> > These need to be kept in sync with the _signal_levels() functions, so
> > moving them to a separate file is a bit questionable.
> > 
> > I suggest that we should attempt to restructure this information as
> > some kind of tables, from which we can look up the max values as
> > well as the hardware specific values for each setting.
> > 
> > That of course won't solve your problem. So far I don't know what your
> > test even does and why does it need this information, so I guess I'll
> > need to read on...
> > 
> > > ---
> > >   drivers/gpu/drm/i915/intel_dp.c               | 99 ---------------------
> > > ------
> > >   drivers/gpu/drm/i915/intel_dp_link_training.c | 92
> > > +++++++++++++++++++++++++
> > >   drivers/gpu/drm/i915/intel_drv.h              | 11 +--
> > >   3 files changed, 99 insertions(+), 103 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 5778059..da87aef 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -111,13 +111,6 @@ static bool is_edp(struct intel_dp *intel_dp)
> > >   	return intel_dig_port->base.type == INTEL_OUTPUT_EDP;
> > >   }
> > >   
> > > -static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
> > > -{
> > > -	struct intel_digital_port *intel_dig_port =
> > > dp_to_dig_port(intel_dp);
> > > -
> > > -	return intel_dig_port->base.base.dev;
> > > -}
> > > -
> > >   static struct intel_dp *intel_attached_dp(struct drm_connector
> > > *connector)
> > >   {
> > >   	return enc_to_intel_dp(&intel_attached_encoder(connector)
> > > ->base);
> > > @@ -3054,98 +3047,6 @@ intel_dp_get_link_status(struct intel_dp *intel_dp,
> > > uint8_t link_status[DP_LINK_
> > >   				       DP_LINK_STATUS_SIZE) ==
> > > DP_LINK_STATUS_SIZE;
> > >   }
> > >   
> > > -/* These are source-specific values. */
> > > -uint8_t
> > > -intel_dp_voltage_max(struct intel_dp *intel_dp)
> > > -{
> > > -	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > > -	enum port port = dp_to_dig_port(intel_dp)->port;
> > > -
> > > -	if (IS_BROXTON(dev))
> > > -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> > > -	else if (INTEL_INFO(dev)->gen >= 9) {
> > > -		if (dev_priv->edp_low_vswing && port == PORT_A)
> > > -			return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> > > -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> > > -	} else if (IS_VALLEYVIEW(dev))
> > > -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> > > -	else if (IS_GEN7(dev) && port == PORT_A)
> > > -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> > > -	else if (HAS_PCH_CPT(dev) && port != PORT_A)
> > > -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> > > -	else
> > > -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> > > -}
> > > -
> > > -uint8_t
> > > -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t
> > > voltage_swing)
> > > -{
> > > -	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > > -	enum port port = dp_to_dig_port(intel_dp)->port;
> > > -
> > > -	if (INTEL_INFO(dev)->gen >= 9) {
> > > -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_3;
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > -		default:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > -		}
> > > -	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> > > -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_3;
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> > > -		default:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > -		}
> > > -	} else if (IS_VALLEYVIEW(dev)) {
> > > -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_3;
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> > > -		default:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > -		}
> > > -	} else if (IS_GEN7(dev) && port == PORT_A) {
> > > -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > > -		default:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > -		}
> > > -	} else {
> > > -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > > -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> > > -		default:
> > > -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > -		}
> > > -	}
> > > -}
> > > -
> > >   static uint32_t vlv_signal_levels(struct intel_dp *intel_dp)
> > >   {
> > >   	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > index f33cbbb..8d27dce 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > @@ -23,6 +23,98 @@
> > >   
> > >   #include "intel_drv.h"
> > >   
> > > +/* These are source-specific values. */
> > > +static uint8_t
> > > +intel_dp_voltage_max(struct intel_dp *intel_dp)
> > > +{
> > > +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > > +	enum port port = dp_to_dig_port(intel_dp)->port;
> > > +
> > > +	if (IS_BROXTON(dev))
> > > +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> > > +	else if (INTEL_INFO(dev)->gen >= 9) {
> > > +		if (dev_priv->edp_low_vswing && port == PORT_A)
> > > +			return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> > > +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> > > +	} else if (IS_VALLEYVIEW(dev))
> > > +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> > > +	else if (IS_GEN7(dev) && port == PORT_A)
> > > +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> > > +	else if (HAS_PCH_CPT(dev) && port != PORT_A)
> > > +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> > > +	else
> > > +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> > > +}
> > > +
> > > +static uint8_t
> > > +intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t
> > > voltage_swing)
> > > +{
> > > +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > > +	enum port port = dp_to_dig_port(intel_dp)->port;
> > > +
> > > +	if (INTEL_INFO(dev)->gen >= 9) {
> > > +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_3;
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > +		default:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > +		}
> > > +	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> > > +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_3;
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> > > +		default:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > +		}
> > > +	} else if (IS_VALLEYVIEW(dev)) {
> > > +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_3;
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> > > +		default:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > +		}
> > > +	} else if (IS_GEN7(dev) && port == PORT_A) {
> > > +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > > +		default:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > +		}
> > > +	} else {
> > > +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > > +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> > > +		default:
> > > +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > +		}
> > > +	}
> > > +}
> > > +
> > >   static void
> > >   intel_get_adjust_train(struct intel_dp *intel_dp,
> > >   		       const uint8_t link_status[DP_LINK_STATUS_SIZE])
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 29ae4bb..671a20f 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1216,15 +1216,18 @@ intel_dp_program_link_training_pattern(struct
> > > intel_dp *intel_dp,
> > >   void
> > >   intel_dp_update_signal_levels(struct intel_dp *intel_dp);
> > >   void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
> > > -uint8_t
> > > -intel_dp_voltage_max(struct intel_dp *intel_dp);
> > > -uint8_t
> > > -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t
> > > voltage_swing);
> > >   void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
> > >   			   uint8_t *link_bw, uint8_t *rate_select);
> > >   bool intel_dp_source_supports_hbr2(struct drm_device *dev);
> > >   bool
> > >   intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t
> > > link_status[DP_LINK_STATUS_SIZE]);
> > > +static inline struct drm_device *intel_dp_to_dev(struct intel_dp
> > > *intel_dp)
> > > +{
> > > +	struct intel_digital_port *intel_dig_port =
> > > dp_to_dig_port(intel_dp);
> > > +
> > > +	return intel_dig_port->base.base.dev;
> > > +}
> > > +
> > >   
> > >   /* intel_dp_mst.c */
> > >   int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port,
> > > int conn_id);
Sivakumar Thulasimani Oct. 19, 2015, 6:13 a.m. UTC | #4
On 10/19/2015 11:29 AM, Ander Conselvan De Oliveira wrote:
> On Mon, 2015-10-19 at 10:44 +0530, Thulasimani, Sivakumar wrote:
>> As an FYI, both of these functions need to be rewritten when we want the
>> code to be
>> compliant to DP spec.
>> We should read the pre-emphasis given by the panel and if vwing exeeds
>> max value
>> we should use the max vswing supported for that pre-emphasis but current
>> code
>> is opposite of that.  you can read more detailed logic in section
>> "3.5.1.2 Link Training"
>> in DP Spec.
> Yeah, the spec is pretty clear on that.
>
>
>> by the way same is true for much of the link training logic and related
>> functions
> Do you have a list of other spec violations? I'm aware that we don't fallback to
> lower bandwidth when link training fails, but that problem can't be solved with
> the driver alone. Since we already pick the lowest bandwidth that can support
> the mode chosen by userspace, the fallback needs to allow it to choose a new
> mode.
>
> There's been some discussion on how to solve this, but I don't know if there was
> any conclusions. One of the possibilities was to send an uevent to userspace to
> tell it to to choose a smaller mode.
here is a list of changes i had to do for local branch, that i should be 
upstreaming in the future

drm/i915: Fix swing & emphasis usage (this is the patch for issue i 
talked about above)
drm/i915: EQ retry must be 5 times only
drm/i915: Start link training from max link rate
drm/i915: Fix Clock recovery sequence ( fixes the whole clock recovery 
to be per spec)
drm/i915: Fix EQ sequence in Link training (fixes the whole EQ to be per 
spec)
drm/i915: Check EQ only if CR passed  (This seems to be in place in your 
changes)

All these are just related to link training, i have been asked for list 
of issues recently
as well, so i will probably file each of them in bugzilla/freedesktop so 
someone
will track it.

regards,
Sivakumar
>
>> i don't know if such rewriting should be done after fixing those or
>> before :(.
> I have posted a new version of the refactoring patches which I believed are
> ready to go. So I'd go for getting those changes in and fix the max voltage vs.
> pre-emphasis problem on top of that.
>
> Thanks,
> Ander
>
>> regards,
>> Sivakumar.
>>
>> On 9/8/2015 6:31 PM, Ville Syrjälä wrote:
>>> On Tue, Sep 08, 2015 at 03:27:58PM +0300, Ander Conselvan de Oliveira wrote:
>>>> So link training tests can use real hardware limits.
>>> These need to be kept in sync with the _signal_levels() functions, so
>>> moving them to a separate file is a bit questionable.
>>>
>>> I suggest that we should attempt to restructure this information as
>>> some kind of tables, from which we can look up the max values as
>>> well as the hardware specific values for each setting.
>>>
>>> That of course won't solve your problem. So far I don't know what your
>>> test even does and why does it need this information, so I guess I'll
>>> need to read on...
>>>
>>>> ---
>>>>    drivers/gpu/drm/i915/intel_dp.c               | 99 ---------------------
>>>> ------
>>>>    drivers/gpu/drm/i915/intel_dp_link_training.c | 92
>>>> +++++++++++++++++++++++++
>>>>    drivers/gpu/drm/i915/intel_drv.h              | 11 +--
>>>>    3 files changed, 99 insertions(+), 103 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>>>> b/drivers/gpu/drm/i915/intel_dp.c
>>>> index 5778059..da87aef 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>> @@ -111,13 +111,6 @@ static bool is_edp(struct intel_dp *intel_dp)
>>>>    	return intel_dig_port->base.type == INTEL_OUTPUT_EDP;
>>>>    }
>>>>    
>>>> -static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
>>>> -{
>>>> -	struct intel_digital_port *intel_dig_port =
>>>> dp_to_dig_port(intel_dp);
>>>> -
>>>> -	return intel_dig_port->base.base.dev;
>>>> -}
>>>> -
>>>>    static struct intel_dp *intel_attached_dp(struct drm_connector
>>>> *connector)
>>>>    {
>>>>    	return enc_to_intel_dp(&intel_attached_encoder(connector)
>>>> ->base);
>>>> @@ -3054,98 +3047,6 @@ intel_dp_get_link_status(struct intel_dp *intel_dp,
>>>> uint8_t link_status[DP_LINK_
>>>>    				       DP_LINK_STATUS_SIZE) ==
>>>> DP_LINK_STATUS_SIZE;
>>>>    }
>>>>    
>>>> -/* These are source-specific values. */
>>>> -uint8_t
>>>> -intel_dp_voltage_max(struct intel_dp *intel_dp)
>>>> -{
>>>> -	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>>>> -	enum port port = dp_to_dig_port(intel_dp)->port;
>>>> -
>>>> -	if (IS_BROXTON(dev))
>>>> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>>>> -	else if (INTEL_INFO(dev)->gen >= 9) {
>>>> -		if (dev_priv->edp_low_vswing && port == PORT_A)
>>>> -			return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>>>> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>>>> -	} else if (IS_VALLEYVIEW(dev))
>>>> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>>>> -	else if (IS_GEN7(dev) && port == PORT_A)
>>>> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>>>> -	else if (HAS_PCH_CPT(dev) && port != PORT_A)
>>>> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>>>> -	else
>>>> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>>>> -}
>>>> -
>>>> -uint8_t
>>>> -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t
>>>> voltage_swing)
>>>> -{
>>>> -	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>>> -	enum port port = dp_to_dig_port(intel_dp)->port;
>>>> -
>>>> -	if (INTEL_INFO(dev)->gen >= 9) {
>>>> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_3;
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> -		default:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> -		}
>>>> -	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>>>> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_3;
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>>>> -		default:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> -		}
>>>> -	} else if (IS_VALLEYVIEW(dev)) {
>>>> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_3;
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>>>> -		default:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> -		}
>>>> -	} else if (IS_GEN7(dev) && port == PORT_A) {
>>>> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>> -		default:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> -		}
>>>> -	} else {
>>>> -		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>> -		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>>>> -		default:
>>>> -			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> -		}
>>>> -	}
>>>> -}
>>>> -
>>>>    static uint32_t vlv_signal_levels(struct intel_dp *intel_dp)
>>>>    {
>>>>    	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>>> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
>>>> b/drivers/gpu/drm/i915/intel_dp_link_training.c
>>>> index f33cbbb..8d27dce 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
>>>> @@ -23,6 +23,98 @@
>>>>    
>>>>    #include "intel_drv.h"
>>>>    
>>>> +/* These are source-specific values. */
>>>> +static uint8_t
>>>> +intel_dp_voltage_max(struct intel_dp *intel_dp)
>>>> +{
>>>> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>>>> +	enum port port = dp_to_dig_port(intel_dp)->port;
>>>> +
>>>> +	if (IS_BROXTON(dev))
>>>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>>>> +	else if (INTEL_INFO(dev)->gen >= 9) {
>>>> +		if (dev_priv->edp_low_vswing && port == PORT_A)
>>>> +			return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>>>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>>>> +	} else if (IS_VALLEYVIEW(dev))
>>>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>>>> +	else if (IS_GEN7(dev) && port == PORT_A)
>>>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>>>> +	else if (HAS_PCH_CPT(dev) && port != PORT_A)
>>>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>>>> +	else
>>>> +		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>>>> +}
>>>> +
>>>> +static uint8_t
>>>> +intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t
>>>> voltage_swing)
>>>> +{
>>>> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>>> +	enum port port = dp_to_dig_port(intel_dp)->port;
>>>> +
>>>> +	if (INTEL_INFO(dev)->gen >= 9) {
>>>> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_3;
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> +		default:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> +		}
>>>> +	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>>>> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_3;
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>>>> +		default:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> +		}
>>>> +	} else if (IS_VALLEYVIEW(dev)) {
>>>> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_3;
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>>>> +		default:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> +		}
>>>> +	} else if (IS_GEN7(dev) && port == PORT_A) {
>>>> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>> +		default:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> +		}
>>>> +	} else {
>>>> +		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>> +		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>>>> +		default:
>>>> +			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> +		}
>>>> +	}
>>>> +}
>>>> +
>>>>    static void
>>>>    intel_get_adjust_train(struct intel_dp *intel_dp,
>>>>    		       const uint8_t link_status[DP_LINK_STATUS_SIZE])
>>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>>>> b/drivers/gpu/drm/i915/intel_drv.h
>>>> index 29ae4bb..671a20f 100644
>>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>>> @@ -1216,15 +1216,18 @@ intel_dp_program_link_training_pattern(struct
>>>> intel_dp *intel_dp,
>>>>    void
>>>>    intel_dp_update_signal_levels(struct intel_dp *intel_dp);
>>>>    void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
>>>> -uint8_t
>>>> -intel_dp_voltage_max(struct intel_dp *intel_dp);
>>>> -uint8_t
>>>> -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t
>>>> voltage_swing);
>>>>    void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
>>>>    			   uint8_t *link_bw, uint8_t *rate_select);
>>>>    bool intel_dp_source_supports_hbr2(struct drm_device *dev);
>>>>    bool
>>>>    intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t
>>>> link_status[DP_LINK_STATUS_SIZE]);
>>>> +static inline struct drm_device *intel_dp_to_dev(struct intel_dp
>>>> *intel_dp)
>>>> +{
>>>> +	struct intel_digital_port *intel_dig_port =
>>>> dp_to_dig_port(intel_dp);
>>>> +
>>>> +	return intel_dig_port->base.base.dev;
>>>> +}
>>>> +
>>>>    
>>>>    /* intel_dp_mst.c */
>>>>    int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port,
>>>> int conn_id);
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5778059..da87aef 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -111,13 +111,6 @@  static bool is_edp(struct intel_dp *intel_dp)
 	return intel_dig_port->base.type == INTEL_OUTPUT_EDP;
 }
 
-static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
-{
-	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-
-	return intel_dig_port->base.base.dev;
-}
-
 static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
 {
 	return enc_to_intel_dp(&intel_attached_encoder(connector)->base);
@@ -3054,98 +3047,6 @@  intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_
 				       DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
 }
 
-/* These are source-specific values. */
-uint8_t
-intel_dp_voltage_max(struct intel_dp *intel_dp)
-{
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum port port = dp_to_dig_port(intel_dp)->port;
-
-	if (IS_BROXTON(dev))
-		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
-	else if (INTEL_INFO(dev)->gen >= 9) {
-		if (dev_priv->edp_low_vswing && port == PORT_A)
-			return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
-		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
-	} else if (IS_VALLEYVIEW(dev))
-		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
-	else if (IS_GEN7(dev) && port == PORT_A)
-		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
-	else if (HAS_PCH_CPT(dev) && port != PORT_A)
-		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
-	else
-		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
-}
-
-uint8_t
-intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
-{
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
-	enum port port = dp_to_dig_port(intel_dp)->port;
-
-	if (INTEL_INFO(dev)->gen >= 9) {
-		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
-			return DP_TRAIN_PRE_EMPH_LEVEL_3;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
-			return DP_TRAIN_PRE_EMPH_LEVEL_2;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
-			return DP_TRAIN_PRE_EMPH_LEVEL_1;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
-			return DP_TRAIN_PRE_EMPH_LEVEL_0;
-		default:
-			return DP_TRAIN_PRE_EMPH_LEVEL_0;
-		}
-	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
-		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
-			return DP_TRAIN_PRE_EMPH_LEVEL_3;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
-			return DP_TRAIN_PRE_EMPH_LEVEL_2;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
-			return DP_TRAIN_PRE_EMPH_LEVEL_1;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
-		default:
-			return DP_TRAIN_PRE_EMPH_LEVEL_0;
-		}
-	} else if (IS_VALLEYVIEW(dev)) {
-		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
-			return DP_TRAIN_PRE_EMPH_LEVEL_3;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
-			return DP_TRAIN_PRE_EMPH_LEVEL_2;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
-			return DP_TRAIN_PRE_EMPH_LEVEL_1;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
-		default:
-			return DP_TRAIN_PRE_EMPH_LEVEL_0;
-		}
-	} else if (IS_GEN7(dev) && port == PORT_A) {
-		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
-			return DP_TRAIN_PRE_EMPH_LEVEL_2;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
-			return DP_TRAIN_PRE_EMPH_LEVEL_1;
-		default:
-			return DP_TRAIN_PRE_EMPH_LEVEL_0;
-		}
-	} else {
-		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
-			return DP_TRAIN_PRE_EMPH_LEVEL_2;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
-			return DP_TRAIN_PRE_EMPH_LEVEL_2;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
-			return DP_TRAIN_PRE_EMPH_LEVEL_1;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
-		default:
-			return DP_TRAIN_PRE_EMPH_LEVEL_0;
-		}
-	}
-}
-
 static uint32_t vlv_signal_levels(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
index f33cbbb..8d27dce 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -23,6 +23,98 @@ 
 
 #include "intel_drv.h"
 
+/* These are source-specific values. */
+static uint8_t
+intel_dp_voltage_max(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum port port = dp_to_dig_port(intel_dp)->port;
+
+	if (IS_BROXTON(dev))
+		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
+	else if (INTEL_INFO(dev)->gen >= 9) {
+		if (dev_priv->edp_low_vswing && port == PORT_A)
+			return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
+		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
+	} else if (IS_VALLEYVIEW(dev))
+		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
+	else if (IS_GEN7(dev) && port == PORT_A)
+		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
+	else if (HAS_PCH_CPT(dev) && port != PORT_A)
+		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
+	else
+		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
+}
+
+static uint8_t
+intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	enum port port = dp_to_dig_port(intel_dp)->port;
+
+	if (INTEL_INFO(dev)->gen >= 9) {
+		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
+			return DP_TRAIN_PRE_EMPH_LEVEL_3;
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
+			return DP_TRAIN_PRE_EMPH_LEVEL_2;
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
+			return DP_TRAIN_PRE_EMPH_LEVEL_1;
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
+			return DP_TRAIN_PRE_EMPH_LEVEL_0;
+		default:
+			return DP_TRAIN_PRE_EMPH_LEVEL_0;
+		}
+	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
+		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
+			return DP_TRAIN_PRE_EMPH_LEVEL_3;
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
+			return DP_TRAIN_PRE_EMPH_LEVEL_2;
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
+			return DP_TRAIN_PRE_EMPH_LEVEL_1;
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
+		default:
+			return DP_TRAIN_PRE_EMPH_LEVEL_0;
+		}
+	} else if (IS_VALLEYVIEW(dev)) {
+		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
+			return DP_TRAIN_PRE_EMPH_LEVEL_3;
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
+			return DP_TRAIN_PRE_EMPH_LEVEL_2;
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
+			return DP_TRAIN_PRE_EMPH_LEVEL_1;
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
+		default:
+			return DP_TRAIN_PRE_EMPH_LEVEL_0;
+		}
+	} else if (IS_GEN7(dev) && port == PORT_A) {
+		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
+			return DP_TRAIN_PRE_EMPH_LEVEL_2;
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
+			return DP_TRAIN_PRE_EMPH_LEVEL_1;
+		default:
+			return DP_TRAIN_PRE_EMPH_LEVEL_0;
+		}
+	} else {
+		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
+			return DP_TRAIN_PRE_EMPH_LEVEL_2;
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
+			return DP_TRAIN_PRE_EMPH_LEVEL_2;
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
+			return DP_TRAIN_PRE_EMPH_LEVEL_1;
+		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
+		default:
+			return DP_TRAIN_PRE_EMPH_LEVEL_0;
+		}
+	}
+}
+
 static void
 intel_get_adjust_train(struct intel_dp *intel_dp,
 		       const uint8_t link_status[DP_LINK_STATUS_SIZE])
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 29ae4bb..671a20f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1216,15 +1216,18 @@  intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
 void
 intel_dp_update_signal_levels(struct intel_dp *intel_dp);
 void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
-uint8_t
-intel_dp_voltage_max(struct intel_dp *intel_dp);
-uint8_t
-intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing);
 void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
 			   uint8_t *link_bw, uint8_t *rate_select);
 bool intel_dp_source_supports_hbr2(struct drm_device *dev);
 bool
 intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]);
+static inline struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+
+	return intel_dig_port->base.base.dev;
+}
+
 
 /* intel_dp_mst.c */
 int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);