| Message ID | 20200924184805.294493-6-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Add support for LTTPR non-transparent link training mode | expand |
On Thu, Sep 24, 2020 at 09:48:04PM +0300, Imre Deak wrote: > By default LTTPRs should be in transparent link training mode, > nevertheless in this patch we switch to this default mode explicitly. > > The DP Standard recommends this, supposedly because an LTTPR may be left > in the non-transparent mode (by BIOS, previous kernel, or after reset > due to a firmware bug). I haven't seen this happening, but let's follow > the DP Standard. > > v2: > - Add a code comment about the explicit disabling of non-transparent > mode. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > .../drm/i915/display/intel_display_types.h | 1 + > drivers/gpu/drm/i915/display/intel_dp.c | 3 ++ > .../drm/i915/display/intel_dp_link_training.c | 52 +++++++++++++++++++ > .../drm/i915/display/intel_dp_link_training.h | 2 + > 4 files changed, 58 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index 3d4bf9b6a0a2..b04921eba73b 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1280,6 +1280,7 @@ struct intel_dp { > u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; > u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE]; > u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]; > + u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE]; > u8 fec_capable; > /* source rates */ > int num_source_rates; > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index b21f42193a11..64bf4aa384d3 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -4726,6 +4726,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > { > int ret; > > + if (!intel_dp_is_edp(intel_dp)) > + intel_dp_lttpr_init(intel_dp); I was initially a bit confused why this is before we read dpcd[], but looks like the spec says that is the expected order. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > + > if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd)) > return false; > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > index 38d4553670a1..4f8f42cc25fa 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > @@ -34,6 +34,52 @@ intel_dp_dump_link_status(const u8 link_status[DP_LINK_STATUS_SIZE]) > link_status[3], link_status[4], link_status[5]); > } > > +static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp) > +{ > + if (drm_dp_read_lttpr_common_caps(&intel_dp->aux, > + intel_dp->lttpr_common_caps) < 0) { > + memset(intel_dp->lttpr_common_caps, 0, > + sizeof(intel_dp->lttpr_common_caps)); > + return false; > + } > + > + drm_dbg_kms(&dp_to_i915(intel_dp)->drm, > + "LTTPR common capabilities: %*ph\n", > + (int)sizeof(intel_dp->lttpr_common_caps), > + intel_dp->lttpr_common_caps); > + > + return true; > +} > + > +static bool > +intel_dp_set_lttpr_transparent_mode(struct intel_dp *intel_dp, bool enable) > +{ > + u8 val = enable ? DP_PHY_REPEATER_MODE_TRANSPARENT : > + DP_PHY_REPEATER_MODE_NON_TRANSPARENT; > + > + return drm_dp_dpcd_write(&intel_dp->aux, DP_PHY_REPEATER_MODE, &val, 1) == 1; > +} > + > +/** > + * intel_dp_lttpr_init - detect LTTPRs and init the LTTPR link training mode > + * @intel_dp: Intel DP struct > + * > + * Read the LTTPR common capabilities and switch to transparent link training > + * mode. > + */ > +int intel_dp_lttpr_init(struct intel_dp *intel_dp) > +{ > + intel_dp_read_lttpr_common_caps(intel_dp); > + > + /* > + * See DP Standard v2.0 3.6.6.1. about the explicit disabling of > + * non-transparent mode. > + */ > + intel_dp_set_lttpr_transparent_mode(intel_dp, true); > + > + return 0; > +} > + > static u8 dp_voltage_max(u8 preemph) > { > switch (preemph & DP_TRAIN_PRE_EMPHASIS_MASK) { > @@ -471,6 +517,12 @@ static void intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp) > */ > void intel_dp_start_link_train(struct intel_dp *intel_dp) > { > + /* > + * TODO: Reiniting LTTPRs here won't be needed once proper connector > + * HW state readout is added. > + */ > + intel_dp_lttpr_init(intel_dp); > + > if (!intel_dp_link_train(intel_dp)) > intel_dp_schedule_fallback_link_training(intel_dp); > } > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.h b/drivers/gpu/drm/i915/display/intel_dp_link_training.h > index 518d834dbc98..3536ce73a123 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.h > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.h > @@ -10,6 +10,8 @@ > > struct intel_dp; > > +int intel_dp_lttpr_init(struct intel_dp *intel_dp); > + > void intel_dp_get_adjust_train(struct intel_dp *intel_dp, > const u8 link_status[DP_LINK_STATUS_SIZE]); > void intel_dp_start_link_train(struct intel_dp *intel_dp); > -- > 2.25.1
On Fri, Sep 25, 2020 at 07:03:50PM +0300, Ville Syrjälä wrote: > On Thu, Sep 24, 2020 at 09:48:04PM +0300, Imre Deak wrote: > > By default LTTPRs should be in transparent link training mode, > > nevertheless in this patch we switch to this default mode explicitly. > > > > The DP Standard recommends this, supposedly because an LTTPR may be left > > in the non-transparent mode (by BIOS, previous kernel, or after reset > > due to a firmware bug). I haven't seen this happening, but let's follow > > the DP Standard. > > > > v2: > > - Add a code comment about the explicit disabling of non-transparent > > mode. > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > .../drm/i915/display/intel_display_types.h | 1 + > > drivers/gpu/drm/i915/display/intel_dp.c | 3 ++ > > .../drm/i915/display/intel_dp_link_training.c | 52 +++++++++++++++++++ > > .../drm/i915/display/intel_dp_link_training.h | 2 + > > 4 files changed, 58 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > > index 3d4bf9b6a0a2..b04921eba73b 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -1280,6 +1280,7 @@ struct intel_dp { > > u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; > > u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE]; > > u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]; > > + u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE]; > > u8 fec_capable; > > /* source rates */ > > int num_source_rates; > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > index b21f42193a11..64bf4aa384d3 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -4726,6 +4726,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > > { > > int ret; > > > > + if (!intel_dp_is_edp(intel_dp)) > > + intel_dp_lttpr_init(intel_dp); > > I was initially a bit confused why this is before we read dpcd[], but > looks like the spec says that is the expected order. Yes. It's strange but reading the spec makes me think that even reading regs could have side-effects, here the LTTPR caps reading is the first according to the spec. Btw, the other less clearly specified things are the order of (explicitly) switching to transparent mode vs. reading the LTTPR common caps, and in what case to perform the switch. I decided the correct order is to read the caps first, and switch unconditionally. The latter has the downside of an AUX timeout/NAK on platforms w/o LTTPRs, but this seemed to be the more robust way. > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Thanks. I mucked up the eDP check, missing it from intel_dp_start_link_train(), will move it to intel_dp_lttpr_init() instead. > > > + > > if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd)) > > return false; > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > index 38d4553670a1..4f8f42cc25fa 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > @@ -34,6 +34,52 @@ intel_dp_dump_link_status(const u8 link_status[DP_LINK_STATUS_SIZE]) > > link_status[3], link_status[4], link_status[5]); > > } > > > > +static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp) > > +{ > > + if (drm_dp_read_lttpr_common_caps(&intel_dp->aux, > > + intel_dp->lttpr_common_caps) < 0) { > > + memset(intel_dp->lttpr_common_caps, 0, > > + sizeof(intel_dp->lttpr_common_caps)); > > + return false; > > + } > > + > > + drm_dbg_kms(&dp_to_i915(intel_dp)->drm, > > + "LTTPR common capabilities: %*ph\n", > > + (int)sizeof(intel_dp->lttpr_common_caps), > > + intel_dp->lttpr_common_caps); > > + > > + return true; > > +} > > + > > +static bool > > +intel_dp_set_lttpr_transparent_mode(struct intel_dp *intel_dp, bool enable) > > +{ > > + u8 val = enable ? DP_PHY_REPEATER_MODE_TRANSPARENT : > > + DP_PHY_REPEATER_MODE_NON_TRANSPARENT; > > + > > + return drm_dp_dpcd_write(&intel_dp->aux, DP_PHY_REPEATER_MODE, &val, 1) == 1; > > +} > > + > > +/** > > + * intel_dp_lttpr_init - detect LTTPRs and init the LTTPR link training mode > > + * @intel_dp: Intel DP struct > > + * > > + * Read the LTTPR common capabilities and switch to transparent link training > > + * mode. > > + */ > > +int intel_dp_lttpr_init(struct intel_dp *intel_dp) > > +{ > > + intel_dp_read_lttpr_common_caps(intel_dp); > > + > > + /* > > + * See DP Standard v2.0 3.6.6.1. about the explicit disabling of > > + * non-transparent mode. > > + */ > > + intel_dp_set_lttpr_transparent_mode(intel_dp, true); > > + > > + return 0; > > +} > > + > > static u8 dp_voltage_max(u8 preemph) > > { > > switch (preemph & DP_TRAIN_PRE_EMPHASIS_MASK) { > > @@ -471,6 +517,12 @@ static void intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp) > > */ > > void intel_dp_start_link_train(struct intel_dp *intel_dp) > > { > > + /* > > + * TODO: Reiniting LTTPRs here won't be needed once proper connector > > + * HW state readout is added. > > + */ > > + intel_dp_lttpr_init(intel_dp); > > + > > if (!intel_dp_link_train(intel_dp)) > > intel_dp_schedule_fallback_link_training(intel_dp); > > } > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.h b/drivers/gpu/drm/i915/display/intel_dp_link_training.h > > index 518d834dbc98..3536ce73a123 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.h > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.h > > @@ -10,6 +10,8 @@ > > > > struct intel_dp; > > > > +int intel_dp_lttpr_init(struct intel_dp *intel_dp); > > + > > void intel_dp_get_adjust_train(struct intel_dp *intel_dp, > > const u8 link_status[DP_LINK_STATUS_SIZE]); > > void intel_dp_start_link_train(struct intel_dp *intel_dp); > > -- > > 2.25.1 > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 3d4bf9b6a0a2..b04921eba73b 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1280,6 +1280,7 @@ struct intel_dp { u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE]; u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]; + u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE]; u8 fec_capable; /* source rates */ int num_source_rates; diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index b21f42193a11..64bf4aa384d3 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -4726,6 +4726,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) { int ret; + if (!intel_dp_is_edp(intel_dp)) + intel_dp_lttpr_init(intel_dp); + if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd)) return false; diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c index 38d4553670a1..4f8f42cc25fa 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c @@ -34,6 +34,52 @@ intel_dp_dump_link_status(const u8 link_status[DP_LINK_STATUS_SIZE]) link_status[3], link_status[4], link_status[5]); } +static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp) +{ + if (drm_dp_read_lttpr_common_caps(&intel_dp->aux, + intel_dp->lttpr_common_caps) < 0) { + memset(intel_dp->lttpr_common_caps, 0, + sizeof(intel_dp->lttpr_common_caps)); + return false; + } + + drm_dbg_kms(&dp_to_i915(intel_dp)->drm, + "LTTPR common capabilities: %*ph\n", + (int)sizeof(intel_dp->lttpr_common_caps), + intel_dp->lttpr_common_caps); + + return true; +} + +static bool +intel_dp_set_lttpr_transparent_mode(struct intel_dp *intel_dp, bool enable) +{ + u8 val = enable ? DP_PHY_REPEATER_MODE_TRANSPARENT : + DP_PHY_REPEATER_MODE_NON_TRANSPARENT; + + return drm_dp_dpcd_write(&intel_dp->aux, DP_PHY_REPEATER_MODE, &val, 1) == 1; +} + +/** + * intel_dp_lttpr_init - detect LTTPRs and init the LTTPR link training mode + * @intel_dp: Intel DP struct + * + * Read the LTTPR common capabilities and switch to transparent link training + * mode. + */ +int intel_dp_lttpr_init(struct intel_dp *intel_dp) +{ + intel_dp_read_lttpr_common_caps(intel_dp); + + /* + * See DP Standard v2.0 3.6.6.1. about the explicit disabling of + * non-transparent mode. + */ + intel_dp_set_lttpr_transparent_mode(intel_dp, true); + + return 0; +} + static u8 dp_voltage_max(u8 preemph) { switch (preemph & DP_TRAIN_PRE_EMPHASIS_MASK) { @@ -471,6 +517,12 @@ static void intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp) */ void intel_dp_start_link_train(struct intel_dp *intel_dp) { + /* + * TODO: Reiniting LTTPRs here won't be needed once proper connector + * HW state readout is added. + */ + intel_dp_lttpr_init(intel_dp); + if (!intel_dp_link_train(intel_dp)) intel_dp_schedule_fallback_link_training(intel_dp); } diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.h b/drivers/gpu/drm/i915/display/intel_dp_link_training.h index 518d834dbc98..3536ce73a123 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.h +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.h @@ -10,6 +10,8 @@ struct intel_dp; +int intel_dp_lttpr_init(struct intel_dp *intel_dp); + void intel_dp_get_adjust_train(struct intel_dp *intel_dp, const u8 link_status[DP_LINK_STATUS_SIZE]); void intel_dp_start_link_train(struct intel_dp *intel_dp);
By default LTTPRs should be in transparent link training mode, nevertheless in this patch we switch to this default mode explicitly. The DP Standard recommends this, supposedly because an LTTPR may be left in the non-transparent mode (by BIOS, previous kernel, or after reset due to a firmware bug). I haven't seen this happening, but let's follow the DP Standard. v2: - Add a code comment about the explicit disabling of non-transparent mode. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> --- .../drm/i915/display/intel_display_types.h | 1 + drivers/gpu/drm/i915/display/intel_dp.c | 3 ++ .../drm/i915/display/intel_dp_link_training.c | 52 +++++++++++++++++++ .../drm/i915/display/intel_dp_link_training.h | 2 + 4 files changed, 58 insertions(+)