[PATCH 25/30] drm/amd/display: Fix dynamic link encoder access.
Timo Aaltonen
tjaalton at ubuntu.com
Mon Nov 22 18:34:13 UTC 2021
From: Jimmy Kizito <Jimmy.Kizito at amd.com>
BugLink: https://bugs.launchpad.net/bugs/1951868
[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 at amd.com>
Acked-by: Mikita Lipski <mikita.lipski at amd.com>
Signed-off-by: Jimmy Kizito <Jimmy.Kizito at amd.com>
Tested-by: Daniel Wheeler <daniel.wheeler at amd.com>
Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
(backported from commit 64d283cb379eadcb412ebba3b61808b58d0c6193 - fix conflicts in dc_link.c, dc-link_dp.c, dce110_hw_sequencer.c, dcn20_hwseq.c, dcn31_hwseq.c)
Signed-off-by: Timo Aaltonen <timo.aaltonen at canonical.com>
---
.../display/dc/clk_mgr/dcn31/dcn31_clk_mgr.c | 2 +-
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 36 ++++++++++++++-----
.../gpu/drm/amd/display/dc/core/dc_link_dp.c | 36 ++++++++++++++++---
.../drm/amd/display/dc/core/dc_link_enc_cfg.c | 25 +++++++------
.../drm/amd/display/dc/core/dc_link_hwss.c | 7 ++--
.../gpu/drm/amd/display/dc/core/dc_resource.c | 3 +-
.../display/dc/dce110/dce110_hw_sequencer.c | 10 ++++--
.../drm/amd/display/dc/dcn10/dcn10_resource.c | 2 +-
.../drm/amd/display/dc/dcn20/dcn20_hwseq.c | 7 ++++
.../drm/amd/display/dc/dcn20/dcn20_resource.c | 20 ++++++++++-
.../drm/amd/display/dc/dcn31/dcn31_hwseq.c | 6 +++-
.../gpu/drm/amd/display/dc/inc/link_enc_cfg.h | 5 +++
12 files changed, 126 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn31/dcn31_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn31/dcn31_clk_mgr.c
index 377c4e53a2b3..3378e1695d13 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn31/dcn31_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn31/dcn31_clk_mgr.c
@@ -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++;
}
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 43971f3759eb..342a8adb94ee 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -3320,6 +3320,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 =
@@ -3331,8 +3335,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;
#endif
config.dpms_off = dpms_off;
config.dm_stream_ctx = pipe_ctx->stream->dm_stream_context;
@@ -3352,6 +3369,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
@@ -3362,9 +3380,10 @@ void core_link_enable_stream(
return;
if (!dc_is_virtual_signal(pipe_ctx->stream->signal)) {
- 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,
@@ -3495,9 +3514,10 @@ void core_link_enable_stream(
* from transmitter control.
*/
if (!dc_is_virtual_signal(pipe_ctx->stream->signal))
- 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);
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 299ab7b92cb6..607285c0c3cd 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
@@ -2055,13 +2055,27 @@ bool dc_link_dp_sync_lt_end(struct dc_link *link, bool link_down)
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;
}
@@ -2074,9 +2088,23 @@ bool dc_link_dp_get_max_link_enc_cap(const struct dc_link *link, struct dc_link_
static struct dc_link_settings get_max_link_cap(struct dc_link *link)
{
struct dc_link_settings max_link_cap = {0};
+ 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);
/* Lower link settings based on sink's link cap */
if (link->reported_link_cap.lane_count < max_link_cap.lane_count)
@@ -2279,7 +2307,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);
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 de80a9ea4cfa..49b17bbea8d1 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
@@ -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;
+}
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
index 2a064f7b669e..4ca1002c9d01 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
@@ -362,9 +362,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,
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index a60396d5be44..82981a1845c7 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -2871,7 +2871,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;
}
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
index db32b519f735..3234338bd086 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
@@ -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)
@@ -1165,6 +1166,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(
@@ -1179,7 +1181,8 @@ void dce110_disable_stream(struct pipe_ctx *pipe_ctx)
dc->hwss.disable_audio_stream(pipe_ctx);
- 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);
@@ -1560,8 +1563,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,
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
index 7daadb6a5233..f37551e00023 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
@@ -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];
}
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
index f15c1944bd1b..47cf2c14ab9a 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
@@ -2376,6 +2376,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
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index e3e01b17c164..36a5c5c07c58 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -86,6 +86,7 @@
#include "dce/dce_aux.h"
#include "dce/dce_i2c.h"
#include "vm_helper.h"
+#include "link_enc_cfg.h"
#include "amdgpu_socbb.h"
@@ -1595,12 +1596,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*/
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 d15bcc086890..7b448dcc171a 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
@@ -156,6 +156,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 */
@@ -220,7 +223,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);
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 883dd8733ea4..2472c9aed095 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
@@ -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_ */
--
2.32.0
More information about the kernel-team
mailing list