[Intel-gfx] [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 at intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at 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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>