* [Intel-gfx] [PATCH] drm/i915/dg2: update link training for 128b/132b
@ 2021-10-07 10:31 Jani Nikula
2021-10-07 10:46 ` Ville Syrjälä
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jani Nikula @ 2021-10-07 10:31 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Ville Syrjälä
The 128b/132b channel coding link training uses more straightforward TX
FFE preset values. Reuse voltage tries and max vswing for retry logic.
The delays for 128b/132b are still all wrong, but this is regardless a
step forward.
v2: Fix UHBR rate checks, use intel_dp_is_uhbr() helper
v3:
- Rebase
- Modify intel_dp_adjust_request_changed() and
intel_dp_link_max_vswing_reached() to take 128b/132b into
account. (Ville)
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 18 ++-
.../drm/i915/display/intel_dp_link_training.c | 124 +++++++++++++-----
2 files changed, 106 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 3f7bbeb3e3cd..59428ce4f8c1 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1338,13 +1338,20 @@ static int translate_signal_level(struct intel_dp *intel_dp,
return 0;
}
-static int intel_ddi_dp_level(struct intel_dp *intel_dp, int lane)
+static int intel_ddi_dp_level(struct intel_dp *intel_dp,
+ const struct intel_crtc_state *crtc_state,
+ int lane)
{
u8 train_set = intel_dp->train_set[lane];
- u8 signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
- DP_TRAIN_PRE_EMPHASIS_MASK);
- return translate_signal_level(intel_dp, signal_levels);
+ if (intel_dp_is_uhbr(crtc_state)) {
+ return train_set & DP_TX_FFE_PRESET_VALUE_MASK;
+ } else {
+ u8 signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
+ DP_TRAIN_PRE_EMPHASIS_MASK);
+
+ return translate_signal_level(intel_dp, signal_levels);
+ }
}
int intel_ddi_level(struct intel_encoder *encoder,
@@ -1362,7 +1369,8 @@ int intel_ddi_level(struct intel_encoder *encoder,
if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
level = intel_ddi_hdmi_level(encoder, trans);
else
- level = intel_ddi_dp_level(enc_to_intel_dp(encoder), lane);
+ level = intel_ddi_dp_level(enc_to_intel_dp(encoder), crtc_state,
+ lane);
if (drm_WARN_ON_ONCE(&i915->drm, level >= n_entries))
level = n_entries - 1;
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 1a943ae38a6b..c54b7df56c9f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -304,11 +304,31 @@ static bool has_per_lane_signal_levels(struct intel_dp *intel_dp,
return !intel_dp_phy_is_downstream_of_source(intel_dp, dp_phy);
}
-static u8 intel_dp_get_lane_adjust_train(struct intel_dp *intel_dp,
- const struct intel_crtc_state *crtc_state,
- enum drm_dp_phy dp_phy,
- const u8 link_status[DP_LINK_STATUS_SIZE],
- int lane)
+
+static u8 intel_dp_get_lane_adjust_train_128b132b(struct intel_dp *intel_dp,
+ const struct intel_crtc_state *crtc_state,
+ enum drm_dp_phy dp_phy,
+ const u8 link_status[DP_LINK_STATUS_SIZE],
+ int lane)
+{
+ u8 tx_ffe = 0;
+
+ if (has_per_lane_signal_levels(intel_dp, dp_phy)) {
+ lane = min(lane, crtc_state->lane_count - 1);
+ tx_ffe = drm_dp_get_adjust_tx_ffe_preset(link_status, lane);
+ } else {
+ for (lane = 0; lane < crtc_state->lane_count; lane++)
+ tx_ffe = max(tx_ffe, drm_dp_get_adjust_tx_ffe_preset(link_status, lane));
+ }
+
+ return tx_ffe;
+}
+
+static u8 intel_dp_get_lane_adjust_train_8b10b(struct intel_dp *intel_dp,
+ const struct intel_crtc_state *crtc_state,
+ enum drm_dp_phy dp_phy,
+ const u8 link_status[DP_LINK_STATUS_SIZE],
+ int lane)
{
u8 v = 0;
u8 p = 0;
@@ -340,6 +360,20 @@ static u8 intel_dp_get_lane_adjust_train(struct intel_dp *intel_dp,
return v | p;
}
+static u8 intel_dp_get_lane_adjust_train(struct intel_dp *intel_dp,
+ const struct intel_crtc_state *crtc_state,
+ enum drm_dp_phy dp_phy,
+ const u8 link_status[DP_LINK_STATUS_SIZE],
+ int lane)
+{
+ if (intel_dp_is_uhbr(crtc_state))
+ return intel_dp_get_lane_adjust_train_128b132b(intel_dp, crtc_state,
+ dp_phy, link_status, lane);
+ else
+ return intel_dp_get_lane_adjust_train_8b10b(intel_dp, crtc_state,
+ dp_phy, link_status, lane);
+}
+
#define TRAIN_REQ_FMT "%d/%d/%d/%d"
#define _TRAIN_REQ_VSWING_ARGS(link_status, lane) \
(drm_dp_get_adjust_request_voltage((link_status), (lane)) >> DP_TRAIN_VOLTAGE_SWING_SHIFT)
@@ -464,6 +498,13 @@ intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
_TRAIN_SET_PREEMPH_ARGS((train_set)[1]), \
_TRAIN_SET_PREEMPH_ARGS((train_set)[2]), \
_TRAIN_SET_PREEMPH_ARGS((train_set)[3])
+#define _TRAIN_SET_TX_FFE_ARGS(train_set) \
+ ((train_set) & DP_TX_FFE_PRESET_VALUE_MASK), ""
+#define TRAIN_SET_TX_FFE_ARGS(train_set) \
+ _TRAIN_SET_TX_FFE_ARGS((train_set)[0]), \
+ _TRAIN_SET_TX_FFE_ARGS((train_set)[1]), \
+ _TRAIN_SET_TX_FFE_ARGS((train_set)[2]), \
+ _TRAIN_SET_TX_FFE_ARGS((train_set)[3])
void intel_dp_set_signal_levels(struct intel_dp *intel_dp,
const struct intel_crtc_state *crtc_state,
@@ -473,14 +514,23 @@ void intel_dp_set_signal_levels(struct intel_dp *intel_dp,
struct drm_i915_private *i915 = to_i915(encoder->base.dev);
char phy_name[10];
- drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] lanes: %d, "
- "vswing levels: " TRAIN_SET_FMT ", "
- "pre-emphasis levels: " TRAIN_SET_FMT "\n",
- encoder->base.base.id, encoder->base.name,
- intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)),
- crtc_state->lane_count,
- TRAIN_SET_VSWING_ARGS(intel_dp->train_set),
- TRAIN_SET_PREEMPH_ARGS(intel_dp->train_set));
+ if (intel_dp_is_uhbr(crtc_state)) {
+ drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] lanes: %d, "
+ "128b/132b TX FFE presets: " TRAIN_SET_FMT "\n",
+ encoder->base.base.id, encoder->base.name,
+ intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)),
+ crtc_state->lane_count,
+ TRAIN_SET_TX_FFE_ARGS(intel_dp->train_set));
+ } else {
+ drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] lanes: %d, "
+ "vswing levels: " TRAIN_SET_FMT ", "
+ "pre-emphasis levels: " TRAIN_SET_FMT "\n",
+ encoder->base.base.id, encoder->base.name,
+ intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)),
+ crtc_state->lane_count,
+ TRAIN_SET_VSWING_ARGS(intel_dp->train_set),
+ TRAIN_SET_PREEMPH_ARGS(intel_dp->train_set));
+ }
if (intel_dp_phy_is_downstream_of_source(intel_dp, dp_phy))
encoder->set_signal_levels(encoder, crtc_state);
@@ -529,16 +579,22 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp,
* 2+1 or 3+0 depending on whether vswing level 3 is supported or not.
*/
for (lane = 0; lane < crtc_state->lane_count; lane++) {
- u8 v = (intel_dp->train_set[lane] & DP_TRAIN_VOLTAGE_SWING_MASK) >>
- DP_TRAIN_VOLTAGE_SWING_SHIFT;
- u8 p = (intel_dp->train_set[lane] & DP_TRAIN_PRE_EMPHASIS_MASK) >>
- DP_TRAIN_PRE_EMPHASIS_SHIFT;
-
- if ((intel_dp->train_set[lane] & DP_TRAIN_MAX_SWING_REACHED) == 0)
- return false;
-
- if (v + p != 3)
- return false;
+ if (intel_dp_is_uhbr(crtc_state)) {
+ if ((intel_dp->train_set[lane] & DP_TX_FFE_PRESET_VALUE_MASK) !=
+ DP_TX_FFE_PRESET_VALUE_MASK)
+ return false;
+ } else {
+ u8 v = (intel_dp->train_set[lane] & DP_TRAIN_VOLTAGE_SWING_MASK) >>
+ DP_TRAIN_VOLTAGE_SWING_SHIFT;
+ u8 p = (intel_dp->train_set[lane] & DP_TRAIN_PRE_EMPHASIS_MASK) >>
+ DP_TRAIN_PRE_EMPHASIS_SHIFT;
+
+ if ((intel_dp->train_set[lane] & DP_TRAIN_MAX_SWING_REACHED) == 0)
+ return false;
+
+ if (v + p != 3)
+ return false;
+ }
}
return true;
@@ -601,17 +657,24 @@ static void intel_dp_link_training_clock_recovery_delay(struct intel_dp *intel_d
drm_dp_lttpr_link_train_clock_recovery_delay();
}
-static bool intel_dp_adjust_request_changed(int lane_count,
+static bool intel_dp_adjust_request_changed(const struct intel_crtc_state *crtc_state,
const u8 old_link_status[DP_LINK_STATUS_SIZE],
const u8 new_link_status[DP_LINK_STATUS_SIZE])
{
int lane;
- for (lane = 0; lane < lane_count; lane++) {
- u8 old = drm_dp_get_adjust_request_voltage(old_link_status, lane) |
- drm_dp_get_adjust_request_pre_emphasis(old_link_status, lane);
- u8 new = drm_dp_get_adjust_request_voltage(new_link_status, lane) |
- drm_dp_get_adjust_request_pre_emphasis(new_link_status, lane);
+ for (lane = 0; lane < crtc_state->lane_count; lane++) {
+ u8 old, new;
+
+ if (intel_dp_is_uhbr(crtc_state)) {
+ old = drm_dp_get_adjust_tx_ffe_preset(old_link_status, lane);
+ new = drm_dp_get_adjust_tx_ffe_preset(new_link_status, lane);
+ } else {
+ old = drm_dp_get_adjust_request_voltage(old_link_status, lane) |
+ drm_dp_get_adjust_request_pre_emphasis(old_link_status, lane);
+ new = drm_dp_get_adjust_request_voltage(new_link_status, lane) |
+ drm_dp_get_adjust_request_pre_emphasis(new_link_status, lane);
+ }
if (old != new)
return true;
@@ -721,8 +784,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
return false;
}
- if (!intel_dp_adjust_request_changed(crtc_state->lane_count,
- old_link_status, link_status))
+ if (!intel_dp_adjust_request_changed(crtc_state, old_link_status, link_status))
++voltage_tries;
else
voltage_tries = 1;
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dg2: update link training for 128b/132b
2021-10-07 10:31 [Intel-gfx] [PATCH] drm/i915/dg2: update link training for 128b/132b Jani Nikula
@ 2021-10-07 10:46 ` Ville Syrjälä
2021-10-07 11:43 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dg2: update link training for 128b/132b (rev2) Patchwork
2021-10-07 12:17 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2021-10-07 10:46 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Thu, Oct 07, 2021 at 01:31:08PM +0300, Jani Nikula wrote:
> The 128b/132b channel coding link training uses more straightforward TX
> FFE preset values. Reuse voltage tries and max vswing for retry logic.
>
> The delays for 128b/132b are still all wrong, but this is regardless a
> step forward.
>
> v2: Fix UHBR rate checks, use intel_dp_is_uhbr() helper
>
> v3:
> - Rebase
> - Modify intel_dp_adjust_request_changed() and
> intel_dp_link_max_vswing_reached() to take 128b/132b into
> account. (Ville)
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 18 ++-
> .../drm/i915/display/intel_dp_link_training.c | 124 +++++++++++++-----
> 2 files changed, 106 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 3f7bbeb3e3cd..59428ce4f8c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1338,13 +1338,20 @@ static int translate_signal_level(struct intel_dp *intel_dp,
> return 0;
> }
>
> -static int intel_ddi_dp_level(struct intel_dp *intel_dp, int lane)
> +static int intel_ddi_dp_level(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *crtc_state,
> + int lane)
> {
> u8 train_set = intel_dp->train_set[lane];
> - u8 signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
> - DP_TRAIN_PRE_EMPHASIS_MASK);
>
> - return translate_signal_level(intel_dp, signal_levels);
> + if (intel_dp_is_uhbr(crtc_state)) {
> + return train_set & DP_TX_FFE_PRESET_VALUE_MASK;
BTW I just noticed dg2_get_snps_buf_trans() does not check for
HDMI vs. DP. Also it isn't using intel_dp_is_uhbr() and has
the > vs. >= bug in the open coded check.
> + } else {
> + u8 signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
> + DP_TRAIN_PRE_EMPHASIS_MASK);
> +
> + return translate_signal_level(intel_dp, signal_levels);
> + }
> }
>
> int intel_ddi_level(struct intel_encoder *encoder,
> @@ -1362,7 +1369,8 @@ int intel_ddi_level(struct intel_encoder *encoder,
> if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> level = intel_ddi_hdmi_level(encoder, trans);
> else
> - level = intel_ddi_dp_level(enc_to_intel_dp(encoder), lane);
> + level = intel_ddi_dp_level(enc_to_intel_dp(encoder), crtc_state,
> + lane);
>
> if (drm_WARN_ON_ONCE(&i915->drm, level >= n_entries))
> level = n_entries - 1;
> 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 1a943ae38a6b..c54b7df56c9f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -304,11 +304,31 @@ static bool has_per_lane_signal_levels(struct intel_dp *intel_dp,
> return !intel_dp_phy_is_downstream_of_source(intel_dp, dp_phy);
> }
>
> -static u8 intel_dp_get_lane_adjust_train(struct intel_dp *intel_dp,
> - const struct intel_crtc_state *crtc_state,
> - enum drm_dp_phy dp_phy,
> - const u8 link_status[DP_LINK_STATUS_SIZE],
> - int lane)
> +
> +static u8 intel_dp_get_lane_adjust_train_128b132b(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *crtc_state,
> + enum drm_dp_phy dp_phy,
> + const u8 link_status[DP_LINK_STATUS_SIZE],
> + int lane)
> +{
> + u8 tx_ffe = 0;
> +
> + if (has_per_lane_signal_levels(intel_dp, dp_phy)) {
> + lane = min(lane, crtc_state->lane_count - 1);
> + tx_ffe = drm_dp_get_adjust_tx_ffe_preset(link_status, lane);
> + } else {
> + for (lane = 0; lane < crtc_state->lane_count; lane++)
> + tx_ffe = max(tx_ffe, drm_dp_get_adjust_tx_ffe_preset(link_status, lane));
> + }
> +
> + return tx_ffe;
> +}
> +
> +static u8 intel_dp_get_lane_adjust_train_8b10b(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *crtc_state,
> + enum drm_dp_phy dp_phy,
> + const u8 link_status[DP_LINK_STATUS_SIZE],
> + int lane)
> {
> u8 v = 0;
> u8 p = 0;
> @@ -340,6 +360,20 @@ static u8 intel_dp_get_lane_adjust_train(struct intel_dp *intel_dp,
> return v | p;
> }
>
> +static u8 intel_dp_get_lane_adjust_train(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *crtc_state,
> + enum drm_dp_phy dp_phy,
> + const u8 link_status[DP_LINK_STATUS_SIZE],
> + int lane)
> +{
> + if (intel_dp_is_uhbr(crtc_state))
> + return intel_dp_get_lane_adjust_train_128b132b(intel_dp, crtc_state,
> + dp_phy, link_status, lane);
> + else
> + return intel_dp_get_lane_adjust_train_8b10b(intel_dp, crtc_state,
> + dp_phy, link_status, lane);
> +}
> +
> #define TRAIN_REQ_FMT "%d/%d/%d/%d"
> #define _TRAIN_REQ_VSWING_ARGS(link_status, lane) \
> (drm_dp_get_adjust_request_voltage((link_status), (lane)) >> DP_TRAIN_VOLTAGE_SWING_SHIFT)
^ those would need doing too
> @@ -464,6 +498,13 @@ intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
> _TRAIN_SET_PREEMPH_ARGS((train_set)[1]), \
> _TRAIN_SET_PREEMPH_ARGS((train_set)[2]), \
> _TRAIN_SET_PREEMPH_ARGS((train_set)[3])
> +#define _TRAIN_SET_TX_FFE_ARGS(train_set) \
> + ((train_set) & DP_TX_FFE_PRESET_VALUE_MASK), ""
Sure, why not.
> +#define TRAIN_SET_TX_FFE_ARGS(train_set) \
> + _TRAIN_SET_TX_FFE_ARGS((train_set)[0]), \
> + _TRAIN_SET_TX_FFE_ARGS((train_set)[1]), \
> + _TRAIN_SET_TX_FFE_ARGS((train_set)[2]), \
> + _TRAIN_SET_TX_FFE_ARGS((train_set)[3])
>
> void intel_dp_set_signal_levels(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state,
> @@ -473,14 +514,23 @@ void intel_dp_set_signal_levels(struct intel_dp *intel_dp,
> struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> char phy_name[10];
>
> - drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] lanes: %d, "
> - "vswing levels: " TRAIN_SET_FMT ", "
> - "pre-emphasis levels: " TRAIN_SET_FMT "\n",
> - encoder->base.base.id, encoder->base.name,
> - intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)),
> - crtc_state->lane_count,
> - TRAIN_SET_VSWING_ARGS(intel_dp->train_set),
> - TRAIN_SET_PREEMPH_ARGS(intel_dp->train_set));
> + if (intel_dp_is_uhbr(crtc_state)) {
> + drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] lanes: %d, "
> + "128b/132b TX FFE presets: " TRAIN_SET_FMT "\n",
> + encoder->base.base.id, encoder->base.name,
> + intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)),
> + crtc_state->lane_count,
> + TRAIN_SET_TX_FFE_ARGS(intel_dp->train_set));
> + } else {
> + drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] lanes: %d, "
> + "vswing levels: " TRAIN_SET_FMT ", "
> + "pre-emphasis levels: " TRAIN_SET_FMT "\n",
> + encoder->base.base.id, encoder->base.name,
> + intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)),
> + crtc_state->lane_count,
> + TRAIN_SET_VSWING_ARGS(intel_dp->train_set),
> + TRAIN_SET_PREEMPH_ARGS(intel_dp->train_set));
Wonder if we should say "8b/10b" here just to be super clear?
> + }
>
> if (intel_dp_phy_is_downstream_of_source(intel_dp, dp_phy))
> encoder->set_signal_levels(encoder, crtc_state);
> @@ -529,16 +579,22 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp,
> * 2+1 or 3+0 depending on whether vswing level 3 is supported or not.
> */
> for (lane = 0; lane < crtc_state->lane_count; lane++) {
> - u8 v = (intel_dp->train_set[lane] & DP_TRAIN_VOLTAGE_SWING_MASK) >>
> - DP_TRAIN_VOLTAGE_SWING_SHIFT;
> - u8 p = (intel_dp->train_set[lane] & DP_TRAIN_PRE_EMPHASIS_MASK) >>
> - DP_TRAIN_PRE_EMPHASIS_SHIFT;
> -
> - if ((intel_dp->train_set[lane] & DP_TRAIN_MAX_SWING_REACHED) == 0)
> - return false;
> -
> - if (v + p != 3)
> - return false;
> + if (intel_dp_is_uhbr(crtc_state)) {
> + if ((intel_dp->train_set[lane] & DP_TX_FFE_PRESET_VALUE_MASK) !=
> + DP_TX_FFE_PRESET_VALUE_MASK)
> + return false;
Seems sane. Maybe we shuld suck these little things into
128b/132b vs. 8b/10b specific helper function just to
declutter intel_dp_link_max_vswing_reached() a bit?
> + } else {
> + u8 v = (intel_dp->train_set[lane] & DP_TRAIN_VOLTAGE_SWING_MASK) >>
> + DP_TRAIN_VOLTAGE_SWING_SHIFT;
> + u8 p = (intel_dp->train_set[lane] & DP_TRAIN_PRE_EMPHASIS_MASK) >>
> + DP_TRAIN_PRE_EMPHASIS_SHIFT;
> +
> + if ((intel_dp->train_set[lane] & DP_TRAIN_MAX_SWING_REACHED) == 0)
> + return false;
> +
> + if (v + p != 3)
> + return false;
> + }
> }
>
> return true;
> @@ -601,17 +657,24 @@ static void intel_dp_link_training_clock_recovery_delay(struct intel_dp *intel_d
> drm_dp_lttpr_link_train_clock_recovery_delay();
> }
>
> -static bool intel_dp_adjust_request_changed(int lane_count,
> +static bool intel_dp_adjust_request_changed(const struct intel_crtc_state *crtc_state,
> const u8 old_link_status[DP_LINK_STATUS_SIZE],
> const u8 new_link_status[DP_LINK_STATUS_SIZE])
> {
> int lane;
>
> - for (lane = 0; lane < lane_count; lane++) {
> - u8 old = drm_dp_get_adjust_request_voltage(old_link_status, lane) |
> - drm_dp_get_adjust_request_pre_emphasis(old_link_status, lane);
> - u8 new = drm_dp_get_adjust_request_voltage(new_link_status, lane) |
> - drm_dp_get_adjust_request_pre_emphasis(new_link_status, lane);
> + for (lane = 0; lane < crtc_state->lane_count; lane++) {
> + u8 old, new;
> +
> + if (intel_dp_is_uhbr(crtc_state)) {
> + old = drm_dp_get_adjust_tx_ffe_preset(old_link_status, lane);
> + new = drm_dp_get_adjust_tx_ffe_preset(new_link_status, lane);
> + } else {
> + old = drm_dp_get_adjust_request_voltage(old_link_status, lane) |
> + drm_dp_get_adjust_request_pre_emphasis(old_link_status, lane);
> + new = drm_dp_get_adjust_request_voltage(new_link_status, lane) |
> + drm_dp_get_adjust_request_pre_emphasis(new_link_status, lane);
> + }
I like it.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> if (old != new)
> return true;
> @@ -721,8 +784,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
> return false;
> }
>
> - if (!intel_dp_adjust_request_changed(crtc_state->lane_count,
> - old_link_status, link_status))
> + if (!intel_dp_adjust_request_changed(crtc_state, old_link_status, link_status))
> ++voltage_tries;
> else
> voltage_tries = 1;
> --
> 2.30.2
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dg2: update link training for 128b/132b (rev2)
2021-10-07 10:31 [Intel-gfx] [PATCH] drm/i915/dg2: update link training for 128b/132b Jani Nikula
2021-10-07 10:46 ` Ville Syrjälä
@ 2021-10-07 11:43 ` Patchwork
2021-10-07 12:17 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2021-10-07 11:43 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/dg2: update link training for 128b/132b (rev2)
URL : https://patchwork.freedesktop.org/series/95317/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
eda15c6a42a6 drm/i915/dg2: update link training for 128b/132b
-:47: WARNING:UNNECESSARY_ELSE: else is not generally useful after a break or return
#47: FILE: drivers/gpu/drm/i915/display/intel_ddi.c:1349:
+ return train_set & DP_TX_FFE_PRESET_VALUE_MASK;
+ } else {
-:79: CHECK:LINE_SPACING: Please don't use multiple blank lines
#79: FILE: drivers/gpu/drm/i915/display/intel_dp_link_training.c:307:
+
-:134: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#134: FILE: drivers/gpu/drm/i915/display/intel_dp_link_training.c:503:
+#define TRAIN_SET_TX_FFE_ARGS(train_set) \
+ _TRAIN_SET_TX_FFE_ARGS((train_set)[0]), \
+ _TRAIN_SET_TX_FFE_ARGS((train_set)[1]), \
+ _TRAIN_SET_TX_FFE_ARGS((train_set)[2]), \
+ _TRAIN_SET_TX_FFE_ARGS((train_set)[3])
-:134: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'train_set' - possible side-effects?
#134: FILE: drivers/gpu/drm/i915/display/intel_dp_link_training.c:503:
+#define TRAIN_SET_TX_FFE_ARGS(train_set) \
+ _TRAIN_SET_TX_FFE_ARGS((train_set)[0]), \
+ _TRAIN_SET_TX_FFE_ARGS((train_set)[1]), \
+ _TRAIN_SET_TX_FFE_ARGS((train_set)[2]), \
+ _TRAIN_SET_TX_FFE_ARGS((train_set)[3])
total: 1 errors, 1 warnings, 2 checks, 204 lines checked
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dg2: update link training for 128b/132b (rev2)
2021-10-07 10:31 [Intel-gfx] [PATCH] drm/i915/dg2: update link training for 128b/132b Jani Nikula
2021-10-07 10:46 ` Ville Syrjälä
2021-10-07 11:43 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dg2: update link training for 128b/132b (rev2) Patchwork
@ 2021-10-07 12:17 ` Patchwork
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2021-10-07 12:17 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 8017 bytes --]
== Series Details ==
Series: drm/i915/dg2: update link training for 128b/132b (rev2)
URL : https://patchwork.freedesktop.org/series/95317/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_10693 -> Patchwork_21275
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_21275 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_21275, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_21275:
### IGT changes ###
#### Possible regressions ####
* igt@core_hotunplug@unbind-rebind:
- fi-tgl-u2: NOTRUN -> [INCOMPLETE][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-u2/igt@core_hotunplug@unbind-rebind.html
Known issues
------------
Here are the changes found in Patchwork_21275 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@amdgpu/amd_basic@query-info:
- fi-tgl-1115g4: NOTRUN -> [SKIP][2] ([fdo#109315])
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-1115g4/igt@amdgpu/amd_basic@query-info.html
* igt@amdgpu/amd_cs_nop@nop-gfx0:
- fi-tgl-1115g4: NOTRUN -> [SKIP][3] ([fdo#109315] / [i915#2575]) +16 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-1115g4/igt@amdgpu/amd_cs_nop@nop-gfx0.html
* igt@amdgpu/amd_cs_nop@sync-fork-compute0:
- fi-snb-2600: NOTRUN -> [SKIP][4] ([fdo#109271]) +17 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-snb-2600/igt@amdgpu/amd_cs_nop@sync-fork-compute0.html
* igt@gem_exec_suspend@basic-s3:
- fi-tgl-u2: NOTRUN -> [FAIL][5] ([i915#1888])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-u2/igt@gem_exec_suspend@basic-s3.html
* igt@gem_huc_copy@huc-copy:
- fi-tgl-u2: NOTRUN -> [SKIP][6] ([i915#2190])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-u2/igt@gem_huc_copy@huc-copy.html
- fi-tgl-1115g4: NOTRUN -> [SKIP][7] ([i915#2190])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-1115g4/igt@gem_huc_copy@huc-copy.html
* igt@i915_pm_backlight@basic-brightness:
- fi-tgl-1115g4: NOTRUN -> [SKIP][8] ([i915#1155])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-1115g4/igt@i915_pm_backlight@basic-brightness.html
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-tgl-1115g4: NOTRUN -> [SKIP][9] ([fdo#111827]) +8 similar issues
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-1115g4/igt@kms_chamelium@common-hpd-after-suspend.html
* igt@kms_chamelium@dp-hpd-fast:
- fi-tgl-u2: NOTRUN -> [SKIP][10] ([fdo#109284] / [fdo#111827]) +8 similar issues
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-u2/igt@kms_chamelium@dp-hpd-fast.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-tgl-u2: NOTRUN -> [SKIP][11] ([i915#4103]) +1 similar issue
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
- fi-tgl-1115g4: NOTRUN -> [SKIP][12] ([i915#4103]) +1 similar issue
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-1115g4/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
* igt@kms_force_connector_basic@force-load-detect:
- fi-tgl-1115g4: NOTRUN -> [SKIP][13] ([fdo#109285])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-1115g4/igt@kms_force_connector_basic@force-load-detect.html
- fi-tgl-u2: NOTRUN -> [SKIP][14] ([fdo#109285])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-u2/igt@kms_force_connector_basic@force-load-detect.html
* igt@kms_psr@primary_mmap_gtt:
- fi-tgl-1115g4: NOTRUN -> [SKIP][15] ([i915#1072]) +3 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-1115g4/igt@kms_psr@primary_mmap_gtt.html
* igt@prime_vgem@basic-userptr:
- fi-tgl-u2: NOTRUN -> [SKIP][16] ([i915#3301])
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-u2/igt@prime_vgem@basic-userptr.html
- fi-tgl-1115g4: NOTRUN -> [SKIP][17] ([i915#3301])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-1115g4/igt@prime_vgem@basic-userptr.html
* igt@runner@aborted:
- fi-tgl-u2: NOTRUN -> [FAIL][18] ([i915#1602] / [i915#2722])
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-tgl-u2/igt@runner@aborted.html
#### Possible fixes ####
* igt@i915_selftest@live@hangcheck:
- fi-snb-2600: [INCOMPLETE][19] ([i915#3921]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10693/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
* igt@kms_chamelium@dp-crc-fast:
- fi-kbl-7500u: [FAIL][21] ([i915#1372]) -> [PASS][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10693/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
* igt@kms_frontbuffer_tracking@basic:
- fi-cml-u2: [DMESG-WARN][23] ([i915#4269]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10693/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
[i915#1372]: https://gitlab.freedesktop.org/drm/intel/issues/1372
[i915#1602]: https://gitlab.freedesktop.org/drm/intel/issues/1602
[i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
[i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722
[i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
[i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4269]: https://gitlab.freedesktop.org/drm/intel/issues/4269
Participating hosts (42 -> 38)
------------------------------
Additional (2): fi-tgl-1115g4 fi-tgl-u2
Missing (6): fi-ilk-m540 bat-dg1-6 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 bat-jsl-1
Build changes
-------------
* Linux: CI_DRM_10693 -> Patchwork_21275
CI-20190529: 20190529
CI_DRM_10693: fd46c2693c04811a4c943cef620190993bf0da76 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6235: 65dd7d484d5d09de196def254afebf41dfde1052 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_21275: eda15c6a42a609def306c3a231b44797a591be83 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
eda15c6a42a6 drm/i915/dg2: update link training for 128b/132b
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21275/index.html
[-- Attachment #2: Type: text/html, Size: 9541 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dg2: update link training for 128b/132b
2021-10-01 10:02 [Intel-gfx] [PATCH] drm/i915/dg2: update link training for 128b/132b Jani Nikula
@ 2021-10-01 15:38 ` Ville Syrjälä
0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2021-10-01 15:38 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Fri, Oct 01, 2021 at 01:02:47PM +0300, Jani Nikula wrote:
> The 128b/132b channel coding link training uses more straightforward TX
> FFE preset values.
>
> v2: Fix UHBR rate checks, use intel_dp_is_uhbr() helper
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 13 ++-
> .../drm/i915/display/intel_dp_link_training.c | 86 +++++++++++++------
> 2 files changed, 70 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 51cd0420e00e..341fda4055ed 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1398,11 +1398,16 @@ static int translate_signal_level(struct intel_dp *intel_dp,
> static int intel_ddi_dp_level(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state)
> {
> - u8 train_set = intel_dp->train_set[0];
> - u8 signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
> - DP_TRAIN_PRE_EMPHASIS_MASK);
> + if (intel_dp_is_uhbr(crtc_state)) {
> + /* FIXME: We'll want independent presets for each lane. */
> + return intel_dp->train_set[0] & DP_TX_FFE_PRESET_VALUE_MASK;
> + } else {
> + u8 train_set = intel_dp->train_set[0];
> + u8 signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
> + DP_TRAIN_PRE_EMPHASIS_MASK);
>
> - return translate_signal_level(intel_dp, signal_levels);
> + return translate_signal_level(intel_dp, signal_levels);
> + }
> }
>
> static void
> 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 053ed9302cda..1dda3d31394e 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -301,6 +301,24 @@ static u8 intel_dp_phy_preemph_max(struct intel_dp *intel_dp,
> return preemph_max;
> }
>
> +static void intel_dp_128b132b_adjust_train(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *crtc_state,
> + const u8 link_status[DP_LINK_STATUS_SIZE])
> +{
> + int lane;
> + u8 tx_ffe = 0;
> +
> + /*
> + * FIXME: We'll want independent presets for each lane. See also
> + * intel_ddi_dp_level() and intel_snps_phy_ddi_vswing_sequence().
> + */
Wait a few patches [1] and we can avoid the FIXMEs ;)
[1] https://patchwork.freedesktop.org/series/95122/
> + for (lane = 0; lane < crtc_state->lane_count; lane++)
> + tx_ffe = max(tx_ffe, drm_dp_get_adjust_tx_ffe_preset(link_status, lane));
> +
> + for (lane = 0; lane < crtc_state->lane_count; lane++)
> + intel_dp->train_set[lane] = tx_ffe;
> +}
> +
> void
> intel_dp_get_adjust_train(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state,
> @@ -313,6 +331,11 @@ intel_dp_get_adjust_train(struct intel_dp *intel_dp,
> u8 voltage_max;
> u8 preemph_max;
>
> + if (intel_dp_is_uhbr(crtc_state)) {
> + intel_dp_128b132b_adjust_train(intel_dp, crtc_state, link_status);
> + return;
> + }
> +
> for (lane = 0; lane < crtc_state->lane_count; lane++) {
> v = max(v, drm_dp_get_adjust_request_voltage(link_status, lane));
> p = max(p, drm_dp_get_adjust_request_pre_emphasis(link_status, lane));
> @@ -402,14 +425,21 @@ void intel_dp_set_signal_levels(struct intel_dp *intel_dp,
> u8 train_set = intel_dp->train_set[0];
> char phy_name[10];
>
> - drm_dbg_kms(&dev_priv->drm, "Using vswing level %d%s, pre-emphasis level %d%s, at %s\n",
> - train_set & DP_TRAIN_VOLTAGE_SWING_MASK,
> - train_set & DP_TRAIN_MAX_SWING_REACHED ? " (max)" : "",
> - (train_set & DP_TRAIN_PRE_EMPHASIS_MASK) >>
> - DP_TRAIN_PRE_EMPHASIS_SHIFT,
> - train_set & DP_TRAIN_MAX_PRE_EMPHASIS_REACHED ?
> - " (max)" : "",
> - intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)));
> + if (intel_dp_is_uhbr(crtc_state)) {
> + /* FIXME: We'll want independent presets for each lane. */
> + drm_dbg_kms(&dev_priv->drm, "%s: Using 128b/132b TX FFE preset %u\n",
> + intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)),
> + train_set & DP_TX_FFE_PRESET_VALUE_MASK);
> + } else {
> + drm_dbg_kms(&dev_priv->drm, "%s: Using 8b/10b vswing level %d%s, pre-emphasis level %d%s\n",
> + intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)),
> + train_set & DP_TRAIN_VOLTAGE_SWING_MASK,
> + train_set & DP_TRAIN_MAX_SWING_REACHED ? " (max)" : "",
> + (train_set & DP_TRAIN_PRE_EMPHASIS_MASK) >>
> + DP_TRAIN_PRE_EMPHASIS_SHIFT,
> + train_set & DP_TRAIN_MAX_PRE_EMPHASIS_REACHED ?
> + " (max)" : "");
> + }
>
> if (intel_dp_phy_is_downstream_of_source(intel_dp, dp_phy))
> intel_dp->set_signal_levels(intel_dp, crtc_state);
> @@ -563,18 +593,21 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
> return true;
> }
>
> - if (voltage_tries == 5) {
> - drm_dbg_kms(&i915->drm,
> - "Same voltage tried 5 times\n");
> - return false;
> - }
> + /* FIXME: 128b/132b needs better abstractions here. */
> + if (!intel_dp_is_uhbr(crtc_state)) {
> + if (voltage_tries == 5) {
> + drm_dbg_kms(&i915->drm,
> + "Same voltage tried 5 times\n");
> + return false;
> + }
>
> - if (max_vswing_reached) {
> - drm_dbg_kms(&i915->drm, "Max Voltage Swing reached\n");
> - return false;
> - }
> + if (max_vswing_reached) {
> + drm_dbg_kms(&i915->drm, "Max Voltage Swing reached\n");
> + return false;
> + }
>
> - voltage = intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
> + voltage = intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
> + }
>
> /* Update training set as requested by target */
> intel_dp_get_adjust_train(intel_dp, crtc_state, dp_phy,
> @@ -585,14 +618,17 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
> return false;
> }
>
> - if ((intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) ==
> - voltage)
> - ++voltage_tries;
> - else
> - voltage_tries = 1;
> + /* FIXME: 128b/132b needs better abstractions here. */
> + if (!intel_dp_is_uhbr(crtc_state)) {
> + if ((intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) ==
> + voltage)
> + ++voltage_tries;
> + else
> + voltage_tries = 1;
Unfortunately the spec seems to totally lack any description of
128b/132b link training sequence. So I have no idea if we should
consider the entire tx_ffe here as the voltage and still do the
"no more than five times with the same setting" thing.
Ugh. Also just realized this code needs further surgery for the
per-lane drive settings...
Ugh2. This code looks totally wrong anyway. At least DP2.0 seems
to say that we should check whether the _request_ stays the same
five times in a row, not whether we happen to transmit the
same voltage level five times...
Also we seem to be missing the 128b/132b AUX_RD_INTERVAL stuff...
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/dg2: update link training for 128b/132b
@ 2021-10-01 10:02 Jani Nikula
2021-10-01 15:38 ` Ville Syrjälä
0 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2021-10-01 10:02 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, ville.syrjala
The 128b/132b channel coding link training uses more straightforward TX
FFE preset values.
v2: Fix UHBR rate checks, use intel_dp_is_uhbr() helper
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 13 ++-
.../drm/i915/display/intel_dp_link_training.c | 86 +++++++++++++------
2 files changed, 70 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 51cd0420e00e..341fda4055ed 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1398,11 +1398,16 @@ static int translate_signal_level(struct intel_dp *intel_dp,
static int intel_ddi_dp_level(struct intel_dp *intel_dp,
const struct intel_crtc_state *crtc_state)
{
- u8 train_set = intel_dp->train_set[0];
- u8 signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
- DP_TRAIN_PRE_EMPHASIS_MASK);
+ if (intel_dp_is_uhbr(crtc_state)) {
+ /* FIXME: We'll want independent presets for each lane. */
+ return intel_dp->train_set[0] & DP_TX_FFE_PRESET_VALUE_MASK;
+ } else {
+ u8 train_set = intel_dp->train_set[0];
+ u8 signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
+ DP_TRAIN_PRE_EMPHASIS_MASK);
- return translate_signal_level(intel_dp, signal_levels);
+ return translate_signal_level(intel_dp, signal_levels);
+ }
}
static void
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 053ed9302cda..1dda3d31394e 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -301,6 +301,24 @@ static u8 intel_dp_phy_preemph_max(struct intel_dp *intel_dp,
return preemph_max;
}
+static void intel_dp_128b132b_adjust_train(struct intel_dp *intel_dp,
+ const struct intel_crtc_state *crtc_state,
+ const u8 link_status[DP_LINK_STATUS_SIZE])
+{
+ int lane;
+ u8 tx_ffe = 0;
+
+ /*
+ * FIXME: We'll want independent presets for each lane. See also
+ * intel_ddi_dp_level() and intel_snps_phy_ddi_vswing_sequence().
+ */
+ for (lane = 0; lane < crtc_state->lane_count; lane++)
+ tx_ffe = max(tx_ffe, drm_dp_get_adjust_tx_ffe_preset(link_status, lane));
+
+ for (lane = 0; lane < crtc_state->lane_count; lane++)
+ intel_dp->train_set[lane] = tx_ffe;
+}
+
void
intel_dp_get_adjust_train(struct intel_dp *intel_dp,
const struct intel_crtc_state *crtc_state,
@@ -313,6 +331,11 @@ intel_dp_get_adjust_train(struct intel_dp *intel_dp,
u8 voltage_max;
u8 preemph_max;
+ if (intel_dp_is_uhbr(crtc_state)) {
+ intel_dp_128b132b_adjust_train(intel_dp, crtc_state, link_status);
+ return;
+ }
+
for (lane = 0; lane < crtc_state->lane_count; lane++) {
v = max(v, drm_dp_get_adjust_request_voltage(link_status, lane));
p = max(p, drm_dp_get_adjust_request_pre_emphasis(link_status, lane));
@@ -402,14 +425,21 @@ void intel_dp_set_signal_levels(struct intel_dp *intel_dp,
u8 train_set = intel_dp->train_set[0];
char phy_name[10];
- drm_dbg_kms(&dev_priv->drm, "Using vswing level %d%s, pre-emphasis level %d%s, at %s\n",
- train_set & DP_TRAIN_VOLTAGE_SWING_MASK,
- train_set & DP_TRAIN_MAX_SWING_REACHED ? " (max)" : "",
- (train_set & DP_TRAIN_PRE_EMPHASIS_MASK) >>
- DP_TRAIN_PRE_EMPHASIS_SHIFT,
- train_set & DP_TRAIN_MAX_PRE_EMPHASIS_REACHED ?
- " (max)" : "",
- intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)));
+ if (intel_dp_is_uhbr(crtc_state)) {
+ /* FIXME: We'll want independent presets for each lane. */
+ drm_dbg_kms(&dev_priv->drm, "%s: Using 128b/132b TX FFE preset %u\n",
+ intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)),
+ train_set & DP_TX_FFE_PRESET_VALUE_MASK);
+ } else {
+ drm_dbg_kms(&dev_priv->drm, "%s: Using 8b/10b vswing level %d%s, pre-emphasis level %d%s\n",
+ intel_dp_phy_name(dp_phy, phy_name, sizeof(phy_name)),
+ train_set & DP_TRAIN_VOLTAGE_SWING_MASK,
+ train_set & DP_TRAIN_MAX_SWING_REACHED ? " (max)" : "",
+ (train_set & DP_TRAIN_PRE_EMPHASIS_MASK) >>
+ DP_TRAIN_PRE_EMPHASIS_SHIFT,
+ train_set & DP_TRAIN_MAX_PRE_EMPHASIS_REACHED ?
+ " (max)" : "");
+ }
if (intel_dp_phy_is_downstream_of_source(intel_dp, dp_phy))
intel_dp->set_signal_levels(intel_dp, crtc_state);
@@ -563,18 +593,21 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
return true;
}
- if (voltage_tries == 5) {
- drm_dbg_kms(&i915->drm,
- "Same voltage tried 5 times\n");
- return false;
- }
+ /* FIXME: 128b/132b needs better abstractions here. */
+ if (!intel_dp_is_uhbr(crtc_state)) {
+ if (voltage_tries == 5) {
+ drm_dbg_kms(&i915->drm,
+ "Same voltage tried 5 times\n");
+ return false;
+ }
- if (max_vswing_reached) {
- drm_dbg_kms(&i915->drm, "Max Voltage Swing reached\n");
- return false;
- }
+ if (max_vswing_reached) {
+ drm_dbg_kms(&i915->drm, "Max Voltage Swing reached\n");
+ return false;
+ }
- voltage = intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
+ voltage = intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
+ }
/* Update training set as requested by target */
intel_dp_get_adjust_train(intel_dp, crtc_state, dp_phy,
@@ -585,14 +618,17 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
return false;
}
- if ((intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) ==
- voltage)
- ++voltage_tries;
- else
- voltage_tries = 1;
+ /* FIXME: 128b/132b needs better abstractions here. */
+ if (!intel_dp_is_uhbr(crtc_state)) {
+ if ((intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) ==
+ voltage)
+ ++voltage_tries;
+ else
+ voltage_tries = 1;
- if (intel_dp_link_max_vswing_reached(intel_dp, crtc_state))
- max_vswing_reached = true;
+ if (intel_dp_link_max_vswing_reached(intel_dp, crtc_state))
+ max_vswing_reached = true;
+ }
}
drm_err(&i915->drm,
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-07 12:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-07 10:31 [Intel-gfx] [PATCH] drm/i915/dg2: update link training for 128b/132b Jani Nikula
2021-10-07 10:46 ` Ville Syrjälä
2021-10-07 11:43 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dg2: update link training for 128b/132b (rev2) Patchwork
2021-10-07 12:17 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2021-10-01 10:02 [Intel-gfx] [PATCH] drm/i915/dg2: update link training for 128b/132b Jani Nikula
2021-10-01 15:38 ` Ville Syrjälä
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.