Re: [PATCH v3] drm/i915: Disable fast link training if DP config changes

[Mika Kahola <mika.kahola@intel.com>]

On Wed, 2015-12-16 at 14:41 +0200, Ville Syrjälä wrote:
> On Wed, Dec 16, 2015 at 02:26:58PM +0200, Mika Kahola wrote:
> > Disable DP fast link training if DP link configuration
> > changes. If one of the DP link parameters i.e. link
> > bandwidth, lane count, rate selection, port clock or bpp
> > changes the link training does no longer apply the
> > previously computed voltage swing and pre-emphasis values.
> > Instead, the link training is started with zero values.
> > 
> > v3: Remove cached old link parameters. Instead, disable
> >     fast link training feature when link parameters are
> >     set (Ville)
> > 
> > v2: Readout DPCD register to check if no aux handshaking is
> >     required in link training (Ander)
> > 
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c               |  3 ++-
> >  drivers/gpu/drm/i915/intel_dp_link_training.c | 16 ++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h              |  1 +
> >  3 files changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 31ba241..92adf21 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1679,6 +1679,7 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
> >  {
> >  	intel_dp->link_rate = pipe_config->port_clock;
> >  	intel_dp->lane_count = pipe_config->lane_count;
> > +	intel_dp->train_set_valid = false;
> 
> This would disable the optimization every time we fire up the link.
> You would have to check the new vs. current params first, and clear
> the flag only if there's a difference.
Yeah, you're right. A step too far..

> >  }
> >  
> >  static void intel_dp_prepare(struct intel_encoder *encoder)
> > @@ -3849,7 +3850,7 @@ intel_dp_link_down(struct intel_dp *intel_dp)
> >  	intel_dp->DP = DP;
> >  }
> >  
> > -static bool
> > +bool
> >  intel_dp_get_dpcd(struct intel_dp *intel_dp)
> >  {
> >  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > index 8888793..0e95a98 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > @@ -85,6 +85,22 @@ static bool
> >  intel_dp_reset_link_train(struct intel_dp *intel_dp,
> >  			uint8_t dp_train_pat)
> >  {
> > +	bool has_dpcd;
> > +	bool flt_supported = false;
> > +
> > +	has_dpcd = intel_dp_get_dpcd(intel_dp);
> > +
> > +	if (has_dpcd) {
> > +		if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11)
> > +			flt_supported = (intel_dp->dpcd[DP_MAX_DOWNSPREAD] &
> > +					 DP_NO_AUX_HANDSHAKE_LINK_TRAINING);
> > +	}
> > +
> > +	intel_dp->train_set_valid &= flt_supported;
> 
> AFAIK what we do isn't really fast link training. Fast link training is
> where you skip the AUX communication entirely, ie. just send the training
> patterns for a shot predefined time, and afterwards checks if the link
> is in good condition or not.
> 
I know this is not exactly what the spec defines as a fast link
training. I should do a better job on naming this.

> > +
> > +	DRM_DEBUG_KMS("fast link training enabled: %s\n",
> > +		      intel_dp->train_set_valid ? "true" : "false");
> > +
> >  	if (!intel_dp->train_set_valid)
> >  		memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set));
> >  	intel_dp_set_signal_levels(intel_dp);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 798463e..94041fd 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1239,6 +1239,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
> >  bool intel_dp_compute_config(struct intel_encoder *encoder,
> >  			     struct intel_crtc_state *pipe_config);
> >  bool intel_dp_is_edp(struct drm_device *dev, enum port port);
> > +bool intel_dp_get_dpcd(struct intel_dp *intel_dp);
> >  enum irqreturn intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port,
> >  				  bool long_hpd);
> >  void intel_edp_backlight_on(struct intel_dp *intel_dp);
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


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