RE: [PATCH 11/22] drm/amd/display: Disable phantom OTG after enable for plane disable
[AMD Official Use Only - General]
Hi Nathan,
Thanks for reporting this issue.
Hi Alvin,
Please see inline.
-Original Message-
From: Nathan Chancellor
Sent: Thursday, November 10, 2022 11:39 PM
To: Liu, HaoPing (Alan)
Cc: amd-gfx@lists.freedesktop.org; Wang, Chao-kai (Stylon)
; Li, Sun peng (Leo) ; Wentland, Harry
; Zhuo, Qingqing (Lillian) ;
Siqueira, Rodrigo ; Li, Roman ;
Chiu, Solomon ; Pillai, Aurabindo
; Lee, Alvin ; Lin, Wayne
; Lei, Jun ; Lakha, Bhawanpreet
; Gutierrez, Agustin ;
Kotarac, Pavle
Subject: Re: [PATCH 11/22] drm/amd/display: Disable phantom OTG after enable
for plane disable
Hi Alan,
On Thu, Nov 03, 2022 at 12:01:06AM +0800, Alan Liu wrote:
> From: Alvin Lee
>
> [Description]
> - Need to disable phantom OTG after it's enabled
> in order to restore it to it's original state.
> - If it's enabled and then an MCLK switch comes in
> we may not prefetch the correct data since the phantom
> OTG could already be in the middle of the frame.
>
> Reviewed-by: Jun Lei
> Acked-by: Alan Liu
> Signed-off-by: Alvin Lee
> ---
> drivers/gpu/drm/amd/display/dc/core/dc.c | 14 +-
> drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c | 8
> .../drm/amd/display/dc/inc/hw/timing_generator.h | 1 +
> 3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index da808996e21d..9c3704c4d7e4 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -1055,6 +1055,7 @@ static void disable_dangling_plane(struct dc *dc,
> struct dc_state *context)
> struct dc_state *dangling_context = dc_create_state(dc);
> struct dc_state *current_ctx;
> struct pipe_ctx *pipe;
> + struct timing_generator *tg;
>
> if (dangling_context == NULL)
> return;
> @@ -1098,6 +1099,7 @@ static void disable_dangling_plane(struct dc
> *dc, struct dc_state *context)
>
> if (should_disable && old_stream) {
> pipe = &dc->current_state->res_ctx.pipe_ctx[i];
> + tg = pipe->stream_res.tg;
> /* When disabling plane for a phantom pipe, we must
> turn on the
>* phantom OTG so the disable programming gets the
> double buffer
>* update. Otherwise the pipe will be left in a
> partially disabled
> @@ -1105,7 +1107,8 @@ static void disable_dangling_plane(struct dc *dc,
> struct dc_state *context)
>* again for different use.
>*/
> if (old_stream->mall_stream_config.type ==
> SUBVP_PHANTOM) {
> -
> pipe->stream_res.tg->funcs->enable_crtc(pipe->stream_res.tg);
> + if (tg->funcs->enable_crtc)
> + tg->funcs->enable_crtc(tg);
> }
> dc_rem_all_planes_for_stream(dc, old_stream,
> dangling_context);
> disable_all_writeback_pipes_for_stream(dc, old_stream,
> dangling_context); @@ -1122,6 +1125,15 @@ static void
> disable_dangling_plane(struct dc *dc, struct dc_state *context)
> dc->hwss.interdependent_update_lock(dc,
> dc->current_state, false);
> dc->hwss.post_unlock_program_front_end(dc,
> dangling_context);
> }
> + /* We need to put the phantom OTG back into it's
> default (disabled) state or we
> + * can get corruption when transition from one SubVP
> config to a different one.
> + * The OTG is set to disable on falling edge of VUPDATE
> so the plane disable
> + * will still get it's double buffer update.
> + */
> + if (old_stream->mall_stream_config.type ==
> SUBVP_PHANTOM) {
> + if (tg->funcs->disable_phantom_crtc)
> + tg->funcs->disable_phantom_crtc(tg);
> + }
> }
> }
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c
> b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c
> index 2b33eeb213e2..2ee798965bc2 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c
> @@ -167,6 +167,13 @@ static void optc32_phantom_crtc_post_enable(struct
> timing_generator *optc)
> REG_WAIT(OTG_CL
Re: [PATCH 11/22] drm/amd/display: Disable phantom OTG after enable for plane disable
Hi Alan,
On Thu, Nov 03, 2022 at 12:01:06AM +0800, Alan Liu wrote:
> From: Alvin Lee
>
> [Description]
> - Need to disable phantom OTG after it's enabled
> in order to restore it to it's original state.
> - If it's enabled and then an MCLK switch comes in
> we may not prefetch the correct data since the phantom
> OTG could already be in the middle of the frame.
>
> Reviewed-by: Jun Lei
> Acked-by: Alan Liu
> Signed-off-by: Alvin Lee
> ---
> drivers/gpu/drm/amd/display/dc/core/dc.c | 14 +-
> drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c | 8
> .../drm/amd/display/dc/inc/hw/timing_generator.h | 1 +
> 3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index da808996e21d..9c3704c4d7e4 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -1055,6 +1055,7 @@ static void disable_dangling_plane(struct dc *dc,
> struct dc_state *context)
> struct dc_state *dangling_context = dc_create_state(dc);
> struct dc_state *current_ctx;
> struct pipe_ctx *pipe;
> + struct timing_generator *tg;
>
> if (dangling_context == NULL)
> return;
> @@ -1098,6 +1099,7 @@ static void disable_dangling_plane(struct dc *dc,
> struct dc_state *context)
>
> if (should_disable && old_stream) {
> pipe = &dc->current_state->res_ctx.pipe_ctx[i];
> + tg = pipe->stream_res.tg;
> /* When disabling plane for a phantom pipe, we must
> turn on the
>* phantom OTG so the disable programming gets the
> double buffer
>* update. Otherwise the pipe will be left in a
> partially disabled
> @@ -1105,7 +1107,8 @@ static void disable_dangling_plane(struct dc *dc,
> struct dc_state *context)
>* again for different use.
>*/
> if (old_stream->mall_stream_config.type ==
> SUBVP_PHANTOM) {
> -
> pipe->stream_res.tg->funcs->enable_crtc(pipe->stream_res.tg);
> + if (tg->funcs->enable_crtc)
> + tg->funcs->enable_crtc(tg);
> }
> dc_rem_all_planes_for_stream(dc, old_stream,
> dangling_context);
> disable_all_writeback_pipes_for_stream(dc, old_stream,
> dangling_context);
> @@ -1122,6 +1125,15 @@ static void disable_dangling_plane(struct dc *dc,
> struct dc_state *context)
> dc->hwss.interdependent_update_lock(dc,
> dc->current_state, false);
> dc->hwss.post_unlock_program_front_end(dc,
> dangling_context);
> }
> + /* We need to put the phantom OTG back into it's
> default (disabled) state or we
> + * can get corruption when transition from one SubVP
> config to a different one.
> + * The OTG is set to disable on falling edge of VUPDATE
> so the plane disable
> + * will still get it's double buffer update.
> + */
> + if (old_stream->mall_stream_config.type ==
> SUBVP_PHANTOM) {
> + if (tg->funcs->disable_phantom_crtc)
> + tg->funcs->disable_phantom_crtc(tg);
> + }
> }
> }
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c
> b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c
> index 2b33eeb213e2..2ee798965bc2 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c
> @@ -167,6 +167,13 @@ static void optc32_phantom_crtc_post_enable(struct
> timing_generator *optc)
> REG_WAIT(OTG_CLOCK_CONTROL, OTG_BUSY, 0, 1, 10);
> }
>
> +static void optc32_disable_phantom_otg(struct timing_generator *optc)
> +{
> + struct optc *optc1 = DCN10TG_FROM_TG(optc);
> +
> + REG_UPDATE(OTG_CONTROL, OTG_MASTER_EN, 0);
> +}
> +
> static void optc32_set_odm_bypass(struct timing_generator *optc,
> const struct dc_crtc_timing *dc_crtc_timing)
> {
> @@ -260,6 +267,7 @@ static struct timing_generator_funcs dcn32_tg_funcs = {
> .enable_crtc = optc32_enable_crtc,
> .disable_crtc = optc32_disable_crtc,
> .phantom_crtc_post_enable = optc32_phantom_crtc_post_enable,
> + .disable_phantom_crtc = optc32_disable_phantom_otg,
> /* used by enable_timing_synchronization. Not need for FPGA */
> .is_counter_moving = optc1_is_counter_moving,
> .get_position = optc1_get_position,
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/timing_generator.h
> b/drivers/gpu/drm/amd/dis
[PATCH 11/22] drm/amd/display: Disable phantom OTG after enable for plane disable
From: Alvin Lee
[Description]
- Need to disable phantom OTG after it's enabled
in order to restore it to it's original state.
- If it's enabled and then an MCLK switch comes in
we may not prefetch the correct data since the phantom
OTG could already be in the middle of the frame.
Reviewed-by: Jun Lei
Acked-by: Alan Liu
Signed-off-by: Alvin Lee
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 14 +-
drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c | 8
.../drm/amd/display/dc/inc/hw/timing_generator.h | 1 +
3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index da808996e21d..9c3704c4d7e4 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1055,6 +1055,7 @@ static void disable_dangling_plane(struct dc *dc, struct
dc_state *context)
struct dc_state *dangling_context = dc_create_state(dc);
struct dc_state *current_ctx;
struct pipe_ctx *pipe;
+ struct timing_generator *tg;
if (dangling_context == NULL)
return;
@@ -1098,6 +1099,7 @@ static void disable_dangling_plane(struct dc *dc, struct
dc_state *context)
if (should_disable && old_stream) {
pipe = &dc->current_state->res_ctx.pipe_ctx[i];
+ tg = pipe->stream_res.tg;
/* When disabling plane for a phantom pipe, we must
turn on the
* phantom OTG so the disable programming gets the
double buffer
* update. Otherwise the pipe will be left in a
partially disabled
@@ -1105,7 +1107,8 @@ static void disable_dangling_plane(struct dc *dc, struct
dc_state *context)
* again for different use.
*/
if (old_stream->mall_stream_config.type ==
SUBVP_PHANTOM) {
-
pipe->stream_res.tg->funcs->enable_crtc(pipe->stream_res.tg);
+ if (tg->funcs->enable_crtc)
+ tg->funcs->enable_crtc(tg);
}
dc_rem_all_planes_for_stream(dc, old_stream,
dangling_context);
disable_all_writeback_pipes_for_stream(dc, old_stream,
dangling_context);
@@ -1122,6 +1125,15 @@ static void disable_dangling_plane(struct dc *dc, struct
dc_state *context)
dc->hwss.interdependent_update_lock(dc,
dc->current_state, false);
dc->hwss.post_unlock_program_front_end(dc,
dangling_context);
}
+ /* We need to put the phantom OTG back into it's
default (disabled) state or we
+* can get corruption when transition from one SubVP
config to a different one.
+* The OTG is set to disable on falling edge of VUPDATE
so the plane disable
+* will still get it's double buffer update.
+*/
+ if (old_stream->mall_stream_config.type ==
SUBVP_PHANTOM) {
+ if (tg->funcs->disable_phantom_crtc)
+ tg->funcs->disable_phantom_crtc(tg);
+ }
}
}
diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c
b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c
index 2b33eeb213e2..2ee798965bc2 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c
@@ -167,6 +167,13 @@ static void optc32_phantom_crtc_post_enable(struct
timing_generator *optc)
REG_WAIT(OTG_CLOCK_CONTROL, OTG_BUSY, 0, 1, 10);
}
+static void optc32_disable_phantom_otg(struct timing_generator *optc)
+{
+ struct optc *optc1 = DCN10TG_FROM_TG(optc);
+
+ REG_UPDATE(OTG_CONTROL, OTG_MASTER_EN, 0);
+}
+
static void optc32_set_odm_bypass(struct timing_generator *optc,
const struct dc_crtc_timing *dc_crtc_timing)
{
@@ -260,6 +267,7 @@ static struct timing_generator_funcs dcn32_tg_funcs = {
.enable_crtc = optc32_enable_crtc,
.disable_crtc = optc32_disable_crtc,
.phantom_crtc_post_enable = optc32_phantom_crtc_post_enable,
+ .disable_phantom_crtc = optc32_disable_phantom_otg,
/* used by enable_timing_synchronization. Not need for FPGA */
.is_counter_moving = optc1_is_counter_moving,
.get_position = optc1_get_position,
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/timing_generator.h
b/drivers/gpu/drm/amd/display/dc/inc/hw/timing_generator.h
index 65f18f9dad34..43eb61961e0f 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw/timing_generator.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw/timing_generator.h
