Re: [PATCH 4/9] drm/i915: Add {preemph, voltage}_max() vfuncs

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

 



On Wed, May 06, 2020 at 02:23:23PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> Different platforms have different max vswing/preemph settings.
> Turn that into a pair vfuncs so we can decouple intel_dp.c and
> intel_ddi.c further.

Forgot to mention that it not only depends on the platform, but also
the link rate which is a runtime thing. So unfortunately can't just
make these into fixed init time assignments.

Though I have been pondering about making an init time split
between eDP vs. eDP low vswing vs. DP vs. HDMI though. That
could potentially make some of the if ladders in the ddi dbuf
trans code less confusing. Haven't actually tried to code
it up to see how it would look in the end.

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c      | 21 ++----
>  drivers/gpu/drm/i915/display/intel_ddi.h      |  3 -
>  .../drm/i915/display/intel_display_types.h    |  3 +
>  drivers/gpu/drm/i915/display/intel_dp.c       | 67 ++++++-------------
>  drivers/gpu/drm/i915/display/intel_dp.h       |  4 --
>  .../drm/i915/display/intel_dp_link_training.c | 20 +++++-
>  6 files changed, 49 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 5601673c3f30..409b8a68570f 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2095,10 +2095,10 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
>  				     ddi_translations[level].deemphasis);
>  }
>  
> -u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
> +static u8 intel_ddi_dp_voltage_max(struct intel_dp *intel_dp)
>  {
> +	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  	enum port port = encoder->port;
>  	enum phy phy = intel_port_to_phy(dev_priv, port);
>  	int n_entries;
> @@ -2151,19 +2151,9 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>   * used on all DDI platforms. Should that change we need to
>   * rethink this code.
>   */
> -u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder, u8 voltage_swing)
> +static u8 intel_ddi_dp_preemph_max(struct intel_dp *intel_dp)
>  {
> -	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;
> -	}
> +	return DP_TRAIN_PRE_EMPH_LEVEL_3;
>  }
>  
>  static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
> @@ -4510,6 +4500,9 @@ intel_ddi_init_dp_connector(struct intel_digital_port *intel_dig_port)
>  	else
>  		intel_dig_port->dp.set_signal_levels = hsw_set_signal_levels;
>  
> +	intel_dig_port->dp.voltage_max = intel_ddi_dp_voltage_max;
> +	intel_dig_port->dp.preemph_max = intel_ddi_dp_preemph_max;
> +
>  	if (INTEL_GEN(dev_priv) < 12) {
>  		intel_dig_port->dp.regs.dp_tp_ctl = DP_TP_CTL(port);
>  		intel_dig_port->dp.regs.dp_tp_status = DP_TP_STATUS(port);
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.h b/drivers/gpu/drm/i915/display/intel_ddi.h
> index fbdf8ddde486..077e9dbbe367 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.h
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.h
> @@ -42,9 +42,6 @@ void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
>  					 struct intel_crtc_state *crtc_state);
>  u32 bxt_signal_levels(struct intel_dp *intel_dp);
>  u32 ddi_signal_levels(struct intel_dp *intel_dp);
> -u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder);
> -u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder,
> -				 u8 voltage_swing);
>  int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
>  				     bool enable);
>  void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 9488449e4b94..e0384af489c2 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1371,6 +1371,9 @@ struct intel_dp {
>  	void (*set_idle_link_train)(struct intel_dp *intel_dp);
>  	void (*set_signal_levels)(struct intel_dp *intel_dp);
>  
> +	u8 (*preemph_max)(struct intel_dp *intel_dp);
> +	u8 (*voltage_max)(struct intel_dp *intel_dp);
> +
>  	/* Displayport compliance testing */
>  	struct intel_dp_compliance compliance;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index a52f01c48644..1b786d5af383 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3947,58 +3947,24 @@ intel_dp_get_link_status(struct intel_dp *intel_dp, u8 link_status[DP_LINK_STATU
>  				DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
>  }
>  
> -/* These are source-specific values. */
> -u8
> -intel_dp_voltage_max(struct intel_dp *intel_dp)
> +static u8 intel_dp_voltage_max_2(struct intel_dp *intel_dp)
>  {
> -	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -	enum port port = encoder->port;
> +	return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> +}
>  
> -	if (HAS_DDI(dev_priv))
> -		return intel_ddi_dp_voltage_max(encoder);
> -	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv) ||
> -		 (HAS_PCH_SPLIT(dev_priv) && port != PORT_A))
> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> -	else
> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> +static u8 intel_dp_voltage_max_3(struct intel_dp *intel_dp)
> +{
> +	return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>  }
>  
> -u8
> -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, u8 voltage_swing)
> +static u8 intel_dp_pre_empemph_max_2(struct intel_dp *intel_dp)
>  {
> -	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -	enum port port = encoder->port;
> +	return DP_TRAIN_PRE_EMPH_LEVEL_2;
> +}
>  
> -	if (HAS_DDI(dev_priv)) {
> -		return intel_ddi_dp_pre_emphasis_max(encoder, voltage_swing);
> -	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv) ||
> -		   (HAS_PCH_SPLIT(dev_priv) && port != PORT_A)) {
> -		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 {
> -		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 u8 intel_dp_pre_empemph_max_3(struct intel_dp *intel_dp)
> +{
> +	return DP_TRAIN_PRE_EMPH_LEVEL_3;
>  }
>  
>  static void vlv_set_signal_levels(struct intel_dp *intel_dp)
> @@ -8491,6 +8457,15 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
>  	else
>  		intel_dig_port->dp.set_signal_levels = g4x_set_signal_levels;
>  
> +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv) ||
> +	    (HAS_PCH_SPLIT(dev_priv) && port != PORT_A)) {
> +		intel_dig_port->dp.preemph_max = intel_dp_pre_empemph_max_3;
> +		intel_dig_port->dp.voltage_max = intel_dp_voltage_max_3;
> +	} else {
> +		intel_dig_port->dp.preemph_max = intel_dp_pre_empemph_max_2;
> +		intel_dig_port->dp.voltage_max = intel_dp_voltage_max_2;
> +	}
> +
>  	intel_dig_port->dp.output_reg = output_reg;
>  	intel_dig_port->max_lanes = 4;
>  	intel_dig_port->dp.regs.dp_tp_ctl = DP_TP_CTL(port);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 6659ce15a693..e8375a75c3ec 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -91,10 +91,6 @@ intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
>  void
>  intel_dp_set_signal_levels(struct intel_dp *intel_dp);
>  void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
> -u8
> -intel_dp_voltage_max(struct intel_dp *intel_dp);
> -u8
> -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, u8 voltage_swing);
>  void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
>  			   u8 *link_bw, u8 *rate_select);
>  bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index e4f1843170b7..171d9e842fc0 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -34,6 +34,21 @@ intel_dp_dump_link_status(const u8 link_status[DP_LINK_STATUS_SIZE])
>  		      link_status[3], link_status[4], link_status[5]);
>  }
>  
> +static u8 dp_pre_emphasis_max(u8 voltage_swing)
> +{
> +	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;
> +	}
> +}
> +
>  void intel_dp_get_adjust_train(struct intel_dp *intel_dp,
>  			       const u8 link_status[DP_LINK_STATUS_SIZE])
>  {
> @@ -53,11 +68,12 @@ void intel_dp_get_adjust_train(struct intel_dp *intel_dp,
>  			p = this_p;
>  	}
>  
> -	voltage_max = intel_dp_voltage_max(intel_dp);
> +	voltage_max = intel_dp->voltage_max(intel_dp);
>  	if (v >= voltage_max)
>  		v = voltage_max | DP_TRAIN_MAX_SWING_REACHED;
>  
> -	preemph_max = intel_dp_pre_emphasis_max(intel_dp, v);
> +	preemph_max = min(intel_dp->preemph_max(intel_dp),
> +			  dp_pre_emphasis_max(v));
>  	if (p >= preemph_max)
>  		p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
>  
> -- 
> 2.24.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux