* [PATCH 0/2] Fixing bad merge in OTG synchronization logic
@ 2022-01-12 14:27 Harry Wentland
2022-01-12 14:27 ` [PATCH 1/2] Revert "drm/amd/display: Fix for otg synchronization logic" Harry Wentland
2022-01-12 14:27 ` [PATCH 2/2] drm/amd/display: Fix for otg synchronization logic Harry Wentland
0 siblings, 2 replies; 5+ messages in thread
From: Harry Wentland @ 2022-01-12 14:27 UTC (permalink / raw)
To: amd-gfx; +Cc: Harry Wentland
A bad merge of
1abaa75bae9e ("drm/amd/display: Fix for otg synchronization logic")
caused Linus to see a lot of underflow on his two 4k displays.
This set pulls his revert and fixes up the original patch.
Linus Torvalds (1):
Revert "drm/amd/display: Fix for otg synchronization logic"
Meenakshikumar Somasundaram (1):
drm/amd/display: Fix for otg synchronization logic
drivers/gpu/drm/amd/display/dc/core/dc.c | 15 +++++++++++----
drivers/gpu/drm/amd/display/dc/inc/resource.h | 1 +
2 files changed, 12 insertions(+), 4 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] Revert "drm/amd/display: Fix for otg synchronization logic"
2022-01-12 14:27 [PATCH 0/2] Fixing bad merge in OTG synchronization logic Harry Wentland
@ 2022-01-12 14:27 ` Harry Wentland
2022-01-12 14:27 ` [PATCH 2/2] drm/amd/display: Fix for otg synchronization logic Harry Wentland
1 sibling, 0 replies; 5+ messages in thread
From: Harry Wentland @ 2022-01-12 14:27 UTC (permalink / raw)
To: amd-gfx
Cc: meenakshikumar somasundaram, Daniel Vetter, Bhawanpreet Lakha,
Linus Torvalds, Daniel Wheeler, Harry Wentland, Alex Deucher,
Jun Lei, Dave Airlie, Christian Koenig, Mustapha Ghaddar
From: Linus Torvalds <torvalds@linux-foundation.org>
This reverts commit a896f870f8a5f23ec961d16baffd3fda1f8be57c.
It causes odd flickering on my Radeon RX580 (PCI ID 1002:67df rev e7,
subsystem ID 1da2:e353).
Bisected right to this commit, and reverting it fixes things.
Link: https://lore.kernel.org/all/CAHk-=wg9hDde_L3bK9tAfdJ4N=TJJ+SjO3ZDONqH5=bVoy_Mzg@mail.gmail.com/
Cc: Alex Deucher <alexdeucher@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Dave Airlie <airlied@gmail.com>
Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Jun Lei <Jun.Lei@amd.com>
Cc: Mustapha Ghaddar <mustapha.ghaddar@amd.com>
Cc: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
Cc: meenakshikumar somasundaram <meenakshikumar.somasundaram@amd.com>
Cc: Daniel Wheeler <daniel.wheeler@amd.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 35 +++++-------
.../gpu/drm/amd/display/dc/core/dc_resource.c | 54 -------------------
drivers/gpu/drm/amd/display/dc/dc.h | 1 -
.../display/dc/dce110/dce110_hw_sequencer.c | 8 ---
.../drm/amd/display/dc/dcn31/dcn31_resource.c | 3 --
.../gpu/drm/amd/display/dc/inc/core_types.h | 1 -
drivers/gpu/drm/amd/display/dc/inc/resource.h | 10 ----
7 files changed, 14 insertions(+), 98 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 91c4874473d6..01c8849b9db2 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1404,29 +1404,22 @@ static void program_timing_sync(
status->timing_sync_info.master = false;
}
+ /* remove any other unblanked pipes as they have already been synced */
+ for (j = j + 1; j < group_size; j++) {
+ bool is_blanked;
- /* remove any other pipes that are already been synced */
- if (dc->config.use_pipe_ctx_sync_logic) {
- /* check pipe's syncd to decide which pipe to be removed */
- for (j = 1; j < group_size; j++) {
- if (pipe_set[j]->pipe_idx_syncd == pipe_set[0]->pipe_idx_syncd) {
- group_size--;
- pipe_set[j] = pipe_set[group_size];
- j--;
- } else
- /* link slave pipe's syncd with master pipe */
- pipe_set[j]->pipe_idx_syncd = pipe_set[0]->pipe_idx_syncd;
+ if (pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked)
+ is_blanked =
+ pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked(pipe_set[j]->stream_res.opp);
+ else
+ is_blanked =
+ pipe_set[j]->stream_res.tg->funcs->is_blanked(pipe_set[j]->stream_res.tg);
+ if (!is_blanked) {
+ group_size--;
+ pipe_set[j] = pipe_set[group_size];
+ j--;
}
- } else {
- /* remove any other pipes by checking valid plane */
- for (j = j + 1; j < group_size; j++) {
- if (pipe_set[j]->plane_state) {
- group_size--;
- pipe_set[j] = pipe_set[group_size];
- j--;
- }
- }
- }
+ }
if (group_size > 1) {
if (sync_type == TIMING_SYNCHRONIZABLE) {
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 b3912ff9dc91..d4ff6cc6b8d9 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -3217,60 +3217,6 @@ struct hpo_dp_link_encoder *resource_get_hpo_dp_link_enc_for_det_lt(
}
#endif
-void reset_syncd_pipes_from_disabled_pipes(struct dc *dc,
- struct dc_state *context)
-{
- int i, j;
- struct pipe_ctx *pipe_ctx_old, *pipe_ctx, *pipe_ctx_syncd;
-
- /* If pipe backend is reset, need to reset pipe syncd status */
- for (i = 0; i < dc->res_pool->pipe_count; i++) {
- pipe_ctx_old = &dc->current_state->res_ctx.pipe_ctx[i];
- pipe_ctx = &context->res_ctx.pipe_ctx[i];
-
- if (!pipe_ctx_old->stream)
- continue;
-
- if (pipe_ctx_old->top_pipe || pipe_ctx_old->prev_odm_pipe)
- continue;
-
- if (!pipe_ctx->stream ||
- pipe_need_reprogram(pipe_ctx_old, pipe_ctx)) {
-
- /* Reset all the syncd pipes from the disabled pipe */
- for (j = 0; j < dc->res_pool->pipe_count; j++) {
- pipe_ctx_syncd = &context->res_ctx.pipe_ctx[j];
- if ((GET_PIPE_SYNCD_FROM_PIPE(pipe_ctx_syncd) == pipe_ctx_old->pipe_idx) ||
- !IS_PIPE_SYNCD_VALID(pipe_ctx_syncd))
- SET_PIPE_SYNCD_TO_PIPE(pipe_ctx_syncd, j);
- }
- }
- }
-}
-
-void check_syncd_pipes_for_disabled_master_pipe(struct dc *dc,
- struct dc_state *context,
- uint8_t disabled_master_pipe_idx)
-{
- int i;
- struct pipe_ctx *pipe_ctx, *pipe_ctx_check;
-
- pipe_ctx = &context->res_ctx.pipe_ctx[disabled_master_pipe_idx];
- if ((GET_PIPE_SYNCD_FROM_PIPE(pipe_ctx) != disabled_master_pipe_idx) ||
- !IS_PIPE_SYNCD_VALID(pipe_ctx))
- SET_PIPE_SYNCD_TO_PIPE(pipe_ctx, disabled_master_pipe_idx);
-
- /* for the pipe disabled, check if any slave pipe exists and assert */
- for (i = 0; i < dc->res_pool->pipe_count; i++) {
- pipe_ctx_check = &context->res_ctx.pipe_ctx[i];
-
- if ((GET_PIPE_SYNCD_FROM_PIPE(pipe_ctx_check) == disabled_master_pipe_idx) &&
- IS_PIPE_SYNCD_VALID(pipe_ctx_check) && (i != disabled_master_pipe_idx))
- DC_ERR("DC: Failure: pipe_idx[%d] syncd with disabled master pipe_idx[%d]\n",
- i, disabled_master_pipe_idx);
- }
-}
-
uint8_t resource_transmitter_to_phy_idx(const struct dc *dc, enum transmitter transmitter)
{
/* TODO - get transmitter to phy idx mapping from DMUB */
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index 288e7b01f561..da2c78ce14d6 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -344,7 +344,6 @@ struct dc_config {
uint8_t vblank_alignment_max_frame_time_diff;
bool is_asymmetric_memory;
bool is_single_rank_dimm;
- bool use_pipe_ctx_sync_logic;
};
enum visual_confirm {
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 f1593186e964..78192ecba102 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
@@ -1566,10 +1566,6 @@ static enum dc_status apply_single_controller_ctx_to_hw(
&pipe_ctx->stream->audio_info);
}
- /* make sure no pipes syncd to the pipe being enabled */
- if (!pipe_ctx->stream->apply_seamless_boot_optimization && dc->config.use_pipe_ctx_sync_logic)
- check_syncd_pipes_for_disabled_master_pipe(dc, context, pipe_ctx->pipe_idx);
-
#if defined(CONFIG_DRM_AMD_DC_DCN)
/* DCN3.1 FPGA Workaround
* Need to enable HPO DP Stream Encoder before setting OTG master enable.
@@ -2301,10 +2297,6 @@ enum dc_status dce110_apply_ctx_to_hw(
enum dc_status status;
int i;
- /* reset syncd pipes from disabled pipes */
- if (dc->config.use_pipe_ctx_sync_logic)
- reset_syncd_pipes_from_disabled_pipes(dc, context);
-
/* Reset old context */
/* look up the targets that have been removed since last commit */
hws->funcs.reset_hw_ctx_wrap(dc, context);
diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
index 40778c05f9b3..1f1c158658ac 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
@@ -2254,9 +2254,6 @@ static bool dcn31_resource_construct(
dc->caps.color.mpc.ogam_rom_caps.hlg = 0;
dc->caps.color.mpc.ocsc = 1;
- /* Use pipe context based otg sync logic */
- dc->config.use_pipe_ctx_sync_logic = true;
-
/* read VBIOS LTTPR caps */
{
if (ctx->dc_bios->funcs->get_lttpr_caps) {
diff --git a/drivers/gpu/drm/amd/display/dc/inc/core_types.h b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
index 943240e2809e..890280026e69 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/core_types.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
@@ -382,7 +382,6 @@ struct pipe_ctx {
struct pll_settings pll_settings;
uint8_t pipe_idx;
- uint8_t pipe_idx_syncd;
struct pipe_ctx *top_pipe;
struct pipe_ctx *bottom_pipe;
diff --git a/drivers/gpu/drm/amd/display/dc/inc/resource.h b/drivers/gpu/drm/amd/display/dc/inc/resource.h
index 028180f58f71..4249bf306e09 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/resource.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/resource.h
@@ -34,10 +34,6 @@
#define MEMORY_TYPE_HBM 2
-#define IS_PIPE_SYNCD_VALID(pipe) ((((pipe)->pipe_idx_syncd) & 0x80)?1:0)
-#define GET_PIPE_SYNCD_FROM_PIPE(pipe) ((pipe)->pipe_idx_syncd & 0x7F)
-#define SET_PIPE_SYNCD_TO_PIPE(pipe, pipe_syncd) ((pipe)->pipe_idx_syncd = (0x80 | pipe_syncd))
-
enum dce_version resource_parse_asic_id(
struct hw_asic_id asic_id);
@@ -212,12 +208,6 @@ struct hpo_dp_link_encoder *resource_get_hpo_dp_link_enc_for_det_lt(
const struct dc_link *link);
#endif
-void reset_syncd_pipes_from_disabled_pipes(struct dc *dc,
- struct dc_state *context);
-
-void check_syncd_pipes_for_disabled_master_pipe(struct dc *dc,
- struct dc_state *context,
- uint8_t disabled_master_pipe_idx);
uint8_t resource_transmitter_to_phy_idx(const struct dc *dc, enum transmitter transmitter);
#endif /* DRIVERS_GPU_DRM_AMD_DC_DEV_DC_INC_RESOURCE_H_ */
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] drm/amd/display: Fix for otg synchronization logic
2022-01-12 14:27 [PATCH 0/2] Fixing bad merge in OTG synchronization logic Harry Wentland
2022-01-12 14:27 ` [PATCH 1/2] Revert "drm/amd/display: Fix for otg synchronization logic" Harry Wentland
@ 2022-01-12 14:27 ` Harry Wentland
2022-01-12 15:53 ` Alex Deucher
1 sibling, 1 reply; 5+ messages in thread
From: Harry Wentland @ 2022-01-12 14:27 UTC (permalink / raw)
To: amd-gfx
Cc: Harry Wentland, Meenakshikumar Somasundaram, torvalds,
Daniel Wheeler, Alex Deucher, Jun Lei, Bhawanpreet Lakha,
Mustapha Ghaddar
From: Meenakshikumar Somasundaram <meenakshikumar.somasundaram@amd.com>
[Why]
During otg sync trigger, plane states are used to decide whether the otg
is already synchronized or not. There are scenarions when otgs are
disabled without plane state getting disabled and in such case the otg is
excluded from synchronization.
[How]
Introduced pipe_idx_syncd in pipe_ctx that tracks each otgs master pipe.
When a otg is disabled/enabled, pipe_idx_syncd is reset to itself.
On sync trigger, pipe_idx_syncd is checked to decide whether a otg is
already synchronized and the otg is further included or excluded from
synchronization.
v2:
Don't drop is_blanked logic
Reviewed-by: Jun Lei <Jun.Lei@amd.com>
Reviewed-by: Mustapha Ghaddar <mustapha.ghaddar@amd.com>
Acked-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
Signed-off-by: meenakshikumar somasundaram <meenakshikumar.somasundaram@amd.com>
Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Harry Wentland <harry.wentland@amd.com>
Cc: torvalds@linux-foundation.org
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 40 +++++++++-----
.../gpu/drm/amd/display/dc/core/dc_resource.c | 54 +++++++++++++++++++
drivers/gpu/drm/amd/display/dc/dc.h | 1 +
.../display/dc/dce110/dce110_hw_sequencer.c | 8 +++
.../drm/amd/display/dc/dcn31/dcn31_resource.c | 3 ++
.../gpu/drm/amd/display/dc/inc/core_types.h | 1 +
drivers/gpu/drm/amd/display/dc/inc/resource.h | 11 ++++
7 files changed, 105 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 01c8849b9db2..6f5528d34093 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1404,20 +1404,34 @@ static void program_timing_sync(
status->timing_sync_info.master = false;
}
- /* remove any other unblanked pipes as they have already been synced */
- for (j = j + 1; j < group_size; j++) {
- bool is_blanked;
- if (pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked)
- is_blanked =
- pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked(pipe_set[j]->stream_res.opp);
- else
- is_blanked =
- pipe_set[j]->stream_res.tg->funcs->is_blanked(pipe_set[j]->stream_res.tg);
- if (!is_blanked) {
- group_size--;
- pipe_set[j] = pipe_set[group_size];
- j--;
+ /* remove any other pipes that are already been synced */
+ if (dc->config.use_pipe_ctx_sync_logic) {
+ /* check pipe's syncd to decide which pipe to be removed */
+ for (j = 1; j < group_size; j++) {
+ if (pipe_set[j]->pipe_idx_syncd == pipe_set[0]->pipe_idx_syncd) {
+ group_size--;
+ pipe_set[j] = pipe_set[group_size];
+ j--;
+ } else
+ /* link slave pipe's syncd with master pipe */
+ pipe_set[j]->pipe_idx_syncd = pipe_set[0]->pipe_idx_syncd;
+ }
+ } else {
+ for (j = j + 1; j < group_size; j++) {
+ bool is_blanked;
+
+ if (pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked)
+ is_blanked =
+ pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked(pipe_set[j]->stream_res.opp);
+ else
+ is_blanked =
+ pipe_set[j]->stream_res.tg->funcs->is_blanked(pipe_set[j]->stream_res.tg);
+ if (!is_blanked) {
+ group_size--;
+ pipe_set[j] = pipe_set[group_size];
+ j--;
+ }
}
}
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 d4ff6cc6b8d9..b3912ff9dc91 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -3217,6 +3217,60 @@ struct hpo_dp_link_encoder *resource_get_hpo_dp_link_enc_for_det_lt(
}
#endif
+void reset_syncd_pipes_from_disabled_pipes(struct dc *dc,
+ struct dc_state *context)
+{
+ int i, j;
+ struct pipe_ctx *pipe_ctx_old, *pipe_ctx, *pipe_ctx_syncd;
+
+ /* If pipe backend is reset, need to reset pipe syncd status */
+ for (i = 0; i < dc->res_pool->pipe_count; i++) {
+ pipe_ctx_old = &dc->current_state->res_ctx.pipe_ctx[i];
+ pipe_ctx = &context->res_ctx.pipe_ctx[i];
+
+ if (!pipe_ctx_old->stream)
+ continue;
+
+ if (pipe_ctx_old->top_pipe || pipe_ctx_old->prev_odm_pipe)
+ continue;
+
+ if (!pipe_ctx->stream ||
+ pipe_need_reprogram(pipe_ctx_old, pipe_ctx)) {
+
+ /* Reset all the syncd pipes from the disabled pipe */
+ for (j = 0; j < dc->res_pool->pipe_count; j++) {
+ pipe_ctx_syncd = &context->res_ctx.pipe_ctx[j];
+ if ((GET_PIPE_SYNCD_FROM_PIPE(pipe_ctx_syncd) == pipe_ctx_old->pipe_idx) ||
+ !IS_PIPE_SYNCD_VALID(pipe_ctx_syncd))
+ SET_PIPE_SYNCD_TO_PIPE(pipe_ctx_syncd, j);
+ }
+ }
+ }
+}
+
+void check_syncd_pipes_for_disabled_master_pipe(struct dc *dc,
+ struct dc_state *context,
+ uint8_t disabled_master_pipe_idx)
+{
+ int i;
+ struct pipe_ctx *pipe_ctx, *pipe_ctx_check;
+
+ pipe_ctx = &context->res_ctx.pipe_ctx[disabled_master_pipe_idx];
+ if ((GET_PIPE_SYNCD_FROM_PIPE(pipe_ctx) != disabled_master_pipe_idx) ||
+ !IS_PIPE_SYNCD_VALID(pipe_ctx))
+ SET_PIPE_SYNCD_TO_PIPE(pipe_ctx, disabled_master_pipe_idx);
+
+ /* for the pipe disabled, check if any slave pipe exists and assert */
+ for (i = 0; i < dc->res_pool->pipe_count; i++) {
+ pipe_ctx_check = &context->res_ctx.pipe_ctx[i];
+
+ if ((GET_PIPE_SYNCD_FROM_PIPE(pipe_ctx_check) == disabled_master_pipe_idx) &&
+ IS_PIPE_SYNCD_VALID(pipe_ctx_check) && (i != disabled_master_pipe_idx))
+ DC_ERR("DC: Failure: pipe_idx[%d] syncd with disabled master pipe_idx[%d]\n",
+ i, disabled_master_pipe_idx);
+ }
+}
+
uint8_t resource_transmitter_to_phy_idx(const struct dc *dc, enum transmitter transmitter)
{
/* TODO - get transmitter to phy idx mapping from DMUB */
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index da2c78ce14d6..288e7b01f561 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -344,6 +344,7 @@ struct dc_config {
uint8_t vblank_alignment_max_frame_time_diff;
bool is_asymmetric_memory;
bool is_single_rank_dimm;
+ bool use_pipe_ctx_sync_logic;
};
enum visual_confirm {
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 78192ecba102..f1593186e964 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
@@ -1566,6 +1566,10 @@ static enum dc_status apply_single_controller_ctx_to_hw(
&pipe_ctx->stream->audio_info);
}
+ /* make sure no pipes syncd to the pipe being enabled */
+ if (!pipe_ctx->stream->apply_seamless_boot_optimization && dc->config.use_pipe_ctx_sync_logic)
+ check_syncd_pipes_for_disabled_master_pipe(dc, context, pipe_ctx->pipe_idx);
+
#if defined(CONFIG_DRM_AMD_DC_DCN)
/* DCN3.1 FPGA Workaround
* Need to enable HPO DP Stream Encoder before setting OTG master enable.
@@ -2297,6 +2301,10 @@ enum dc_status dce110_apply_ctx_to_hw(
enum dc_status status;
int i;
+ /* reset syncd pipes from disabled pipes */
+ if (dc->config.use_pipe_ctx_sync_logic)
+ reset_syncd_pipes_from_disabled_pipes(dc, context);
+
/* Reset old context */
/* look up the targets that have been removed since last commit */
hws->funcs.reset_hw_ctx_wrap(dc, context);
diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
index 1f1c158658ac..40778c05f9b3 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
@@ -2254,6 +2254,9 @@ static bool dcn31_resource_construct(
dc->caps.color.mpc.ogam_rom_caps.hlg = 0;
dc->caps.color.mpc.ocsc = 1;
+ /* Use pipe context based otg sync logic */
+ dc->config.use_pipe_ctx_sync_logic = true;
+
/* read VBIOS LTTPR caps */
{
if (ctx->dc_bios->funcs->get_lttpr_caps) {
diff --git a/drivers/gpu/drm/amd/display/dc/inc/core_types.h b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
index 890280026e69..943240e2809e 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/core_types.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
@@ -382,6 +382,7 @@ struct pipe_ctx {
struct pll_settings pll_settings;
uint8_t pipe_idx;
+ uint8_t pipe_idx_syncd;
struct pipe_ctx *top_pipe;
struct pipe_ctx *bottom_pipe;
diff --git a/drivers/gpu/drm/amd/display/dc/inc/resource.h b/drivers/gpu/drm/amd/display/dc/inc/resource.h
index 4249bf306e09..dbfe6690ded8 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/resource.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/resource.h
@@ -34,6 +34,10 @@
#define MEMORY_TYPE_HBM 2
+#define IS_PIPE_SYNCD_VALID(pipe) ((((pipe)->pipe_idx_syncd) & 0x80)?1:0)
+#define GET_PIPE_SYNCD_FROM_PIPE(pipe) ((pipe)->pipe_idx_syncd & 0x7F)
+#define SET_PIPE_SYNCD_TO_PIPE(pipe, pipe_syncd) ((pipe)->pipe_idx_syncd = (0x80 | pipe_syncd))
+
enum dce_version resource_parse_asic_id(
struct hw_asic_id asic_id);
@@ -208,6 +212,13 @@ struct hpo_dp_link_encoder *resource_get_hpo_dp_link_enc_for_det_lt(
const struct dc_link *link);
#endif
+void reset_syncd_pipes_from_disabled_pipes(struct dc *dc,
+ struct dc_state *context);
+
+void check_syncd_pipes_for_disabled_master_pipe(struct dc *dc,
+ struct dc_state *context,
+ uint8_t disabled_master_pipe_idx);
+
uint8_t resource_transmitter_to_phy_idx(const struct dc *dc, enum transmitter transmitter);
#endif /* DRIVERS_GPU_DRM_AMD_DC_DEV_DC_INC_RESOURCE_H_ */
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/amd/display: Fix for otg synchronization logic
2022-01-12 14:27 ` [PATCH 2/2] drm/amd/display: Fix for otg synchronization logic Harry Wentland
@ 2022-01-12 15:53 ` Alex Deucher
2022-01-12 16:25 ` Harry Wentland
0 siblings, 1 reply; 5+ messages in thread
From: Alex Deucher @ 2022-01-12 15:53 UTC (permalink / raw)
To: Harry Wentland
Cc: Bhawanpreet Lakha, amd-gfx list, Meenakshikumar Somasundaram,
Daniel Wheeler, Alex Deucher, Jun Lei, Linus Torvalds,
Mustapha Ghaddar
On Wed, Jan 12, 2022 at 9:28 AM Harry Wentland <harry.wentland@amd.com> wrote:
>
> From: Meenakshikumar Somasundaram <meenakshikumar.somasundaram@amd.com>
>
> [Why]
> During otg sync trigger, plane states are used to decide whether the otg
> is already synchronized or not. There are scenarions when otgs are
> disabled without plane state getting disabled and in such case the otg is
> excluded from synchronization.
>
> [How]
> Introduced pipe_idx_syncd in pipe_ctx that tracks each otgs master pipe.
> When a otg is disabled/enabled, pipe_idx_syncd is reset to itself.
> On sync trigger, pipe_idx_syncd is checked to decide whether a otg is
> already synchronized and the otg is further included or excluded from
> synchronization.
>
> v2:
> Don't drop is_blanked logic
>
> Reviewed-by: Jun Lei <Jun.Lei@amd.com>
> Reviewed-by: Mustapha Ghaddar <mustapha.ghaddar@amd.com>
> Acked-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
> Signed-off-by: meenakshikumar somasundaram <meenakshikumar.somasundaram@amd.com>
> Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> Cc: torvalds@linux-foundation.org
Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/display/dc/core/dc.c | 40 +++++++++-----
> .../gpu/drm/amd/display/dc/core/dc_resource.c | 54 +++++++++++++++++++
> drivers/gpu/drm/amd/display/dc/dc.h | 1 +
> .../display/dc/dce110/dce110_hw_sequencer.c | 8 +++
> .../drm/amd/display/dc/dcn31/dcn31_resource.c | 3 ++
> .../gpu/drm/amd/display/dc/inc/core_types.h | 1 +
> drivers/gpu/drm/amd/display/dc/inc/resource.h | 11 ++++
> 7 files changed, 105 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 01c8849b9db2..6f5528d34093 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -1404,20 +1404,34 @@ static void program_timing_sync(
> status->timing_sync_info.master = false;
>
> }
> - /* remove any other unblanked pipes as they have already been synced */
> - for (j = j + 1; j < group_size; j++) {
> - bool is_blanked;
>
> - if (pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked)
> - is_blanked =
> - pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked(pipe_set[j]->stream_res.opp);
> - else
> - is_blanked =
> - pipe_set[j]->stream_res.tg->funcs->is_blanked(pipe_set[j]->stream_res.tg);
> - if (!is_blanked) {
> - group_size--;
> - pipe_set[j] = pipe_set[group_size];
> - j--;
> + /* remove any other pipes that are already been synced */
> + if (dc->config.use_pipe_ctx_sync_logic) {
> + /* check pipe's syncd to decide which pipe to be removed */
> + for (j = 1; j < group_size; j++) {
> + if (pipe_set[j]->pipe_idx_syncd == pipe_set[0]->pipe_idx_syncd) {
> + group_size--;
> + pipe_set[j] = pipe_set[group_size];
> + j--;
> + } else
> + /* link slave pipe's syncd with master pipe */
> + pipe_set[j]->pipe_idx_syncd = pipe_set[0]->pipe_idx_syncd;
> + }
> + } else {
> + for (j = j + 1; j < group_size; j++) {
> + bool is_blanked;
> +
> + if (pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked)
> + is_blanked =
> + pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked(pipe_set[j]->stream_res.opp);
> + else
> + is_blanked =
> + pipe_set[j]->stream_res.tg->funcs->is_blanked(pipe_set[j]->stream_res.tg);
> + if (!is_blanked) {
> + group_size--;
> + pipe_set[j] = pipe_set[group_size];
> + j--;
> + }
> }
> }
>
> 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 d4ff6cc6b8d9..b3912ff9dc91 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -3217,6 +3217,60 @@ struct hpo_dp_link_encoder *resource_get_hpo_dp_link_enc_for_det_lt(
> }
> #endif
>
> +void reset_syncd_pipes_from_disabled_pipes(struct dc *dc,
> + struct dc_state *context)
> +{
> + int i, j;
> + struct pipe_ctx *pipe_ctx_old, *pipe_ctx, *pipe_ctx_syncd;
> +
> + /* If pipe backend is reset, need to reset pipe syncd status */
> + for (i = 0; i < dc->res_pool->pipe_count; i++) {
> + pipe_ctx_old = &dc->current_state->res_ctx.pipe_ctx[i];
> + pipe_ctx = &context->res_ctx.pipe_ctx[i];
> +
> + if (!pipe_ctx_old->stream)
> + continue;
> +
> + if (pipe_ctx_old->top_pipe || pipe_ctx_old->prev_odm_pipe)
> + continue;
> +
> + if (!pipe_ctx->stream ||
> + pipe_need_reprogram(pipe_ctx_old, pipe_ctx)) {
> +
> + /* Reset all the syncd pipes from the disabled pipe */
> + for (j = 0; j < dc->res_pool->pipe_count; j++) {
> + pipe_ctx_syncd = &context->res_ctx.pipe_ctx[j];
> + if ((GET_PIPE_SYNCD_FROM_PIPE(pipe_ctx_syncd) == pipe_ctx_old->pipe_idx) ||
> + !IS_PIPE_SYNCD_VALID(pipe_ctx_syncd))
> + SET_PIPE_SYNCD_TO_PIPE(pipe_ctx_syncd, j);
> + }
> + }
> + }
> +}
> +
> +void check_syncd_pipes_for_disabled_master_pipe(struct dc *dc,
> + struct dc_state *context,
> + uint8_t disabled_master_pipe_idx)
> +{
> + int i;
> + struct pipe_ctx *pipe_ctx, *pipe_ctx_check;
> +
> + pipe_ctx = &context->res_ctx.pipe_ctx[disabled_master_pipe_idx];
> + if ((GET_PIPE_SYNCD_FROM_PIPE(pipe_ctx) != disabled_master_pipe_idx) ||
> + !IS_PIPE_SYNCD_VALID(pipe_ctx))
> + SET_PIPE_SYNCD_TO_PIPE(pipe_ctx, disabled_master_pipe_idx);
> +
> + /* for the pipe disabled, check if any slave pipe exists and assert */
> + for (i = 0; i < dc->res_pool->pipe_count; i++) {
> + pipe_ctx_check = &context->res_ctx.pipe_ctx[i];
> +
> + if ((GET_PIPE_SYNCD_FROM_PIPE(pipe_ctx_check) == disabled_master_pipe_idx) &&
> + IS_PIPE_SYNCD_VALID(pipe_ctx_check) && (i != disabled_master_pipe_idx))
> + DC_ERR("DC: Failure: pipe_idx[%d] syncd with disabled master pipe_idx[%d]\n",
> + i, disabled_master_pipe_idx);
> + }
> +}
> +
> uint8_t resource_transmitter_to_phy_idx(const struct dc *dc, enum transmitter transmitter)
> {
> /* TODO - get transmitter to phy idx mapping from DMUB */
> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
> index da2c78ce14d6..288e7b01f561 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
> @@ -344,6 +344,7 @@ struct dc_config {
> uint8_t vblank_alignment_max_frame_time_diff;
> bool is_asymmetric_memory;
> bool is_single_rank_dimm;
> + bool use_pipe_ctx_sync_logic;
> };
>
> enum visual_confirm {
> 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 78192ecba102..f1593186e964 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
> @@ -1566,6 +1566,10 @@ static enum dc_status apply_single_controller_ctx_to_hw(
> &pipe_ctx->stream->audio_info);
> }
>
> + /* make sure no pipes syncd to the pipe being enabled */
> + if (!pipe_ctx->stream->apply_seamless_boot_optimization && dc->config.use_pipe_ctx_sync_logic)
> + check_syncd_pipes_for_disabled_master_pipe(dc, context, pipe_ctx->pipe_idx);
> +
> #if defined(CONFIG_DRM_AMD_DC_DCN)
> /* DCN3.1 FPGA Workaround
> * Need to enable HPO DP Stream Encoder before setting OTG master enable.
> @@ -2297,6 +2301,10 @@ enum dc_status dce110_apply_ctx_to_hw(
> enum dc_status status;
> int i;
>
> + /* reset syncd pipes from disabled pipes */
> + if (dc->config.use_pipe_ctx_sync_logic)
> + reset_syncd_pipes_from_disabled_pipes(dc, context);
> +
> /* Reset old context */
> /* look up the targets that have been removed since last commit */
> hws->funcs.reset_hw_ctx_wrap(dc, context);
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
> index 1f1c158658ac..40778c05f9b3 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
> @@ -2254,6 +2254,9 @@ static bool dcn31_resource_construct(
> dc->caps.color.mpc.ogam_rom_caps.hlg = 0;
> dc->caps.color.mpc.ocsc = 1;
>
> + /* Use pipe context based otg sync logic */
> + dc->config.use_pipe_ctx_sync_logic = true;
> +
> /* read VBIOS LTTPR caps */
> {
> if (ctx->dc_bios->funcs->get_lttpr_caps) {
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/core_types.h b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
> index 890280026e69..943240e2809e 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/core_types.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
> @@ -382,6 +382,7 @@ struct pipe_ctx {
> struct pll_settings pll_settings;
>
> uint8_t pipe_idx;
> + uint8_t pipe_idx_syncd;
>
> struct pipe_ctx *top_pipe;
> struct pipe_ctx *bottom_pipe;
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/resource.h b/drivers/gpu/drm/amd/display/dc/inc/resource.h
> index 4249bf306e09..dbfe6690ded8 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/resource.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/resource.h
> @@ -34,6 +34,10 @@
> #define MEMORY_TYPE_HBM 2
>
>
> +#define IS_PIPE_SYNCD_VALID(pipe) ((((pipe)->pipe_idx_syncd) & 0x80)?1:0)
> +#define GET_PIPE_SYNCD_FROM_PIPE(pipe) ((pipe)->pipe_idx_syncd & 0x7F)
> +#define SET_PIPE_SYNCD_TO_PIPE(pipe, pipe_syncd) ((pipe)->pipe_idx_syncd = (0x80 | pipe_syncd))
> +
> enum dce_version resource_parse_asic_id(
> struct hw_asic_id asic_id);
>
> @@ -208,6 +212,13 @@ struct hpo_dp_link_encoder *resource_get_hpo_dp_link_enc_for_det_lt(
> const struct dc_link *link);
> #endif
>
> +void reset_syncd_pipes_from_disabled_pipes(struct dc *dc,
> + struct dc_state *context);
> +
> +void check_syncd_pipes_for_disabled_master_pipe(struct dc *dc,
> + struct dc_state *context,
> + uint8_t disabled_master_pipe_idx);
> +
> uint8_t resource_transmitter_to_phy_idx(const struct dc *dc, enum transmitter transmitter);
>
> #endif /* DRIVERS_GPU_DRM_AMD_DC_DEV_DC_INC_RESOURCE_H_ */
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/amd/display: Fix for otg synchronization logic
2022-01-12 15:53 ` Alex Deucher
@ 2022-01-12 16:25 ` Harry Wentland
0 siblings, 0 replies; 5+ messages in thread
From: Harry Wentland @ 2022-01-12 16:25 UTC (permalink / raw)
To: Alex Deucher
Cc: Bhawanpreet Lakha, amd-gfx list, Meenakshikumar Somasundaram,
Daniel Wheeler, Alex Deucher, Jun Lei, Linus Torvalds,
Mustapha Ghaddar
On 2022-01-12 10:53, Alex Deucher wrote:
> On Wed, Jan 12, 2022 at 9:28 AM Harry Wentland <harry.wentland@amd.com> wrote:
>>
>> From: Meenakshikumar Somasundaram <meenakshikumar.somasundaram@amd.com>
>>
>> [Why]
>> During otg sync trigger, plane states are used to decide whether the otg
>> is already synchronized or not. There are scenarions when otgs are
>> disabled without plane state getting disabled and in such case the otg is
>> excluded from synchronization.
>>
>> [How]
>> Introduced pipe_idx_syncd in pipe_ctx that tracks each otgs master pipe.
>> When a otg is disabled/enabled, pipe_idx_syncd is reset to itself.
>> On sync trigger, pipe_idx_syncd is checked to decide whether a otg is
>> already synchronized and the otg is further included or excluded from
>> synchronization.
>>
>> v2:
>> Don't drop is_blanked logic
>>
>> Reviewed-by: Jun Lei <Jun.Lei@amd.com>
>> Reviewed-by: Mustapha Ghaddar <mustapha.ghaddar@amd.com>
>> Acked-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
>> Signed-off-by: meenakshikumar somasundaram <meenakshikumar.somasundaram@amd.com>
>> Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>> Cc: torvalds@linux-foundation.org
>
> Series is:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
And merged. Thanks.
Harry
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-12 16:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-12 14:27 [PATCH 0/2] Fixing bad merge in OTG synchronization logic Harry Wentland
2022-01-12 14:27 ` [PATCH 1/2] Revert "drm/amd/display: Fix for otg synchronization logic" Harry Wentland
2022-01-12 14:27 ` [PATCH 2/2] drm/amd/display: Fix for otg synchronization logic Harry Wentland
2022-01-12 15:53 ` Alex Deucher
2022-01-12 16:25 ` Harry Wentland
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.