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 
<[email protected]>
---
  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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to