[PATCH] drm/i915/cnl: Fix DP max voltage

[PATCH] drm/i915/cnl: Fix DP max voltage

[Rodrigo Vivi]

From: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>

On clock recovery this function is called to find out
the max voltage swing level that we could go.

However gen 9 functions use the old buffer translation tables
to figure that out. That table is not valid for CNL
causing an invalid number of entries and an invalid selection
on the max voltage swing level.

v2: Let's use same approach that previous platforms.
v3: Actually use n_entries and avoid duplicated -1.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 48 +++++++++++++++++++++++++++++++---------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index d962552e2ccc..9aa508616284 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -649,6 +649,29 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
 	}
 }
 
+static int cnl_max_level(struct drm_i915_private *dev_priv,
+			 enum intel_output_type type)
+{
+	int n_entries = 0;
+
+	switch (type) {
+	case INTEL_OUTPUT_DP:
+		cnl_get_buf_trans_dp(dev_priv, &n_entries);
+		break;
+	case INTEL_OUTPUT_EDP:
+		cnl_get_buf_trans_edp(dev_priv, &n_entries);
+		break;
+	case INTEL_OUTPUT_HDMI:
+		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
+		break;
+	default:
+		MISSING_CASE(type);
+		return 0;
+	}
+
+	return n_entries;
+}
+
 static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
 {
 	int n_hdmi_entries;
@@ -1877,19 +1900,24 @@ static void bxt_ddi_vswing_sequence(struct drm_i915_private *dev_priv,
 u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	int n_entries;
+	int n_entries, level;
 
-	if (encoder->type == INTEL_OUTPUT_EDP)
-		intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
-	else
-		intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
+	if (IS_CANNONLAKE(dev_priv)) {
+		level = cnl_max_level(dev_priv, encoder->type);
+	} else {
+		if (encoder->type == INTEL_OUTPUT_EDP)
+			intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
+		else
+			intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
+		level = n_entries - 1;
+	}
 
-	if (WARN_ON(n_entries < 1))
-		n_entries = 1;
-	if (WARN_ON(n_entries > ARRAY_SIZE(index_to_dp_signal_levels)))
-		n_entries = ARRAY_SIZE(index_to_dp_signal_levels);
+	if (WARN_ON(level < 0))
+		level = 0;
+	if (WARN_ON(level > ARRAY_SIZE(index_to_dp_signal_levels) - 1))
+		level = ARRAY_SIZE(index_to_dp_signal_levels) - 1;
 
-	return index_to_dp_signal_levels[n_entries - 1] &
+	return index_to_dp_signal_levels[level] &
 		DP_TRAIN_VOLTAGE_SWING_MASK;
 }
 
-- 
2.13.2

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

Re: [PATCH] drm/i915/cnl: Fix DP max voltage

[Ville Syrjälä]

On Wed, Aug 30, 2017 at 05:00:50PM -0700, Rodrigo Vivi wrote:
> From: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
> 
> On clock recovery this function is called to find out
> the max voltage swing level that we could go.
> 
> However gen 9 functions use the old buffer translation tables
> to figure that out. That table is not valid for CNL
> causing an invalid number of entries and an invalid selection
> on the max voltage swing level.
> 
> v2: Let's use same approach that previous platforms.
> v3: Actually use n_entries and avoid duplicated -1.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 48 +++++++++++++++++++++++++++++++---------
>  1 file changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index d962552e2ccc..9aa508616284 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -649,6 +649,29 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
>  	}
>  }
>  
> +static int cnl_max_level(struct drm_i915_private *dev_priv,
> +			 enum intel_output_type type)
> +{
> +	int n_entries = 0;
> +
> +	switch (type) {
> +	case INTEL_OUTPUT_DP:
> +		cnl_get_buf_trans_dp(dev_priv, &n_entries);
> +		break;
> +	case INTEL_OUTPUT_EDP:
> +		cnl_get_buf_trans_edp(dev_priv, &n_entries);
> +		break;
> +	case INTEL_OUTPUT_HDMI:
> +		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
> +		break;
> +	default:
> +		MISSING_CASE(type);
> +		return 0;
> +	}
> +
> +	return n_entries;
> +}
> +
>  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
>  {
>  	int n_hdmi_entries;
> @@ -1877,19 +1900,24 @@ static void bxt_ddi_vswing_sequence(struct drm_i915_private *dev_priv,
>  u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	int n_entries;
> +	int n_entries, level;
>  
> -	if (encoder->type == INTEL_OUTPUT_EDP)
> -		intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> -	else
> -		intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> +	if (IS_CANNONLAKE(dev_priv)) {
> +		level = cnl_max_level(dev_priv, encoder->type);
> +	} else {
> +		if (encoder->type == INTEL_OUTPUT_EDP)
> +			intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> +		else
> +			intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> +		level = n_entries - 1;

You removed the -1 from the cnl path but then added it to the other path?

In fact, I think to keep things looking a bit more consistent I'd just
open code the cnl stuff in intel_ddi_dp_voltage_max() the same way as
the other platforms are handled. And we don't even need to care about HDMI
here.

> +	}
>  
> -	if (WARN_ON(n_entries < 1))
> -		n_entries = 1;
> -	if (WARN_ON(n_entries > ARRAY_SIZE(index_to_dp_signal_levels)))
> -		n_entries = ARRAY_SIZE(index_to_dp_signal_levels);
> +	if (WARN_ON(level < 0))
> +		level = 0;
> +	if (WARN_ON(level > ARRAY_SIZE(index_to_dp_signal_levels) - 1))
> +		level = ARRAY_SIZE(index_to_dp_signal_levels) - 1;
>  
> -	return index_to_dp_signal_levels[n_entries - 1] &
> +	return index_to_dp_signal_levels[level] &
>  		DP_TRAIN_VOLTAGE_SWING_MASK;
>  }
>  
> -- 
> 2.13.2

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

[PATCH] drm/i915/cnl: Fix DP max voltage

[Rodrigo Vivi]

From: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>

On clock recovery this function is called to find out
the max voltage swing level that we could go.

However gen 9 functions use the old buffer translation tables
to figure that out. That table is not valid for CNL
causing an invalid number of entries and an invalid selection
on the max voltage swing level.

v2: Let's use same approach that previous platforms.
v3: Actually use n_entries and avoid duplicated -1.
v4: Avoid cnl_max_level and use current style.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f1757a8e481a..1da3bb2cc4b4 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1879,10 +1879,17 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	int n_entries;
 
-	if (encoder->type == INTEL_OUTPUT_EDP)
-		intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
-	else
-		intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
+	if (IS_CANNONLAKE(dev_priv)) {
+		if (encoder->type == INTEL_OUTPUT_EDP)
+			cnl_get_buf_trans_edp(dev_priv, &n_entries);
+		else
+			cnl_get_buf_trans_dp(dev_priv, &n_entries);
+	} else {
+		if (encoder->type == INTEL_OUTPUT_EDP)
+			intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
+		else
+			intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
+	}
 
 	if (WARN_ON(n_entries < 1))
 		n_entries = 1;
-- 
2.13.2

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

Re: [PATCH] drm/i915/cnl: Fix DP max voltage

[Ville Syrjälä]

On Thu, Aug 31, 2017 at 07:53:56AM -0700, Rodrigo Vivi wrote:
> From: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
> 
> On clock recovery this function is called to find out
> the max voltage swing level that we could go.
> 
> However gen 9 functions use the old buffer translation tables
> to figure that out. That table is not valid for CNL
> causing an invalid number of entries and an invalid selection
> on the max voltage swing level.
> 
> v2: Let's use same approach that previous platforms.
> v3: Actually use n_entries and avoid duplicated -1.
> v4: Avoid cnl_max_level and use current style.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f1757a8e481a..1da3bb2cc4b4 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1879,10 +1879,17 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	int n_entries;
>  
> -	if (encoder->type == INTEL_OUTPUT_EDP)
> -		intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> -	else
> -		intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> +	if (IS_CANNONLAKE(dev_priv)) {
> +		if (encoder->type == INTEL_OUTPUT_EDP)
> +			cnl_get_buf_trans_edp(dev_priv, &n_entries);
> +		else
> +			cnl_get_buf_trans_dp(dev_priv, &n_entries);
> +	} else {
> +		if (encoder->type == INTEL_OUTPUT_EDP)
> +			intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> +		else
> +			intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> +	}
>  
>  	if (WARN_ON(n_entries < 1))
>  		n_entries = 1;
> -- 
> 2.13.2

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

Re: [PATCH] drm/i915/cnl: Fix DP max voltage

[Vivi, Rodrigo]

On Thu, 2017-08-31 at 18:06 +0300, Ville Syrjälä wrote:
> On Thu, Aug 31, 2017 at 07:53:56AM -0700, Rodrigo Vivi wrote:
> > From: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
> > 
> > On clock recovery this function is called to find out
> > the max voltage swing level that we could go.
> > 
> > However gen 9 functions use the old buffer translation tables
> > to figure that out. That table is not valid for CNL
> > causing an invalid number of entries and an invalid selection
> > on the max voltage swing level.
> > 
> > v2: Let's use same approach that previous platforms.
> > v3: Actually use n_entries and avoid duplicated -1.
> > v4: Avoid cnl_max_level and use current style.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Clint Taylor <clinton.a.taylor@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks for the quick review.
Series merged to dinq. Hopefully we will be able to get CI happier
with drm-tip.


> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index f1757a8e481a..1da3bb2cc4b4 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1879,10 +1879,17 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	int n_entries;
> >  
> > -	if (encoder->type == INTEL_OUTPUT_EDP)
> > -		intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> > -	else
> > -		intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> > +	if (IS_CANNONLAKE(dev_priv)) {
> > +		if (encoder->type == INTEL_OUTPUT_EDP)
> > +			cnl_get_buf_trans_edp(dev_priv, &n_entries);
> > +		else
> > +			cnl_get_buf_trans_dp(dev_priv, &n_entries);
> > +	} else {
> > +		if (encoder->type == INTEL_OUTPUT_EDP)
> > +			intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> > +		else
> > +			intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> > +	}
> >  
> >  	if (WARN_ON(n_entries < 1))
> >  		n_entries = 1;
> > -- 
> > 2.13.2
> 

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

[PATCH 1/8] drm/i915: Introduce intel_ddi_dp_level.

[Rodrigo Vivi]

No functional changes. This only moves the DP level
selection to a separated function that will be later
used to organize better the vswing sequences.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0a316a6ccb50..7e875e05d053 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2054,18 +2054,22 @@ static uint32_t translate_signal_level(int signal_levels)
 	return 0;
 }
 
+static uint32_t intel_ddi_dp_level(struct intel_dp *intel_dp)
+{
+	uint8_t train_set = intel_dp->train_set[0];
+	int signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
+					 DP_TRAIN_PRE_EMPHASIS_MASK);
+
+	return translate_signal_level(signal_levels);
+}
+
 uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
 	struct intel_encoder *encoder = &dport->base;
-	uint8_t train_set = intel_dp->train_set[0];
-	int signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
-					 DP_TRAIN_PRE_EMPHASIS_MASK);
 	enum port port = dport->port;
-	uint32_t level;
-
-	level = translate_signal_level(signal_levels);
+	uint32_t level = intel_ddi_dp_level(intel_dp);
 
 	if (IS_GEN9_BC(dev_priv))
 		skl_ddi_set_iboost(encoder, level);
-- 
2.13.2

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

[PATCH 2/8] drm/i915: decouple gen9 and gen10 dp signal levels.

[Rodrigo Vivi]

Let's decouple bxt, glk and cnl dp signal levels
from other DDIs to avoid confusion.

No functional change. Only a reorg to avoid messing
with currently working DP signal levels when
moving voltage swing sequences around to match spec.

v2: ddi_signal_levels is also called from other ddi
    platforms, so don't remove IS_GEN9_BC check from
    skl_ddi_set_iboos. (Ville).

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 27 ++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_dp.c  | 10 ++++------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 3 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 7e875e05d053..9a887780f99f 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2063,23 +2063,32 @@ static uint32_t intel_ddi_dp_level(struct intel_dp *intel_dp)
 	return translate_signal_level(signal_levels);
 }
 
-uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
+u32 bxt_signal_levels(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
 	struct intel_encoder *encoder = &dport->base;
 	enum port port = dport->port;
+	u32 level = intel_ddi_dp_level(intel_dp);
+
+	if (IS_CANNONLAKE(dev_priv))
+		cnl_ddi_vswing_sequence(encoder, level);
+	else
+		bxt_ddi_vswing_sequence(dev_priv, level, port, encoder->type);
+
+	return 0;
+}
+
+uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
+	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
+	struct intel_encoder *encoder = &dport->base;
 	uint32_t level = intel_ddi_dp_level(intel_dp);
 
 	if (IS_GEN9_BC(dev_priv))
-		skl_ddi_set_iboost(encoder, level);
-	else if (IS_GEN9_LP(dev_priv))
-		bxt_ddi_vswing_sequence(dev_priv, level, port, encoder->type);
-	else if (IS_CANNONLAKE(dev_priv)) {
-		cnl_ddi_vswing_sequence(encoder, level);
-		/* DDI_BUF_CTL bits 27:24 are reserved on CNL */
-		return 0;
-	}
+	    skl_ddi_set_iboost(encoder, level);
+
 	return DDI_BUF_TRANS_SELECT(level);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d3e5fdf0d2fa..49a8c339b2b0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3506,13 +3506,11 @@ intel_dp_set_signal_levels(struct intel_dp *intel_dp)
 	uint32_t signal_levels, mask = 0;
 	uint8_t train_set = intel_dp->train_set[0];
 
-	if (HAS_DDI(dev_priv)) {
+	if (IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(dev_priv)) {
+		signal_levels = bxt_signal_levels(intel_dp);
+	} else if (HAS_DDI(dev_priv)) {
 		signal_levels = ddi_signal_levels(intel_dp);
-
-		if (IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(dev_priv))
-			signal_levels = 0;
-		else
-			mask = DDI_BUF_EMP_MASK;
+		mask = DDI_BUF_EMP_MASK;
 	} else if (IS_CHERRYVIEW(dev_priv)) {
 		signal_levels = chv_signal_levels(intel_dp);
 	} else if (IS_VALLEYVIEW(dev_priv)) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 17649f13091c..469c06000774 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1271,6 +1271,7 @@ void intel_ddi_clock_get(struct intel_encoder *encoder,
 			 struct intel_crtc_state *pipe_config);
 void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
 				    bool state);
+u32 bxt_signal_levels(struct intel_dp *intel_dp);
 uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
 u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder);
 
-- 
2.13.2

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

Re: [PATCH 2/8] drm/i915: decouple gen9 and gen10 dp signal levels.

[Ville Syrjälä]

On Tue, Aug 29, 2017 at 04:22:24PM -0700, Rodrigo Vivi wrote:
> Let's decouple bxt, glk and cnl dp signal levels
> from other DDIs to avoid confusion.
> 
> No functional change. Only a reorg to avoid messing
> with currently working DP signal levels when
> moving voltage swing sequences around to match spec.
> 
> v2: ddi_signal_levels is also called from other ddi
>     platforms, so don't remove IS_GEN9_BC check from
>     skl_ddi_set_iboos. (Ville).
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 27 ++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_dp.c  | 10 ++++------
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  3 files changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 7e875e05d053..9a887780f99f 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2063,23 +2063,32 @@ static uint32_t intel_ddi_dp_level(struct intel_dp *intel_dp)
>  	return translate_signal_level(signal_levels);
>  }
>  
> -uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
> +u32 bxt_signal_levels(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
>  	struct intel_encoder *encoder = &dport->base;
>  	enum port port = dport->port;
> +	u32 level = intel_ddi_dp_level(intel_dp);
> +
> +	if (IS_CANNONLAKE(dev_priv))
> +		cnl_ddi_vswing_sequence(encoder, level);
> +	else
> +		bxt_ddi_vswing_sequence(dev_priv, level, port, encoder->type);
> +
> +	return 0;
> +}
> +
> +uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
> +{
> +	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
> +	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
> +	struct intel_encoder *encoder = &dport->base;
>  	uint32_t level = intel_ddi_dp_level(intel_dp);
>  
>  	if (IS_GEN9_BC(dev_priv))
> -		skl_ddi_set_iboost(encoder, level);
> -	else if (IS_GEN9_LP(dev_priv))
> -		bxt_ddi_vswing_sequence(dev_priv, level, port, encoder->type);
> -	else if (IS_CANNONLAKE(dev_priv)) {
> -		cnl_ddi_vswing_sequence(encoder, level);
> -		/* DDI_BUF_CTL bits 27:24 are reserved on CNL */
> -		return 0;
> -	}
> +	    skl_ddi_set_iboost(encoder, level);
	^^^^
Tab got changed into spaces somehow.

With that fixed
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
>  	return DDI_BUF_TRANS_SELECT(level);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d3e5fdf0d2fa..49a8c339b2b0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3506,13 +3506,11 @@ intel_dp_set_signal_levels(struct intel_dp *intel_dp)
>  	uint32_t signal_levels, mask = 0;
>  	uint8_t train_set = intel_dp->train_set[0];
>  
> -	if (HAS_DDI(dev_priv)) {
> +	if (IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> +		signal_levels = bxt_signal_levels(intel_dp);
> +	} else if (HAS_DDI(dev_priv)) {
>  		signal_levels = ddi_signal_levels(intel_dp);
> -
> -		if (IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(dev_priv))
> -			signal_levels = 0;
> -		else
> -			mask = DDI_BUF_EMP_MASK;
> +		mask = DDI_BUF_EMP_MASK;
>  	} else if (IS_CHERRYVIEW(dev_priv)) {
>  		signal_levels = chv_signal_levels(intel_dp);
>  	} else if (IS_VALLEYVIEW(dev_priv)) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 17649f13091c..469c06000774 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1271,6 +1271,7 @@ void intel_ddi_clock_get(struct intel_encoder *encoder,
>  			 struct intel_crtc_state *pipe_config);
>  void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
>  				    bool state);
> +u32 bxt_signal_levels(struct intel_dp *intel_dp);
>  uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
>  u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder);
>  
> -- 
> 2.13.2

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

[PATCH 3/8] drm/i915: Align vswing sequences with old ddi buffer registers.

[Rodrigo Vivi]

Vswing sequences on BXT and CNL are equivalent
to the ddi buffer registers setting on other platforms.

For some reason it got aligned with skl_ddi_set_iboost what
is semantically incorrect. This forced us to keep skipping
ddi buffer translation tables on the platforms that has
the vswing sequences.

v2: Don't mess with DP signal levels on this patch.

Cc: Vandana Kannan <vandana.kannan@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9a887780f99f..eedd29487e0b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -688,9 +688,6 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
 	enum port port = intel_ddi_get_encoder_port(encoder);
 	const struct ddi_buf_trans *ddi_translations;
 
-	if (IS_GEN9_LP(dev_priv))
-		return;
-
 	switch (encoder->type) {
 	case INTEL_OUTPUT_EDP:
 		ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv,
@@ -741,9 +738,6 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
 	enum port port = intel_ddi_get_encoder_port(encoder);
 	const struct ddi_buf_trans *ddi_translations_hdmi;
 
-	if (IS_GEN9_LP(dev_priv))
-		return;
-
 	hdmi_level = intel_ddi_hdmi_level(dev_priv, port);
 
 	if (IS_GEN9_BC(dev_priv)) {
@@ -2154,7 +2148,9 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 
 	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
-	intel_prepare_dp_ddi_buffers(encoder);
+	if (!IS_GEN9_LP(dev_priv) && !IS_CANNONLAKE(dev_priv))
+		intel_prepare_dp_ddi_buffers(encoder);
+
 	intel_ddi_init_dp_buf_reg(encoder);
 	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
 	intel_dp_start_link_train(intel_dp);
@@ -2180,14 +2176,16 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 
 	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
-	intel_prepare_hdmi_ddi_buffers(encoder);
-	if (IS_GEN9_BC(dev_priv))
-		skl_ddi_set_iboost(encoder, level);
+	if (IS_CANNONLAKE(dev_priv))
+		cnl_ddi_vswing_sequence(encoder, level);
 	else if (IS_GEN9_LP(dev_priv))
 		bxt_ddi_vswing_sequence(dev_priv, level, port,
 					INTEL_OUTPUT_HDMI);
-	else if (IS_CANNONLAKE(dev_priv))
-		cnl_ddi_vswing_sequence(encoder, level);
+	else
+		intel_prepare_hdmi_ddi_buffers(encoder);
+
+	if (IS_GEN9_BC(dev_priv))
+		skl_ddi_set_iboost(encoder, level);
 
 	intel_dig_port->set_infoframes(&encoder->base,
 				       has_infoframe,
-- 
2.13.2

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

Re: [PATCH 3/8] drm/i915: Align vswing sequences with old ddi buffer registers.

[Ville Syrjälä]

On Tue, Aug 29, 2017 at 04:22:25PM -0700, Rodrigo Vivi wrote:
> Vswing sequences on BXT and CNL are equivalent
> to the ddi buffer registers setting on other platforms.
> 
> For some reason it got aligned with skl_ddi_set_iboost what
> is semantically incorrect. This forced us to keep skipping
> ddi buffer translation tables on the platforms that has
> the vswing sequences.
> 
> v2: Don't mess with DP signal levels on this patch.
> 
> Cc: Vandana Kannan <vandana.kannan@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 9a887780f99f..eedd29487e0b 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -688,9 +688,6 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
>  	enum port port = intel_ddi_get_encoder_port(encoder);
>  	const struct ddi_buf_trans *ddi_translations;
>  
> -	if (IS_GEN9_LP(dev_priv))
> -		return;
> -
>  	switch (encoder->type) {
>  	case INTEL_OUTPUT_EDP:
>  		ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv,
> @@ -741,9 +738,6 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
>  	enum port port = intel_ddi_get_encoder_port(encoder);
>  	const struct ddi_buf_trans *ddi_translations_hdmi;
>  
> -	if (IS_GEN9_LP(dev_priv))
> -		return;
> -
>  	hdmi_level = intel_ddi_hdmi_level(dev_priv, port);
>  
>  	if (IS_GEN9_BC(dev_priv)) {
> @@ -2154,7 +2148,9 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  
>  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
>  
> -	intel_prepare_dp_ddi_buffers(encoder);
> +	if (!IS_GEN9_LP(dev_priv) && !IS_CANNONLAKE(dev_priv))
> +		intel_prepare_dp_ddi_buffers(encoder);
> +
>  	intel_ddi_init_dp_buf_reg(encoder);
>  	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
>  	intel_dp_start_link_train(intel_dp);
> @@ -2180,14 +2176,16 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>  
>  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
>  
> -	intel_prepare_hdmi_ddi_buffers(encoder);
> -	if (IS_GEN9_BC(dev_priv))
> -		skl_ddi_set_iboost(encoder, level);
> +	if (IS_CANNONLAKE(dev_priv))
> +		cnl_ddi_vswing_sequence(encoder, level);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_vswing_sequence(dev_priv, level, port,
>  					INTEL_OUTPUT_HDMI);
> -	else if (IS_CANNONLAKE(dev_priv))
> -		cnl_ddi_vswing_sequence(encoder, level);
> +	else
> +		intel_prepare_hdmi_ddi_buffers(encoder);
> +
> +	if (IS_GEN9_BC(dev_priv))
> +		skl_ddi_set_iboost(encoder, level);
>  
>  	intel_dig_port->set_infoframes(&encoder->base,
>  				       has_infoframe,
> -- 
> 2.13.2

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

[PATCH 4/8] drm/i915: Enable voltage swing before enabling DDI_BUF_CTL.

[Rodrigo Vivi]

Sequences for DisplayPort asks us to
" Configure voltage swing and related IO settings.
Refer to DDI Buffer section."

before "Configure and enable DDI_BUF_CTL"

On BXT and CNL this means to execute the ddi vswing sequences.

At this point these sequences calls are getting duplicated for DP
because they are all called from DP link trainning sequences.

However this patch is not yet removing it before a futher discussion
since spec also allows that during link training without disabling
anything:

"
Notes
Changing voltage swing during link training:
Change the swing setting following the DDI Buffer section.
The port does not need to be disabled.
"

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index eedd29487e0b..506782c1a62a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2136,6 +2136,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	enum port port = intel_ddi_get_encoder_port(encoder);
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+	uint32_t level = intel_ddi_dp_level(intel_dp);
 
 	WARN_ON(link_mst && (port == PORT_A || port == PORT_E));
 
@@ -2148,7 +2149,11 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 
 	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
-	if (!IS_GEN9_LP(dev_priv) && !IS_CANNONLAKE(dev_priv))
+	if (IS_CANNONLAKE(dev_priv))
+		cnl_ddi_vswing_sequence(encoder, level);
+	else if (IS_GEN9_LP(dev_priv))
+		bxt_ddi_vswing_sequence(dev_priv, level, port, encoder->type);
+	else
 		intel_prepare_dp_ddi_buffers(encoder);
 
 	intel_ddi_init_dp_buf_reg(encoder);
-- 
2.13.2

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

Re: [PATCH 4/8] drm/i915: Enable voltage swing before enabling DDI_BUF_CTL.

[Ville Syrjälä]

On Tue, Aug 29, 2017 at 04:22:26PM -0700, Rodrigo Vivi wrote:
> Sequences for DisplayPort asks us to
> " Configure voltage swing and related IO settings.
> Refer to DDI Buffer section."
> 
> before "Configure and enable DDI_BUF_CTL"
> 
> On BXT and CNL this means to execute the ddi vswing sequences.
> 
> At this point these sequences calls are getting duplicated for DP
> because they are all called from DP link trainning sequences.
> 
> However this patch is not yet removing it before a futher discussion
> since spec also allows that during link training without disabling
> anything:
> 
> "
> Notes
> Changing voltage swing during link training:
> Change the swing setting following the DDI Buffer section.
> The port does not need to be disabled.
> "
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Imre is out atm so we didn't get his opinion, but I'm fine with this so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index eedd29487e0b..506782c1a62a 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2136,6 +2136,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	enum port port = intel_ddi_get_encoder_port(encoder);
>  	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> +	uint32_t level = intel_ddi_dp_level(intel_dp);
>  
>  	WARN_ON(link_mst && (port == PORT_A || port == PORT_E));
>  
> @@ -2148,7 +2149,11 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  
>  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
>  
> -	if (!IS_GEN9_LP(dev_priv) && !IS_CANNONLAKE(dev_priv))
> +	if (IS_CANNONLAKE(dev_priv))
> +		cnl_ddi_vswing_sequence(encoder, level);
> +	else if (IS_GEN9_LP(dev_priv))
> +		bxt_ddi_vswing_sequence(dev_priv, level, port, encoder->type);
> +	else
>  		intel_prepare_dp_ddi_buffers(encoder);
>  
>  	intel_ddi_init_dp_buf_reg(encoder);
> -- 
> 2.13.2

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

[PATCH 5/8] drm/i915/cnl: Move voltage check into ddi buf trans functions.

[Rodrigo Vivi]

Let's start converging CNL buf translations to same style
used on previous platforms. So first thing is to use the
standard signature so we don't need to propagate the voltage
check into other parts of the code, but only on the parts
that it is really useful.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 48 ++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 506782c1a62a..7b547a7f6c2b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1830,9 +1830,10 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 }
 
 static const struct cnl_ddi_buf_trans *
-cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv,
-		       u32 voltage, int *n_entries)
+cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
 {
+	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
+
 	if (voltage == VOLTAGE_INFO_0_85V) {
 		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_0_85V);
 		return cnl_ddi_translations_hdmi_0_85V;
@@ -1842,14 +1843,16 @@ cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv,
 	} else if (voltage == VOLTAGE_INFO_1_05V) {
 		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_1_05V);
 		return cnl_ddi_translations_hdmi_1_05V;
-	}
+	} else
+		MISSING_CASE(voltage);
 	return NULL;
 }
 
 static const struct cnl_ddi_buf_trans *
-cnl_get_buf_trans_dp(struct drm_i915_private *dev_priv,
-		     u32 voltage, int *n_entries)
+cnl_get_buf_trans_dp(struct drm_i915_private *dev_priv, int *n_entries)
 {
+	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
+
 	if (voltage == VOLTAGE_INFO_0_85V) {
 		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_0_85V);
 		return cnl_ddi_translations_dp_0_85V;
@@ -1859,14 +1862,16 @@ cnl_get_buf_trans_dp(struct drm_i915_private *dev_priv,
 	} else if (voltage == VOLTAGE_INFO_1_05V) {
 		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_1_05V);
 		return cnl_ddi_translations_dp_1_05V;
-	}
+	} else
+		MISSING_CASE(voltage);
 	return NULL;
 }
 
 static const struct cnl_ddi_buf_trans *
-cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv,
-		      u32 voltage, int *n_entries)
+cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
 {
+	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
+
 	if (dev_priv->vbt.edp.low_vswing) {
 		if (voltage == VOLTAGE_INFO_0_85V) {
 			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_85V);
@@ -1877,10 +1882,11 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv,
 		} else if (voltage == VOLTAGE_INFO_1_05V) {
 			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_1_05V);
 			return cnl_ddi_translations_edp_1_05V;
-		}
+		} else
+			MISSING_CASE(voltage);
 		return NULL;
 	} else {
-		return cnl_get_buf_trans_dp(dev_priv, voltage, n_entries);
+		return cnl_get_buf_trans_dp(dev_priv, n_entries);
 	}
 }
 
@@ -1888,31 +1894,19 @@ static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
 				    u32 level, enum port port, int type)
 {
 	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
-	u32 n_entries, val, voltage;
+	u32 n_entries, val;
 	int ln;
 
-	/*
-	 * Values for each port type are listed in
-	 * voltage swing programming tables.
-	 * Vccio voltage found in PORT_COMP_DW3.
-	 */
-	voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
-
 	if (type == INTEL_OUTPUT_HDMI) {
-		ddi_translations = cnl_get_buf_trans_hdmi(dev_priv,
-							  voltage, &n_entries);
+		ddi_translations = cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
 	} else if (type == INTEL_OUTPUT_DP) {
-		ddi_translations = cnl_get_buf_trans_dp(dev_priv,
-							voltage, &n_entries);
+		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
 	} else if (type == INTEL_OUTPUT_EDP) {
-		ddi_translations = cnl_get_buf_trans_edp(dev_priv,
-							 voltage, &n_entries);
+		ddi_translations = cnl_get_buf_trans_edp(dev_priv, &n_entries);
 	}
 
-	if (ddi_translations == NULL) {
-		MISSING_CASE(voltage);
+	if (WARN_ON(ddi_translations == NULL))
 		return;
-	}
 
 	if (level >= n_entries) {
 		DRM_DEBUG_KMS("DDI translation not found for level %d. Using %d instead.", level, n_entries - 1);
-- 
2.13.2

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

Re: [PATCH 5/8] drm/i915/cnl: Move voltage check into ddi buf trans functions.

[Ville Syrjälä]

On Tue, Aug 29, 2017 at 04:22:27PM -0700, Rodrigo Vivi wrote:
> Let's start converging CNL buf translations to same style
> used on previous platforms. So first thing is to use the
> standard signature so we don't need to propagate the voltage
> check into other parts of the code, but only on the parts
> that it is really useful.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 48 ++++++++++++++++++----------------------
>  1 file changed, 21 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 506782c1a62a..7b547a7f6c2b 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1830,9 +1830,10 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  }
>  
>  static const struct cnl_ddi_buf_trans *
> -cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv,
> -		       u32 voltage, int *n_entries)
> +cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
>  {
> +	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;

I wonder if we should just cache that somewhere. My main worry is
whether the device is always awake when we call this code.

> +
>  	if (voltage == VOLTAGE_INFO_0_85V) {
>  		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_0_85V);
>  		return cnl_ddi_translations_hdmi_0_85V;
> @@ -1842,14 +1843,16 @@ cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv,
>  	} else if (voltage == VOLTAGE_INFO_1_05V) {
>  		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_1_05V);
>  		return cnl_ddi_translations_hdmi_1_05V;
> -	}
> +	} else
> +		MISSING_CASE(voltage);

nit: Looks like these if ladders could be turned into switch statements.


Anyways, patch lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	return NULL;
>  }
>  
>  static const struct cnl_ddi_buf_trans *
> -cnl_get_buf_trans_dp(struct drm_i915_private *dev_priv,
> -		     u32 voltage, int *n_entries)
> +cnl_get_buf_trans_dp(struct drm_i915_private *dev_priv, int *n_entries)
>  {
> +	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
> +
>  	if (voltage == VOLTAGE_INFO_0_85V) {
>  		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_0_85V);
>  		return cnl_ddi_translations_dp_0_85V;
> @@ -1859,14 +1862,16 @@ cnl_get_buf_trans_dp(struct drm_i915_private *dev_priv,
>  	} else if (voltage == VOLTAGE_INFO_1_05V) {
>  		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_1_05V);
>  		return cnl_ddi_translations_dp_1_05V;
> -	}
> +	} else
> +		MISSING_CASE(voltage);
>  	return NULL;
>  }
>  
>  static const struct cnl_ddi_buf_trans *
> -cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv,
> -		      u32 voltage, int *n_entries)
> +cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
>  {
> +	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
> +
>  	if (dev_priv->vbt.edp.low_vswing) {
>  		if (voltage == VOLTAGE_INFO_0_85V) {
>  			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_85V);
> @@ -1877,10 +1882,11 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv,
>  		} else if (voltage == VOLTAGE_INFO_1_05V) {
>  			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_1_05V);
>  			return cnl_ddi_translations_edp_1_05V;
> -		}
> +		} else
> +			MISSING_CASE(voltage);
>  		return NULL;
>  	} else {
> -		return cnl_get_buf_trans_dp(dev_priv, voltage, n_entries);
> +		return cnl_get_buf_trans_dp(dev_priv, n_entries);
>  	}
>  }
>  
> @@ -1888,31 +1894,19 @@ static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
>  				    u32 level, enum port port, int type)
>  {
>  	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
> -	u32 n_entries, val, voltage;
> +	u32 n_entries, val;
>  	int ln;
>  
> -	/*
> -	 * Values for each port type are listed in
> -	 * voltage swing programming tables.
> -	 * Vccio voltage found in PORT_COMP_DW3.
> -	 */
> -	voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
> -
>  	if (type == INTEL_OUTPUT_HDMI) {
> -		ddi_translations = cnl_get_buf_trans_hdmi(dev_priv,
> -							  voltage, &n_entries);
> +		ddi_translations = cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
>  	} else if (type == INTEL_OUTPUT_DP) {
> -		ddi_translations = cnl_get_buf_trans_dp(dev_priv,
> -							voltage, &n_entries);
> +		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
>  	} else if (type == INTEL_OUTPUT_EDP) {
> -		ddi_translations = cnl_get_buf_trans_edp(dev_priv,
> -							 voltage, &n_entries);
> +		ddi_translations = cnl_get_buf_trans_edp(dev_priv, &n_entries);
>  	}
>  
> -	if (ddi_translations == NULL) {
> -		MISSING_CASE(voltage);
> +	if (WARN_ON(ddi_translations == NULL))
>  		return;
> -	}
>  
>  	if (level >= n_entries) {
>  		DRM_DEBUG_KMS("DDI translation not found for level %d. Using %d instead.", level, n_entries - 1);
> -- 
> 2.13.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 6/8] drm/i915/cnl: Move ddi buf trans related functions up.

[Rodrigo Vivi]

No functional changes. But those functions will be needed
to get max level for HDMI and DP, so let's move those
up closer to other similar functions existent for previous
platforms.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 122 +++++++++++++++++++--------------------
 1 file changed, 61 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 7b547a7f6c2b..3ce02cbd4483 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -588,6 +588,67 @@ skl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
 	}
 }
 
+static const struct cnl_ddi_buf_trans *
+cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
+{
+	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
+
+	if (voltage == VOLTAGE_INFO_0_85V) {
+		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_0_85V);
+		return cnl_ddi_translations_hdmi_0_85V;
+	} else if (voltage == VOLTAGE_INFO_0_95V) {
+		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_0_95V);
+		return cnl_ddi_translations_hdmi_0_95V;
+	} else if (voltage == VOLTAGE_INFO_1_05V) {
+		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_1_05V);
+		return cnl_ddi_translations_hdmi_1_05V;
+	} else
+		MISSING_CASE(voltage);
+	return NULL;
+}
+
+static const struct cnl_ddi_buf_trans *
+cnl_get_buf_trans_dp(struct drm_i915_private *dev_priv, int *n_entries)
+{
+	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
+
+	if (voltage == VOLTAGE_INFO_0_85V) {
+		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_0_85V);
+		return cnl_ddi_translations_dp_0_85V;
+	} else if (voltage == VOLTAGE_INFO_0_95V) {
+		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_0_95V);
+		return cnl_ddi_translations_dp_0_95V;
+	} else if (voltage == VOLTAGE_INFO_1_05V) {
+		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_1_05V);
+		return cnl_ddi_translations_dp_1_05V;
+	} else
+		MISSING_CASE(voltage);
+	return NULL;
+}
+
+static const struct cnl_ddi_buf_trans *
+cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
+{
+	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
+
+	if (dev_priv->vbt.edp.low_vswing) {
+		if (voltage == VOLTAGE_INFO_0_85V) {
+			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_85V);
+			return cnl_ddi_translations_edp_0_85V;
+		} else if (voltage == VOLTAGE_INFO_0_95V) {
+			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_95V);
+			return cnl_ddi_translations_edp_0_95V;
+		} else if (voltage == VOLTAGE_INFO_1_05V) {
+			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_1_05V);
+			return cnl_ddi_translations_edp_1_05V;
+		} else
+			MISSING_CASE(voltage);
+		return NULL;
+	} else {
+		return cnl_get_buf_trans_dp(dev_priv, n_entries);
+	}
+}
+
 static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
 {
 	int n_hdmi_entries;
@@ -1829,67 +1890,6 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 		DP_TRAIN_VOLTAGE_SWING_MASK;
 }
 
-static const struct cnl_ddi_buf_trans *
-cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
-{
-	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
-
-	if (voltage == VOLTAGE_INFO_0_85V) {
-		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_0_85V);
-		return cnl_ddi_translations_hdmi_0_85V;
-	} else if (voltage == VOLTAGE_INFO_0_95V) {
-		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_0_95V);
-		return cnl_ddi_translations_hdmi_0_95V;
-	} else if (voltage == VOLTAGE_INFO_1_05V) {
-		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_1_05V);
-		return cnl_ddi_translations_hdmi_1_05V;
-	} else
-		MISSING_CASE(voltage);
-	return NULL;
-}
-
-static const struct cnl_ddi_buf_trans *
-cnl_get_buf_trans_dp(struct drm_i915_private *dev_priv, int *n_entries)
-{
-	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
-
-	if (voltage == VOLTAGE_INFO_0_85V) {
-		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_0_85V);
-		return cnl_ddi_translations_dp_0_85V;
-	} else if (voltage == VOLTAGE_INFO_0_95V) {
-		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_0_95V);
-		return cnl_ddi_translations_dp_0_95V;
-	} else if (voltage == VOLTAGE_INFO_1_05V) {
-		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_1_05V);
-		return cnl_ddi_translations_dp_1_05V;
-	} else
-		MISSING_CASE(voltage);
-	return NULL;
-}
-
-static const struct cnl_ddi_buf_trans *
-cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
-{
-	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
-
-	if (dev_priv->vbt.edp.low_vswing) {
-		if (voltage == VOLTAGE_INFO_0_85V) {
-			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_85V);
-			return cnl_ddi_translations_edp_0_85V;
-		} else if (voltage == VOLTAGE_INFO_0_95V) {
-			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_95V);
-			return cnl_ddi_translations_edp_0_95V;
-		} else if (voltage == VOLTAGE_INFO_1_05V) {
-			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_1_05V);
-			return cnl_ddi_translations_edp_1_05V;
-		} else
-			MISSING_CASE(voltage);
-		return NULL;
-	} else {
-		return cnl_get_buf_trans_dp(dev_priv, n_entries);
-	}
-}
-
 static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
 				    u32 level, enum port port, int type)
 {
-- 
2.13.2

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

Re: [PATCH 6/8] drm/i915/cnl: Move ddi buf trans related functions up.

[Ville Syrjälä]

On Tue, Aug 29, 2017 at 04:22:28PM -0700, Rodrigo Vivi wrote:
> No functional changes. But those functions will be needed
> to get max level for HDMI and DP, so let's move those
> up closer to other similar functions existent for previous
> platforms.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 122 +++++++++++++++++++--------------------
>  1 file changed, 61 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 7b547a7f6c2b..3ce02cbd4483 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -588,6 +588,67 @@ skl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
>  	}
>  }
>  
> +static const struct cnl_ddi_buf_trans *
> +cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
> +{
> +	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
> +
> +	if (voltage == VOLTAGE_INFO_0_85V) {
> +		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_0_85V);
> +		return cnl_ddi_translations_hdmi_0_85V;
> +	} else if (voltage == VOLTAGE_INFO_0_95V) {
> +		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_0_95V);
> +		return cnl_ddi_translations_hdmi_0_95V;
> +	} else if (voltage == VOLTAGE_INFO_1_05V) {
> +		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_1_05V);
> +		return cnl_ddi_translations_hdmi_1_05V;
> +	} else
> +		MISSING_CASE(voltage);
> +	return NULL;
> +}
> +
> +static const struct cnl_ddi_buf_trans *
> +cnl_get_buf_trans_dp(struct drm_i915_private *dev_priv, int *n_entries)
> +{
> +	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
> +
> +	if (voltage == VOLTAGE_INFO_0_85V) {
> +		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_0_85V);
> +		return cnl_ddi_translations_dp_0_85V;
> +	} else if (voltage == VOLTAGE_INFO_0_95V) {
> +		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_0_95V);
> +		return cnl_ddi_translations_dp_0_95V;
> +	} else if (voltage == VOLTAGE_INFO_1_05V) {
> +		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_1_05V);
> +		return cnl_ddi_translations_dp_1_05V;
> +	} else
> +		MISSING_CASE(voltage);
> +	return NULL;
> +}
> +
> +static const struct cnl_ddi_buf_trans *
> +cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
> +{
> +	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
> +
> +	if (dev_priv->vbt.edp.low_vswing) {
> +		if (voltage == VOLTAGE_INFO_0_85V) {
> +			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_85V);
> +			return cnl_ddi_translations_edp_0_85V;
> +		} else if (voltage == VOLTAGE_INFO_0_95V) {
> +			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_95V);
> +			return cnl_ddi_translations_edp_0_95V;
> +		} else if (voltage == VOLTAGE_INFO_1_05V) {
> +			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_1_05V);
> +			return cnl_ddi_translations_edp_1_05V;
> +		} else
> +			MISSING_CASE(voltage);
> +		return NULL;
> +	} else {
> +		return cnl_get_buf_trans_dp(dev_priv, n_entries);
> +	}
> +}
> +
>  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
>  {
>  	int n_hdmi_entries;
> @@ -1829,67 +1890,6 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  		DP_TRAIN_VOLTAGE_SWING_MASK;
>  }
>  
> -static const struct cnl_ddi_buf_trans *
> -cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
> -{
> -	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
> -
> -	if (voltage == VOLTAGE_INFO_0_85V) {
> -		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_0_85V);
> -		return cnl_ddi_translations_hdmi_0_85V;
> -	} else if (voltage == VOLTAGE_INFO_0_95V) {
> -		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_0_95V);
> -		return cnl_ddi_translations_hdmi_0_95V;
> -	} else if (voltage == VOLTAGE_INFO_1_05V) {
> -		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_1_05V);
> -		return cnl_ddi_translations_hdmi_1_05V;
> -	} else
> -		MISSING_CASE(voltage);
> -	return NULL;
> -}
> -
> -static const struct cnl_ddi_buf_trans *
> -cnl_get_buf_trans_dp(struct drm_i915_private *dev_priv, int *n_entries)
> -{
> -	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
> -
> -	if (voltage == VOLTAGE_INFO_0_85V) {
> -		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_0_85V);
> -		return cnl_ddi_translations_dp_0_85V;
> -	} else if (voltage == VOLTAGE_INFO_0_95V) {
> -		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_0_95V);
> -		return cnl_ddi_translations_dp_0_95V;
> -	} else if (voltage == VOLTAGE_INFO_1_05V) {
> -		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_1_05V);
> -		return cnl_ddi_translations_dp_1_05V;
> -	} else
> -		MISSING_CASE(voltage);
> -	return NULL;
> -}
> -
> -static const struct cnl_ddi_buf_trans *
> -cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
> -{
> -	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
> -
> -	if (dev_priv->vbt.edp.low_vswing) {
> -		if (voltage == VOLTAGE_INFO_0_85V) {
> -			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_85V);
> -			return cnl_ddi_translations_edp_0_85V;
> -		} else if (voltage == VOLTAGE_INFO_0_95V) {
> -			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_95V);
> -			return cnl_ddi_translations_edp_0_95V;
> -		} else if (voltage == VOLTAGE_INFO_1_05V) {
> -			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_1_05V);
> -			return cnl_ddi_translations_edp_1_05V;
> -		} else
> -			MISSING_CASE(voltage);
> -		return NULL;
> -	} else {
> -		return cnl_get_buf_trans_dp(dev_priv, n_entries);
> -	}
> -}
> -
>  static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
>  				    u32 level, enum port port, int type)
>  {
> -- 
> 2.13.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 7/8] drm/i915/cnl: Fix DDI hdmi level selection.

[Rodrigo Vivi]

Let's get a proper HDMI DDI entry level for vswing programming
sequences on CNL.

Spec doesn't specify any default for HDMI tables,
so let's pick the last entry as the default for now.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 3ce02cbd4483..f1757a8e481a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -660,7 +660,10 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
 	if (IS_GEN9_LP(dev_priv))
 		return hdmi_level;
 
-	if (IS_GEN9_BC(dev_priv)) {
+	if (IS_CANNONLAKE(dev_priv)) {
+		cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
+		hdmi_default_entry = n_hdmi_entries - 1;
+	} else if (IS_GEN9_BC(dev_priv)) {
 		skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
 		hdmi_default_entry = 8;
 	} else if (IS_BROADWELL(dev_priv)) {
-- 
2.13.2

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

Re: [PATCH 7/8] drm/i915/cnl: Fix DDI hdmi level selection.

[Ville Syrjälä]

On Tue, Aug 29, 2017 at 04:22:29PM -0700, Rodrigo Vivi wrote:
> Let's get a proper HDMI DDI entry level for vswing programming
> sequences on CNL.
> 
> Spec doesn't specify any default for HDMI tables,
> so let's pick the last entry as the default for now.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 3ce02cbd4483..f1757a8e481a 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -660,7 +660,10 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
>  	if (IS_GEN9_LP(dev_priv))
>  		return hdmi_level;
>  
> -	if (IS_GEN9_BC(dev_priv)) {
> +	if (IS_CANNONLAKE(dev_priv)) {
> +		cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> +		hdmi_default_entry = n_hdmi_entries - 1;

Hmm. I guess we might try to do the same thing for BXT, for extra
consistency. But that's a separate issue.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	} else if (IS_GEN9_BC(dev_priv)) {
>  		skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
>  		hdmi_default_entry = 8;
>  	} else if (IS_BROADWELL(dev_priv)) {
> -- 
> 2.13.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 8/8] drm/i915/cnl: Fix DP max voltage

[Rodrigo Vivi]

On clock recovery this function is called to find out
the max voltage swing level that we could go.

However gen 9 functions use the old buffer translation tables
to figure that out. That table is not valid for CNL
causing an invalid number of entries and an invalid selection
on the max voltage swing level.

v2: Let's use same approach that previous platforms.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f1757a8e481a..97ff082c28a7 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -649,6 +649,29 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
 	}
 }
 
+static int cnl_max_level(struct drm_i915_private *dev_priv,
+			 enum intel_output_type type)
+{
+	int n_entries = 0;
+
+	switch (type) {
+	case INTEL_OUTPUT_DP:
+		cnl_get_buf_trans_dp(dev_priv, &n_entries);
+		break;
+	case INTEL_OUTPUT_EDP:
+		cnl_get_buf_trans_edp(dev_priv, &n_entries);
+		break;
+	case INTEL_OUTPUT_HDMI:
+		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
+		break;
+	default:
+		MISSING_CASE(type);
+		return 0;
+	}
+
+	return n_entries - 1;
+}
+
 static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
 {
 	int n_hdmi_entries;
@@ -1879,10 +1902,14 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	int n_entries;
 
-	if (encoder->type == INTEL_OUTPUT_EDP)
-		intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
-	else
-		intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
+	if (IS_CANNONLAKE(dev_priv)) {
+		cnl_max_level(dev_priv, encoder->type);
+	} else {
+		if (encoder->type == INTEL_OUTPUT_EDP)
+			intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
+		else
+			intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
+	}
 
 	if (WARN_ON(n_entries < 1))
 		n_entries = 1;
-- 
2.13.2

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

Re: [PATCH 8/8] drm/i915/cnl: Fix DP max voltage

[Ville Syrjälä]

On Tue, Aug 29, 2017 at 04:22:30PM -0700, Rodrigo Vivi wrote:
> On clock recovery this function is called to find out
> the max voltage swing level that we could go.
> 
> However gen 9 functions use the old buffer translation tables
> to figure that out. That table is not valid for CNL
> causing an invalid number of entries and an invalid selection
> on the max voltage swing level.
> 
> v2: Let's use same approach that previous platforms.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 35 +++++++++++++++++++++++++++++++----
>  1 file changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f1757a8e481a..97ff082c28a7 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -649,6 +649,29 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
>  	}
>  }
>  
> +static int cnl_max_level(struct drm_i915_private *dev_priv,
> +			 enum intel_output_type type)
> +{
> +	int n_entries = 0;
> +
> +	switch (type) {
> +	case INTEL_OUTPUT_DP:
> +		cnl_get_buf_trans_dp(dev_priv, &n_entries);
> +		break;
> +	case INTEL_OUTPUT_EDP:
> +		cnl_get_buf_trans_edp(dev_priv, &n_entries);
> +		break;
> +	case INTEL_OUTPUT_HDMI:
> +		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
> +		break;
> +	default:
> +		MISSING_CASE(type);
> +		return 0;
> +	}
> +
> +	return n_entries - 1;
> +}
> +
>  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
>  {
>  	int n_hdmi_entries;
> @@ -1879,10 +1902,14 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	int n_entries;
>  
> -	if (encoder->type == INTEL_OUTPUT_EDP)
> -		intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> -	else
> -		intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> +	if (IS_CANNONLAKE(dev_priv)) {
> +		cnl_max_level(dev_priv, encoder->type);

You're not actually using the return value. Also the return value has -1
already applied, whereas here we just need the n_entries w/o -1.

> +	} else {
> +		if (encoder->type == INTEL_OUTPUT_EDP)
> +			intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> +		else
> +			intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> +	}
>  
>  	if (WARN_ON(n_entries < 1))
>  		n_entries = 1;
> -- 
> 2.13.2

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

Re: [PATCH 8/8] drm/i915/cnl: Fix DP max voltage

[Ville Syrjälä]

On Tue, Aug 29, 2017 at 04:22:30PM -0700, Rodrigo Vivi wrote:
> On clock recovery this function is called to find out
> the max voltage swing level that we could go.
> 
> However gen 9 functions use the old buffer translation tables
> to figure that out. That table is not valid for CNL
> causing an invalid number of entries and an invalid selection
> on the max voltage swing level.
> 
> v2: Let's use same approach that previous platforms.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 35 +++++++++++++++++++++++++++++++----
>  1 file changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f1757a8e481a..97ff082c28a7 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -649,6 +649,29 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
>  	}
>  }
>  
> +static int cnl_max_level(struct drm_i915_private *dev_priv,
> +			 enum intel_output_type type)
> +{
> +	int n_entries = 0;
> +
> +	switch (type) {
> +	case INTEL_OUTPUT_DP:

These encoder->type checks are a bit problematic due to the DDI encoder
type changing dynamically. But to fix that I thunk I'll just need to
resurrect my old patches to get rid of that type changing. But I'll
wait until you cand land these since I need to rebase my stuff anyway.

> +		cnl_get_buf_trans_dp(dev_priv, &n_entries);
> +		break;
> +	case INTEL_OUTPUT_EDP:
> +		cnl_get_buf_trans_edp(dev_priv, &n_entries);
> +		break;
> +	case INTEL_OUTPUT_HDMI:
> +		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
> +		break;
> +	default:
> +		MISSING_CASE(type);
> +		return 0;
> +	}
> +
> +	return n_entries - 1;
> +}
> +
>  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
>  {
>  	int n_hdmi_entries;
> @@ -1879,10 +1902,14 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	int n_entries;
>  
> -	if (encoder->type == INTEL_OUTPUT_EDP)
> -		intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> -	else
> -		intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> +	if (IS_CANNONLAKE(dev_priv)) {
> +		cnl_max_level(dev_priv, encoder->type);
> +	} else {
> +		if (encoder->type == INTEL_OUTPUT_EDP)
> +			intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> +		else
> +			intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> +	}
>  
>  	if (WARN_ON(n_entries < 1))
>  		n_entries = 1;
> -- 
> 2.13.2

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

✓ Fi.CI.BAT: success for series starting with [1/8] drm/i915: Introduce intel_ddi_dp_level.

[Patchwork]

== Series Details ==

Series: series starting with [1/8] drm/i915: Introduce intel_ddi_dp_level.
URL   : https://patchwork.freedesktop.org/series/29508/
State : success

== Summary ==

Series 29508v1 series starting with [1/8] drm/i915: Introduce intel_ddi_dp_level.
https://patchwork.freedesktop.org/api/1.0/series/29508/revisions/1/mbox/

Test gem_ringfill:
        Subgroup basic-default-hang:
                incomplete -> DMESG-WARN (fi-pnv-d510) fdo#101600
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> FAIL       (fi-snb-2600) fdo#100215
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> SKIP       (fi-skl-x1585l) fdo#101781
Test kms_frontbuffer_tracking:
        Subgroup basic:
                dmesg-warn -> PASS       (fi-bdw-5557u)

fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:457s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:442s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:363s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:561s
fi-bwr-2160      total:279  pass:184  dwarn:0   dfail:0   fail:0   skip:95  time:254s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:505s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:518s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:510s
fi-elk-e7500     total:279  pass:230  dwarn:0   dfail:0   fail:0   skip:49  time:440s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:607s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:442s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:429s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:420s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:489s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:477s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:480s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:597s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:586s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:527s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:467s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:491s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:441s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:477s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:551s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:405s
fi-skl-6700k failed to connect after reboot

428ed27345fbf9be530d01ca6dc862eb5895db81 drm-tip: 2017y-08m-29d-17h-43m-11s UTC integration manifest
b866ac4a3412 drm/i915/cnl: Fix DP max voltage
ca4a613265e9 drm/i915/cnl: Fix DDI hdmi level selection.
0c42dd5b2cf1 drm/i915/cnl: Move ddi buf trans related functions up.
b1cfb8242e86 drm/i915/cnl: Move voltage check into ddi buf trans functions.
1f0aea39bf70 drm/i915: Enable voltage swing before enabling DDI_BUF_CTL.
bd7fcd99ee83 drm/i915: Align vswing sequences with old ddi buffer registers.
d652d935d01a drm/i915: decouple gen9 and gen10 dp signal levels.
38d9face694c drm/i915: Introduce intel_ddi_dp_level.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5534/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/8] drm/i915: Introduce intel_ddi_dp_level.

[Patchwork]

== Series Details ==

Series: series starting with [1/8] drm/i915: Introduce intel_ddi_dp_level.
URL   : https://patchwork.freedesktop.org/series/29508/
State : success

== Summary ==

Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test perf:
        Subgroup polling:
                pass       -> FAIL       (shard-hsw) fdo#102252 +1
Test kms_cursor_legacy:
        Subgroup short-flip-before-cursor-atomic-transitions-varying-size:
                skip       -> PASS       (shard-hsw)

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-hsw        total:2230 pass:1231 dwarn:0   dfail:0   fail:17  skip:982 time:9671s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5534/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/8] drm/i915: Introduce intel_ddi_dp_level.

[Ville Syrjälä]

On Tue, Aug 29, 2017 at 04:22:23PM -0700, Rodrigo Vivi wrote:
> No functional changes. This only moves the DP level
> selection to a separated function that will be later
> used to organize better the vswing sequences.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 0a316a6ccb50..7e875e05d053 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2054,18 +2054,22 @@ static uint32_t translate_signal_level(int signal_levels)
>  	return 0;
>  }
>  
> +static uint32_t intel_ddi_dp_level(struct intel_dp *intel_dp)
> +{
> +	uint8_t train_set = intel_dp->train_set[0];
> +	int signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
> +					 DP_TRAIN_PRE_EMPHASIS_MASK);
> +
> +	return translate_signal_level(signal_levels);
> +}
> +
>  uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
>  	struct intel_encoder *encoder = &dport->base;
> -	uint8_t train_set = intel_dp->train_set[0];
> -	int signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
> -					 DP_TRAIN_PRE_EMPHASIS_MASK);
>  	enum port port = dport->port;
> -	uint32_t level;
> -
> -	level = translate_signal_level(signal_levels);
> +	uint32_t level = intel_ddi_dp_level(intel_dp);
>  
>  	if (IS_GEN9_BC(dev_priv))
>  		skl_ddi_set_iboost(encoder, level);
> -- 
> 2.13.2

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

[PATCH] drm/i915/cnl: Fix DP max voltage

[Rodrigo Vivi]

On clock recovery this function is called to find out
the max voltage swing level that we could go.

However gen 9 functions use the old buffer translation tables
to figure that out. That table is not valid for CNL
causing an invalid number of entries and an invalid selection
on the max voltage swing level.

Always picking Level 3 on CNL seems to be the safest way to
go instead of doing something similar to gen9 look-up.

So, unless we find a good reason let's simplify and follow
the most used way to get the max DP voltage.

Cc: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 80b5dcb..ab81be4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3116,7 +3116,7 @@ static bool intel_dp_get_alpm_status(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
 	enum port port = dp_to_dig_port(intel_dp)->port;
 
-	if (IS_GEN9_LP(dev_priv))
+	if (IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(dev_priv))
 		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
 	else if (INTEL_GEN(dev_priv) >= 9) {
 		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
-- 
1.9.1

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

Re: [PATCH] drm/i915/cnl: Fix DP max voltage

[Ville Syrjälä]

On Mon, Jul 10, 2017 at 11:18:14AM -0700, Rodrigo Vivi wrote:
> On clock recovery this function is called to find out
> the max voltage swing level that we could go.
> 
> However gen 9 functions use the old buffer translation tables
> to figure that out. That table is not valid for CNL
> causing an invalid number of entries and an invalid selection
> on the max voltage swing level.
> 
> Always picking Level 3 on CNL seems to be the safest way to
> go instead of doing something similar to gen9 look-up.
> 
> So, unless we find a good reason let's simplify and follow
> the most used way to get the max DP voltage.
>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 80b5dcb..ab81be4 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3116,7 +3116,7 @@ static bool intel_dp_get_alpm_status(struct intel_dp *intel_dp)
                                    ^^^^^^^^^^^^^^^^^^^^^^^^
I wonder what's the deal with these broken looking diffs recently?
Is there some buggy version of git around that people are using?

>  	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
>  	enum port port = dp_to_dig_port(intel_dp)->port;
>  
> -	if (IS_GEN9_LP(dev_priv))
> +	if (IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(dev_priv))
>  		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;

This looks wrong to me. If I'm reading the cnl buf trans tables right
then eDP doesn't go up to level 3.

The HDMI case also looks suscpect since intel_ddi_hdmi_level() doesn't
have anything for CNL.

IMO the best way would be to change all platforms to use a
similar approach as what I did for SKL/KBL. In fact I'd like to see all
the link training vswing/pre-emph stuff converted into a more data
driven approach. Otherwise all the information is spread rather wide
making rather hard to cross check the max vs. the actual values.
This is especially true for DDI platforms since the buf trans tables
aren't even in the same file. Any takers?

>
>  	else if (INTEL_GEN(dev_priv) >= 9) {
>  		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH] drm/i915/cnl: Fix DP max voltage

[Vivi, Rodrigo]

On Mon, 2017-07-10 at 21:40 +0300, Ville Syrjälä wrote:
> On Mon, Jul 10, 2017 at 11:18:14AM -0700, Rodrigo Vivi wrote:
> > On clock recovery this function is called to find out
> > the max voltage swing level that we could go.
> > 
> > However gen 9 functions use the old buffer translation tables
> > to figure that out. That table is not valid for CNL
> > causing an invalid number of entries and an invalid selection
> > on the max voltage swing level.
> > 
> > Always picking Level 3 on CNL seems to be the safest way to
> > go instead of doing something similar to gen9 look-up.
> > 
> > So, unless we find a good reason let's simplify and follow
> > the most used way to get the max DP voltage.
> >
> > Cc: Clint Taylor <clinton.a.taylor@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 80b5dcb..ab81be4 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3116,7 +3116,7 @@ static bool intel_dp_get_alpm_status(struct intel_dp *intel_dp)
>                                     ^^^^^^^^^^^^^^^^^^^^^^^^
> I wonder what's the deal with these broken looking diffs recently?
> Is there some buggy version of git around that people are using?

that's a good question... it is a fact that I need to update my git
anyways...

> 
> >  	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> >  	enum port port = dp_to_dig_port(intel_dp)->port;
> >  
> > -	if (IS_GEN9_LP(dev_priv))
> > +	if (IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(dev_priv))
> >  		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> 
> This looks wrong to me. If I'm reading the cnl buf trans tables right
> then eDP doesn't go up to level 3.

ouch!

> 
> The HDMI case also looks suscpect since intel_ddi_hdmi_level() doesn't
> have anything for CNL.

I will check...

> 
> IMO the best way would be to change all platforms to use a
> similar approach as what I did for SKL/KBL.

hm... I will take a look on that path for now...

>  In fact I'd like to see all
> the link training vswing/pre-emph stuff converted into a more data
> driven approach. Otherwise all the information is spread rather wide
> making rather hard to cross check the max vs. the actual values.
> This is especially true for DDI platforms since the buf trans tables
> aren't even in the same file. Any takers?

that's a good idea.
it would get cleaner.

> 
> >
> >  	else if (INTEL_GEN(dev_priv) >= 9) {
> >  		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

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