public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/cnl: Fix DP max voltage
@ 2017-07-10 18:18 Rodrigo Vivi
  2017-07-10 18:40 ` Ville Syrjälä
  2017-07-10 18:56 ` ✓ Fi.CI.BAT: success for " Patchwork
  0 siblings, 2 replies; 9+ messages in thread
From: Rodrigo Vivi @ 2017-07-10 18:18 UTC (permalink / raw)
  To: intel-gfx; +Cc: 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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915/cnl: Fix DP max voltage
  2017-07-10 18:18 [PATCH] drm/i915/cnl: Fix DP max voltage Rodrigo Vivi
@ 2017-07-10 18:40 ` Ville Syrjälä
  2017-07-10 19:25   ` Vivi, Rodrigo
  2017-07-10 18:56 ` ✓ Fi.CI.BAT: success for " Patchwork
  1 sibling, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2017-07-10 18:40 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/cnl: Fix DP max voltage
  2017-07-10 18:18 [PATCH] drm/i915/cnl: Fix DP max voltage Rodrigo Vivi
  2017-07-10 18:40 ` Ville Syrjälä
@ 2017-07-10 18:56 ` Patchwork
  1 sibling, 0 replies; 9+ messages in thread
From: Patchwork @ 2017-07-10 18:56 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/cnl: Fix DP max voltage
URL   : https://patchwork.freedesktop.org/series/27076/
State : success

== Summary ==

Series 27076v1 drm/i915/cnl: Fix DP max voltage
https://patchwork.freedesktop.org/api/1.0/series/27076/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100125
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:442s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:436s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:355s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:529s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:517s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:486s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:476s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:599s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:437s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:413s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:428s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:491s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:475s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:458s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:571s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:581s
fi-pnv-d510      total:279  pass:221  dwarn:3   dfail:0   fail:0   skip:55  time:565s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:461s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:587s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:463s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:482s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:437s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:542s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:405s

623a0a89056dd13ef29a80f415adbb28837685f8 drm-tip: 2017y-07m-10d-18h-19m-45s UTC integration manifest
ddea8a1 drm/i915/cnl: Fix DP max voltage

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5157/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915/cnl: Fix DP max voltage
  2017-07-10 18:40 ` Ville Syrjälä
@ 2017-07-10 19:25   ` Vivi, Rodrigo
  0 siblings, 0 replies; 9+ messages in thread
From: Vivi, Rodrigo @ 2017-07-10 19:25 UTC (permalink / raw)
  To: ville.syrjala@linux.intel.com; +Cc: intel-gfx@lists.freedesktop.org

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] drm/i915/cnl: Fix DP max voltage
       [not found] <Message-id: <20170830142037.GH4914@intel.com>
@ 2017-08-31  0:00 ` Rodrigo Vivi
  2017-08-31 12:34   ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Rodrigo Vivi @ 2017-08-31  0:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Vivi, Rodrigo

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915/cnl: Fix DP max voltage
  2017-08-31  0:00 ` [PATCH] " Rodrigo Vivi
@ 2017-08-31 12:34   ` Ville Syrjälä
  0 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2017-08-31 12:34 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] drm/i915/cnl: Fix DP max voltage
       [not found] <Message-id: <20170831123436.GO4914@intel.com>
@ 2017-08-31 14:53 ` Rodrigo Vivi
  2017-08-31 15:06   ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Rodrigo Vivi @ 2017-08-31 14:53 UTC (permalink / raw)
  To: intel-gfx; +Cc: Vivi, Rodrigo

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915/cnl: Fix DP max voltage
  2017-08-31 14:53 ` Rodrigo Vivi
@ 2017-08-31 15:06   ` Ville Syrjälä
  2017-08-31 16:49     ` Vivi, Rodrigo
  0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2017-08-31 15:06 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915/cnl: Fix DP max voltage
  2017-08-31 15:06   ` Ville Syrjälä
@ 2017-08-31 16:49     ` Vivi, Rodrigo
  0 siblings, 0 replies; 9+ messages in thread
From: Vivi, Rodrigo @ 2017-08-31 16:49 UTC (permalink / raw)
  To: ville.syrjala@linux.intel.com; +Cc: intel-gfx@lists.freedesktop.org

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-08-31 16:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-10 18:18 [PATCH] drm/i915/cnl: Fix DP max voltage Rodrigo Vivi
2017-07-10 18:40 ` Ville Syrjälä
2017-07-10 19:25   ` Vivi, Rodrigo
2017-07-10 18:56 ` ✓ Fi.CI.BAT: success for " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2017-08-30 14:20 [PATCH 8/8] " Ville Syrjälä
     [not found] <Message-id: <20170830142037.GH4914@intel.com>
2017-08-31  0:00 ` [PATCH] " Rodrigo Vivi
2017-08-31 12:34   ` Ville Syrjälä
     [not found] <Message-id: <20170831123436.GO4914@intel.com>
2017-08-31 14:53 ` Rodrigo Vivi
2017-08-31 15:06   ` Ville Syrjälä
2017-08-31 16:49     ` Vivi, Rodrigo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox