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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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