Re: [PATCH 16/22] drm/i915: Introduce struct intel_dp_signal_levels

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

 



This is a good simplification but for a wrong implementation :). we know that our current
implementation is wrong and needs to be done the opposite way to make
our code compliant with spec. i assume that it will then be a question of when this will be removed rather than if. In such a scenario is this and following
patches/optimizations needed "at present" ?

regards,
Sivakumar
On 10/23/2015 3:31 PM, Ander Conselvan de Oliveira wrote:
In order to clarify which platforms support which combination of voltage
swing and pre emphasis level, introduce struct intel_dp_signal_levels.
With the new struct, the if ladder to determine the values used is put
in one place, intel_dp_init_signal_levels().

This also wires gens 4, 5 and non-eDP ports on gen 6 and 7 to use the
new struct.

v2: Rebase
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_dp.c               |   2 +
  drivers/gpu/drm/i915/intel_dp_signal_levels.c | 103 ++++++++++++++++++++++----
  drivers/gpu/drm/i915/intel_drv.h              |  11 +++
  3 files changed, 101 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1236791..e640b59 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5220,6 +5220,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
  	if (HAS_DDI(dev))
  		intel_dp->prepare_link_retrain = intel_ddi_prepare_link_retrain;
+ intel_dp_init_signal_levels(intel_dp);
+
  	/* Preserve the current hw state. */
  	intel_dp->DP = I915_READ(intel_dp->output_reg);
  	intel_dp->attached_connector = intel_connector;
diff --git a/drivers/gpu/drm/i915/intel_dp_signal_levels.c b/drivers/gpu/drm/i915/intel_dp_signal_levels.c
index e516dd2..365bdc4 100644
--- a/drivers/gpu/drm/i915/intel_dp_signal_levels.c
+++ b/drivers/gpu/drm/i915/intel_dp_signal_levels.c
@@ -24,8 +24,8 @@
  #include "intel_drv.h"
/* These are source-specific values. */
-uint8_t
-intel_dp_voltage_max(struct intel_dp *intel_dp)
+static uint8_t
+_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;
@@ -47,8 +47,8 @@ intel_dp_voltage_max(struct intel_dp *intel_dp)
  		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
  }
-uint8_t
-intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
+static uint8_t
+_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;
@@ -389,10 +389,10 @@ static uint32_t chv_signal_levels(struct intel_dp *intel_dp)
  	return 0;
  }
-static uint32_t
-gen4_signal_levels(uint8_t train_set)
+static void
+gen4_set_signal_levels(struct intel_dp *intel_dp, uint8_t train_set)
  {
-	uint32_t	signal_levels = 0;
+	uint32_t signal_levels = 0;
switch (train_set & DP_TRAIN_VOLTAGE_SWING_MASK) {
  	case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
@@ -424,9 +424,38 @@ gen4_signal_levels(uint8_t train_set)
  		signal_levels |= DP_PRE_EMPHASIS_9_5;
  		break;
  	}
-	return signal_levels;
+
+	DRM_DEBUG_KMS("Using signal levels %08x\n", signal_levels);
+
+	intel_dp->DP &= ~(DP_VOLTAGE_MASK | DP_PRE_EMPHASIS_MASK);
+	intel_dp->DP |= signal_levels;
  }
+static const struct signal_levels gen4_signal_levels = {
+	.max_voltage = DP_TRAIN_VOLTAGE_SWING_LEVEL_2,
+	.max_pre_emph = {
+		DP_TRAIN_PRE_EMPH_LEVEL_2,
+		DP_TRAIN_PRE_EMPH_LEVEL_2,
+		DP_TRAIN_PRE_EMPH_LEVEL_1,
+		DP_TRAIN_PRE_EMPH_LEVEL_0,
+	},
+
+	.set = gen4_set_signal_levels,
+};
+
+/* Not for eDP */
+static const struct signal_levels snb_signal_levels = {
+	.max_voltage = DP_TRAIN_VOLTAGE_SWING_LEVEL_3,
+	.max_pre_emph = {
+		DP_TRAIN_PRE_EMPH_LEVEL_2,
+		DP_TRAIN_PRE_EMPH_LEVEL_2,
+		DP_TRAIN_PRE_EMPH_LEVEL_1,
+		DP_TRAIN_PRE_EMPH_LEVEL_0,
+	},
+
+	.set = gen4_set_signal_levels,
+};
+
  /* Gen6's DP voltage swing and pre-emphasis control */
  static uint32_t
  gen6_edp_signal_levels(uint8_t train_set)
@@ -486,13 +515,12 @@ gen7_edp_signal_levels(uint8_t train_set)
  	}
  }
-void
-intel_dp_set_signal_levels(struct intel_dp *intel_dp)
+static void
+_update_signal_levels(struct intel_dp *intel_dp)
  {
  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
  	enum port port = intel_dig_port->port;
  	struct drm_device *dev = intel_dig_port->base.base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
  	uint32_t signal_levels, mask = 0;
  	uint8_t train_set = intel_dp->train_set[0];
@@ -514,21 +542,66 @@ intel_dp_set_signal_levels(struct intel_dp *intel_dp)
  		signal_levels = gen6_edp_signal_levels(train_set);
  		mask = EDP_LINK_TRAIN_VOL_EMP_MASK_SNB;
  	} else {
-		signal_levels = gen4_signal_levels(train_set);
-		mask = DP_VOLTAGE_MASK | DP_PRE_EMPHASIS_MASK;
+		WARN(1, "Should be calling intel_dp->signal_levels->set instead.");
+		return;
  	}
if (mask)
  		DRM_DEBUG_KMS("Using signal levels %08x\n", signal_levels);
+ intel_dp->DP = (intel_dp->DP & ~mask) | signal_levels;
+}
+
+uint8_t
+intel_dp_voltage_max(struct intel_dp *intel_dp)
+{
+	if (intel_dp->signal_levels)
+		return intel_dp->signal_levels->max_voltage;
+	else
+		return _dp_voltage_max(intel_dp);
+}
+
+uint8_t
+intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
+{
+	if (intel_dp->signal_levels)
+		return intel_dp->signal_levels->max_pre_emph[voltage_swing];
+	else
+		return _dp_pre_emphasis_max(intel_dp, voltage_swing);
+}
+
+void
+intel_dp_set_signal_levels(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	uint8_t train_set = intel_dp->train_set[0];
+
+	if (intel_dp->signal_levels)
+		intel_dp->signal_levels->set(intel_dp, train_set);
+	else
+		_update_signal_levels(intel_dp);
+
  	DRM_DEBUG_KMS("Using vswing level %d\n",
  		train_set & DP_TRAIN_VOLTAGE_SWING_MASK);
  	DRM_DEBUG_KMS("Using pre-emphasis level %d\n",
  		(train_set & DP_TRAIN_PRE_EMPHASIS_MASK) >>
  			DP_TRAIN_PRE_EMPHASIS_SHIFT);
- intel_dp->DP = (intel_dp->DP & ~mask) | signal_levels;
-
  	I915_WRITE(intel_dp->output_reg, intel_dp->DP);
  	POSTING_READ(intel_dp->output_reg);
  }
+
+void
+intel_dp_init_signal_levels(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	enum port port = intel_dig_port->port;
+
+	if (port != PORT_A && (IS_IVYBRIDGE(dev) || IS_GEN6(dev)))
+		intel_dp->signal_levels = &snb_signal_levels;
+
+	if (INTEL_INFO(dev)->gen <= 5)
+		intel_dp->signal_levels = &gen4_signal_levels;
+}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d758e94..e00ce6c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -726,6 +726,14 @@ struct sink_crc {
  	int last_count;
  };
+struct intel_dp;
+struct signal_levels {
+	uint8_t max_voltage;
+	uint8_t max_pre_emph[4];
+
+	void (*set)(struct intel_dp *intel_dp, uint8_t train_set);
+};
+
  struct intel_dp {
  	uint32_t output_reg;
  	uint32_t aux_ch_ctl_reg;
@@ -744,6 +752,7 @@ struct intel_dp {
  	int sink_rates[DP_MAX_SUPPORTED_RATES];
  	struct sink_crc sink_crc;
  	struct drm_dp_aux aux;
+	const struct signal_levels *signal_levels;
  	uint8_t train_set[4];
  	int panel_power_up_delay;
  	int panel_power_down_delay;
@@ -1263,6 +1272,8 @@ bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
  bool
  intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]);
+void intel_dp_init_signal_levels(struct intel_dp *intel_dp);
+
  /* intel_dp_mst.c */
  int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
  void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux