drm/amd/display: make enable link independent from verified link caps
authorWenjing Liu <wenjing.liu@amd.com>
Mon, 13 Jun 2022 15:45:31 +0000 (11:45 -0400)
committerAlex Deucher <alexander.deucher@amd.com>
Wed, 13 Jul 2022 15:25:16 +0000 (11:25 -0400)
[why]
Ideally link capability should be independent from the link
configuration that we decide to use in enable link. Otherwise if link
capability is changed after validation has completed, we could end up
enabling a link configuration with invalid configuration. This would
lead to over link bandwidth subscription or in the extreme case
causes us to enable HPO link to a DIO stream.

[how]
Add a new struct in pipe ctx called link config. This structure will
contain link configuration to enable a link. It will be populated
during map pool resources after we validate link bandwidth. Remove
the reference of verified link cap during enable link process and
use link config in pipe ctx instead.

Reviewed-by: George Shen <George.Shen@amd.com>
Acked-by: Solomon Chiu <solomon.chiu@amd.com>
Signed-off-by: Wenjing Liu <wenjing.liu@amd.com>
Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
drivers/gpu/drm/amd/display/dc/core/dc_debug.c
drivers/gpu/drm/amd/display/dc/core/dc_link.c
drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
drivers/gpu/drm/amd/display/dc/core/dc_resource.c
drivers/gpu/drm/amd/display/dc/inc/core_status.h
drivers/gpu/drm/amd/display/dc/inc/core_types.h
drivers/gpu/drm/amd/display/dc/inc/dc_link_dp.h

index 283957dbdf93af4dd79b58afe611b9eedad7f114..69f1c2b89a573e7a3cf4321f74b3a9bb21fa8fb6 100644 (file)
@@ -424,6 +424,8 @@ char *dc_status_to_str(enum dc_status status)
                return "No link encoder resource";
        case DC_FAIL_DP_PAYLOAD_ALLOCATION:
                return "Fail dp payload allocation";
+       case DC_FAIL_DP_LINK_BANDWIDTH:
+               return "Insufficient DP link bandwidth";
        case DC_ERROR_UNEXPECTED:
                return "Unexpected error";
        }
index dbdeda60e9e25f90f77ebc17bb226f27890fb03d..65269cd8cb787b3487d34f9c08d00f174b27c293 100644 (file)
@@ -1969,7 +1969,8 @@ static enum dc_status enable_link_dp(struct dc_state *state,
        enum dc_status status;
        bool skip_video_pattern;
        struct dc_link *link = stream->link;
-       struct dc_link_settings link_settings = {0};
+       const struct dc_link_settings *link_settings =
+                       &pipe_ctx->link_config.dp_link_settings;
        bool fec_enable;
        int i;
        bool apply_seamless_boot_optimization = false;
@@ -1986,9 +1987,6 @@ static enum dc_status enable_link_dp(struct dc_state *state,
                }
        }
 
-       /* get link settings for video mode timing */
-       decide_link_settings(stream, &link_settings);
-
        /* Train with fallback when enabling DPIA link. Conventional links are
         * trained with fallback during sink detection.
         */
@@ -1999,7 +1997,7 @@ static enum dc_status enable_link_dp(struct dc_state *state,
         * Temporary w/a to get DP2.0 link rates to work with SST.
         * TODO DP2.0 - Workaround: Remove w/a if and when the issue is resolved.
         */
-       if (dp_get_link_encoding_format(&link_settings) == DP_128b_132b_ENCODING &&
+       if (dp_get_link_encoding_format(link_settings) == DP_128b_132b_ENCODING &&
                        pipe_ctx->stream->signal == SIGNAL_TYPE_DISPLAY_PORT &&
                        link->dc->debug.set_mst_en_for_sst) {
                dp_enable_mst_on_sink(link, true);
@@ -2012,11 +2010,11 @@ static enum dc_status enable_link_dp(struct dc_state *state,
                link->dc->hwss.edp_wait_for_hpd_ready(link, true);
        }
 
-       if (dp_get_link_encoding_format(&link_settings) == DP_128b_132b_ENCODING) {
+       if (dp_get_link_encoding_format(link_settings) == DP_128b_132b_ENCODING) {
                /* TODO - DP2.0 HW: calculate 32 symbol clock for HPO encoder */
        } else {
                pipe_ctx->stream_res.pix_clk_params.requested_sym_clk =
-                               link_settings.link_rate * LINK_RATE_REF_FREQ_IN_KHZ;
+                               link_settings->link_rate * LINK_RATE_REF_FREQ_IN_KHZ;
                if (state->clk_mgr && !apply_seamless_boot_optimization)
                        state->clk_mgr->funcs->update_clocks(state->clk_mgr,
                                        state, false);
@@ -2032,16 +2030,15 @@ static enum dc_status enable_link_dp(struct dc_state *state,
 
        skip_video_pattern = true;
 
-       if (link_settings.link_rate == LINK_RATE_LOW)
+       if (link_settings->link_rate == LINK_RATE_LOW)
                skip_video_pattern = false;
 
-       if (perform_link_training_with_retries(&link_settings,
+       if (perform_link_training_with_retries(link_settings,
                                               skip_video_pattern,
                                               LINK_TRAINING_ATTEMPTS,
                                               pipe_ctx,
                                               pipe_ctx->stream->signal,
                                               do_fallback)) {
-               link->cur_link_settings = link_settings;
                status = DC_OK;
        } else {
                status = DC_FAIL_DP_LINK_TRAINING;
@@ -2052,7 +2049,7 @@ static enum dc_status enable_link_dp(struct dc_state *state,
        else
                fec_enable = true;
 
-       if (dp_get_link_encoding_format(&link_settings) == DP_8b_10b_ENCODING)
+       if (dp_get_link_encoding_format(link_settings) == DP_8b_10b_ENCODING)
                dp_set_fec_enable(link, fec_enable);
 
        // during mode set we do DP_SET_POWER off then on, aux writes are lost
@@ -4124,11 +4121,10 @@ static void fpga_dp_hpo_enable_link_and_stream(struct dc_state *state, struct pi
        struct fixed31_32 avg_time_slots_per_mtp;
        uint8_t req_slot_count = 0;
        uint8_t vc_id = 1; /// VC ID always 1 for SST
-       struct dc_link_settings link_settings = {0};
+       struct dc_link_settings link_settings = pipe_ctx->link_config.dp_link_settings;
        const struct link_hwss *link_hwss = get_link_hwss(stream->link, &pipe_ctx->link_res);
        DC_LOGGER_INIT(pipe_ctx->stream->ctx->logger);
 
-       decide_link_settings(stream, &link_settings);
        stream->link->cur_link_settings = link_settings;
 
        if (link_hwss->ext.enable_dp_link_output)
@@ -4602,10 +4598,7 @@ void dc_link_set_preferred_link_settings(struct dc *dc,
        if (link_stream->dpms_off)
                return;
 
-       decide_link_settings(link_stream, &store_settings);
-
-       if ((store_settings.lane_count != LANE_COUNT_UNKNOWN) &&
-               (store_settings.link_rate != LINK_RATE_UNKNOWN))
+       if (decide_link_settings(link_stream, &store_settings))
                dp_retrain_link_dp_test(link, &store_settings, false);
 }
 
index a126921c664ac77cace0584af88f966a7381fe90..d380b8bc6f3952ce15d5b432b2cbff61b6985d23 100644 (file)
@@ -3880,15 +3880,13 @@ static bool decide_mst_link_settings(const struct dc_link *link, struct dc_link_
        return true;
 }
 
-void decide_link_settings(struct dc_stream_state *stream,
+bool decide_link_settings(struct dc_stream_state *stream,
        struct dc_link_settings *link_setting)
 {
-       struct dc_link *link;
-       uint32_t req_bw;
-
-       req_bw = dc_bandwidth_in_kbps_from_timing(&stream->timing);
+       struct dc_link *link = stream->link;
+       uint32_t req_bw = dc_bandwidth_in_kbps_from_timing(&stream->timing);
 
-       link = stream->link;
+       memset(link_setting, 0, sizeof(*link_setting));
 
        /* if preferred is specified through AMDDP, use it, if it's enough
         * to drive the mode
@@ -3897,16 +3895,15 @@ void decide_link_settings(struct dc_stream_state *stream,
                        LANE_COUNT_UNKNOWN &&
                        link->preferred_link_setting.link_rate !=
                                        LINK_RATE_UNKNOWN) {
-               *link_setting =  link->preferred_link_setting;
-               return;
+               *link_setting = link->preferred_link_setting;
+               return true;
        }
 
        /* MST doesn't perform link training for now
         * TODO: add MST specific link training routine
         */
        if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST) {
-               if (decide_mst_link_settings(link, link_setting))
-                       return;
+               decide_mst_link_settings(link, link_setting);
        } else if (link->connector_signal == SIGNAL_TYPE_EDP) {
                /* enable edp link optimization for DSC eDP case */
                if (stream->timing.flags.DSC) {
@@ -3924,17 +3921,16 @@ void decide_link_settings(struct dc_stream_state *stream,
                                decide_edp_link_settings(link, &tmp_link_setting, orig_req_bw);
                                max_link_rate = tmp_link_setting.link_rate;
                        }
-                       if (decide_edp_link_settings_with_dsc(link, link_setting, req_bw, max_link_rate))
-                               return;
-               } else if (decide_edp_link_settings(link, link_setting, req_bw))
-                       return;
-       } else if (decide_dp_link_settings(link, link_setting, req_bw))
-               return;
-
-       BREAK_TO_DEBUGGER();
-       ASSERT(link->verified_link_cap.lane_count != LANE_COUNT_UNKNOWN);
+                       decide_edp_link_settings_with_dsc(link, link_setting, req_bw, max_link_rate);
+               } else {
+                       decide_edp_link_settings(link, link_setting, req_bw);
+               }
+       } else {
+               decide_dp_link_settings(link, link_setting, req_bw);
+       }
 
-       *link_setting = link->verified_link_cap;
+       return link_setting->lane_count != LANE_COUNT_UNKNOWN &&
+                       link_setting->link_rate != LINK_RATE_UNKNOWN;
 }
 
 /*************************Short Pulse IRQ***************************/
@@ -4509,7 +4505,6 @@ void dc_link_dp_handle_link_loss(struct dc_link *link)
 {
        int i;
        struct pipe_ctx *pipe_ctx;
-       struct dc_link_settings prev_link_settings = link->preferred_link_setting;
 
        for (i = 0; i < MAX_PIPES; i++) {
                pipe_ctx = &link->dc->current_state->res_ctx.pipe_ctx[i];
@@ -4520,10 +4515,6 @@ void dc_link_dp_handle_link_loss(struct dc_link *link)
        if (pipe_ctx == NULL || pipe_ctx->stream == NULL)
                return;
 
-       /* toggle stream state with the preference for current link settings */
-       dc_link_set_preferred_training_settings((struct dc *)link->dc,
-                                       &link->cur_link_settings, NULL, link, true);
-
        for (i = 0; i < MAX_PIPES; i++) {
                pipe_ctx = &link->dc->current_state->res_ctx.pipe_ctx[i];
                if (pipe_ctx && pipe_ctx->stream && !pipe_ctx->stream->dpms_off &&
@@ -4539,10 +4530,6 @@ void dc_link_dp_handle_link_loss(struct dc_link *link)
                        core_link_enable_stream(link->dc->current_state, pipe_ctx);
                }
        }
-
-       /* restore previous link settings preference */
-       dc_link_set_preferred_training_settings((struct dc *)link->dc,
-                                       &prev_link_settings, NULL, link, true);
 }
 
 bool dc_link_handle_hpd_rx_irq(struct dc_link *link, union hpd_irq_data *out_hpd_irq_dpcd_data, bool *out_link_loss,
index 9eb7e702762260c51c063d3b4fade0df7339b534..9db50ed5460bb80dc9804f815bc42c86ac063991 100644 (file)
@@ -2374,12 +2374,10 @@ enum dc_status resource_map_pool_resources(
        /* Allocate DP HPO Stream Encoder based on signal, hw capabilities
         * and link settings
         */
-       if (dc_is_dp_signal(stream->signal) &&
-                       dc->caps.dp_hpo) {
-               struct dc_link_settings link_settings = {0};
-
-               decide_link_settings(stream, &link_settings);
-               if (dp_get_link_encoding_format(&link_settings) == DP_128b_132b_ENCODING) {
+       if (dc_is_dp_signal(stream->signal)) {
+               if (!decide_link_settings(stream, &pipe_ctx->link_config.dp_link_settings))
+                       return DC_FAIL_DP_LINK_BANDWIDTH;
+               if (dp_get_link_encoding_format(&pipe_ctx->link_config.dp_link_settings) == DP_128b_132b_ENCODING) {
                        pipe_ctx->stream_res.hpo_dp_stream_enc =
                                        find_first_free_match_hpo_dp_stream_enc_for_link(
                                                        &context->res_ctx, pool, stream);
index 8eb8d4afa876e810ef0ea145979ec6976947372c..fa5edd03d00439e751a5fb021ed174f0e1bbad2f 100644 (file)
@@ -55,6 +55,7 @@ enum dc_status {
 
        DC_NO_LINK_ENC_RESOURCE = 26,
        DC_FAIL_DP_PAYLOAD_ALLOCATION = 27,
+       DC_FAIL_DP_LINK_BANDWIDTH = 28,
        DC_ERROR_UNEXPECTED = -1
 };
 
index 078c0c3ca2c56c3711955ad7d0b19ad54b18a98f..2e158b1f5980db7491a52c20d19cd155522a86db 100644 (file)
@@ -342,6 +342,9 @@ struct link_resource {
        struct hpo_dp_link_encoder *hpo_dp_link_enc;
 };
 
+struct link_config {
+       struct dc_link_settings dp_link_settings;
+};
 union pipe_update_flags {
        struct {
                uint32_t enable : 1;
@@ -375,6 +378,13 @@ struct pipe_ctx {
 
        struct pll_settings pll_settings;
 
+       /* link config records software decision for what link config should be
+        * enabled given current link capability and stream during hw resource
+        * mapping. This is to decouple the dependency on link capability during
+        * dc commit or update.
+        */
+       struct link_config link_config;
+
        uint8_t pipe_idx;
        uint8_t pipe_idx_syncd;
 
index aaa17d4dd46b925e1292b26fb7935fd9ea9a3b17..b44c7b43f7db2175dbb0cea9ceec43341ccbbb6e 100644 (file)
@@ -70,7 +70,7 @@ bool decide_edp_link_settings(struct dc_link *link,
                struct dc_link_settings *link_setting,
                uint32_t req_bw);
 
-void decide_link_settings(
+bool decide_link_settings(
        struct dc_stream_state *stream,
        struct dc_link_settings *link_setting);