drm/amd/display: Fix entry into transient encoder assignment mode.

[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@amd.com>
Reviewed-by: Jun Lei <Jun.Lei@amd.com>
Acked-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
Signed-off-by: Jimmy Kizito <Jimmy.Kizito@amd.com>
Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

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 0496828e86739da07cb9c43e1b899ade36b20d11..c1207049dbc599835a7e9182fdbb6e0821d54816 100644 (file)
--- 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 42da7f4301132f92b9787f815daff7355787bdd0..639a0a276a08eb336648f44764e09c7c5a434680 100644 (file)
--- 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 8b12b4111c8870a54bda0561c8d5b076720e83d9..a788d160953b5e3689708bac5ff864b034ea80c2 100644 (file)
--- 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 55f2e30b8e5aaf1c0bab7e9f747c7b545c51c90f..1ed1404e969df2e82ab14fb2f56860527e6fb8a0 100644 (file)
--- 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 c6f6baa6e6774d7e0cee3ecd4fdf7a736626cffd..7beb14169f92c8e422d65eb2794f9b062beb06e2 100644 (file)
--- 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_ */