From: Jimmy Kizito <Jimmy.Kizito@xxxxxxx>
[Why]
In some scenarios it is possible for the encoder assignment module to be
set to "transient" mode even though there are no new encoder
assignments.
This can lead to incorrect results when querying encoder assignment,
which in turn can cause incorrect displays to be manipulated.
[How]
Only allow encoder assignment to be in transient mode of operation when
there are valid new encoder assignments.
Reviewed-by: Meenakshikumar Somasundaram <Meenakshikumar.Somasundaram@xxxxxxx>
Reviewed-by: Jun Lei <Jun.Lei@xxxxxxx>
Acked-by: Hamza Mahfooz <hamza.mahfooz@xxxxxxx>
Signed-off-by: Jimmy Kizito <Jimmy.Kizito@xxxxxxx>
---
.../gpu/drm/amd/display/dc/core/dc_link_dp.c | 17 ++++---
.../drm/amd/display/dc/core/dc_link_enc_cfg.c | 45 ++++++++++++++++++-
.../display/dc/dcn31/dcn31_dio_link_encoder.c | 6 +++
.../drm/amd/display/dc/dcn31/dcn31_hwseq.c | 2 +-
.../gpu/drm/amd/display/dc/inc/link_enc_cfg.h | 7 +++
5 files changed, 69 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index b70fdab5a97f..9b20f340c21f 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -2812,8 +2812,8 @@ bool perform_link_training_with_retries(
j = 0;
while (j < attempts && fail_count < (attempts * 10)) {
- DC_LOG_HW_LINK_TRAINING("%s: Beginning link training attempt %u of %d @ rate(%d) x lane(%d)\n",
- __func__, (unsigned int)j + 1, attempts, cur_link_settings.link_rate,
+ DC_LOG_HW_LINK_TRAINING("%s: Beginning link(%d) training attempt %u of %d @ rate(%d) x lane(%d)\n",
+ __func__, link->link_index, (unsigned int)j + 1, attempts, cur_link_settings.link_rate,
cur_link_settings.lane_count);
dp_enable_link_phy(
@@ -2883,8 +2883,8 @@ bool perform_link_training_with_retries(
break;
}
- DC_LOG_WARNING("%s: Link training attempt %u of %d failed @ rate(%d) x lane(%d)\n",
- __func__, (unsigned int)j + 1, attempts, cur_link_settings.link_rate,
+ DC_LOG_WARNING("%s: Link(%d) training attempt %u of %d failed @ rate(%d) x lane(%d)\n",
+ __func__, link->link_index, (unsigned int)j + 1, attempts, cur_link_settings.link_rate,
cur_link_settings.lane_count);
dp_disable_link_phy(link, &pipe_ctx->link_res, signal);
@@ -2927,8 +2927,13 @@ bool perform_link_training_with_retries(
*/
req_bw = dc_bandwidth_in_kbps_from_timing(&stream->timing);
link_bw = dc_link_bandwidth_kbps(link, &cur_link_settings);
- if (req_bw > link_bw)
- break;
+ is_link_bw_low = (req_bw > link_bw);
+ is_link_bw_min = ((cur_link_settings.link_rate <= LINK_RATE_LOW) &&
+ (cur_link_settings.lane_count <= LANE_COUNT_ONE));
+ if (is_link_bw_low)
+ DC_LOG_WARNING(
+ "%s: Link(%d) bandwidth too low after fallback req_bw(%d) > link_bw(%d)\n",
+ __func__, link->link_index, req_bw, link_bw);
}
msleep(delay_between_attempts);
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
index 42da7f430113..639a0a276a08 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
@@ -26,6 +26,8 @@
#include "resource.h"
#include "dc_link_dp.h"
+#define DC_LOGGER dc->ctx->logger
+
/* Check whether stream is supported by DIG link encoders. */
static bool is_dig_link_enc_stream(struct dc_stream_state *stream)
{
@@ -383,6 +385,30 @@ void link_enc_cfg_link_encs_assign(
state->res_ctx.link_enc_cfg_ctx.link_enc_assignments[i];
}
+ /* Log encoder assignments. */
+ for (i = 0; i < MAX_PIPES; i++) {
+ struct link_enc_assignment assignment =
+ dc->current_state->res_ctx.link_enc_cfg_ctx.link_enc_assignments[i];
+
+ if (assignment.valid)
+ DC_LOG_DEBUG("%s: CUR %s(%d) - enc_id(%d)\n",
+ __func__,
+ assignment.ep_id.ep_type == DISPLAY_ENDPOINT_PHY ? "PHY" : "DPIA",
+ assignment.ep_id.link_id.enum_id - 1,
+ assignment.eng_id);
+ }
+ for (i = 0; i < MAX_PIPES; i++) {
+ struct link_enc_assignment assignment =
+ state->res_ctx.link_enc_cfg_ctx.link_enc_assignments[i];
+
+ if (assignment.valid)
+ DC_LOG_DEBUG("%s: NEW %s(%d) - enc_id(%d)\n",
+ __func__,
+ assignment.ep_id.ep_type == DISPLAY_ENDPOINT_PHY ? "PHY" : "DPIA",
+ assignment.ep_id.link_id.enum_id - 1,
+ assignment.eng_id);
+ }
+
/* Current state mode will be set to steady once this state committed. */
state->res_ctx.link_enc_cfg_ctx.mode = LINK_ENC_CFG_STEADY;
}
@@ -658,8 +684,25 @@ bool link_enc_cfg_validate(struct dc *dc, struct dc_state *state)
((valid_uniqueness & 0x1) << 2) |
((valid_avail & 0x1) << 3) |
((valid_streams & 0x1) << 4);
- dm_error("Invalid link encoder assignments: 0x%x\n", valid_bitmap);
+ DC_LOG_ERROR("%s: Invalid link encoder assignments - 0x%x\n", __func__, valid_bitmap);
}
return is_valid;
}
+
+void link_enc_cfg_set_transient_mode(struct dc *dc, struct dc_state *current_state, struct dc_state *new_state)
+{
+ int i = 0;
+ int num_transient_assignments = 0;
+
+ for (i = 0; i < MAX_PIPES; i++) {
+ if (current_state->res_ctx.link_enc_cfg_ctx.transient_assignments[i].valid)
+ num_transient_assignments++;
+ }
+
+ /* Only enter transient mode if the new encoder assignments are valid. */
+ if (new_state->stream_count == num_transient_assignments) {
+ current_state->res_ctx.link_enc_cfg_ctx.mode = LINK_ENC_CFG_TRANSIENT;
+ DC_LOG_DEBUG("%s: current_state(%p) mode(%d)\n", __func__, current_state, LINK_ENC_CFG_TRANSIENT);
+ }
+}
diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c
index 8b12b4111c88..a788d160953b 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c
@@ -458,6 +458,7 @@ void dcn31_link_encoder_enable_dp_output(
/* Enable transmitter and encoder. */
if (!link_enc_cfg_is_transmitter_mappable(enc->ctx->dc, enc)) {
+ DC_LOG_DEBUG("%s: enc_id(%d)\n", __func__, enc->preferred_engine);
dcn20_link_encoder_enable_dp_output(enc, link_settings, clock_source);
} else {
@@ -489,6 +490,7 @@ void dcn31_link_encoder_enable_dp_output(
return;
}
+ DC_LOG_DEBUG("%s: DPIA(%d) - enc_id(%d)\n", __func__, dpia_control.dpia_id, dpia_control.enc_id);
link_dpia_control(enc->ctx, &dpia_control);
}
}
@@ -503,6 +505,7 @@ void dcn31_link_encoder_enable_dp_mst_output(
/* Enable transmitter and encoder. */
if (!link_enc_cfg_is_transmitter_mappable(enc->ctx->dc, enc)) {
+ DC_LOG_DEBUG("%s: enc_id(%d)\n", __func__, enc->preferred_engine);
dcn10_link_encoder_enable_dp_mst_output(enc, link_settings, clock_source);
} else {
@@ -534,6 +537,7 @@ void dcn31_link_encoder_enable_dp_mst_output(
return;
}
+ DC_LOG_DEBUG("%s: DPIA(%d) - enc_id(%d)\n", __func__, dpia_control.dpia_id, dpia_control.enc_id);
link_dpia_control(enc->ctx, &dpia_control);
}
}
@@ -547,6 +551,7 @@ void dcn31_link_encoder_disable_output(
/* Disable transmitter and encoder. */
if (!link_enc_cfg_is_transmitter_mappable(enc->ctx->dc, enc)) {
+ DC_LOG_DEBUG("%s: enc_id(%d)\n", __func__, enc->preferred_engine);
dcn10_link_encoder_disable_output(enc, signal);
} else {
@@ -578,6 +583,7 @@ void dcn31_link_encoder_disable_output(
return;
}
+ DC_LOG_DEBUG("%s: DPIA(%d) - enc_id(%d)\n", __func__, dpia_control.dpia_id, dpia_control.enc_id);
link_dpia_control(enc->ctx, &dpia_control);
link_encoder_disable(enc10);
diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
index 55f2e30b8e5a..1ed1404e969d 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
@@ -613,7 +613,7 @@ void dcn31_reset_hw_ctx_wrap(
}
/* New dc_state in the process of being applied to hardware. */
- dc->current_state->res_ctx.link_enc_cfg_ctx.mode = LINK_ENC_CFG_TRANSIENT;
+ link_enc_cfg_set_transient_mode(dc, dc->current_state, context);
}
void dcn31_setup_hpo_hw_control(const struct dce_hwseq *hws, bool enable)
diff --git a/drivers/gpu/drm/amd/display/dc/inc/link_enc_cfg.h b/drivers/gpu/drm/amd/display/dc/inc/link_enc_cfg.h
index c6f6baa6e677..7beb14169f92 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/link_enc_cfg.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/link_enc_cfg.h
@@ -110,4 +110,11 @@ bool link_enc_cfg_is_link_enc_avail(struct dc *dc, enum engine_id eng_id, struct
/* Returns true if encoder assignments in supplied state pass validity checks. */
bool link_enc_cfg_validate(struct dc *dc, struct dc_state *state);
+/* Set the link encoder assignment mode for the current_state to LINK_ENC_CFG_TRANSIENT mode.
+ * This indicates that a new_state is in the process of being applied to hardware.
+ * During this transition, old and new encoder assignments should be accessible from the old_state.
+ * Only allow transition into transient mode if new encoder assignments are valid.
+ */
+void link_enc_cfg_set_transient_mode(struct dc *dc, struct dc_state *current_state, struct dc_state *new_state);
+
#endif /* DC_INC_LINK_ENC_CFG_H_ */
--
2.36.1
