* [PATCH] drm/amd/display: Use DCN30 watermark calc for DCN301
@ 2021-08-13 19:21 Liu, Zhan
2021-08-13 19:22 ` Liu, Zhan
2021-08-16 13:59 ` Leo Li
0 siblings, 2 replies; 5+ messages in thread
From: Liu, Zhan @ 2021-08-13 19:21 UTC (permalink / raw)
To: amd-gfx; +Cc: Cornij, Nikola, Liu, Zhan, Logush, Oliver
[AMD Official Use Only]
[why]
dcn301_calculate_wm_and_dl() causes flickering when external monitor is
connected.
This issue has been fixed before by commit 0e4c0ae59d7e
("drm/amdgpu/display: drop dcn301_calculate_wm_and_dl for now"), however
part of the fix was gone after commit 2cbcb78c9ee5 ("Merge tag
'amd-drm-next-5.13-2021-03-23' of
https://gitlab.freedesktop.org/agd5f/linux into drm-next").
[how]
Use dcn30_calculate_wm_and_dlg() instead as in the original fix.
Fixes: 2cbcb78c9ee5 ("Merge tag 'amd-drm-next-5.13-2021-03-23' of https://gitlab.freedesktop.org/agd5f/linux into drm-next")
Signed-off-by: Nikola Cornij mailto:nikola.cornij@amd.com
---
.../amd/display/dc/dcn301/dcn301_resource.c | 96 +------------------
1 file changed, 1 insertion(+), 95 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
index 9776d1737818..912285fdce18 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
@@ -1622,106 +1622,12 @@ static void dcn301_update_bw_bounding_box(struct dc *dc, struct clk_bw_params *b
dml_init_instance(&dc->dml, &dcn3_01_soc, &dcn3_01_ip, DML_PROJECT_DCN30);
}
-static void calculate_wm_set_for_vlevel(
- int vlevel,
- struct wm_range_table_entry *table_entry,
- struct dcn_watermarks *wm_set,
- struct display_mode_lib *dml,
- display_e2e_pipe_params_st *pipes,
- int pipe_cnt)
-{
- double dram_clock_change_latency_cached = dml->soc.dram_clock_change_latency_us;
-
- ASSERT(vlevel < dml->soc.num_states);
- /* only pipe 0 is read for voltage and dcf/soc clocks */
- pipes[0].clks_cfg.voltage = vlevel;
- pipes[0].clks_cfg.dcfclk_mhz = dml->soc.clock_limits[vlevel].dcfclk_mhz;
- pipes[0].clks_cfg.socclk_mhz = dml->soc.clock_limits[vlevel].socclk_mhz;
-
- dml->soc.dram_clock_change_latency_us = table_entry->pstate_latency_us;
- dml->soc.sr_exit_time_us = table_entry->sr_exit_time_us;
- dml->soc.sr_enter_plus_exit_time_us = table_entry->sr_enter_plus_exit_time_us;
-
- wm_set->urgent_ns = get_wm_urgent(dml, pipes, pipe_cnt) * 1000;
- wm_set->cstate_pstate.cstate_enter_plus_exit_ns = get_wm_stutter_enter_exit(dml, pipes, pipe_cnt) * 1000;
- wm_set->cstate_pstate.cstate_exit_ns = get_wm_stutter_exit(dml, pipes, pipe_cnt) * 1000;
- wm_set->cstate_pstate.pstate_change_ns = get_wm_dram_clock_change(dml, pipes, pipe_cnt) * 1000;
- wm_set->pte_meta_urgent_ns = get_wm_memory_trip(dml, pipes, pipe_cnt) * 1000;
- wm_set->frac_urg_bw_nom = get_fraction_of_urgent_bandwidth(dml, pipes, pipe_cnt) * 1000;
- wm_set->frac_urg_bw_flip = get_fraction_of_urgent_bandwidth_imm_flip(dml, pipes, pipe_cnt) * 1000;
- wm_set->urgent_latency_ns = get_urgent_latency(dml, pipes, pipe_cnt) * 1000;
- dml->soc.dram_clock_change_latency_us = dram_clock_change_latency_cached;
-
-}
-
-static void dcn301_calculate_wm_and_dlg(
- struct dc *dc, struct dc_state *context,
- display_e2e_pipe_params_st *pipes,
- int pipe_cnt,
- int vlevel_req)
-{
- int i, pipe_idx;
- int vlevel, vlevel_max;
- struct wm_range_table_entry *table_entry;
- struct clk_bw_params *bw_params = dc->clk_mgr->bw_params;
-
- ASSERT(bw_params);
-
- vlevel_max = bw_params->clk_table.num_entries - 1;
-
- /* WM Set D */
- table_entry = &bw_params->wm_table.entries[WM_D];
- if (table_entry->wm_type == WM_TYPE_RETRAINING)
- vlevel = 0;
- else
- vlevel = vlevel_max;
- calculate_wm_set_for_vlevel(vlevel, table_entry, &context->bw_ctx.bw.dcn.watermarks.d,
- &context->bw_ctx.dml, pipes, pipe_cnt);
- /* WM Set C */
- table_entry = &bw_params->wm_table.entries[WM_C];
- vlevel = min(max(vlevel_req, 2), vlevel_max);
- calculate_wm_set_for_vlevel(vlevel, table_entry, &context->bw_ctx.bw.dcn.watermarks.c,
- &context->bw_ctx.dml, pipes, pipe_cnt);
- /* WM Set B */
- table_entry = &bw_params->wm_table.entries[WM_B];
- vlevel = min(max(vlevel_req, 1), vlevel_max);
- calculate_wm_set_for_vlevel(vlevel, table_entry, &context->bw_ctx.bw.dcn.watermarks.b,
- &context->bw_ctx.dml, pipes, pipe_cnt);
-
- /* WM Set A */
- table_entry = &bw_params->wm_table.entries[WM_A];
- vlevel = min(vlevel_req, vlevel_max);
- calculate_wm_set_for_vlevel(vlevel, table_entry, &context->bw_ctx.bw.dcn.watermarks.a,
- &context->bw_ctx.dml, pipes, pipe_cnt);
-
- for (i = 0, pipe_idx = 0; i < dc->res_pool->pipe_count; i++) {
- if (!context->res_ctx.pipe_ctx[i].stream)
- continue;
-
- pipes[pipe_idx].clks_cfg.dispclk_mhz = get_dispclk_calculated(&context->bw_ctx.dml, pipes, pipe_cnt);
- pipes[pipe_idx].clks_cfg.dppclk_mhz = get_dppclk_calculated(&context->bw_ctx.dml, pipes, pipe_cnt, pipe_idx);
-
- if (dc->config.forced_clocks) {
- pipes[pipe_idx].clks_cfg.dispclk_mhz = context->bw_ctx.dml.soc.clock_limits[0].dispclk_mhz;
- pipes[pipe_idx].clks_cfg.dppclk_mhz = context->bw_ctx.dml.soc.clock_limits[0].dppclk_mhz;
- }
- if (dc->debug.min_disp_clk_khz > pipes[pipe_idx].clks_cfg.dispclk_mhz * 1000)
- pipes[pipe_idx].clks_cfg.dispclk_mhz = dc->debug.min_disp_clk_khz / 1000.0;
- if (dc->debug.min_dpp_clk_khz > pipes[pipe_idx].clks_cfg.dppclk_mhz * 1000)
- pipes[pipe_idx].clks_cfg.dppclk_mhz = dc->debug.min_dpp_clk_khz / 1000.0;
-
- pipe_idx++;
- }
-
- dcn20_calculate_dlg_params(dc, context, pipes, pipe_cnt, vlevel);
-}
-
static struct resource_funcs dcn301_res_pool_funcs = {
.destroy = dcn301_destroy_resource_pool,
.link_enc_create = dcn301_link_encoder_create,
.panel_cntl_create = dcn301_panel_cntl_create,
.validate_bandwidth = dcn30_validate_bandwidth,
- .calculate_wm_and_dlg = dcn301_calculate_wm_and_dlg,
+ .calculate_wm_and_dlg = dcn30_calculate_wm_and_dlg,
.update_soc_for_wm_a = dcn30_update_soc_for_wm_a,
.populate_dml_pipes = dcn30_populate_dml_pipes_from_context,
.acquire_idle_pipe_for_layer = dcn20_acquire_idle_pipe_for_layer,
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH] drm/amd/display: Use DCN30 watermark calc for DCN301
2021-08-13 19:21 [PATCH] drm/amd/display: Use DCN30 watermark calc for DCN301 Liu, Zhan
@ 2021-08-13 19:22 ` Liu, Zhan
2021-08-16 13:59 ` Leo Li
1 sibling, 0 replies; 5+ messages in thread
From: Liu, Zhan @ 2021-08-13 19:22 UTC (permalink / raw)
To: amd-gfx; +Cc: Cornij, Nikola, Logush, Oliver
[Public]
> -----Original Message-----
> From: Liu, Zhan <Zhan.Liu@amd.com>
> Sent: 2021/August/13, Friday 3:21 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Cornij, Nikola <Nikola.Cornij@amd.com>; Liu, Zhan
> <Zhan.Liu@amd.com>; Logush, Oliver <Oliver.Logush@amd.com>
> Subject: [PATCH] drm/amd/display: Use DCN30 watermark calc for DCN301
>
>
> [why]
> dcn301_calculate_wm_and_dl() causes flickering when external monitor is
> connected.
>
> This issue has been fixed before by commit 0e4c0ae59d7e
> ("drm/amdgpu/display: drop dcn301_calculate_wm_and_dl for now"),
> however part of the fix was gone after commit 2cbcb78c9ee5 ("Merge tag
> 'amd-drm-next-5.13-2021-03-23' of
> https://gitlab.freedesktop.org/agd5f/linux into drm-next").
>
> [how]
> Use dcn30_calculate_wm_and_dlg() instead as in the original fix.
>
> Fixes: 2cbcb78c9ee5 ("Merge tag 'amd-drm-next-5.13-2021-03-23' of
> https://gitlab.freedesktop.org/agd5f/linux into drm-next")
> Signed-off-by: Nikola Cornij <nikola.cornij@amd.com>
Reviewed-by: Zhan Liu <zhan.liu@amd.com>
Tested-by: Zhan Liu <zhan.liu@amd.com>
Tested-by: Oliver Logush <oliver.logush@amd.com>
> ---
> .../amd/display/dc/dcn301/dcn301_resource.c | 96 +------------------
> 1 file changed, 1 insertion(+), 95 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> index 9776d1737818..912285fdce18 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> @@ -1622,106 +1622,12 @@ static void
> dcn301_update_bw_bounding_box(struct dc *dc, struct clk_bw_params *b
> dml_init_instance(&dc->dml, &dcn3_01_soc, &dcn3_01_ip,
> DML_PROJECT_DCN30); }
>
> -static void calculate_wm_set_for_vlevel(
> - int vlevel,
> - struct wm_range_table_entry *table_entry,
> - struct dcn_watermarks *wm_set,
> - struct display_mode_lib *dml,
> - display_e2e_pipe_params_st *pipes,
> - int pipe_cnt)
> -{
> - double dram_clock_change_latency_cached = dml-
> >soc.dram_clock_change_latency_us;
> -
> - ASSERT(vlevel < dml->soc.num_states);
> - /* only pipe 0 is read for voltage and dcf/soc clocks */
> - pipes[0].clks_cfg.voltage = vlevel;
> - pipes[0].clks_cfg.dcfclk_mhz = dml->soc.clock_limits[vlevel].dcfclk_mhz;
> - pipes[0].clks_cfg.socclk_mhz = dml->soc.clock_limits[vlevel].socclk_mhz;
> -
> - dml->soc.dram_clock_change_latency_us = table_entry-
> >pstate_latency_us;
> - dml->soc.sr_exit_time_us = table_entry->sr_exit_time_us;
> - dml->soc.sr_enter_plus_exit_time_us = table_entry-
> >sr_enter_plus_exit_time_us;
> -
> - wm_set->urgent_ns = get_wm_urgent(dml, pipes, pipe_cnt) * 1000;
> - wm_set->cstate_pstate.cstate_enter_plus_exit_ns =
> get_wm_stutter_enter_exit(dml, pipes, pipe_cnt) * 1000;
> - wm_set->cstate_pstate.cstate_exit_ns = get_wm_stutter_exit(dml, pipes,
> pipe_cnt) * 1000;
> - wm_set->cstate_pstate.pstate_change_ns =
> get_wm_dram_clock_change(dml, pipes, pipe_cnt) * 1000;
> - wm_set->pte_meta_urgent_ns = get_wm_memory_trip(dml, pipes,
> pipe_cnt) * 1000;
> - wm_set->frac_urg_bw_nom = get_fraction_of_urgent_bandwidth(dml,
> pipes, pipe_cnt) * 1000;
> - wm_set->frac_urg_bw_flip =
> get_fraction_of_urgent_bandwidth_imm_flip(dml, pipes, pipe_cnt) * 1000;
> - wm_set->urgent_latency_ns = get_urgent_latency(dml, pipes, pipe_cnt)
> * 1000;
> - dml->soc.dram_clock_change_latency_us =
> dram_clock_change_latency_cached;
> -
> -}
> -
> -static void dcn301_calculate_wm_and_dlg(
> - struct dc *dc, struct dc_state *context,
> - display_e2e_pipe_params_st *pipes,
> - int pipe_cnt,
> - int vlevel_req)
> -{
> - int i, pipe_idx;
> - int vlevel, vlevel_max;
> - struct wm_range_table_entry *table_entry;
> - struct clk_bw_params *bw_params = dc->clk_mgr->bw_params;
> -
> - ASSERT(bw_params);
> -
> - vlevel_max = bw_params->clk_table.num_entries - 1;
> -
> - /* WM Set D */
> - table_entry = &bw_params->wm_table.entries[WM_D];
> - if (table_entry->wm_type == WM_TYPE_RETRAINING)
> - vlevel = 0;
> - else
> - vlevel = vlevel_max;
> - calculate_wm_set_for_vlevel(vlevel, table_entry, &context-
> >bw_ctx.bw.dcn.watermarks.d,
> - &context->bw_ctx.dml, pipes, pipe_cnt);
> - /* WM Set C */
> - table_entry = &bw_params->wm_table.entries[WM_C];
> - vlevel = min(max(vlevel_req, 2), vlevel_max);
> - calculate_wm_set_for_vlevel(vlevel, table_entry, &context-
> >bw_ctx.bw.dcn.watermarks.c,
> - &context->bw_ctx.dml, pipes, pipe_cnt);
> - /* WM Set B */
> - table_entry = &bw_params->wm_table.entries[WM_B];
> - vlevel = min(max(vlevel_req, 1), vlevel_max);
> - calculate_wm_set_for_vlevel(vlevel, table_entry, &context-
> >bw_ctx.bw.dcn.watermarks.b,
> - &context->bw_ctx.dml, pipes, pipe_cnt);
> -
> - /* WM Set A */
> - table_entry = &bw_params->wm_table.entries[WM_A];
> - vlevel = min(vlevel_req, vlevel_max);
> - calculate_wm_set_for_vlevel(vlevel, table_entry, &context-
> >bw_ctx.bw.dcn.watermarks.a,
> - &context->bw_ctx.dml, pipes, pipe_cnt);
> -
> - for (i = 0, pipe_idx = 0; i < dc->res_pool->pipe_count; i++) {
> - if (!context->res_ctx.pipe_ctx[i].stream)
> - continue;
> -
> - pipes[pipe_idx].clks_cfg.dispclk_mhz =
> get_dispclk_calculated(&context->bw_ctx.dml, pipes, pipe_cnt);
> - pipes[pipe_idx].clks_cfg.dppclk_mhz =
> get_dppclk_calculated(&context->bw_ctx.dml, pipes, pipe_cnt, pipe_idx);
> -
> - if (dc->config.forced_clocks) {
> - pipes[pipe_idx].clks_cfg.dispclk_mhz = context-
> >bw_ctx.dml.soc.clock_limits[0].dispclk_mhz;
> - pipes[pipe_idx].clks_cfg.dppclk_mhz = context-
> >bw_ctx.dml.soc.clock_limits[0].dppclk_mhz;
> - }
> - if (dc->debug.min_disp_clk_khz >
> pipes[pipe_idx].clks_cfg.dispclk_mhz * 1000)
> - pipes[pipe_idx].clks_cfg.dispclk_mhz = dc-
> >debug.min_disp_clk_khz / 1000.0;
> - if (dc->debug.min_dpp_clk_khz >
> pipes[pipe_idx].clks_cfg.dppclk_mhz * 1000)
> - pipes[pipe_idx].clks_cfg.dppclk_mhz = dc-
> >debug.min_dpp_clk_khz / 1000.0;
> -
> - pipe_idx++;
> - }
> -
> - dcn20_calculate_dlg_params(dc, context, pipes, pipe_cnt, vlevel);
> -}
> -
> static struct resource_funcs dcn301_res_pool_funcs = {
> .destroy = dcn301_destroy_resource_pool,
> .link_enc_create = dcn301_link_encoder_create,
> .panel_cntl_create = dcn301_panel_cntl_create,
> .validate_bandwidth = dcn30_validate_bandwidth,
> - .calculate_wm_and_dlg = dcn301_calculate_wm_and_dlg,
> + .calculate_wm_and_dlg = dcn30_calculate_wm_and_dlg,
> .update_soc_for_wm_a = dcn30_update_soc_for_wm_a,
> .populate_dml_pipes = dcn30_populate_dml_pipes_from_context,
> .acquire_idle_pipe_for_layer = dcn20_acquire_idle_pipe_for_layer,
> --
> 2.31.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amd/display: Use DCN30 watermark calc for DCN301
2021-08-13 19:21 [PATCH] drm/amd/display: Use DCN30 watermark calc for DCN301 Liu, Zhan
2021-08-13 19:22 ` Liu, Zhan
@ 2021-08-16 13:59 ` Leo Li
2021-08-16 20:25 ` Leo Li
1 sibling, 1 reply; 5+ messages in thread
From: Leo Li @ 2021-08-16 13:59 UTC (permalink / raw)
To: Liu, Zhan, amd-gfx; +Cc: Cornij, Nikola, Logush, Oliver
On 2021-08-13 3:21 p.m., Liu, Zhan wrote:
> [AMD Official Use Only]
>
> [AMD Official Use Only]
>
> [why]
> dcn301_calculate_wm_and_dl() causes flickering when external monitor is
> connected.
>
> This issue has been fixed before by commit 0e4c0ae59d7e
> ("drm/amdgpu/display: drop dcn301_calculate_wm_and_dl for now"), however
> part of the fix was gone after commit 2cbcb78c9ee5 ("Merge tag
> 'amd-drm-next-5.13-2021-03-23' of
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fagd5f%2Flinux&data=04%7C01%7Csunpeng.li%40amd.com%7C5f101c8bf2594f79890508d95e8f8f98%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637644792952782170%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=VDcz5SuZ0guBGRKKOlOWdWI%2FDLuIIwYAIs%2F8geu4JLU%3D&reserved=0 into drm-next").
>
> [how]
> Use dcn30_calculate_wm_and_dlg() instead as in the original fix.
>
> Fixes: 2cbcb78c9ee5 ("Merge tag 'amd-drm-next-5.13-2021-03-23' of https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fagd5f%2Flinux&data=04%7C01%7Csunpeng.li%40amd.com%7C5f101c8bf2594f79890508d95e8f8f98%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637644792952782170%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=VDcz5SuZ0guBGRKKOlOWdWI%2FDLuIIwYAIs%2F8geu4JLU%3D&reserved=0 into drm-next")
> Signed-off-by: Nikola Cornij mailto:nikola.cornij@amd.com
> ---
> .../amd/display/dc/dcn301/dcn301_resource.c | 96 +------------------
> 1 file changed, 1 insertion(+), 95 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> index 9776d1737818..912285fdce18 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> @@ -1622,106 +1622,12 @@ static void dcn301_update_bw_bounding_box(struct dc *dc, struct clk_bw_params *b
> dml_init_instance(&dc->dml, &dcn3_01_soc, &dcn3_01_ip, DML_PROJECT_DCN30);
> }
>
> -static void calculate_wm_set_for_vlevel(
> - int vlevel,
> - struct wm_range_table_entry *table_entry,
> - struct dcn_watermarks *wm_set,
> - struct display_mode_lib *dml,
> - display_e2e_pipe_params_st *pipes,
> - int pipe_cnt)
> -{
> - double dram_clock_change_latency_cached = dml->soc.dram_clock_change_latency_us;
> -
> - ASSERT(vlevel < dml->soc.num_states);
> - /* only pipe 0 is read for voltage and dcf/soc clocks */
> - pipes[0].clks_cfg.voltage = vlevel;
> - pipes[0].clks_cfg.dcfclk_mhz = dml->soc.clock_limits[vlevel].dcfclk_mhz;
> - pipes[0].clks_cfg.socclk_mhz = dml->soc.clock_limits[vlevel].socclk_mhz;
> -
> - dml->soc.dram_clock_change_latency_us = table_entry->pstate_latency_us;
> - dml->soc.sr_exit_time_us = table_entry->sr_exit_time_us;
> - dml->soc.sr_enter_plus_exit_time_us = table_entry->sr_enter_plus_exit_time_us;
> -
> - wm_set->urgent_ns = get_wm_urgent(dml, pipes, pipe_cnt) * 1000;
> - wm_set->cstate_pstate.cstate_enter_plus_exit_ns = get_wm_stutter_enter_exit(dml, pipes, pipe_cnt) * 1000;
> - wm_set->cstate_pstate.cstate_exit_ns = get_wm_stutter_exit(dml, pipes, pipe_cnt) * 1000;
> - wm_set->cstate_pstate.pstate_change_ns = get_wm_dram_clock_change(dml, pipes, pipe_cnt) * 1000;
> - wm_set->pte_meta_urgent_ns = get_wm_memory_trip(dml, pipes, pipe_cnt) * 1000;
> - wm_set->frac_urg_bw_nom = get_fraction_of_urgent_bandwidth(dml, pipes, pipe_cnt) * 1000;
> - wm_set->frac_urg_bw_flip = get_fraction_of_urgent_bandwidth_imm_flip(dml, pipes, pipe_cnt) * 1000;
> - wm_set->urgent_latency_ns = get_urgent_latency(dml, pipes, pipe_cnt) * 1000;
> - dml->soc.dram_clock_change_latency_us = dram_clock_change_latency_cached;
> -
> -}
> -
> -static void dcn301_calculate_wm_and_dlg(
> - struct dc *dc, struct dc_state *context,
> - display_e2e_pipe_params_st *pipes,
> - int pipe_cnt,
> - int vlevel_req)
> -{
> - int i, pipe_idx;
> - int vlevel, vlevel_max;
> - struct wm_range_table_entry *table_entry;
> - struct clk_bw_params *bw_params = dc->clk_mgr->bw_params;
> -
> - ASSERT(bw_params);
> -
> - vlevel_max = bw_params->clk_table.num_entries - 1;
> -
> - /* WM Set D */
> - table_entry = &bw_params->wm_table.entries[WM_D];
> - if (table_entry->wm_type == WM_TYPE_RETRAINING)
> - vlevel = 0;
> - else
> - vlevel = vlevel_max;
> - calculate_wm_set_for_vlevel(vlevel, table_entry, &context->bw_ctx.bw.dcn.watermarks.d,
> - &context->bw_ctx.dml, pipes, pipe_cnt);
> - /* WM Set C */
> - table_entry = &bw_params->wm_table.entries[WM_C];
> - vlevel = min(max(vlevel_req, 2), vlevel_max);
> - calculate_wm_set_for_vlevel(vlevel, table_entry, &context->bw_ctx.bw.dcn.watermarks.c,
> - &context->bw_ctx.dml, pipes, pipe_cnt);
> - /* WM Set B */
> - table_entry = &bw_params->wm_table.entries[WM_B];
> - vlevel = min(max(vlevel_req, 1), vlevel_max);
> - calculate_wm_set_for_vlevel(vlevel, table_entry, &context->bw_ctx.bw.dcn.watermarks.b,
> - &context->bw_ctx.dml, pipes, pipe_cnt);
> -
> - /* WM Set A */
> - table_entry = &bw_params->wm_table.entries[WM_A];
> - vlevel = min(vlevel_req, vlevel_max);
> - calculate_wm_set_for_vlevel(vlevel, table_entry, &context->bw_ctx.bw.dcn.watermarks.a,
> - &context->bw_ctx.dml, pipes, pipe_cnt);
> -
> - for (i = 0, pipe_idx = 0; i < dc->res_pool->pipe_count; i++) {
> - if (!context->res_ctx.pipe_ctx[i].stream)
> - continue;
> -
> - pipes[pipe_idx].clks_cfg.dispclk_mhz = get_dispclk_calculated(&context->bw_ctx.dml, pipes, pipe_cnt);
> - pipes[pipe_idx].clks_cfg.dppclk_mhz = get_dppclk_calculated(&context->bw_ctx.dml, pipes, pipe_cnt, pipe_idx);
> -
> - if (dc->config.forced_clocks) {
> - pipes[pipe_idx].clks_cfg.dispclk_mhz = context->bw_ctx.dml.soc.clock_limits[0].dispclk_mhz;
> - pipes[pipe_idx].clks_cfg.dppclk_mhz = context->bw_ctx.dml.soc.clock_limits[0].dppclk_mhz;
> - }
> - if (dc->debug.min_disp_clk_khz > pipes[pipe_idx].clks_cfg.dispclk_mhz * 1000)
> - pipes[pipe_idx].clks_cfg.dispclk_mhz = dc->debug.min_disp_clk_khz / 1000.0;
> - if (dc->debug.min_dpp_clk_khz > pipes[pipe_idx].clks_cfg.dppclk_mhz * 1000)
> - pipes[pipe_idx].clks_cfg.dppclk_mhz = dc->debug.min_dpp_clk_khz / 1000.0;
> -
> - pipe_idx++;
> - }
> -
> - dcn20_calculate_dlg_params(dc, context, pipes, pipe_cnt, vlevel);
> -}
> -
> static struct resource_funcs dcn301_res_pool_funcs = {
> .destroy = dcn301_destroy_resource_pool,
> .link_enc_create = dcn301_link_encoder_create,
> .panel_cntl_create = dcn301_panel_cntl_create,
> .validate_bandwidth = dcn30_validate_bandwidth,
> - .calculate_wm_and_dlg = dcn301_calculate_wm_and_dlg,
> + .calculate_wm_and_dlg = dcn30_calculate_wm_and_dlg,
Hi Zhan,
Using dcn30_calculate_wm_and_dlg smells fishy, IIRC watermark
calculations for DPUG and APU are very different. It's likely that
you're now picking up corrupted values form the wm_table.
Take a look at how struct wm_table is populated in vg_clk_mgr.c v.s.
dcn30_clk_mgr.c. For APU, wm_table.entries are populated, whereas for
DGPU, wm_table.nv_entries are populated. .entries and .nv_entries are
under a union, with very different struct definitions.
Have you taken a look at whether the pstate latency and sr enter/exit
latency values being used after your change are sensible? It could be
that you simply needed to raise these watermarks.
Thanks,
Leo
> .update_soc_for_wm_a = dcn30_update_soc_for_wm_a,
> .populate_dml_pipes = dcn30_populate_dml_pipes_from_context,
> .acquire_idle_pipe_for_layer = dcn20_acquire_idle_pipe_for_layer,
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amd/display: Use DCN30 watermark calc for DCN301
2021-08-16 13:59 ` Leo Li
@ 2021-08-16 20:25 ` Leo Li
2021-08-16 20:27 ` Alex Deucher
0 siblings, 1 reply; 5+ messages in thread
From: Leo Li @ 2021-08-16 20:25 UTC (permalink / raw)
To: Liu, Zhan, amd-gfx; +Cc: Cornij, Nikola, Logush, Oliver
On 2021-08-16 9:59 a.m., Leo Li wrote:
>
>
>
> On 2021-08-13 3:21 p.m., Liu, Zhan wrote:
>> [AMD Official Use Only]
>>
>> [AMD Official Use Only]
>>
>> [why]
>> dcn301_calculate_wm_and_dl() causes flickering when external monitor is
>> connected.
>>
>> This issue has been fixed before by commit 0e4c0ae59d7e
>> ("drm/amdgpu/display: drop dcn301_calculate_wm_and_dl for now"), however
>> part of the fix was gone after commit 2cbcb78c9ee5 ("Merge tag
>> 'amd-drm-next-5.13-2021-03-23' of
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fagd5f%2Flinux&data=04%7C01%7Csunpeng.li%40amd.com%7C723f9131e57b4bd99db508d960be2441%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637647192045690562%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=f2gL9TVAvdXlCbsZCDa2prF1J4l2ZDbpY8L2f6vK7as%3D&reserved=0
>> into drm-next").
>>
>> [how]
>> Use dcn30_calculate_wm_and_dlg() instead as in the original fix.
>>
>> Fixes: 2cbcb78c9ee5 ("Merge tag 'amd-drm-next-5.13-2021-03-23' of
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fagd5f%2Flinux&data=04%7C01%7Csunpeng.li%40amd.com%7C723f9131e57b4bd99db508d960be2441%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637647192045690562%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=f2gL9TVAvdXlCbsZCDa2prF1J4l2ZDbpY8L2f6vK7as%3D&reserved=0
>> into drm-next")
>> Signed-off-by: Nikola Cornij mailto:nikola.cornij@amd.com
>> ---
>> .../amd/display/dc/dcn301/dcn301_resource.c | 96 +------------------
>> 1 file changed, 1 insertion(+), 95 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
>> b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
>> index 9776d1737818..912285fdce18 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
>> @@ -1622,106 +1622,12 @@ static void
>> dcn301_update_bw_bounding_box(struct dc *dc, struct clk_bw_params *b
>> dml_init_instance(&dc->dml, &dcn3_01_soc, &dcn3_01_ip,
>> DML_PROJECT_DCN30);
>> }
>>
>> -static void calculate_wm_set_for_vlevel(
>> - int vlevel,
>> - struct wm_range_table_entry *table_entry,
>> - struct dcn_watermarks *wm_set,
>> - struct display_mode_lib *dml,
>> - display_e2e_pipe_params_st *pipes,
>> - int pipe_cnt)
>> -{
>> - double dram_clock_change_latency_cached =
>> dml->soc.dram_clock_change_latency_us;
>> -
>> - ASSERT(vlevel < dml->soc.num_states);
>> - /* only pipe 0 is read for voltage and dcf/soc clocks */
>> - pipes[0].clks_cfg.voltage = vlevel;
>> - pipes[0].clks_cfg.dcfclk_mhz =
>> dml->soc.clock_limits[vlevel].dcfclk_mhz;
>> - pipes[0].clks_cfg.socclk_mhz =
>> dml->soc.clock_limits[vlevel].socclk_mhz;
>> -
>> - dml->soc.dram_clock_change_latency_us =
>> table_entry->pstate_latency_us;
>> - dml->soc.sr_exit_time_us = table_entry->sr_exit_time_us;
>> - dml->soc.sr_enter_plus_exit_time_us =
>> table_entry->sr_enter_plus_exit_time_us;
>> -
>> - wm_set->urgent_ns = get_wm_urgent(dml, pipes, pipe_cnt) * 1000;
>> - wm_set->cstate_pstate.cstate_enter_plus_exit_ns =
>> get_wm_stutter_enter_exit(dml, pipes, pipe_cnt) * 1000;
>> - wm_set->cstate_pstate.cstate_exit_ns =
>> get_wm_stutter_exit(dml, pipes, pipe_cnt) * 1000;
>> - wm_set->cstate_pstate.pstate_change_ns =
>> get_wm_dram_clock_change(dml, pipes, pipe_cnt) * 1000;
>> - wm_set->pte_meta_urgent_ns = get_wm_memory_trip(dml, pipes,
>> pipe_cnt) * 1000;
>> - wm_set->frac_urg_bw_nom =
>> get_fraction_of_urgent_bandwidth(dml, pipes, pipe_cnt) * 1000;
>> - wm_set->frac_urg_bw_flip =
>> get_fraction_of_urgent_bandwidth_imm_flip(dml, pipes, pipe_cnt) * 1000;
>> - wm_set->urgent_latency_ns = get_urgent_latency(dml, pipes,
>> pipe_cnt) * 1000;
>> - dml->soc.dram_clock_change_latency_us =
>> dram_clock_change_latency_cached;
>> -
>> -}
>> -
>> -static void dcn301_calculate_wm_and_dlg(
>> - struct dc *dc, struct dc_state *context,
>> - display_e2e_pipe_params_st *pipes,
>> - int pipe_cnt,
>> - int vlevel_req)
>> -{
>> - int i, pipe_idx;
>> - int vlevel, vlevel_max;
>> - struct wm_range_table_entry *table_entry;
>> - struct clk_bw_params *bw_params = dc->clk_mgr->bw_params;
>> -
>> - ASSERT(bw_params);
>> -
>> - vlevel_max = bw_params->clk_table.num_entries - 1;
>> -
>> - /* WM Set D */
>> - table_entry = &bw_params->wm_table.entries[WM_D];
>> - if (table_entry->wm_type == WM_TYPE_RETRAINING)
>> - vlevel = 0;
>> - else
>> - vlevel = vlevel_max;
>> - calculate_wm_set_for_vlevel(vlevel, table_entry,
>> &context->bw_ctx.bw.dcn.watermarks.d,
>> - &context->bw_ctx.dml,
>> pipes, pipe_cnt);
>> - /* WM Set C */
>> - table_entry = &bw_params->wm_table.entries[WM_C];
>> - vlevel = min(max(vlevel_req, 2), vlevel_max);
>> - calculate_wm_set_for_vlevel(vlevel, table_entry,
>> &context->bw_ctx.bw.dcn.watermarks.c,
>> - &context->bw_ctx.dml,
>> pipes, pipe_cnt);
>> - /* WM Set B */
>> - table_entry = &bw_params->wm_table.entries[WM_B];
>> - vlevel = min(max(vlevel_req, 1), vlevel_max);
>> - calculate_wm_set_for_vlevel(vlevel, table_entry,
>> &context->bw_ctx.bw.dcn.watermarks.b,
>> - &context->bw_ctx.dml,
>> pipes, pipe_cnt);
>> -
>> - /* WM Set A */
>> - table_entry = &bw_params->wm_table.entries[WM_A];
>> - vlevel = min(vlevel_req, vlevel_max);
>> - calculate_wm_set_for_vlevel(vlevel, table_entry,
>> &context->bw_ctx.bw.dcn.watermarks.a,
>> - &context->bw_ctx.dml,
>> pipes, pipe_cnt);
>> -
>> - for (i = 0, pipe_idx = 0; i < dc->res_pool->pipe_count; i++) {
>> - if (!context->res_ctx.pipe_ctx[i].stream)
>> - continue;
>> -
>> - pipes[pipe_idx].clks_cfg.dispclk_mhz =
>> get_dispclk_calculated(&context->bw_ctx.dml, pipes, pipe_cnt);
>> - pipes[pipe_idx].clks_cfg.dppclk_mhz =
>> get_dppclk_calculated(&context->bw_ctx.dml, pipes, pipe_cnt, pipe_idx);
>> -
>> - if (dc->config.forced_clocks) {
>> - pipes[pipe_idx].clks_cfg.dispclk_mhz =
>> context->bw_ctx.dml.soc.clock_limits[0].dispclk_mhz;
>> - pipes[pipe_idx].clks_cfg.dppclk_mhz =
>> context->bw_ctx.dml.soc.clock_limits[0].dppclk_mhz;
>> - }
>> - if (dc->debug.min_disp_clk_khz >
>> pipes[pipe_idx].clks_cfg.dispclk_mhz * 1000)
>> - pipes[pipe_idx].clks_cfg.dispclk_mhz =
>> dc->debug.min_disp_clk_khz / 1000.0;
>> - if (dc->debug.min_dpp_clk_khz >
>> pipes[pipe_idx].clks_cfg.dppclk_mhz * 1000)
>> - pipes[pipe_idx].clks_cfg.dppclk_mhz =
>> dc->debug.min_dpp_clk_khz / 1000.0;
>> -
>> - pipe_idx++;
>> - }
>> -
>> - dcn20_calculate_dlg_params(dc, context, pipes, pipe_cnt, vlevel);
>> -}
>> -
>> static struct resource_funcs dcn301_res_pool_funcs = {
>> .destroy = dcn301_destroy_resource_pool,
>> .link_enc_create = dcn301_link_encoder_create,
>> .panel_cntl_create = dcn301_panel_cntl_create,
>> .validate_bandwidth = dcn30_validate_bandwidth,
>> - .calculate_wm_and_dlg = dcn301_calculate_wm_and_dlg,
>> + .calculate_wm_and_dlg = dcn30_calculate_wm_and_dlg,
>
> Hi Zhan,
>
> Using dcn30_calculate_wm_and_dlg smells fishy, IIRC watermark
> calculations for DPUG and APU are very different. It's likely that
> you're now picking up corrupted values form the wm_table.
>
> Take a look at how struct wm_table is populated in vg_clk_mgr.c v.s.
> dcn30_clk_mgr.c. For APU, wm_table.entries are populated, whereas for
> DGPU, wm_table.nv_entries are populated. .entries and .nv_entries are
> under a union, with very different struct definitions.
>
> Have you taken a look at whether the pstate latency and sr enter/exit
> latency values being used after your change are sensible? It could be
> that you simply needed to raise these watermarks.
>
> Thanks,
> Leo
After some DMs, it looks like this change is simply restoring an
accidental revert that occurred due to a recent rebase. Given that this
is needed to fix a regression,
Acked-by: Leo Li <sunpeng.li@amd.com>
Nevertheless, this still looks iffy. I'm not sure if the pstate and sr
enter/exit latencies being used here are what you expect.
Thanks,
Leo
>
>> .update_soc_for_wm_a = dcn30_update_soc_for_wm_a,
>> .populate_dml_pipes = dcn30_populate_dml_pipes_from_context,
>> .acquire_idle_pipe_for_layer =
>> dcn20_acquire_idle_pipe_for_layer,
>> --
>> 2.31.1
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amd/display: Use DCN30 watermark calc for DCN301
2021-08-16 20:25 ` Leo Li
@ 2021-08-16 20:27 ` Alex Deucher
0 siblings, 0 replies; 5+ messages in thread
From: Alex Deucher @ 2021-08-16 20:27 UTC (permalink / raw)
To: Leo Li; +Cc: Liu, Zhan, amd-gfx, Cornij, Nikola, Logush, Oliver
On Mon, Aug 16, 2021 at 4:25 PM Leo Li <sunpeng.li@amd.com> wrote:
>
>
>
> On 2021-08-16 9:59 a.m., Leo Li wrote:
> >
> >
> >
> > On 2021-08-13 3:21 p.m., Liu, Zhan wrote:
> >> [AMD Official Use Only]
> >>
> >> [AMD Official Use Only]
> >>
> >> [why]
> >> dcn301_calculate_wm_and_dl() causes flickering when external monitor is
> >> connected.
> >>
> >> This issue has been fixed before by commit 0e4c0ae59d7e
> >> ("drm/amdgpu/display: drop dcn301_calculate_wm_and_dl for now"), however
> >> part of the fix was gone after commit 2cbcb78c9ee5 ("Merge tag
> >> 'amd-drm-next-5.13-2021-03-23' of
> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fagd5f%2Flinux&data=04%7C01%7Csunpeng.li%40amd.com%7C723f9131e57b4bd99db508d960be2441%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637647192045690562%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=f2gL9TVAvdXlCbsZCDa2prF1J4l2ZDbpY8L2f6vK7as%3D&reserved=0
> >> into drm-next").
> >>
> >> [how]
> >> Use dcn30_calculate_wm_and_dlg() instead as in the original fix.
> >>
> >> Fixes: 2cbcb78c9ee5 ("Merge tag 'amd-drm-next-5.13-2021-03-23' of
> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fagd5f%2Flinux&data=04%7C01%7Csunpeng.li%40amd.com%7C723f9131e57b4bd99db508d960be2441%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637647192045690562%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=f2gL9TVAvdXlCbsZCDa2prF1J4l2ZDbpY8L2f6vK7as%3D&reserved=0
> >> into drm-next")
> >> Signed-off-by: Nikola Cornij mailto:nikola.cornij@amd.com
> >> ---
> >> .../amd/display/dc/dcn301/dcn301_resource.c | 96 +------------------
> >> 1 file changed, 1 insertion(+), 95 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> >> b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> >> index 9776d1737818..912285fdce18 100644
> >> --- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> >> +++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> >> @@ -1622,106 +1622,12 @@ static void
> >> dcn301_update_bw_bounding_box(struct dc *dc, struct clk_bw_params *b
> >> dml_init_instance(&dc->dml, &dcn3_01_soc, &dcn3_01_ip,
> >> DML_PROJECT_DCN30);
> >> }
> >>
> >> -static void calculate_wm_set_for_vlevel(
> >> - int vlevel,
> >> - struct wm_range_table_entry *table_entry,
> >> - struct dcn_watermarks *wm_set,
> >> - struct display_mode_lib *dml,
> >> - display_e2e_pipe_params_st *pipes,
> >> - int pipe_cnt)
> >> -{
> >> - double dram_clock_change_latency_cached =
> >> dml->soc.dram_clock_change_latency_us;
> >> -
> >> - ASSERT(vlevel < dml->soc.num_states);
> >> - /* only pipe 0 is read for voltage and dcf/soc clocks */
> >> - pipes[0].clks_cfg.voltage = vlevel;
> >> - pipes[0].clks_cfg.dcfclk_mhz =
> >> dml->soc.clock_limits[vlevel].dcfclk_mhz;
> >> - pipes[0].clks_cfg.socclk_mhz =
> >> dml->soc.clock_limits[vlevel].socclk_mhz;
> >> -
> >> - dml->soc.dram_clock_change_latency_us =
> >> table_entry->pstate_latency_us;
> >> - dml->soc.sr_exit_time_us = table_entry->sr_exit_time_us;
> >> - dml->soc.sr_enter_plus_exit_time_us =
> >> table_entry->sr_enter_plus_exit_time_us;
> >> -
> >> - wm_set->urgent_ns = get_wm_urgent(dml, pipes, pipe_cnt) * 1000;
> >> - wm_set->cstate_pstate.cstate_enter_plus_exit_ns =
> >> get_wm_stutter_enter_exit(dml, pipes, pipe_cnt) * 1000;
> >> - wm_set->cstate_pstate.cstate_exit_ns =
> >> get_wm_stutter_exit(dml, pipes, pipe_cnt) * 1000;
> >> - wm_set->cstate_pstate.pstate_change_ns =
> >> get_wm_dram_clock_change(dml, pipes, pipe_cnt) * 1000;
> >> - wm_set->pte_meta_urgent_ns = get_wm_memory_trip(dml, pipes,
> >> pipe_cnt) * 1000;
> >> - wm_set->frac_urg_bw_nom =
> >> get_fraction_of_urgent_bandwidth(dml, pipes, pipe_cnt) * 1000;
> >> - wm_set->frac_urg_bw_flip =
> >> get_fraction_of_urgent_bandwidth_imm_flip(dml, pipes, pipe_cnt) * 1000;
> >> - wm_set->urgent_latency_ns = get_urgent_latency(dml, pipes,
> >> pipe_cnt) * 1000;
> >> - dml->soc.dram_clock_change_latency_us =
> >> dram_clock_change_latency_cached;
> >> -
> >> -}
> >> -
> >> -static void dcn301_calculate_wm_and_dlg(
> >> - struct dc *dc, struct dc_state *context,
> >> - display_e2e_pipe_params_st *pipes,
> >> - int pipe_cnt,
> >> - int vlevel_req)
> >> -{
> >> - int i, pipe_idx;
> >> - int vlevel, vlevel_max;
> >> - struct wm_range_table_entry *table_entry;
> >> - struct clk_bw_params *bw_params = dc->clk_mgr->bw_params;
> >> -
> >> - ASSERT(bw_params);
> >> -
> >> - vlevel_max = bw_params->clk_table.num_entries - 1;
> >> -
> >> - /* WM Set D */
> >> - table_entry = &bw_params->wm_table.entries[WM_D];
> >> - if (table_entry->wm_type == WM_TYPE_RETRAINING)
> >> - vlevel = 0;
> >> - else
> >> - vlevel = vlevel_max;
> >> - calculate_wm_set_for_vlevel(vlevel, table_entry,
> >> &context->bw_ctx.bw.dcn.watermarks.d,
> >> - &context->bw_ctx.dml,
> >> pipes, pipe_cnt);
> >> - /* WM Set C */
> >> - table_entry = &bw_params->wm_table.entries[WM_C];
> >> - vlevel = min(max(vlevel_req, 2), vlevel_max);
> >> - calculate_wm_set_for_vlevel(vlevel, table_entry,
> >> &context->bw_ctx.bw.dcn.watermarks.c,
> >> - &context->bw_ctx.dml,
> >> pipes, pipe_cnt);
> >> - /* WM Set B */
> >> - table_entry = &bw_params->wm_table.entries[WM_B];
> >> - vlevel = min(max(vlevel_req, 1), vlevel_max);
> >> - calculate_wm_set_for_vlevel(vlevel, table_entry,
> >> &context->bw_ctx.bw.dcn.watermarks.b,
> >> - &context->bw_ctx.dml,
> >> pipes, pipe_cnt);
> >> -
> >> - /* WM Set A */
> >> - table_entry = &bw_params->wm_table.entries[WM_A];
> >> - vlevel = min(vlevel_req, vlevel_max);
> >> - calculate_wm_set_for_vlevel(vlevel, table_entry,
> >> &context->bw_ctx.bw.dcn.watermarks.a,
> >> - &context->bw_ctx.dml,
> >> pipes, pipe_cnt);
> >> -
> >> - for (i = 0, pipe_idx = 0; i < dc->res_pool->pipe_count; i++) {
> >> - if (!context->res_ctx.pipe_ctx[i].stream)
> >> - continue;
> >> -
> >> - pipes[pipe_idx].clks_cfg.dispclk_mhz =
> >> get_dispclk_calculated(&context->bw_ctx.dml, pipes, pipe_cnt);
> >> - pipes[pipe_idx].clks_cfg.dppclk_mhz =
> >> get_dppclk_calculated(&context->bw_ctx.dml, pipes, pipe_cnt, pipe_idx);
> >> -
> >> - if (dc->config.forced_clocks) {
> >> - pipes[pipe_idx].clks_cfg.dispclk_mhz =
> >> context->bw_ctx.dml.soc.clock_limits[0].dispclk_mhz;
> >> - pipes[pipe_idx].clks_cfg.dppclk_mhz =
> >> context->bw_ctx.dml.soc.clock_limits[0].dppclk_mhz;
> >> - }
> >> - if (dc->debug.min_disp_clk_khz >
> >> pipes[pipe_idx].clks_cfg.dispclk_mhz * 1000)
> >> - pipes[pipe_idx].clks_cfg.dispclk_mhz =
> >> dc->debug.min_disp_clk_khz / 1000.0;
> >> - if (dc->debug.min_dpp_clk_khz >
> >> pipes[pipe_idx].clks_cfg.dppclk_mhz * 1000)
> >> - pipes[pipe_idx].clks_cfg.dppclk_mhz =
> >> dc->debug.min_dpp_clk_khz / 1000.0;
> >> -
> >> - pipe_idx++;
> >> - }
> >> -
> >> - dcn20_calculate_dlg_params(dc, context, pipes, pipe_cnt, vlevel);
> >> -}
> >> -
> >> static struct resource_funcs dcn301_res_pool_funcs = {
> >> .destroy = dcn301_destroy_resource_pool,
> >> .link_enc_create = dcn301_link_encoder_create,
> >> .panel_cntl_create = dcn301_panel_cntl_create,
> >> .validate_bandwidth = dcn30_validate_bandwidth,
> >> - .calculate_wm_and_dlg = dcn301_calculate_wm_and_dlg,
> >> + .calculate_wm_and_dlg = dcn30_calculate_wm_and_dlg,
> >
> > Hi Zhan,
> >
> > Using dcn30_calculate_wm_and_dlg smells fishy, IIRC watermark
> > calculations for DPUG and APU are very different. It's likely that
> > you're now picking up corrupted values form the wm_table.
> >
> > Take a look at how struct wm_table is populated in vg_clk_mgr.c v.s.
> > dcn30_clk_mgr.c. For APU, wm_table.entries are populated, whereas for
> > DGPU, wm_table.nv_entries are populated. .entries and .nv_entries are
> > under a union, with very different struct definitions.
> >
> > Have you taken a look at whether the pstate latency and sr enter/exit
> > latency values being used after your change are sensible? It could be
> > that you simply needed to raise these watermarks.
> >
> > Thanks,
> > Leo
>
> After some DMs, it looks like this change is simply restoring an
> accidental revert that occurred due to a recent rebase. Given that this
> is needed to fix a regression,
>
> Acked-by: Leo Li <sunpeng.li@amd.com>
>
> Nevertheless, this still looks iffy. I'm not sure if the pstate and sr
> enter/exit latencies being used here are what you expect.
I agree. The original patch that fixed this looks like a bit of a hack:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0e4c0ae59d7e
We should figure out what parameters are causing problems.
Alex
>
> Thanks,
> Leo
>
> >
> >> .update_soc_for_wm_a = dcn30_update_soc_for_wm_a,
> >> .populate_dml_pipes = dcn30_populate_dml_pipes_from_context,
> >> .acquire_idle_pipe_for_layer =
> >> dcn20_acquire_idle_pipe_for_layer,
> >> --
> >> 2.31.1
> >>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-16 20:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-13 19:21 [PATCH] drm/amd/display: Use DCN30 watermark calc for DCN301 Liu, Zhan
2021-08-13 19:22 ` Liu, Zhan
2021-08-16 13:59 ` Leo Li
2021-08-16 20:25 ` Leo Li
2021-08-16 20:27 ` Alex Deucher
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.