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

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

On Wed, Aug 16, 2017 at 01:19:51PM -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>
> ---
>  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 a6056bb4f801..8ea0368e15b1 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2133,6 +2133,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));
>  
> @@ -2145,7 +2146,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);

Hmm. Yeah, I guess it would make sense to set these up already before we
enable DDI_BUF_CTL, which I think would happend from the
.prepare_link_retrain() hook on DDI, and that does get called before the
signal levels have been set up.

So I'm fine with this change. Imre, any thoughts?

> +	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