OSDN Git Service

drm/amd/display: Fix dynamic link encoder access.
authorJimmy Kizito <Jimmy.Kizito@amd.com>
Tue, 10 Aug 2021 23:07:43 +0000 (19:07 -0400)
committerAlex Deucher <alexander.deucher@amd.com>
Tue, 14 Sep 2021 19:57:09 +0000 (15:57 -0400)
[Why]
Assuming DIG link encoders are statically mapped to links can cause
system instability due to null pointer accesses.

[How]
- Add checks for non-null link encoder pointers before trying to access
them.
- When a hardware platform uses dynamic DIG assignment (i.e. resource
function 'link_encs_assign' defined) and a link supports flexible
mapping to DIGs, use the link_enc_cfg API to access the DIG assigned to
a link or stream.

Reviewed-by: Meenakshikumar Somasundaram <meenakshikumar.somasundaram@amd.com>
Acked-by: Mikita Lipski <mikita.lipski@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>
12 files changed:
drivers/gpu/drm/amd/display/dc/clk_mgr/dcn31/dcn31_clk_mgr.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_link_enc_cfg.c
drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
drivers/gpu/drm/amd/display/dc/core/dc_resource.c
drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
drivers/gpu/drm/amd/display/dc/inc/link_enc_cfg.h

index 15491e3..1414da4 100644 (file)
@@ -87,7 +87,7 @@ int dcn31_get_active_display_cnt_wa(
                const struct dc_link *link = dc->links[i];
 
                /* abusing the fact that the dig and phy are coupled to see if the phy is enabled */
-               if (link->link_enc->funcs->is_dig_enabled &&
+               if (link->link_enc && link->link_enc->funcs->is_dig_enabled &&
                                link->link_enc->funcs->is_dig_enabled(link->link_enc))
                        display_count++;
        }
index 5d9460e..3c8eb3e 100644 (file)
@@ -3457,6 +3457,10 @@ static enum dc_status deallocate_mst_payload(struct pipe_ctx *pipe_ctx)
 static void update_psp_stream_config(struct pipe_ctx *pipe_ctx, bool dpms_off)
 {
        struct cp_psp *cp_psp = &pipe_ctx->stream->ctx->cp_psp;
+#if defined(CONFIG_DRM_AMD_DC_DCN)
+       struct link_encoder *link_enc = NULL;
+#endif
+
        if (cp_psp && cp_psp->funcs.update_stream_config) {
                struct cp_psp_stream_config config = {0};
                enum dp_panel_mode panel_mode =
@@ -3468,8 +3472,21 @@ static void update_psp_stream_config(struct pipe_ctx *pipe_ctx, bool dpms_off)
                config.dig_be = pipe_ctx->stream->link->link_enc_hw_inst;
 #if defined(CONFIG_DRM_AMD_DC_DCN)
                config.stream_enc_idx = pipe_ctx->stream_res.stream_enc->id - ENGINE_ID_DIGA;
-               config.link_enc_idx = pipe_ctx->stream->link->link_enc->transmitter - TRANSMITTER_UNIPHY_A;
-               config.phy_idx = pipe_ctx->stream->link->link_enc->transmitter - TRANSMITTER_UNIPHY_A;
+               if (pipe_ctx->stream->link->ep_type == DISPLAY_ENDPOINT_PHY) {
+                       link_enc = pipe_ctx->stream->link->link_enc;
+                       config.phy_idx = link_enc->transmitter - TRANSMITTER_UNIPHY_A;
+               } else if (pipe_ctx->stream->link->dc->res_pool->funcs->link_encs_assign) {
+                       /* Use link encoder assignment from current DC state - which may differ from the DC state to be
+                        * committed - when updating PSP config.
+                        */
+                       link_enc = link_enc_cfg_get_link_enc_used_by_stream(
+                                       pipe_ctx->stream->link->dc->current_state,
+                                       pipe_ctx->stream);
+                       config.phy_idx = 0; /* Clear phy_idx for non-physical display endpoints. */
+               }
+               ASSERT(link_enc);
+               if (link_enc)
+                       config.link_enc_idx = link_enc->transmitter - TRANSMITTER_UNIPHY_A;
                if (is_dp_128b_132b_signal(pipe_ctx)) {
                        config.stream_enc_idx = pipe_ctx->stream_res.hpo_dp_stream_enc->id - ENGINE_ID_HPO_DP_0;
                        config.link_enc_idx = pipe_ctx->stream->link->hpo_dp_link_enc->inst;
@@ -3576,6 +3593,7 @@ void core_link_enable_stream(
        struct dc_stream_state *stream = pipe_ctx->stream;
        struct dc_link *link = stream->sink->link;
        enum dc_status status;
+       struct link_encoder *link_enc;
 #if defined(CONFIG_DRM_AMD_DC_DCN)
        enum otg_out_mux_dest otg_out_dest = OUT_MUX_DIO;
 #endif
@@ -3585,15 +3603,22 @@ void core_link_enable_stream(
                        dc_is_virtual_signal(pipe_ctx->stream->signal))
                return;
 
+       if (dc->res_pool->funcs->link_encs_assign && stream->link->ep_type != DISPLAY_ENDPOINT_PHY)
+               link_enc = stream->link_enc;
+       else
+               link_enc = stream->link->link_enc;
+       ASSERT(link_enc);
+
 #if defined(CONFIG_DRM_AMD_DC_DCN)
        if (!dc_is_virtual_signal(pipe_ctx->stream->signal)
                        && !is_dp_128b_132b_signal(pipe_ctx)) {
 #else
        if (!dc_is_virtual_signal(pipe_ctx->stream->signal)) {
 #endif
-               stream->link->link_enc->funcs->setup(
-                       stream->link->link_enc,
-                       pipe_ctx->stream->signal);
+               if (link_enc)
+                       link_enc->funcs->setup(
+                               link_enc,
+                               pipe_ctx->stream->signal);
                pipe_ctx->stream_res.stream_enc->funcs->setup_stereo_sync(
                        pipe_ctx->stream_res.stream_enc,
                        pipe_ctx->stream_res.tg->inst,
@@ -3748,9 +3773,10 @@ void core_link_enable_stream(
 #else
                if (!dc_is_virtual_signal(pipe_ctx->stream->signal))
 #endif
-                       stream->link->link_enc->funcs->setup(
-                               stream->link->link_enc,
-                               pipe_ctx->stream->signal);
+                       if (link_enc)
+                               link_enc->funcs->setup(
+                                       link_enc,
+                                       pipe_ctx->stream->signal);
 
                dc->hwss.enable_stream(pipe_ctx);
 
index e573b0c..07c2343 100644 (file)
@@ -2624,13 +2624,27 @@ static enum dc_link_rate get_lttpr_max_link_rate(struct dc_link *link)
 
 bool dc_link_dp_get_max_link_enc_cap(const struct dc_link *link, struct dc_link_settings *max_link_enc_cap)
 {
+       struct link_encoder *link_enc = NULL;
+
        if (!max_link_enc_cap) {
                DC_LOG_ERROR("%s: Could not return max link encoder caps", __func__);
                return false;
        }
 
-       if (link->link_enc->funcs->get_max_link_cap) {
-               link->link_enc->funcs->get_max_link_cap(link->link_enc, max_link_enc_cap);
+       /* Links supporting dynamically assigned link encoder will be assigned next
+        * available encoder if one not already assigned.
+        */
+       if (link->is_dig_mapping_flexible &&
+                       link->dc->res_pool->funcs->link_encs_assign) {
+               link_enc = link_enc_cfg_get_link_enc_used_by_link(link->dc->current_state, link);
+               if (link_enc == NULL)
+                       link_enc = link_enc_cfg_get_next_avail_link_enc(link->dc, link->dc->current_state);
+       } else
+               link_enc = link->link_enc;
+       ASSERT(link_enc);
+
+       if (link_enc && link_enc->funcs->get_max_link_cap) {
+               link_enc->funcs->get_max_link_cap(link_enc, max_link_enc_cap);
                return true;
        }
 
@@ -2646,9 +2660,23 @@ static struct dc_link_settings get_max_link_cap(struct dc_link *link)
 #if defined(CONFIG_DRM_AMD_DC_DCN)
        enum dc_link_rate lttpr_max_link_rate;
 #endif
+       struct link_encoder *link_enc = NULL;
+
+       /* Links supporting dynamically assigned link encoder will be assigned next
+        * available encoder if one not already assigned.
+        */
+       if (link->is_dig_mapping_flexible &&
+                       link->dc->res_pool->funcs->link_encs_assign) {
+               link_enc = link_enc_cfg_get_link_enc_used_by_link(link->dc->current_state, link);
+               if (link_enc == NULL)
+                       link_enc = link_enc_cfg_get_next_avail_link_enc(link->dc, link->dc->current_state);
+       } else
+               link_enc = link->link_enc;
+       ASSERT(link_enc);
 
        /* get max link encoder capability */
-       link->link_enc->funcs->get_max_link_cap(link->link_enc, &max_link_cap);
+       if (link_enc)
+               link_enc->funcs->get_max_link_cap(link_enc, &max_link_cap);
 #if defined(CONFIG_DRM_AMD_DC_DCN)
        if (max_link_cap.link_rate >= LINK_RATE_UHBR10 &&
                        !link->hpo_dp_link_enc)
@@ -2867,7 +2895,7 @@ bool dp_verify_link_cap(
         * PHY will sometimes be in bad state on hotplugging display from certain USB-C dongle,
         * so add extra cycle of enabling and disabling the PHY before first link training.
         */
-       if (link->link_enc->features.flags.bits.DP_IS_USB_C &&
+       if (link->link_enc && link->link_enc->features.flags.bits.DP_IS_USB_C &&
                        link->dc->debug.usbc_combo_phy_reset_wa) {
                dp_enable_link_phy(link, link->connector_signal, dp_cs_id, cur);
                dp_disable_link_phy(link, link->connector_signal);
index de80a9e..49b17bb 100644 (file)
@@ -139,7 +139,7 @@ static struct dc_stream_state *get_stream_using_link_enc(
        for (i = 0; i < state->stream_count; i++) {
                struct link_enc_assignment assignment = state->res_ctx.link_enc_assignments[i];
 
-               if (assignment.valid && (assignment.eng_id == eng_id)) {
+               if ((assignment.valid == true) && (assignment.eng_id == eng_id)) {
                        stream_idx = i;
                        break;
                }
@@ -254,7 +254,7 @@ struct dc_link *link_enc_cfg_get_link_using_link_enc(
        for (i = 0; i < state->stream_count; i++) {
                struct link_enc_assignment assignment = state->res_ctx.link_enc_assignments[i];
 
-               if (assignment.valid && (assignment.eng_id == eng_id)) {
+               if ((assignment.valid == true) && (assignment.eng_id == eng_id)) {
                        stream_idx = i;
                        break;
                }
@@ -274,7 +274,6 @@ struct link_encoder *link_enc_cfg_get_link_enc_used_by_link(
 {
        struct link_encoder *link_enc = NULL;
        struct display_endpoint_id ep_id;
-       int stream_idx = -1;
        int i;
 
        ep_id = (struct display_endpoint_id) {
@@ -283,20 +282,15 @@ struct link_encoder *link_enc_cfg_get_link_enc_used_by_link(
 
        for (i = 0; i < state->stream_count; i++) {
                struct link_enc_assignment assignment = state->res_ctx.link_enc_assignments[i];
-
-               if (assignment.valid &&
+               if (assignment.valid == true &&
                                assignment.ep_id.link_id.id == ep_id.link_id.id &&
                                assignment.ep_id.link_id.enum_id == ep_id.link_id.enum_id &&
                                assignment.ep_id.link_id.type == ep_id.link_id.type &&
                                assignment.ep_id.ep_type == ep_id.ep_type) {
-                       stream_idx = i;
-                       break;
+                       link_enc = link->dc->res_pool->link_encoders[assignment.eng_id - ENGINE_ID_DIGA];
                }
        }
 
-       if (stream_idx != -1)
-               link_enc = state->streams[stream_idx]->link_enc;
-
        return link_enc;
 }
 
@@ -313,3 +307,14 @@ struct link_encoder *link_enc_cfg_get_next_avail_link_enc(
 
        return link_enc;
 }
+
+struct link_encoder *link_enc_cfg_get_link_enc_used_by_stream(
+               struct dc_state *state,
+               const struct dc_stream_state *stream)
+{
+       struct link_encoder *link_enc;
+
+       link_enc = link_enc_cfg_get_link_enc_used_by_link(state, stream->link);
+
+       return link_enc;
+}
index b9570b7..dae0ab7 100644 (file)
@@ -452,9 +452,10 @@ void dp_retrain_link_dp_test(struct dc_link *link,
                        if ((&pipes[i])->stream_res.audio && !link->dc->debug.az_endpoint_mute_only)
                                (&pipes[i])->stream_res.audio->funcs->az_disable((&pipes[i])->stream_res.audio);
 
-                       link->link_enc->funcs->disable_output(
-                                       link->link_enc,
-                                       SIGNAL_TYPE_DISPLAY_PORT);
+                       if (link->link_enc)
+                               link->link_enc->funcs->disable_output(
+                                               link->link_enc,
+                                               SIGNAL_TYPE_DISPLAY_PORT);
 
                        /* Clear current link setting. */
                        memset(&link->cur_link_settings, 0,
index 57420bf..8766b12 100644 (file)
@@ -2970,7 +2970,8 @@ enum dc_status dc_validate_stream(struct dc *dc, struct dc_stream_state *stream)
                res = DC_FAIL_CONTROLLER_VALIDATE;
 
        if (res == DC_OK) {
-               if (!link->link_enc->funcs->validate_output_with_stream(
+               if (link->ep_type == DISPLAY_ENDPOINT_PHY &&
+                               !link->link_enc->funcs->validate_output_with_stream(
                                                link->link_enc, stream))
                        res = DC_FAIL_ENC_VALIDATE;
        }
index 2ce668a..102f764 100644 (file)
@@ -46,6 +46,7 @@
 #include "transform.h"
 #include "stream_encoder.h"
 #include "link_encoder.h"
+#include "link_enc_cfg.h"
 #include "link_hwss.h"
 #include "dc_link_dp.h"
 #if defined(CONFIG_DRM_AMD_DC_DCN)
@@ -1192,6 +1193,7 @@ void dce110_disable_stream(struct pipe_ctx *pipe_ctx)
        struct dc_stream_state *stream = pipe_ctx->stream;
        struct dc_link *link = stream->link;
        struct dc *dc = pipe_ctx->stream->ctx->dc;
+       struct link_encoder *link_enc = NULL;
 
        if (dc_is_hdmi_tmds_signal(pipe_ctx->stream->signal)) {
                pipe_ctx->stream_res.stream_enc->funcs->stop_hdmi_info_packets(
@@ -1213,6 +1215,13 @@ void dce110_disable_stream(struct pipe_ctx *pipe_ctx)
 
        dc->hwss.disable_audio_stream(pipe_ctx);
 
+       /* Link encoder may have been dynamically assigned to non-physical display endpoint. */
+       if (link->ep_type == DISPLAY_ENDPOINT_PHY)
+               link_enc = link->link_enc;
+       else if (dc->res_pool->funcs->link_encs_assign)
+               link_enc = link_enc_cfg_get_link_enc_used_by_link(link->dc->current_state, link);
+       ASSERT(link_enc);
+
 #if defined(CONFIG_DRM_AMD_DC_DCN)
        if (is_dp_128b_132b_signal(pipe_ctx)) {
                pipe_ctx->stream_res.hpo_dp_stream_enc->funcs->disable(
@@ -1220,13 +1229,15 @@ void dce110_disable_stream(struct pipe_ctx *pipe_ctx)
                setup_dp_hpo_stream(pipe_ctx, false);
        /* TODO - DP2.0 HW: unmap stream from link encoder here */
        } else {
-               link->link_enc->funcs->connect_dig_be_to_fe(
-                               link->link_enc,
+               if (link_enc)
+                       link_enc->funcs->connect_dig_be_to_fe(
+                               link_enc,
                                pipe_ctx->stream_res.stream_enc->id,
                                false);
        }
 #else
-       link->link_enc->funcs->connect_dig_be_to_fe(
+       if (link_enc)
+               link_enc->funcs->connect_dig_be_to_fe(
                        link->link_enc,
                        pipe_ctx->stream_res.stream_enc->id,
                        false);
@@ -1666,8 +1677,9 @@ static void power_down_encoders(struct dc *dc)
                if (signal != SIGNAL_TYPE_EDP)
                        signal = SIGNAL_TYPE_NONE;
 
-               dc->links[i]->link_enc->funcs->disable_output(
-                               dc->links[i]->link_enc, signal);
+               if (dc->links[i]->ep_type == DISPLAY_ENDPOINT_PHY)
+                       dc->links[i]->link_enc->funcs->disable_output(
+                                       dc->links[i]->link_enc, signal);
 
                dc->links[i]->link_status.link_active = false;
                memset(&dc->links[i]->cur_link_settings, 0,
index 7daadb6..f37551e 100644 (file)
@@ -1296,7 +1296,7 @@ struct stream_encoder *dcn10_find_first_free_match_stream_enc_for_link(
                         * in daisy chain use case
                         */
                        j = i;
-                       if (pool->stream_enc[i]->id ==
+                       if (link->ep_type == DISPLAY_ENDPOINT_PHY && pool->stream_enc[i]->id ==
                                        link->link_enc->preferred_engine)
                                return pool->stream_enc[i];
                }
index ffc0b9a..c0aed7d 100644 (file)
@@ -2381,6 +2381,13 @@ void dcn20_enable_stream(struct pipe_ctx *pipe_ctx)
        uint32_t active_total_with_borders;
        uint32_t early_control = 0;
        struct timing_generator *tg = pipe_ctx->stream_res.tg;
+       struct link_encoder *link_enc;
+
+       if (link->is_dig_mapping_flexible &&
+                       link->dc->res_pool->funcs->link_encs_assign)
+               link_enc = pipe_ctx->stream->link_enc;
+       else
+               link_enc = link->link_enc;
 
        /* For MST, there are multiply stream go to only one link.
         * connect DIG back_end to front_end while enable_stream and
@@ -2397,9 +2404,9 @@ void dcn20_enable_stream(struct pipe_ctx *pipe_ctx)
                                link->hpo_dp_link_enc->inst);
        }
 
-       if (!is_dp_128b_132b_signal(pipe_ctx))
-               link->link_enc->funcs->connect_dig_be_to_fe(
-                       link->link_enc, pipe_ctx->stream_res.stream_enc->id, true);
+       if (!is_dp_128b_132b_signal(pipe_ctx) && link_enc)
+               link_enc->funcs->connect_dig_be_to_fe(
+                       link_enc, pipe_ctx->stream_res.stream_enc->id, true);
 
        if (dc_is_dp_signal(pipe_ctx->stream->signal))
                dp_source_sequence_trace(link, DPCD_SOURCE_SEQ_AFTER_CONNECT_DIG_FE_BE);
index 7bab883..e69f553 100644 (file)
@@ -87,6 +87,7 @@
 #include "dce/dce_aux.h"
 #include "dce/dce_i2c.h"
 #include "vm_helper.h"
+#include "link_enc_cfg.h"
 
 #include "amdgpu_socbb.h"
 
@@ -1596,12 +1597,29 @@ static void get_pixel_clock_parameters(
        const struct dc_stream_state *stream = pipe_ctx->stream;
        struct pipe_ctx *odm_pipe;
        int opp_cnt = 1;
+       struct dc_link *link = stream->link;
+       struct link_encoder *link_enc = NULL;
 
        for (odm_pipe = pipe_ctx->next_odm_pipe; odm_pipe; odm_pipe = odm_pipe->next_odm_pipe)
                opp_cnt++;
 
        pixel_clk_params->requested_pix_clk_100hz = stream->timing.pix_clk_100hz;
-       pixel_clk_params->encoder_object_id = stream->link->link_enc->id;
+
+       /* Links supporting dynamically assigned link encoder will be assigned next
+        * available encoder if one not already assigned.
+        */
+       if (link->is_dig_mapping_flexible &&
+                       link->dc->res_pool->funcs->link_encs_assign) {
+               link_enc = link_enc_cfg_get_link_enc_used_by_stream(stream->link->dc->current_state, stream);
+               if (link_enc == NULL)
+                       link_enc = link_enc_cfg_get_next_avail_link_enc(stream->link->dc,
+                               stream->link->dc->current_state);
+       } else
+               link_enc = stream->link->link_enc;
+       ASSERT(link_enc);
+
+       if (link_enc)
+               pixel_clk_params->encoder_object_id = link_enc->id;
        pixel_clk_params->signal_type = pipe_ctx->stream->signal;
        pixel_clk_params->controller_id = pipe_ctx->stream_res.tg->inst + 1;
        /* TODO: un-hardcode*/
index 2434232..83f223d 100644 (file)
@@ -157,6 +157,9 @@ void dcn31_init_hw(struct dc *dc)
                 */
                struct dc_link *link = dc->links[i];
 
+               if (link->ep_type != DISPLAY_ENDPOINT_PHY)
+                       continue;
+
                link->link_enc->funcs->hw_init(link->link_enc);
 
                /* Check for enabled DIG to identify enabled display */
@@ -184,7 +187,8 @@ void dcn31_init_hw(struct dc *dc)
                                                     &dpcd_power_state, sizeof(dpcd_power_state));
                        if (status == DC_OK && dpcd_power_state == DP_POWER_STATE_D0) {
                                /* blank dp stream before power off receiver*/
-                               if (dc->links[i]->link_enc->funcs->get_dig_frontend) {
+                               if (dc->links[i]->ep_type == DISPLAY_ENDPOINT_PHY &&
+                                               dc->links[i]->link_enc->funcs->get_dig_frontend) {
                                        unsigned int fe;
 
                                        fe = dc->links[i]->link_enc->funcs->get_dig_frontend(
@@ -248,7 +252,8 @@ void dcn31_init_hw(struct dc *dc)
                        for (i = 0; i < dc->link_count; i++) {
                                struct dc_link *link = dc->links[i];
 
-                               if (link->link_enc->funcs->is_dig_enabled &&
+                               if (link->ep_type == DISPLAY_ENDPOINT_PHY &&
+                                               link->link_enc->funcs->is_dig_enabled &&
                                                link->link_enc->funcs->is_dig_enabled(link->link_enc) &&
                                                dc->hwss.power_down) {
                                        dc->hwss.power_down(dc);
index 883dd87..2472c9a 100644 (file)
@@ -88,4 +88,9 @@ struct link_encoder *link_enc_cfg_get_next_avail_link_enc(
        const struct dc *dc,
        const struct dc_state *state);
 
+/* Return DIG link encoder used by stream. NULL if unused. */
+struct link_encoder *link_enc_cfg_get_link_enc_used_by_stream(
+               struct dc_state *state,
+               const struct dc_stream_state *stream);
+
 #endif /* DC_INC_LINK_ENC_CFG_H_ */