On Thu, 04 May 2023, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Thu, May 04, 2023 at 02:10:43AM +0300, Imre Deak wrote:
>> Add functions for printing link training debug and error messages, both
>> to prepare for the next patch, which downgrades an error to a debug
>> message if the sink is disconnected and to remove some code duplication.
>>
>> v2: (Ville)
>> - Always print the connector prefix.
>> - Preserve the drm_dbg_kms() debug category.
>>
>> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
>
> While I don't really like the custom debug infrastructure I can't
> immediately see any really better way to do this.
To avoid any confusion, NAK on this patch.
For one thing, I'm firmly in the same camp with the custom debug
wrappers. I hate it that gem/gt has started adding their own wrappers,
and as you add one as an example, it just expands from there. We now
have tons. I think it's enough that we have *drm* specific wrappers.
That said, I could've lived with this one, as it's limited to within a
file (though I know that can change all too quickly if we're not
vigilant).
The reason for rejecting this altogether is in the
implementation. Having an extra function in there makes the function
name debugging in __drm_dev_dbg() (where all this leads to after the
onion has been peeled) result in:
<7>[ 35.817968] i915 0000:00:02.0: [drm:lt_msg [i915]] [CONNECTOR:119:DP-2][ENCODER:118:DDI E/PHY E][DPRX] Using LINK_BW_SET value 0a
<7>[ 35.818535] i915 0000:00:02.0: [drm:lt_msg [i915]] [CONNECTOR:119:DP-2][ENCODER:118:DDI E/PHY E][DPRX] 8b/10b, lanes: 2, vswing levels: 0/0/0/0, pre-emphasis levels: 0/0/0/0
<7>[ 35.818685] i915 0000:00:02.0: [drm:hsw_set_signal_levels [i915]] Using signal levels 00000000
<7>[ 35.818841] i915 0000:00:02.0: [drm:lt_msg [i915]] [CONNECTOR:119:DP-2][ENCODER:118:DDI E/PHY E][DPRX] Using DP training pattern TPS1
<7>[ 35.819990] i915 0000:00:02.0: [drm:lt_msg [i915]] [CONNECTOR:119:DP-2][ENCODER:118:DDI E/PHY E][DPRX] Clock recovery OK
<7>[ 35.820146] i915 0000:00:02.0: [drm:lt_msg [i915]] [CONNECTOR:119:DP-2][ENCODER:118:DDI E/PHY E][DPRX] Using DP training pattern TPS2
<7>[ 35.821886] i915 0000:00:02.0: [drm:lt_msg [i915]] [CONNECTOR:119:DP-2][ENCODER:118:DDI E/PHY E][DPRX] Channel EQ done. DP Training successful
<7>[ 35.822035] i915 0000:00:02.0: [drm:lt_msg [i915]] [CONNECTOR:119:DP-2][ENCODER:118:DDI E/PHY E][DPRX] Link Training passed at link rate = 270000, lane count = 2
And that's just useless.
I don't want it fixed with some __always_inline hack either.
Back to the drawing board, I'm afraid.
BR,
Jani.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
>> ---
>> .../drm/i915/display/intel_dp_link_training.c | 387 +++++++-----------
>> 1 file changed, 140 insertions(+), 247 deletions(-)
>>
>> 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 e92c62bcc9b85..f83944eaac128 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> @@ -26,6 +26,43 @@
>> #include "intel_dp.h"
>> #include "intel_dp_link_training.h"
>>
>> +__printf(4, 5)
>> +static void lt_msg(struct intel_dp *intel_dp, enum drm_dp_phy dp_phy,
>> + bool is_error, const char *format, ...)
>> +{
>> + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> + struct intel_connector *connector = intel_dp->attached_connector;
>> + struct va_format vaf;
>> + va_list args;
>> +
>> + if (drm_WARN_ON(&i915->drm, !connector))
>> + return;
>> +
>> + va_start(args, format);
>> + vaf.fmt = format;
>> + vaf.va = &args;
>> +
>> + if (is_error)
>> + drm_err(&i915->drm, "[CONNECTOR:%d:%s][ENCODER:%d:%s][%s] %pV\n",
>> + connector->base.base.id, connector->base.name,
>> + encoder->base.base.id, encoder->base.name,
>> + drm_dp_phy_name(dp_phy),
>> + &vaf);
>> + else
>> + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s][ENCODER:%d:%s][%s] %pV\n",
>> + connector->base.base.id, connector->base.name,
>> + encoder->base.base.id, encoder->base.name,
>> + drm_dp_phy_name(dp_phy),
>> + &vaf);
>> +}
>> +
>> +#define lt_err(intel_dp, dp_phy, format, ...) \
>> + lt_msg(intel_dp, dp_phy, true, format, ## __VA_ARGS__)
>> +
>> +#define lt_dbg(intel_dp, dp_phy, format, ...) \
>> + lt_msg(intel_dp, dp_phy, false, format, ## __VA_ARGS__)
>> +
>> static void intel_dp_reset_lttpr_common_caps(struct intel_dp *intel_dp)
>> {
>> memset(intel_dp->lttpr_common_caps, 0, sizeof(intel_dp->lttpr_common_caps));
>> @@ -47,29 +84,21 @@ static void intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
>> const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>> enum drm_dp_phy dp_phy)
>> {
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> u8 *phy_caps = intel_dp_lttpr_phy_caps(intel_dp, dp_phy);
>>
>> if (drm_dp_read_lttpr_phy_caps(&intel_dp->aux, dpcd, dp_phy, phy_caps) < 0) {
>> - drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
>> - "[ENCODER:%d:%s][%s] failed to read the PHY caps\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy));
>> + lt_dbg(intel_dp, dp_phy, "failed to read the PHY caps\n");
>> return;
>> }
>>
>> - drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
>> - "[ENCODER:%d:%s][%s] PHY capabilities: %*ph\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy),
>> - (int)sizeof(intel_dp->lttpr_phy_caps[0]),
>> - phy_caps);
>> + lt_dbg(intel_dp, dp_phy, "PHY capabilities: %*ph\n",
>> + (int)sizeof(intel_dp->lttpr_phy_caps[0]),
>> + phy_caps);
>> }
>>
>> static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp,
>> const u8 dpcd[DP_RECEIVER_CAP_SIZE])
>> {
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> int ret;
>>
>> ret = drm_dp_read_lttpr_common_caps(&intel_dp->aux, dpcd,
>> @@ -77,11 +106,9 @@ static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp,
>> if (ret < 0)
>> goto reset_caps;
>>
>> - drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
>> - "[ENCODER:%d:%s] LTTPR common capabilities: %*ph\n",
>> - encoder->base.base.id, encoder->base.name,
>> - (int)sizeof(intel_dp->lttpr_common_caps),
>> - intel_dp->lttpr_common_caps);
>> + lt_dbg(intel_dp, DP_PHY_DPRX, "LTTPR common capabilities: %*ph\n",
>> + (int)sizeof(intel_dp->lttpr_common_caps),
>> + intel_dp->lttpr_common_caps);
>>
>> /* The minimum value of LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV is 1.4 */
>> if (intel_dp->lttpr_common_caps[0] < 0x14)
>> @@ -105,8 +132,6 @@ intel_dp_set_lttpr_transparent_mode(struct intel_dp *intel_dp, bool enable)
>>
>> static int intel_dp_init_lttpr(struct intel_dp *intel_dp, const u8 dpcd[DP_RECEIVER_CAP_SIZE])
>> {
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> - struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>> int lttpr_count;
>> int i;
>>
>> @@ -138,9 +163,8 @@ static int intel_dp_init_lttpr(struct intel_dp *intel_dp, const u8 dpcd[DP_RECEI
>> return 0;
>>
>> if (!intel_dp_set_lttpr_transparent_mode(intel_dp, false)) {
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s] Switching to LTTPR non-transparent LT mode failed, fall-back to transparent mode\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_dbg(intel_dp, DP_PHY_DPRX,
>> + "Switching to LTTPR non-transparent LT mode failed, fall-back to transparent mode\n");
>>
>> intel_dp_set_lttpr_transparent_mode(intel_dp, true);
>> intel_dp_reset_lttpr_count(intel_dp);
>> @@ -409,26 +433,22 @@ intel_dp_get_adjust_train(struct intel_dp *intel_dp,
>> enum drm_dp_phy dp_phy,
>> const u8 link_status[DP_LINK_STATUS_SIZE])
>> {
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> - struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>> int lane;
>>
>> if (intel_dp_is_uhbr(crtc_state)) {
>> - drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 128b/132b, lanes: %d, "
>> - "TX FFE request: " TRAIN_REQ_FMT "\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy),
>> - crtc_state->lane_count,
>> - TRAIN_REQ_TX_FFE_ARGS(link_status));
>> + lt_dbg(intel_dp, dp_phy,
>> + "128b/132b, lanes: %d, "
>> + "TX FFE request: " TRAIN_REQ_FMT "\n",
>> + crtc_state->lane_count,
>> + TRAIN_REQ_TX_FFE_ARGS(link_status));
>> } else {
>> - drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 8b/10b, lanes: %d, "
>> - "vswing request: " TRAIN_REQ_FMT ", "
>> - "pre-emphasis request: " TRAIN_REQ_FMT "\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy),
>> - crtc_state->lane_count,
>> - TRAIN_REQ_VSWING_ARGS(link_status),
>> - TRAIN_REQ_PREEMPH_ARGS(link_status));
>> + lt_dbg(intel_dp, dp_phy,
>> + "8b/10b, lanes: %d, "
>> + "vswing request: " TRAIN_REQ_FMT ", "
>> + "pre-emphasis request: " TRAIN_REQ_FMT "\n",
>> + crtc_state->lane_count,
>> + TRAIN_REQ_VSWING_ARGS(link_status),
>> + TRAIN_REQ_PREEMPH_ARGS(link_status));
>> }
>>
>> for (lane = 0; lane < 4; lane++)
>> @@ -487,16 +507,11 @@ intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
>> enum drm_dp_phy dp_phy,
>> u8 dp_train_pat)
>> {
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> - struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>> u8 train_pat = intel_dp_training_pattern_symbol(dp_train_pat);
>>
>> if (train_pat != DP_TRAINING_PATTERN_DISABLE)
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s][%s] Using DP training pattern TPS%c\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy),
>> - dp_training_pattern_name(train_pat));
>> + lt_dbg(intel_dp, dp_phy, "Using DP training pattern TPS%c\n",
>> + dp_training_pattern_name(train_pat));
>>
>> intel_dp->set_link_train(intel_dp, crtc_state, dp_train_pat);
>> }
>> @@ -531,24 +546,21 @@ void intel_dp_set_signal_levels(struct intel_dp *intel_dp,
>> enum drm_dp_phy dp_phy)
>> {
>> struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> - struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>>
>> if (intel_dp_is_uhbr(crtc_state)) {
>> - drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 128b/132b, lanes: %d, "
>> - "TX FFE presets: " TRAIN_SET_FMT "\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy),
>> - crtc_state->lane_count,
>> - TRAIN_SET_TX_FFE_ARGS(intel_dp->train_set));
>> + lt_dbg(intel_dp, dp_phy,
>> + "128b/132b, lanes: %d, "
>> + "TX FFE presets: " TRAIN_SET_FMT "\n",
>> + crtc_state->lane_count,
>> + TRAIN_SET_TX_FFE_ARGS(intel_dp->train_set));
>> } else {
>> - drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 8b/10b, lanes: %d, "
>> - "vswing levels: " TRAIN_SET_FMT ", "
>> - "pre-emphasis levels: " TRAIN_SET_FMT "\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy),
>> - crtc_state->lane_count,
>> - TRAIN_SET_VSWING_ARGS(intel_dp->train_set),
>> - TRAIN_SET_PREEMPH_ARGS(intel_dp->train_set));
>> + lt_dbg(intel_dp, dp_phy,
>> + "8b/10b, lanes: %d, "
>> + "vswing levels: " TRAIN_SET_FMT ", "
>> + "pre-emphasis levels: " TRAIN_SET_FMT "\n",
>> + 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))
>> @@ -677,8 +689,6 @@ static bool
>> intel_dp_prepare_link_train(struct intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state)
>> {
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> - struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>> u8 link_bw, rate_select;
>>
>> if (intel_dp->prepare_link_retrain)
>> @@ -699,24 +709,21 @@ intel_dp_prepare_link_train(struct intel_dp *intel_dp,
>> * link rates are not stable.
>> */
>> if (!link_bw) {
>> - struct intel_connector *connector = intel_dp->attached_connector;
>> __le16 sink_rates[DP_MAX_SUPPORTED_RATES];
>>
>> - drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Reloading eDP link rates\n",
>> - connector->base.base.id, connector->base.name);
>> + lt_dbg(intel_dp, DP_PHY_DPRX, "Reloading eDP link rates\n");
>>
>> drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES,
>> sink_rates, sizeof(sink_rates));
>> }
>>
>> if (link_bw)
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s] Using LINK_BW_SET value %02x\n",
>> - encoder->base.base.id, encoder->base.name, link_bw);
>> + lt_dbg(intel_dp, DP_PHY_DPRX, "Using LINK_BW_SET value %02x\n",
>> + link_bw);
>> else
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s] Using LINK_RATE_SET value %02x\n",
>> - encoder->base.base.id, encoder->base.name, rate_select);
>> + lt_dbg(intel_dp, DP_PHY_DPRX,
>> + "Using LINK_RATE_SET value %02x\n",
>> + rate_select);
>> /*
>> * Spec DP2.1 Section 3.5.2.16
>> * Prior to LT DPTX should set 128b/132b DP Channel coding and then set link rate
>> @@ -758,15 +765,10 @@ void
>> intel_dp_dump_link_status(struct intel_dp *intel_dp, enum drm_dp_phy dp_phy,
>> const u8 link_status[DP_LINK_STATUS_SIZE])
>> {
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> - struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>> -
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s][%s] ln0_1:0x%x ln2_3:0x%x align:0x%x sink:0x%x adj_req0_1:0x%x adj_req2_3:0x%x\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy),
>> - link_status[0], link_status[1], link_status[2],
>> - link_status[3], link_status[4], link_status[5]);
>> + lt_dbg(intel_dp, dp_phy,
>> + "ln0_1:0x%x ln2_3:0x%x align:0x%x sink:0x%x adj_req0_1:0x%x adj_req2_3:0x%x\n",
>> + link_status[0], link_status[1], link_status[2],
>> + link_status[3], link_status[4], link_status[5]);
>> }
>>
>> /*
>> @@ -778,8 +780,6 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state,
>> enum drm_dp_phy dp_phy)
>> {
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> - struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>> u8 old_link_status[DP_LINK_STATUS_SIZE] = {};
>> int voltage_tries, cr_tries, max_cr_tries;
>> u8 link_status[DP_LINK_STATUS_SIZE];
>> @@ -794,9 +794,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
>> if (!intel_dp_reset_link_train(intel_dp, crtc_state, dp_phy,
>> DP_TRAINING_PATTERN_1 |
>> DP_LINK_SCRAMBLING_DISABLE)) {
>> - drm_err(&i915->drm, "[ENCODER:%d:%s][%s] Failed to enable link training\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy));
>> + lt_err(intel_dp, dp_phy, "Failed to enable link training\n");
>> return false;
>> }
>>
>> @@ -819,35 +817,24 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
>>
>> if (drm_dp_dpcd_read_phy_link_status(&intel_dp->aux, dp_phy,
>> link_status) < 0) {
>> - drm_err(&i915->drm, "[ENCODER:%d:%s][%s] Failed to get link status\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy));
>> + lt_err(intel_dp, dp_phy, "Failed to get link status\n");
>> return false;
>> }
>>
>> if (drm_dp_clock_recovery_ok(link_status, crtc_state->lane_count)) {
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s][%s] Clock recovery OK\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy));
>> + lt_dbg(intel_dp, dp_phy, "Clock recovery OK\n");
>> return true;
>> }
>>
>> if (voltage_tries == 5) {
>> intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s][%s] Same voltage tried 5 times\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy));
>> + lt_dbg(intel_dp, dp_phy, "Same voltage tried 5 times\n");
>> return false;
>> }
>>
>> if (max_vswing_reached) {
>> intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s][%s] Max Voltage Swing reached\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy));
>> + lt_dbg(intel_dp, dp_phy, "Max Voltage Swing reached\n");
>> return false;
>> }
>>
>> @@ -855,10 +842,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
>> intel_dp_get_adjust_train(intel_dp, crtc_state, dp_phy,
>> link_status);
>> if (!intel_dp_update_link_train(intel_dp, crtc_state, dp_phy)) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s][%s] Failed to update link training\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy));
>> + lt_err(intel_dp, dp_phy, "Failed to update link training\n");
>> return false;
>> }
>>
>> @@ -874,10 +858,8 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
>> }
>>
>> intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s][%s] Failed clock recovery %d times, giving up!\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy), max_cr_tries);
>> + lt_err(intel_dp, dp_phy, "Failed clock recovery %d times, giving up!\n",
>> + max_cr_tries);
>>
>> return false;
>> }
>> @@ -911,11 +893,11 @@ static u32 intel_dp_training_pattern(struct intel_dp *intel_dp,
>> return DP_TRAINING_PATTERN_4;
>> } else if (crtc_state->port_clock == 810000) {
>> if (!source_tps4)
>> - drm_dbg_kms(&i915->drm,
>> - "8.1 Gbps link rate without source TPS4 support\n");
>> + lt_dbg(intel_dp, dp_phy,
>> + "8.1 Gbps link rate without source TPS4 support\n");
>> if (!sink_tps4)
>> - drm_dbg_kms(&i915->drm,
>> - "8.1 Gbps link rate without sink TPS4 support\n");
>> + lt_dbg(intel_dp, dp_phy,
>> + "8.1 Gbps link rate without sink TPS4 support\n");
>> }
>>
>> /*
>> @@ -929,11 +911,11 @@ static u32 intel_dp_training_pattern(struct intel_dp *intel_dp,
>> return DP_TRAINING_PATTERN_3;
>> } else if (crtc_state->port_clock >= 540000) {
>> if (!source_tps3)
>> - drm_dbg_kms(&i915->drm,
>> - ">=5.4/6.48 Gbps link rate without source TPS3 support\n");
>> + lt_dbg(intel_dp, dp_phy,
>> + ">=5.4/6.48 Gbps link rate without source TPS3 support\n");
>> if (!sink_tps3)
>> - drm_dbg_kms(&i915->drm,
>> - ">=5.4/6.48 Gbps link rate without sink TPS3 support\n");
>> + lt_dbg(intel_dp, dp_phy,
>> + ">=5.4/6.48 Gbps link rate without sink TPS3 support\n");
>> }
>>
>> return DP_TRAINING_PATTERN_2;
>> @@ -949,8 +931,6 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state,
>> enum drm_dp_phy dp_phy)
>> {
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> - struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>> int tries;
>> u32 training_pattern;
>> u8 link_status[DP_LINK_STATUS_SIZE];
>> @@ -969,10 +949,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
>> /* channel equalization */
>> if (!intel_dp_set_link_train(intel_dp, crtc_state, dp_phy,
>> training_pattern)) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s][%s] Failed to start channel equalization\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy));
>> + lt_err(intel_dp, dp_phy, "Failed to start channel equalization\n");
>> return false;
>> }
>>
>> @@ -981,10 +958,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
>>
>> if (drm_dp_dpcd_read_phy_link_status(&intel_dp->aux, dp_phy,
>> link_status) < 0) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s][%s] Failed to get link status\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy));
>> + lt_err(intel_dp, dp_phy, "Failed to get link status\n");
>> break;
>> }
>>
>> @@ -992,21 +966,15 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
>> if (!drm_dp_clock_recovery_ok(link_status,
>> crtc_state->lane_count)) {
>> intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s][%s] Clock recovery check failed, cannot "
>> - "continue channel equalization\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy));
>> + lt_dbg(intel_dp, dp_phy,
>> + "Clock recovery check failed, cannot continue channel equalization\n");
>> break;
>> }
>>
>> if (drm_dp_channel_eq_ok(link_status,
>> crtc_state->lane_count)) {
>> channel_eq = true;
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s][%s] Channel EQ done. DP Training successful\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy));
>> + lt_dbg(intel_dp, dp_phy, "Channel EQ done. DP Training successful\n");
>> break;
>> }
>>
>> @@ -1014,10 +982,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
>> intel_dp_get_adjust_train(intel_dp, crtc_state, dp_phy,
>> link_status);
>> if (!intel_dp_update_link_train(intel_dp, crtc_state, dp_phy)) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s][%s] Failed to update link training\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy));
>> + lt_err(intel_dp, dp_phy, "Failed to update link training\n");
>> break;
>> }
>> }
>> @@ -1025,10 +990,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
>> /* Try 5 times, else fail and try at lower BW */
>> if (tries == 5) {
>> intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s][%s] Channel equalization failed 5 times\n",
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy));
>> + lt_dbg(intel_dp, dp_phy, "Channel equalization failed 5 times\n");
>> }
>>
>> return channel_eq;
>> @@ -1047,13 +1009,12 @@ static int
>> intel_dp_128b132b_intra_hop(struct intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state)
>> {
>> - struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> u8 sink_status;
>> int ret;
>>
>> ret = drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_STATUS, &sink_status);
>> if (ret != 1) {
>> - drm_dbg_kms(&i915->drm, "Failed to read sink status\n");
>> + lt_dbg(intel_dp, DP_PHY_DPRX, "Failed to read sink status\n");
>> return ret < 0 ? ret : -EIO;
>> }
>>
>> @@ -1079,9 +1040,6 @@ intel_dp_128b132b_intra_hop(struct intel_dp *intel_dp,
>> void intel_dp_stop_link_train(struct intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state)
>> {
>> - struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> -
>> intel_dp->link_trained = true;
>>
>> intel_dp_disable_dpcd_training_pattern(intel_dp, DP_PHY_DPRX);
>> @@ -1090,9 +1048,7 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp,
>>
>> if (intel_dp_is_uhbr(crtc_state) &&
>> wait_for(intel_dp_128b132b_intra_hop(intel_dp, crtc_state) == 0, 500)) {
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s] 128b/132b intra-hop not clearing\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_dbg(intel_dp, DP_PHY_DPRX, "128b/132b intra-hop not clearing\n");
>> }
>> }
>>
>> @@ -1101,8 +1057,6 @@ intel_dp_link_train_phy(struct intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state,
>> enum drm_dp_phy dp_phy)
>> {
>> - struct intel_connector *connector = intel_dp->attached_connector;
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> bool ret = false;
>>
>> if (!intel_dp_link_training_clock_recovery(intel_dp, crtc_state, dp_phy))
>> @@ -1114,13 +1068,10 @@ intel_dp_link_train_phy(struct intel_dp *intel_dp,
>> ret = true;
>>
>> out:
>> - drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
>> - "[CONNECTOR:%d:%s][ENCODER:%d:%s][%s] Link Training %s at link rate = %d, lane count = %d\n",
>> - connector->base.base.id, connector->base.name,
>> - encoder->base.base.id, encoder->base.name,
>> - drm_dp_phy_name(dp_phy),
>> - ret ? "passed" : "failed",
>> - crtc_state->port_clock, crtc_state->lane_count);
>> + lt_dbg(intel_dp, dp_phy,
>> + "Link Training %s at link rate = %d, lane count = %d\n",
>> + ret ? "passed" : "failed",
>> + crtc_state->port_clock, crtc_state->lane_count);
>>
>> return ret;
>> }
>> @@ -1129,13 +1080,10 @@ static void intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state)
>> {
>> struct intel_connector *intel_connector = intel_dp->attached_connector;
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>>
>> if (intel_dp->hobl_active) {
>> - drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
>> - "[ENCODER:%d:%s] Link Training failed with HOBL active, "
>> - "not enabling it from now on",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_dbg(intel_dp, DP_PHY_DPRX,
>> + "Link Training failed with HOBL active, not enabling it from now on\n");
>> intel_dp->hobl_failed = true;
>> } else if (intel_dp_get_link_train_fallback_values(intel_dp,
>> crtc_state->port_clock,
>> @@ -1182,8 +1130,6 @@ static bool
>> intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state)
>> {
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> - struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>> u8 link_status[DP_LINK_STATUS_SIZE];
>> int delay_us;
>> int try, max_tries = 20;
>> @@ -1198,9 +1144,7 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>> */
>> if (!intel_dp_reset_link_train(intel_dp, crtc_state, DP_PHY_DPRX,
>> DP_TRAINING_PATTERN_1)) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Failed to start 128b/132b TPS1\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Failed to start 128b/132b TPS1\n");
>> return false;
>> }
>>
>> @@ -1208,27 +1152,21 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>>
>> /* Read the initial TX FFE settings. */
>> if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Failed to read TX FFE presets\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Failed to read TX FFE presets\n");
>> return false;
>> }
>>
>> /* Update signal levels and training set as requested. */
>> intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX, link_status);
>> if (!intel_dp_update_link_train(intel_dp, crtc_state, DP_PHY_DPRX)) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Failed to set initial TX FFE settings\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Failed to set initial TX FFE settings\n");
>> return false;
>> }
>>
>> /* Start transmitting 128b/132b TPS2. */
>> if (!intel_dp_set_link_train(intel_dp, crtc_state, DP_PHY_DPRX,
>> DP_TRAINING_PATTERN_2)) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Failed to start 128b/132b TPS2\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Failed to start 128b/132b TPS2\n");
>> return false;
>> }
>>
>> @@ -1245,32 +1183,25 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>> delay_us = drm_dp_128b132b_read_aux_rd_interval(&intel_dp->aux);
>>
>> if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Failed to read link status\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
>> return false;
>> }
>>
>> if (drm_dp_128b132b_link_training_failed(link_status)) {
>> intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Downstream link training failure\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX,
>> + "Downstream link training failure\n");
>> return false;
>> }
>>
>> if (drm_dp_128b132b_lane_channel_eq_done(link_status, crtc_state->lane_count)) {
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s] Lane channel eq done\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_dbg(intel_dp, DP_PHY_DPRX, "Lane channel eq done\n");
>> break;
>> }
>>
>> if (timeout) {
>> intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Lane channel eq timeout\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Lane channel eq timeout\n");
>> return false;
>> }
>>
>> @@ -1280,18 +1211,14 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>> /* Update signal levels and training set as requested. */
>> intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX, link_status);
>> if (!intel_dp_update_link_train(intel_dp, crtc_state, DP_PHY_DPRX)) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Failed to update TX FFE settings\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Failed to update TX FFE settings\n");
>> return false;
>> }
>> }
>>
>> if (try == max_tries) {
>> intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Max loop count reached\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Max loop count reached\n");
>> return false;
>> }
>>
>> @@ -1300,32 +1227,24 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>> timeout = true; /* try one last time after deadline */
>>
>> if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Failed to read link status\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
>> return false;
>> }
>>
>> if (drm_dp_128b132b_link_training_failed(link_status)) {
>> intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Downstream link training failure\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Downstream link training failure\n");
>> return false;
>> }
>>
>> if (drm_dp_128b132b_eq_interlane_align_done(link_status)) {
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s] Interlane align done\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_dbg(intel_dp, DP_PHY_DPRX, "Interlane align done\n");
>> break;
>> }
>>
>> if (timeout) {
>> intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Interlane align timeout\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Interlane align timeout\n");
>> return false;
>> }
>>
>> @@ -1343,16 +1262,12 @@ intel_dp_128b132b_lane_cds(struct intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state,
>> int lttpr_count)
>> {
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> - struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>> u8 link_status[DP_LINK_STATUS_SIZE];
>> unsigned long deadline;
>>
>> if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TRAINING_PATTERN_SET,
>> DP_TRAINING_PATTERN_2_CDS) != 1) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Failed to start 128b/132b TPS2 CDS\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Failed to start 128b/132b TPS2 CDS\n");
>> return false;
>> }
>>
>> @@ -1368,34 +1283,26 @@ intel_dp_128b132b_lane_cds(struct intel_dp *intel_dp,
>> usleep_range(2000, 3000);
>>
>> if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Failed to read link status\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
>> return false;
>> }
>>
>> if (drm_dp_128b132b_eq_interlane_align_done(link_status) &&
>> drm_dp_128b132b_cds_interlane_align_done(link_status) &&
>> drm_dp_128b132b_lane_symbol_locked(link_status, crtc_state->lane_count)) {
>> - drm_dbg_kms(&i915->drm,
>> - "[ENCODER:%d:%s] CDS interlane align done\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "CDS interlane align done\n");
>> break;
>> }
>>
>> if (drm_dp_128b132b_link_training_failed(link_status)) {
>> intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] Downstream link training failure\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "Downstream link training failure\n");
>> return false;
>> }
>>
>> if (timeout) {
>> intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] CDS timeout\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "CDS timeout\n");
>> return false;
>> }
>> }
>> @@ -1411,15 +1318,10 @@ intel_dp_128b132b_link_train(struct intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state,
>> int lttpr_count)
>> {
>> - struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> - struct intel_connector *connector = intel_dp->attached_connector;
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> bool passed = false;
>>
>> if (wait_for(intel_dp_128b132b_intra_hop(intel_dp, crtc_state) == 0, 500)) {
>> - drm_err(&i915->drm,
>> - "[ENCODER:%d:%s] 128b/132b intra-hop not clear\n",
>> - encoder->base.base.id, encoder->base.name);
>> + lt_err(intel_dp, DP_PHY_DPRX, "128b/132b intra-hop not clear\n");
>> return false;
>> }
>>
>> @@ -1427,12 +1329,10 @@ intel_dp_128b132b_link_train(struct intel_dp *intel_dp,
>> intel_dp_128b132b_lane_cds(intel_dp, crtc_state, lttpr_count))
>> passed = true;
>>
>> - drm_dbg_kms(&i915->drm,
>> - "[CONNECTOR:%d:%s][ENCODER:%d:%s] 128b/132b Link Training %s at link rate = %d, lane count = %d\n",
>> - connector->base.base.id, connector->base.name,
>> - encoder->base.base.id, encoder->base.name,
>> - passed ? "passed" : "failed",
>> - crtc_state->port_clock, crtc_state->lane_count);
>> + lt_dbg(intel_dp, DP_PHY_DPRX,
>> + "128b/132b Link Training %s at link rate = %d, lane count = %d\n",
>> + passed ? "passed" : "failed",
>> + crtc_state->port_clock, crtc_state->lane_count);
>>
>> return passed;
>> }
>> @@ -1451,8 +1351,6 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state)
>> {
>> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> - struct intel_connector *connector = intel_dp->attached_connector;
>> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> bool passed;
>>
>> /*
>> @@ -1485,10 +1383,7 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
>> * ignore_long_hpd flag can unset from the testcase.
>> */
>> if (!passed && i915->display.hotplug.ignore_long_hpd) {
>> - drm_dbg_kms(&i915->drm,
>> - "[CONNECTOR:%d:%s][ENCODER:%d:%s] Ignore the link failure\n",
>> - connector->base.base.id, connector->base.name,
>> - encoder->base.base.id, encoder->base.name);
>> + lt_dbg(intel_dp, DP_PHY_DPRX, "Ignore the link failure\n");
>> return;
>> }
>>
>> @@ -1499,8 +1394,6 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
>> void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state)
>> {
>> - struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> -
>> /*
>> * VIDEO_DIP_CTL register bit 31 should be set to '0' to not
>> * disable SDP CRC. This is applicable for Display version 13.
>> @@ -1513,5 +1406,5 @@ void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
>> DP_SDP_ERROR_DETECTION_CONFIGURATION,
>> DP_SDP_CRC16_128B132B_EN);
>>
>> - drm_dbg_kms(&i915->drm, "DP2.0 SDP CRC16 for 128b/132b enabled\n");
>> + lt_dbg(intel_dp, DP_PHY_DPRX, "DP2.0 SDP CRC16 for 128b/132b enabled\n");
>> }
>> --
>> 2.37.2
--
Jani Nikula, Intel Open Source Graphics Center
