2022-09-23 14:50:22

by Daniel Gomez

[permalink] [raw]
Subject: [RFC][PATCH] drm/amd/display: Restore DC_FP_* wrapper in dml/calcs

Commit [1] removes DC_FP_* wrappers from dml. However, this generates
the BUG [2] on the amdgpu driver. Restore DC_FP_* wrappers in dml/calcs
but only for the functions dcn_bw_update_from_pplib and
dcn_bw_notify_pplib_of_wm_ranges.

[1] 9696679bf7ac40a8fb6a488a75bd66d4414cd3c3 drm/amd/display: remove
DC_FP_* wrapper from dml folder

[2] BUG: sleeping function called from invalid context at
kernel/locking/mutex.c:283

Signed-off-by: Daniel Gomez <[email protected]>
---

Hi,

The patch [1] introduces BUG [2] since linux 5.18. The reason seems to be
wrapping entirely the functions dcn_bw_update_from_pplib and
dcn_bw_notify_pplib_of_wm_ranges in the dcn10.

On dcn_bw_update_from_pplib function the problem seems to be the
dm_pp_get_clock_levels_by_type_with_voltage call.

Any suggestions on what should we do here?

BUG: sleeping function called from invalid context at kernel/locking/mutex.c:283
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 227, name: systemd-udevd
preempt_count: 1, expected: 0
CPU: 4 PID: 227 Comm: systemd-udevd Not tainted 6.0.0-rc6-qtec-standard #2
Hardware name: Qtechnology A/S QT5222/QT5221, BIOS v1.0.1 06/07/2021
Call Trace:
<TASK>
dump_stack_lvl+0x33/0x42
__might_resched.cold.172+0xa5/0xb3
mutex_lock+0x1a/0x40
amdgpu_dpm_get_clock_by_type_with_voltage+0x38/0x70 [amdgpu]
dm_pp_get_clock_levels_by_type_with_voltage+0x64/0xa0 [amdgpu]
dcn_bw_update_from_pplib+0x70/0x340 [amdgpu]
dcn10_create_resource_pool+0x8c8/0xd20 [amdgpu]
? __kmalloc+0x1c7/0x4a0
dc_create_resource_pool+0xe7/0x190 [amdgpu]
dc_create+0x212/0x5d0 [amdgpu]
amdgpu_dm_init+0x246/0x370 [amdgpu]
? schedule_hrtimeout_range_clock+0x93/0x120
? phm_wait_for_register_unequal.part.1+0x4a/0x80 [amdgpu]
dm_hw_init+0xe/0x20 [amdgpu]
amdgpu_device_init.cold.56+0x1324/0x1653 [amdgpu]
? pci_bus_read_config_word+0x43/0x80
amdgpu_driver_load_kms+0x15/0x120 [amdgpu]
amdgpu_pci_probe+0x116/0x320 [amdgpu]
pci_device_probe+0x97/0x110
really_probe+0xdd/0x340
__driver_probe_device+0x80/0x170
driver_probe_device+0x1f/0x90
__driver_attach+0xdc/0x180
? __device_attach_driver+0x100/0x100
? __device_attach_driver+0x100/0x100
bus_for_each_dev+0x74/0xc0
bus_add_driver+0x19e/0x210
? kset_find_obj+0x30/0xa0
? 0xffffffffa0a5b000
driver_register+0x6b/0xc0
? 0xffffffffa0a5b000
do_one_initcall+0x4a/0x1f0
? __vunmap+0x28e/0x2f0
? __cond_resched+0x15/0x30
? kmem_cache_alloc_trace+0x3d/0x440
do_init_module+0x4a/0x1e0
load_module+0x1cba/0x1e10
? __do_sys_finit_module+0xb7/0x120
__do_sys_finit_module+0xb7/0x120
do_syscall_64+0x3c/0x80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7ff2b5f5422d
Code: 5d c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 ab 0e 00 f7 d8 64 89 01 48
RSP: 002b:00007ffc44ab28e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
RAX: ffffffffffffffda RBX: 0000555c566a9240 RCX: 00007ff2b5f5422d
RDX: 0000000000000000 RSI: 00007ff2b60bb353 RDI: 0000000000000019
RBP: 00007ff2b60bb353 R08: 0000000000000000 R09: 0000555c566a9240
R10: 0000000000000019 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000020000 R14: 0000000000000000 R15: 0000000000000000
</TASK>

drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 2 --
drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c | 10 ++++++++++
2 files changed, 10 insertions(+), 2 deletions(-)

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 174eebbe8b4f..a6ef20b43f3a 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
@@ -1505,7 +1505,6 @@ static bool dcn10_resource_construct(
&& pool->base.pp_smu->rv_funcs.set_pme_wa_enable != NULL)
dc->debug.az_endpoint_mute_only = false;

- DC_FP_START();
if (!dc->debug.disable_pplib_clock_request)
dcn_bw_update_from_pplib(dc);
dcn_bw_sync_calcs_and_dml(dc);
@@ -1513,7 +1512,6 @@ static bool dcn10_resource_construct(
dc->res_pool = &pool->base;
dcn_bw_notify_pplib_of_wm_ranges(dc);
}
- DC_FP_END();

{
struct irq_service_init_data init_data;
diff --git a/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
index db3b16b77034..a3c71d875adb 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
@@ -1490,6 +1490,8 @@ void dcn_bw_update_from_pplib(struct dc *dc)
res = dm_pp_get_clock_levels_by_type_with_voltage(
ctx, DM_PP_CLOCK_TYPE_FCLK, &fclks);

+ DC_FP_START();
+
if (res)
res = verify_clock_values(&fclks);

@@ -1519,9 +1521,13 @@ void dcn_bw_update_from_pplib(struct dc *dc)
} else
BREAK_TO_DEBUGGER();

+ DC_FP_END();
+
res = dm_pp_get_clock_levels_by_type_with_voltage(
ctx, DM_PP_CLOCK_TYPE_DCFCLK, &dcfclks);

+ DC_FP_START();
+
if (res)
res = verify_clock_values(&dcfclks);

@@ -1532,6 +1538,8 @@ void dcn_bw_update_from_pplib(struct dc *dc)
dc->dcn_soc->dcfclkv_max0p9 = dcfclks.data[dcfclks.num_levels - 1].clocks_in_khz / 1000.0;
} else
BREAK_TO_DEBUGGER();
+
+ DC_FP_END();
}

void dcn_bw_notify_pplib_of_wm_ranges(struct dc *dc)
@@ -1546,9 +1554,11 @@ void dcn_bw_notify_pplib_of_wm_ranges(struct dc *dc)
if (!pp || !pp->set_wm_ranges)
return;

+ DC_FP_START();
min_fclk_khz = dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 * 1000000 / 32;
min_dcfclk_khz = dc->dcn_soc->dcfclkv_min0p65 * 1000;
socclk_khz = dc->dcn_soc->socclk * 1000;
+ DC_FP_END();

/* Now notify PPLib/SMU about which Watermarks sets they should select
* depending on DPM state they are in. And update BW MGR GFX Engine and
--
2.35.1


2022-09-23 15:22:33

by Christian König

[permalink] [raw]
Subject: Re: [RFC][PATCH] drm/amd/display: Restore DC_FP_* wrapper in dml/calcs

Am 23.09.22 um 16:17 schrieb Daniel Gomez:
> Commit [1] removes DC_FP_* wrappers from dml. However, this generates
> the BUG [2] on the amdgpu driver. Restore DC_FP_* wrappers in dml/calcs
> but only for the functions dcn_bw_update_from_pplib and
> dcn_bw_notify_pplib_of_wm_ranges.

That's a really bad idea. The wrappers must be *outside* of the code
file which uses floating point code.

The code should probably just be re-aranged to not try to lock the mutex
inside the critical section.

Regards,
Christian.

>
> [1] 9696679bf7ac40a8fb6a488a75bd66d4414cd3c3 drm/amd/display: remove
> DC_FP_* wrapper from dml folder
>
> [2] BUG: sleeping function called from invalid context at
> kernel/locking/mutex.c:283
>
> Signed-off-by: Daniel Gomez <[email protected]>
> ---
>
> Hi,
>
> The patch [1] introduces BUG [2] since linux 5.18. The reason seems to be
> wrapping entirely the functions dcn_bw_update_from_pplib and
> dcn_bw_notify_pplib_of_wm_ranges in the dcn10.
>
> On dcn_bw_update_from_pplib function the problem seems to be the
> dm_pp_get_clock_levels_by_type_with_voltage call.
>
> Any suggestions on what should we do here?
>
> BUG: sleeping function called from invalid context at kernel/locking/mutex.c:283
> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 227, name: systemd-udevd
> preempt_count: 1, expected: 0
> CPU: 4 PID: 227 Comm: systemd-udevd Not tainted 6.0.0-rc6-qtec-standard #2
> Hardware name: Qtechnology A/S QT5222/QT5221, BIOS v1.0.1 06/07/2021
> Call Trace:
> <TASK>
> dump_stack_lvl+0x33/0x42
> __might_resched.cold.172+0xa5/0xb3
> mutex_lock+0x1a/0x40
> amdgpu_dpm_get_clock_by_type_with_voltage+0x38/0x70 [amdgpu]
> dm_pp_get_clock_levels_by_type_with_voltage+0x64/0xa0 [amdgpu]
> dcn_bw_update_from_pplib+0x70/0x340 [amdgpu]
> dcn10_create_resource_pool+0x8c8/0xd20 [amdgpu]
> ? __kmalloc+0x1c7/0x4a0
> dc_create_resource_pool+0xe7/0x190 [amdgpu]
> dc_create+0x212/0x5d0 [amdgpu]
> amdgpu_dm_init+0x246/0x370 [amdgpu]
> ? schedule_hrtimeout_range_clock+0x93/0x120
> ? phm_wait_for_register_unequal.part.1+0x4a/0x80 [amdgpu]
> dm_hw_init+0xe/0x20 [amdgpu]
> amdgpu_device_init.cold.56+0x1324/0x1653 [amdgpu]
> ? pci_bus_read_config_word+0x43/0x80
> amdgpu_driver_load_kms+0x15/0x120 [amdgpu]
> amdgpu_pci_probe+0x116/0x320 [amdgpu]
> pci_device_probe+0x97/0x110
> really_probe+0xdd/0x340
> __driver_probe_device+0x80/0x170
> driver_probe_device+0x1f/0x90
> __driver_attach+0xdc/0x180
> ? __device_attach_driver+0x100/0x100
> ? __device_attach_driver+0x100/0x100
> bus_for_each_dev+0x74/0xc0
> bus_add_driver+0x19e/0x210
> ? kset_find_obj+0x30/0xa0
> ? 0xffffffffa0a5b000
> driver_register+0x6b/0xc0
> ? 0xffffffffa0a5b000
> do_one_initcall+0x4a/0x1f0
> ? __vunmap+0x28e/0x2f0
> ? __cond_resched+0x15/0x30
> ? kmem_cache_alloc_trace+0x3d/0x440
> do_init_module+0x4a/0x1e0
> load_module+0x1cba/0x1e10
> ? __do_sys_finit_module+0xb7/0x120
> __do_sys_finit_module+0xb7/0x120
> do_syscall_64+0x3c/0x80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> RIP: 0033:0x7ff2b5f5422d
> Code: 5d c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 ab 0e 00 f7 d8 64 89 01 48
> RSP: 002b:00007ffc44ab28e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
> RAX: ffffffffffffffda RBX: 0000555c566a9240 RCX: 00007ff2b5f5422d
> RDX: 0000000000000000 RSI: 00007ff2b60bb353 RDI: 0000000000000019
> RBP: 00007ff2b60bb353 R08: 0000000000000000 R09: 0000555c566a9240
> R10: 0000000000000019 R11: 0000000000000246 R12: 0000000000000000
> R13: 0000000000020000 R14: 0000000000000000 R15: 0000000000000000
> </TASK>
>
> drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 2 --
> drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c | 10 ++++++++++
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> 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 174eebbe8b4f..a6ef20b43f3a 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> @@ -1505,7 +1505,6 @@ static bool dcn10_resource_construct(
> && pool->base.pp_smu->rv_funcs.set_pme_wa_enable != NULL)
> dc->debug.az_endpoint_mute_only = false;
>
> - DC_FP_START();
> if (!dc->debug.disable_pplib_clock_request)
> dcn_bw_update_from_pplib(dc);
> dcn_bw_sync_calcs_and_dml(dc);
> @@ -1513,7 +1512,6 @@ static bool dcn10_resource_construct(
> dc->res_pool = &pool->base;
> dcn_bw_notify_pplib_of_wm_ranges(dc);
> }
> - DC_FP_END();
>
> {
> struct irq_service_init_data init_data;
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
> index db3b16b77034..a3c71d875adb 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
> @@ -1490,6 +1490,8 @@ void dcn_bw_update_from_pplib(struct dc *dc)
> res = dm_pp_get_clock_levels_by_type_with_voltage(
> ctx, DM_PP_CLOCK_TYPE_FCLK, &fclks);
>
> + DC_FP_START();
> +
> if (res)
> res = verify_clock_values(&fclks);
>
> @@ -1519,9 +1521,13 @@ void dcn_bw_update_from_pplib(struct dc *dc)
> } else
> BREAK_TO_DEBUGGER();
>
> + DC_FP_END();
> +
> res = dm_pp_get_clock_levels_by_type_with_voltage(
> ctx, DM_PP_CLOCK_TYPE_DCFCLK, &dcfclks);
>
> + DC_FP_START();
> +
> if (res)
> res = verify_clock_values(&dcfclks);
>
> @@ -1532,6 +1538,8 @@ void dcn_bw_update_from_pplib(struct dc *dc)
> dc->dcn_soc->dcfclkv_max0p9 = dcfclks.data[dcfclks.num_levels - 1].clocks_in_khz / 1000.0;
> } else
> BREAK_TO_DEBUGGER();
> +
> + DC_FP_END();
> }
>
> void dcn_bw_notify_pplib_of_wm_ranges(struct dc *dc)
> @@ -1546,9 +1554,11 @@ void dcn_bw_notify_pplib_of_wm_ranges(struct dc *dc)
> if (!pp || !pp->set_wm_ranges)
> return;
>
> + DC_FP_START();
> min_fclk_khz = dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 * 1000000 / 32;
> min_dcfclk_khz = dc->dcn_soc->dcfclkv_min0p65 * 1000;
> socclk_khz = dc->dcn_soc->socclk * 1000;
> + DC_FP_END();
>
> /* Now notify PPLib/SMU about which Watermarks sets they should select
> * depending on DPM state they are in. And update BW MGR GFX Engine and
> --
> 2.35.1
>

2022-09-25 22:10:33

by Daniel Gomez

[permalink] [raw]
Subject: [PATCH] drm/amd/display: Fix mutex lock in dcn10

Removal of DC_FP_* wrappers from dml (9696679bf7ac) provokes a mutex
lock [2] on the amdgpu driver. Re-arrange the dcn10 code to avoid
locking the mutex by placing the DC_FP_* wrappers around the proper
functions.

This fixes the following WARN/stacktrace:

BUG: sleeping function called from invalid context at kernel/locking/mutex.c:283
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 227, name: systemd-udevd
preempt_count: 1, expected: 0
CPU: 4 PID: 227 Comm: systemd-udevd Not tainted 6.0.0-rc6-qtec-standard #2
Hardware name: Qtechnology A/S QT5222/QT5221, BIOS v1.0.1 06/07/2021
Call Trace:
<TASK>
dump_stack_lvl+0x33/0x42
__might_resched.cold.172+0xa5/0xb3
mutex_lock+0x1a/0x40
amdgpu_dpm_get_clock_by_type_with_voltage+0x38/0x70 [amdgpu]
dm_pp_get_clock_levels_by_type_with_voltage+0x64/0xa0 [amdgpu]
dcn_bw_update_from_pplib+0x70/0x340 [amdgpu]
dcn10_create_resource_pool+0x8c8/0xd20 [amdgpu]
? __kmalloc+0x1c7/0x4a0
dc_create_resource_pool+0xe7/0x190 [amdgpu]
dc_create+0x212/0x5d0 [amdgpu]
amdgpu_dm_init+0x246/0x370 [amdgpu]
? schedule_hrtimeout_range_clock+0x93/0x120
? phm_wait_for_register_unequal.part.1+0x4a/0x80 [amdgpu]
dm_hw_init+0xe/0x20 [amdgpu]
amdgpu_device_init.cold.56+0x1324/0x1653 [amdgpu]
? pci_bus_read_config_word+0x43/0x80
amdgpu_driver_load_kms+0x15/0x120 [amdgpu]
amdgpu_pci_probe+0x116/0x320 [amdgpu]
pci_device_probe+0x97/0x110
really_probe+0xdd/0x340
__driver_probe_device+0x80/0x170
driver_probe_device+0x1f/0x90
__driver_attach+0xdc/0x180
? __device_attach_driver+0x100/0x100
? __device_attach_driver+0x100/0x100
bus_for_each_dev+0x74/0xc0
bus_add_driver+0x19e/0x210
? kset_find_obj+0x30/0xa0
? 0xffffffffa0a5b000
driver_register+0x6b/0xc0
? 0xffffffffa0a5b000
do_one_initcall+0x4a/0x1f0
? __vunmap+0x28e/0x2f0
? __cond_resched+0x15/0x30
? kmem_cache_alloc_trace+0x3d/0x440
do_init_module+0x4a/0x1e0
load_module+0x1cba/0x1e10
? __do_sys_finit_module+0xb7/0x120
__do_sys_finit_module+0xb7/0x120
do_syscall_64+0x3c/0x80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7ff2b5f5422d
Code: 5d c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48
89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48>
3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 ab 0e 00 f7 d8 64 89 01 48
RSP: 002b:00007ffc44ab28e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
RAX: ffffffffffffffda RBX: 0000555c566a9240 RCX: 00007ff2b5f5422d
RDX: 0000000000000000 RSI: 00007ff2b60bb353 RDI: 0000000000000019
RBP: 00007ff2b60bb353 R08: 0000000000000000 R09: 0000555c566a9240
R10: 0000000000000019 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000020000 R14: 0000000000000000 R15: 0000000000000000
</TASK>

Fixes: 9696679bf7ac ("drm/amd/display: remove DC_FP_* wrapper from
dml folder")
Signed-off-by: Daniel Gomez <[email protected]>
---
.../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 12 +-
.../drm/amd/display/dc/dcn10/dcn10_resource.c | 66 +++++++++-
.../drm/amd/display/dc/dml/calcs/dcn_calcs.c | 118 ++++++++----------
.../gpu/drm/amd/display/dc/inc/dcn_calcs.h | 19 ++-
4 files changed, 138 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index 5b5d952b2b8c..cb1e06d62841 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -2994,6 +2994,7 @@ void dcn10_prepare_bandwidth(
{
struct dce_hwseq *hws = dc->hwseq;
struct hubbub *hubbub = dc->res_pool->hubbub;
+ int min_fclk_khz, min_dcfclk_khz, socclk_khz;

if (dc->debug.sanity_checks)
hws->funcs.verify_allow_pstate_change_high(dc);
@@ -3016,8 +3017,11 @@ void dcn10_prepare_bandwidth(

if (dc->debug.pplib_wm_report_mode == WM_REPORT_OVERRIDE) {
DC_FP_START();
- dcn_bw_notify_pplib_of_wm_ranges(dc);
+ dcn_get_soc_clks(
+ dc, &min_fclk_khz, &min_dcfclk_khz, &socclk_khz);
DC_FP_END();
+ dcn_bw_notify_pplib_of_wm_ranges(
+ dc, min_fclk_khz, min_dcfclk_khz, socclk_khz);
}

if (dc->debug.sanity_checks)
@@ -3030,6 +3034,7 @@ void dcn10_optimize_bandwidth(
{
struct dce_hwseq *hws = dc->hwseq;
struct hubbub *hubbub = dc->res_pool->hubbub;
+ int min_fclk_khz, min_dcfclk_khz, socclk_khz;

if (dc->debug.sanity_checks)
hws->funcs.verify_allow_pstate_change_high(dc);
@@ -3053,8 +3058,11 @@ void dcn10_optimize_bandwidth(

if (dc->debug.pplib_wm_report_mode == WM_REPORT_OVERRIDE) {
DC_FP_START();
- dcn_bw_notify_pplib_of_wm_ranges(dc);
+ dcn_get_soc_clks(
+ dc, &min_fclk_khz, &min_dcfclk_khz, &socclk_khz);
DC_FP_END();
+ dcn_bw_notify_pplib_of_wm_ranges(
+ dc, min_fclk_khz, min_dcfclk_khz, socclk_khz);
}

if (dc->debug.sanity_checks)
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 174eebbe8b4f..a18a5b56ca7d 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
@@ -1336,6 +1336,21 @@ static noinline void dcn10_resource_construct_fp(
}
}

+static bool verify_clock_values(struct dm_pp_clock_levels_with_voltage *clks)
+{
+ int i;
+
+ if (clks->num_levels == 0)
+ return false;
+
+ for (i = 0; i < clks->num_levels; i++)
+ /* Ensure that the result is sane */
+ if (clks->data[i].clocks_in_khz == 0)
+ return false;
+
+ return true;
+}
+
static bool dcn10_resource_construct(
uint8_t num_virtual_links,
struct dc *dc,
@@ -1345,6 +1360,9 @@ static bool dcn10_resource_construct(
int j;
struct dc_context *ctx = dc->ctx;
uint32_t pipe_fuses = read_pipe_fuses(ctx);
+ struct dm_pp_clock_levels_with_voltage fclks = {0}, dcfclks = {0};
+ int min_fclk_khz, min_dcfclk_khz, socclk_khz;
+ bool res;

ctx->dc_bios->regs = &bios_regs;

@@ -1505,15 +1523,53 @@ static bool dcn10_resource_construct(
&& pool->base.pp_smu->rv_funcs.set_pme_wa_enable != NULL)
dc->debug.az_endpoint_mute_only = false;

- DC_FP_START();
- if (!dc->debug.disable_pplib_clock_request)
- dcn_bw_update_from_pplib(dc);
+
+ if (!dc->debug.disable_pplib_clock_request) {
+ /*
+ * TODO: This is not the proper way to obtain
+ * fabric_and_dram_bandwidth, should be min(fclk, memclk).
+ */
+ res = dm_pp_get_clock_levels_by_type_with_voltage(
+ ctx, DM_PP_CLOCK_TYPE_FCLK, &fclks);
+
+ DC_FP_START();
+
+ if (res)
+ res = verify_clock_values(&fclks);
+
+ if (res)
+ dcn_bw_update_from_pplib_fclks(dc, &fclks);
+ else
+ BREAK_TO_DEBUGGER();
+
+ DC_FP_END();
+
+ res = dm_pp_get_clock_levels_by_type_with_voltage(
+ ctx, DM_PP_CLOCK_TYPE_DCFCLK, &dcfclks);
+
+ DC_FP_START();
+
+ if (res)
+ res = verify_clock_values(&dcfclks);
+
+ if (res)
+ dcn_bw_update_from_pplib_dcfclks(dc, &dcfclks);
+ else
+ BREAK_TO_DEBUGGER();
+
+ DC_FP_END();
+ }
+
dcn_bw_sync_calcs_and_dml(dc);
if (!dc->debug.disable_pplib_wm_range) {
dc->res_pool = &pool->base;
- dcn_bw_notify_pplib_of_wm_ranges(dc);
+ DC_FP_START();
+ dcn_get_soc_clks(
+ dc, &min_fclk_khz, &min_dcfclk_khz, &socclk_khz);
+ DC_FP_END();
+ dcn_bw_notify_pplib_of_wm_ranges(
+ dc, min_fclk_khz, min_dcfclk_khz, socclk_khz);
}
- DC_FP_END();

{
struct irq_service_init_data init_data;
diff --git a/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
index db3b16b77034..7d3394470352 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
@@ -1464,81 +1464,67 @@ unsigned int dcn_find_dcfclk_suits_all(
return dcf_clk;
}

-static bool verify_clock_values(struct dm_pp_clock_levels_with_voltage *clks)
+void dcn_bw_update_from_pplib_fclks(
+ struct dc *dc,
+ struct dm_pp_clock_levels_with_voltage *fclks)
{
- int i;
-
- if (clks->num_levels == 0)
- return false;
-
- for (i = 0; i < clks->num_levels; i++)
- /* Ensure that the result is sane */
- if (clks->data[i].clocks_in_khz == 0)
- return false;
+ unsigned vmin0p65_idx, vmid0p72_idx, vnom0p8_idx, vmax0p9_idx;

- return true;
+ ASSERT(fclks->num_levels);
+
+ vmin0p65_idx = 0;
+ vmid0p72_idx = fclks->num_levels -
+ (fclks->num_levels > 2 ? 3 : (fclks->num_levels > 1 ? 2 : 1));
+ vnom0p8_idx = fclks->num_levels - (fclks->num_levels > 1 ? 2 : 1);
+ vmax0p9_idx = fclks->num_levels - 1;
+
+ dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 =
+ 32 * (fclks->data[vmin0p65_idx].clocks_in_khz / 1000.0) / 1000.0;
+ dc->dcn_soc->fabric_and_dram_bandwidth_vmid0p72 =
+ dc->dcn_soc->number_of_channels *
+ (fclks->data[vmid0p72_idx].clocks_in_khz / 1000.0)
+ * ddr4_dram_factor_single_Channel / 1000.0;
+ dc->dcn_soc->fabric_and_dram_bandwidth_vnom0p8 =
+ dc->dcn_soc->number_of_channels *
+ (fclks->data[vnom0p8_idx].clocks_in_khz / 1000.0)
+ * ddr4_dram_factor_single_Channel / 1000.0;
+ dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9 =
+ dc->dcn_soc->number_of_channels *
+ (fclks->data[vmax0p9_idx].clocks_in_khz / 1000.0)
+ * ddr4_dram_factor_single_Channel / 1000.0;
}

-void dcn_bw_update_from_pplib(struct dc *dc)
+void dcn_bw_update_from_pplib_dcfclks(
+ struct dc *dc,
+ struct dm_pp_clock_levels_with_voltage *dcfclks)
{
- struct dc_context *ctx = dc->ctx;
- struct dm_pp_clock_levels_with_voltage fclks = {0}, dcfclks = {0};
- bool res;
- unsigned vmin0p65_idx, vmid0p72_idx, vnom0p8_idx, vmax0p9_idx;
-
- /* TODO: This is not the proper way to obtain fabric_and_dram_bandwidth, should be min(fclk, memclk) */
- res = dm_pp_get_clock_levels_by_type_with_voltage(
- ctx, DM_PP_CLOCK_TYPE_FCLK, &fclks);
-
- if (res)
- res = verify_clock_values(&fclks);
-
- if (res) {
- ASSERT(fclks.num_levels);
-
- vmin0p65_idx = 0;
- vmid0p72_idx = fclks.num_levels -
- (fclks.num_levels > 2 ? 3 : (fclks.num_levels > 1 ? 2 : 1));
- vnom0p8_idx = fclks.num_levels - (fclks.num_levels > 1 ? 2 : 1);
- vmax0p9_idx = fclks.num_levels - 1;
-
- dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 =
- 32 * (fclks.data[vmin0p65_idx].clocks_in_khz / 1000.0) / 1000.0;
- dc->dcn_soc->fabric_and_dram_bandwidth_vmid0p72 =
- dc->dcn_soc->number_of_channels *
- (fclks.data[vmid0p72_idx].clocks_in_khz / 1000.0)
- * ddr4_dram_factor_single_Channel / 1000.0;
- dc->dcn_soc->fabric_and_dram_bandwidth_vnom0p8 =
- dc->dcn_soc->number_of_channels *
- (fclks.data[vnom0p8_idx].clocks_in_khz / 1000.0)
- * ddr4_dram_factor_single_Channel / 1000.0;
- dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9 =
- dc->dcn_soc->number_of_channels *
- (fclks.data[vmax0p9_idx].clocks_in_khz / 1000.0)
- * ddr4_dram_factor_single_Channel / 1000.0;
- } else
- BREAK_TO_DEBUGGER();
-
- res = dm_pp_get_clock_levels_by_type_with_voltage(
- ctx, DM_PP_CLOCK_TYPE_DCFCLK, &dcfclks);
-
- if (res)
- res = verify_clock_values(&dcfclks);
+ if (dcfclks->num_levels >= 3) {
+ dc->dcn_soc->dcfclkv_min0p65 = dcfclks->data[0].clocks_in_khz / 1000.0;
+ dc->dcn_soc->dcfclkv_mid0p72 = dcfclks->data[dcfclks->num_levels - 3].clocks_in_khz / 1000.0;
+ dc->dcn_soc->dcfclkv_nom0p8 = dcfclks->data[dcfclks->num_levels - 2].clocks_in_khz / 1000.0;
+ dc->dcn_soc->dcfclkv_max0p9 = dcfclks->data[dcfclks->num_levels - 1].clocks_in_khz / 1000.0;
+ }
+}

- if (res && dcfclks.num_levels >= 3) {
- dc->dcn_soc->dcfclkv_min0p65 = dcfclks.data[0].clocks_in_khz / 1000.0;
- dc->dcn_soc->dcfclkv_mid0p72 = dcfclks.data[dcfclks.num_levels - 3].clocks_in_khz / 1000.0;
- dc->dcn_soc->dcfclkv_nom0p8 = dcfclks.data[dcfclks.num_levels - 2].clocks_in_khz / 1000.0;
- dc->dcn_soc->dcfclkv_max0p9 = dcfclks.data[dcfclks.num_levels - 1].clocks_in_khz / 1000.0;
- } else
- BREAK_TO_DEBUGGER();
+void dcn_get_soc_clks(
+ struct dc *dc,
+ int *min_fclk_khz,
+ int *min_dcfclk_khz,
+ int *socclk_khz)
+{
+ *min_fclk_khz = dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 * 1000000 / 32;
+ *min_dcfclk_khz = dc->dcn_soc->dcfclkv_min0p65 * 1000;
+ *socclk_khz = dc->dcn_soc->socclk * 1000;
}

-void dcn_bw_notify_pplib_of_wm_ranges(struct dc *dc)
+void dcn_bw_notify_pplib_of_wm_ranges(
+ struct dc *dc,
+ int min_fclk_khz,
+ int min_dcfclk_khz,
+ int socclk_khz)
{
struct pp_smu_funcs_rv *pp = NULL;
struct pp_smu_wm_range_sets ranges = {0};
- int min_fclk_khz, min_dcfclk_khz, socclk_khz;
const int overdrive = 5000000; /* 5 GHz to cover Overdrive */

if (dc->res_pool->pp_smu)
@@ -1546,10 +1532,6 @@ void dcn_bw_notify_pplib_of_wm_ranges(struct dc *dc)
if (!pp || !pp->set_wm_ranges)
return;

- min_fclk_khz = dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 * 1000000 / 32;
- min_dcfclk_khz = dc->dcn_soc->dcfclkv_min0p65 * 1000;
- socclk_khz = dc->dcn_soc->socclk * 1000;
-
/* Now notify PPLib/SMU about which Watermarks sets they should select
* depending on DPM state they are in. And update BW MGR GFX Engine and
* Memory clock member variables for Watermarks calculations for each
diff --git a/drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h b/drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h
index 806f3041db14..9e4ddc985240 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h
@@ -628,8 +628,23 @@ unsigned int dcn_find_dcfclk_suits_all(
const struct dc *dc,
struct dc_clocks *clocks);

-void dcn_bw_update_from_pplib(struct dc *dc);
-void dcn_bw_notify_pplib_of_wm_ranges(struct dc *dc);
+void dcn_get_soc_clks(
+ struct dc *dc,
+ int *min_fclk_khz,
+ int *min_dcfclk_khz,
+ int *socclk_khz);
+
+void dcn_bw_update_from_pplib_fclks(
+ struct dc *dc,
+ struct dm_pp_clock_levels_with_voltage *fclks);
+void dcn_bw_update_from_pplib_dcfclks(
+ struct dc *dc,
+ struct dm_pp_clock_levels_with_voltage *dcfclks);
+void dcn_bw_notify_pplib_of_wm_ranges(
+ struct dc *dc,
+ int min_fclk_khz,
+ int min_dcfclk_khz,
+ int socclk_khz);
void dcn_bw_sync_calcs_and_dml(struct dc *dc);

enum source_macro_tile_size swizzle_mode_to_macro_tile_size(enum swizzle_mode_values sw_mode);
--
2.35.1

2022-09-26 07:15:35

by Christian König

[permalink] [raw]
Subject: Re: [PATCH] drm/amd/display: Fix mutex lock in dcn10

Am 25.09.22 um 23:53 schrieb Daniel Gomez:
> Removal of DC_FP_* wrappers from dml (9696679bf7ac) provokes a mutex
> lock [2] on the amdgpu driver. Re-arrange the dcn10 code to avoid
> locking the mutex by placing the DC_FP_* wrappers around the proper
> functions.

Of hand that looks correct to me now, but our DC team needs to take a
closer look.

Regards,
Christian.

>
> This fixes the following WARN/stacktrace:
>
> BUG: sleeping function called from invalid context at kernel/locking/mutex.c:283
> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 227, name: systemd-udevd
> preempt_count: 1, expected: 0
> CPU: 4 PID: 227 Comm: systemd-udevd Not tainted 6.0.0-rc6-qtec-standard #2
> Hardware name: Qtechnology A/S QT5222/QT5221, BIOS v1.0.1 06/07/2021
> Call Trace:
> <TASK>
> dump_stack_lvl+0x33/0x42
> __might_resched.cold.172+0xa5/0xb3
> mutex_lock+0x1a/0x40
> amdgpu_dpm_get_clock_by_type_with_voltage+0x38/0x70 [amdgpu]
> dm_pp_get_clock_levels_by_type_with_voltage+0x64/0xa0 [amdgpu]
> dcn_bw_update_from_pplib+0x70/0x340 [amdgpu]
> dcn10_create_resource_pool+0x8c8/0xd20 [amdgpu]
> ? __kmalloc+0x1c7/0x4a0
> dc_create_resource_pool+0xe7/0x190 [amdgpu]
> dc_create+0x212/0x5d0 [amdgpu]
> amdgpu_dm_init+0x246/0x370 [amdgpu]
> ? schedule_hrtimeout_range_clock+0x93/0x120
> ? phm_wait_for_register_unequal.part.1+0x4a/0x80 [amdgpu]
> dm_hw_init+0xe/0x20 [amdgpu]
> amdgpu_device_init.cold.56+0x1324/0x1653 [amdgpu]
> ? pci_bus_read_config_word+0x43/0x80
> amdgpu_driver_load_kms+0x15/0x120 [amdgpu]
> amdgpu_pci_probe+0x116/0x320 [amdgpu]
> pci_device_probe+0x97/0x110
> really_probe+0xdd/0x340
> __driver_probe_device+0x80/0x170
> driver_probe_device+0x1f/0x90
> __driver_attach+0xdc/0x180
> ? __device_attach_driver+0x100/0x100
> ? __device_attach_driver+0x100/0x100
> bus_for_each_dev+0x74/0xc0
> bus_add_driver+0x19e/0x210
> ? kset_find_obj+0x30/0xa0
> ? 0xffffffffa0a5b000
> driver_register+0x6b/0xc0
> ? 0xffffffffa0a5b000
> do_one_initcall+0x4a/0x1f0
> ? __vunmap+0x28e/0x2f0
> ? __cond_resched+0x15/0x30
> ? kmem_cache_alloc_trace+0x3d/0x440
> do_init_module+0x4a/0x1e0
> load_module+0x1cba/0x1e10
> ? __do_sys_finit_module+0xb7/0x120
> __do_sys_finit_module+0xb7/0x120
> do_syscall_64+0x3c/0x80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> RIP: 0033:0x7ff2b5f5422d
> Code: 5d c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48
> 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48>
> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 ab 0e 00 f7 d8 64 89 01 48
> RSP: 002b:00007ffc44ab28e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
> RAX: ffffffffffffffda RBX: 0000555c566a9240 RCX: 00007ff2b5f5422d
> RDX: 0000000000000000 RSI: 00007ff2b60bb353 RDI: 0000000000000019
> RBP: 00007ff2b60bb353 R08: 0000000000000000 R09: 0000555c566a9240
> R10: 0000000000000019 R11: 0000000000000246 R12: 0000000000000000
> R13: 0000000000020000 R14: 0000000000000000 R15: 0000000000000000
> </TASK>
>
> Fixes: 9696679bf7ac ("drm/amd/display: remove DC_FP_* wrapper from
> dml folder")
> Signed-off-by: Daniel Gomez <[email protected]>
> ---
> .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 12 +-
> .../drm/amd/display/dc/dcn10/dcn10_resource.c | 66 +++++++++-
> .../drm/amd/display/dc/dml/calcs/dcn_calcs.c | 118 ++++++++----------
> .../gpu/drm/amd/display/dc/inc/dcn_calcs.h | 19 ++-
> 4 files changed, 138 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> index 5b5d952b2b8c..cb1e06d62841 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> @@ -2994,6 +2994,7 @@ void dcn10_prepare_bandwidth(
> {
> struct dce_hwseq *hws = dc->hwseq;
> struct hubbub *hubbub = dc->res_pool->hubbub;
> + int min_fclk_khz, min_dcfclk_khz, socclk_khz;
>
> if (dc->debug.sanity_checks)
> hws->funcs.verify_allow_pstate_change_high(dc);
> @@ -3016,8 +3017,11 @@ void dcn10_prepare_bandwidth(
>
> if (dc->debug.pplib_wm_report_mode == WM_REPORT_OVERRIDE) {
> DC_FP_START();
> - dcn_bw_notify_pplib_of_wm_ranges(dc);
> + dcn_get_soc_clks(
> + dc, &min_fclk_khz, &min_dcfclk_khz, &socclk_khz);
> DC_FP_END();
> + dcn_bw_notify_pplib_of_wm_ranges(
> + dc, min_fclk_khz, min_dcfclk_khz, socclk_khz);
> }
>
> if (dc->debug.sanity_checks)
> @@ -3030,6 +3034,7 @@ void dcn10_optimize_bandwidth(
> {
> struct dce_hwseq *hws = dc->hwseq;
> struct hubbub *hubbub = dc->res_pool->hubbub;
> + int min_fclk_khz, min_dcfclk_khz, socclk_khz;
>
> if (dc->debug.sanity_checks)
> hws->funcs.verify_allow_pstate_change_high(dc);
> @@ -3053,8 +3058,11 @@ void dcn10_optimize_bandwidth(
>
> if (dc->debug.pplib_wm_report_mode == WM_REPORT_OVERRIDE) {
> DC_FP_START();
> - dcn_bw_notify_pplib_of_wm_ranges(dc);
> + dcn_get_soc_clks(
> + dc, &min_fclk_khz, &min_dcfclk_khz, &socclk_khz);
> DC_FP_END();
> + dcn_bw_notify_pplib_of_wm_ranges(
> + dc, min_fclk_khz, min_dcfclk_khz, socclk_khz);
> }
>
> if (dc->debug.sanity_checks)
> 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 174eebbe8b4f..a18a5b56ca7d 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> @@ -1336,6 +1336,21 @@ static noinline void dcn10_resource_construct_fp(
> }
> }
>
> +static bool verify_clock_values(struct dm_pp_clock_levels_with_voltage *clks)
> +{
> + int i;
> +
> + if (clks->num_levels == 0)
> + return false;
> +
> + for (i = 0; i < clks->num_levels; i++)
> + /* Ensure that the result is sane */
> + if (clks->data[i].clocks_in_khz == 0)
> + return false;
> +
> + return true;
> +}
> +
> static bool dcn10_resource_construct(
> uint8_t num_virtual_links,
> struct dc *dc,
> @@ -1345,6 +1360,9 @@ static bool dcn10_resource_construct(
> int j;
> struct dc_context *ctx = dc->ctx;
> uint32_t pipe_fuses = read_pipe_fuses(ctx);
> + struct dm_pp_clock_levels_with_voltage fclks = {0}, dcfclks = {0};
> + int min_fclk_khz, min_dcfclk_khz, socclk_khz;
> + bool res;
>
> ctx->dc_bios->regs = &bios_regs;
>
> @@ -1505,15 +1523,53 @@ static bool dcn10_resource_construct(
> && pool->base.pp_smu->rv_funcs.set_pme_wa_enable != NULL)
> dc->debug.az_endpoint_mute_only = false;
>
> - DC_FP_START();
> - if (!dc->debug.disable_pplib_clock_request)
> - dcn_bw_update_from_pplib(dc);
> +
> + if (!dc->debug.disable_pplib_clock_request) {
> + /*
> + * TODO: This is not the proper way to obtain
> + * fabric_and_dram_bandwidth, should be min(fclk, memclk).
> + */
> + res = dm_pp_get_clock_levels_by_type_with_voltage(
> + ctx, DM_PP_CLOCK_TYPE_FCLK, &fclks);
> +
> + DC_FP_START();
> +
> + if (res)
> + res = verify_clock_values(&fclks);
> +
> + if (res)
> + dcn_bw_update_from_pplib_fclks(dc, &fclks);
> + else
> + BREAK_TO_DEBUGGER();
> +
> + DC_FP_END();
> +
> + res = dm_pp_get_clock_levels_by_type_with_voltage(
> + ctx, DM_PP_CLOCK_TYPE_DCFCLK, &dcfclks);
> +
> + DC_FP_START();
> +
> + if (res)
> + res = verify_clock_values(&dcfclks);
> +
> + if (res)
> + dcn_bw_update_from_pplib_dcfclks(dc, &dcfclks);
> + else
> + BREAK_TO_DEBUGGER();
> +
> + DC_FP_END();
> + }
> +
> dcn_bw_sync_calcs_and_dml(dc);
> if (!dc->debug.disable_pplib_wm_range) {
> dc->res_pool = &pool->base;
> - dcn_bw_notify_pplib_of_wm_ranges(dc);
> + DC_FP_START();
> + dcn_get_soc_clks(
> + dc, &min_fclk_khz, &min_dcfclk_khz, &socclk_khz);
> + DC_FP_END();
> + dcn_bw_notify_pplib_of_wm_ranges(
> + dc, min_fclk_khz, min_dcfclk_khz, socclk_khz);
> }
> - DC_FP_END();
>
> {
> struct irq_service_init_data init_data;
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
> index db3b16b77034..7d3394470352 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
> @@ -1464,81 +1464,67 @@ unsigned int dcn_find_dcfclk_suits_all(
> return dcf_clk;
> }
>
> -static bool verify_clock_values(struct dm_pp_clock_levels_with_voltage *clks)
> +void dcn_bw_update_from_pplib_fclks(
> + struct dc *dc,
> + struct dm_pp_clock_levels_with_voltage *fclks)
> {
> - int i;
> -
> - if (clks->num_levels == 0)
> - return false;
> -
> - for (i = 0; i < clks->num_levels; i++)
> - /* Ensure that the result is sane */
> - if (clks->data[i].clocks_in_khz == 0)
> - return false;
> + unsigned vmin0p65_idx, vmid0p72_idx, vnom0p8_idx, vmax0p9_idx;
>
> - return true;
> + ASSERT(fclks->num_levels);
> +
> + vmin0p65_idx = 0;
> + vmid0p72_idx = fclks->num_levels -
> + (fclks->num_levels > 2 ? 3 : (fclks->num_levels > 1 ? 2 : 1));
> + vnom0p8_idx = fclks->num_levels - (fclks->num_levels > 1 ? 2 : 1);
> + vmax0p9_idx = fclks->num_levels - 1;
> +
> + dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 =
> + 32 * (fclks->data[vmin0p65_idx].clocks_in_khz / 1000.0) / 1000.0;
> + dc->dcn_soc->fabric_and_dram_bandwidth_vmid0p72 =
> + dc->dcn_soc->number_of_channels *
> + (fclks->data[vmid0p72_idx].clocks_in_khz / 1000.0)
> + * ddr4_dram_factor_single_Channel / 1000.0;
> + dc->dcn_soc->fabric_and_dram_bandwidth_vnom0p8 =
> + dc->dcn_soc->number_of_channels *
> + (fclks->data[vnom0p8_idx].clocks_in_khz / 1000.0)
> + * ddr4_dram_factor_single_Channel / 1000.0;
> + dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9 =
> + dc->dcn_soc->number_of_channels *
> + (fclks->data[vmax0p9_idx].clocks_in_khz / 1000.0)
> + * ddr4_dram_factor_single_Channel / 1000.0;
> }
>
> -void dcn_bw_update_from_pplib(struct dc *dc)
> +void dcn_bw_update_from_pplib_dcfclks(
> + struct dc *dc,
> + struct dm_pp_clock_levels_with_voltage *dcfclks)
> {
> - struct dc_context *ctx = dc->ctx;
> - struct dm_pp_clock_levels_with_voltage fclks = {0}, dcfclks = {0};
> - bool res;
> - unsigned vmin0p65_idx, vmid0p72_idx, vnom0p8_idx, vmax0p9_idx;
> -
> - /* TODO: This is not the proper way to obtain fabric_and_dram_bandwidth, should be min(fclk, memclk) */
> - res = dm_pp_get_clock_levels_by_type_with_voltage(
> - ctx, DM_PP_CLOCK_TYPE_FCLK, &fclks);
> -
> - if (res)
> - res = verify_clock_values(&fclks);
> -
> - if (res) {
> - ASSERT(fclks.num_levels);
> -
> - vmin0p65_idx = 0;
> - vmid0p72_idx = fclks.num_levels -
> - (fclks.num_levels > 2 ? 3 : (fclks.num_levels > 1 ? 2 : 1));
> - vnom0p8_idx = fclks.num_levels - (fclks.num_levels > 1 ? 2 : 1);
> - vmax0p9_idx = fclks.num_levels - 1;
> -
> - dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 =
> - 32 * (fclks.data[vmin0p65_idx].clocks_in_khz / 1000.0) / 1000.0;
> - dc->dcn_soc->fabric_and_dram_bandwidth_vmid0p72 =
> - dc->dcn_soc->number_of_channels *
> - (fclks.data[vmid0p72_idx].clocks_in_khz / 1000.0)
> - * ddr4_dram_factor_single_Channel / 1000.0;
> - dc->dcn_soc->fabric_and_dram_bandwidth_vnom0p8 =
> - dc->dcn_soc->number_of_channels *
> - (fclks.data[vnom0p8_idx].clocks_in_khz / 1000.0)
> - * ddr4_dram_factor_single_Channel / 1000.0;
> - dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9 =
> - dc->dcn_soc->number_of_channels *
> - (fclks.data[vmax0p9_idx].clocks_in_khz / 1000.0)
> - * ddr4_dram_factor_single_Channel / 1000.0;
> - } else
> - BREAK_TO_DEBUGGER();
> -
> - res = dm_pp_get_clock_levels_by_type_with_voltage(
> - ctx, DM_PP_CLOCK_TYPE_DCFCLK, &dcfclks);
> -
> - if (res)
> - res = verify_clock_values(&dcfclks);
> + if (dcfclks->num_levels >= 3) {
> + dc->dcn_soc->dcfclkv_min0p65 = dcfclks->data[0].clocks_in_khz / 1000.0;
> + dc->dcn_soc->dcfclkv_mid0p72 = dcfclks->data[dcfclks->num_levels - 3].clocks_in_khz / 1000.0;
> + dc->dcn_soc->dcfclkv_nom0p8 = dcfclks->data[dcfclks->num_levels - 2].clocks_in_khz / 1000.0;
> + dc->dcn_soc->dcfclkv_max0p9 = dcfclks->data[dcfclks->num_levels - 1].clocks_in_khz / 1000.0;
> + }
> +}
>
> - if (res && dcfclks.num_levels >= 3) {
> - dc->dcn_soc->dcfclkv_min0p65 = dcfclks.data[0].clocks_in_khz / 1000.0;
> - dc->dcn_soc->dcfclkv_mid0p72 = dcfclks.data[dcfclks.num_levels - 3].clocks_in_khz / 1000.0;
> - dc->dcn_soc->dcfclkv_nom0p8 = dcfclks.data[dcfclks.num_levels - 2].clocks_in_khz / 1000.0;
> - dc->dcn_soc->dcfclkv_max0p9 = dcfclks.data[dcfclks.num_levels - 1].clocks_in_khz / 1000.0;
> - } else
> - BREAK_TO_DEBUGGER();
> +void dcn_get_soc_clks(
> + struct dc *dc,
> + int *min_fclk_khz,
> + int *min_dcfclk_khz,
> + int *socclk_khz)
> +{
> + *min_fclk_khz = dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 * 1000000 / 32;
> + *min_dcfclk_khz = dc->dcn_soc->dcfclkv_min0p65 * 1000;
> + *socclk_khz = dc->dcn_soc->socclk * 1000;
> }
>
> -void dcn_bw_notify_pplib_of_wm_ranges(struct dc *dc)
> +void dcn_bw_notify_pplib_of_wm_ranges(
> + struct dc *dc,
> + int min_fclk_khz,
> + int min_dcfclk_khz,
> + int socclk_khz)
> {
> struct pp_smu_funcs_rv *pp = NULL;
> struct pp_smu_wm_range_sets ranges = {0};
> - int min_fclk_khz, min_dcfclk_khz, socclk_khz;
> const int overdrive = 5000000; /* 5 GHz to cover Overdrive */
>
> if (dc->res_pool->pp_smu)
> @@ -1546,10 +1532,6 @@ void dcn_bw_notify_pplib_of_wm_ranges(struct dc *dc)
> if (!pp || !pp->set_wm_ranges)
> return;
>
> - min_fclk_khz = dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 * 1000000 / 32;
> - min_dcfclk_khz = dc->dcn_soc->dcfclkv_min0p65 * 1000;
> - socclk_khz = dc->dcn_soc->socclk * 1000;
> -
> /* Now notify PPLib/SMU about which Watermarks sets they should select
> * depending on DPM state they are in. And update BW MGR GFX Engine and
> * Memory clock member variables for Watermarks calculations for each
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h b/drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h
> index 806f3041db14..9e4ddc985240 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h
> @@ -628,8 +628,23 @@ unsigned int dcn_find_dcfclk_suits_all(
> const struct dc *dc,
> struct dc_clocks *clocks);
>
> -void dcn_bw_update_from_pplib(struct dc *dc);
> -void dcn_bw_notify_pplib_of_wm_ranges(struct dc *dc);
> +void dcn_get_soc_clks(
> + struct dc *dc,
> + int *min_fclk_khz,
> + int *min_dcfclk_khz,
> + int *socclk_khz);
> +
> +void dcn_bw_update_from_pplib_fclks(
> + struct dc *dc,
> + struct dm_pp_clock_levels_with_voltage *fclks);
> +void dcn_bw_update_from_pplib_dcfclks(
> + struct dc *dc,
> + struct dm_pp_clock_levels_with_voltage *dcfclks);
> +void dcn_bw_notify_pplib_of_wm_ranges(
> + struct dc *dc,
> + int min_fclk_khz,
> + int min_dcfclk_khz,
> + int socclk_khz);
> void dcn_bw_sync_calcs_and_dml(struct dc *dc);
>
> enum source_macro_tile_size swizzle_mode_to_macro_tile_size(enum swizzle_mode_values sw_mode);

2022-09-30 15:03:55

by Hamza Mahfooz

[permalink] [raw]
Subject: Re: [PATCH] drm/amd/display: Fix mutex lock in dcn10

Applied. Thanks!

On 2022-09-25 17:53, Daniel Gomez wrote:
> Removal of DC_FP_* wrappers from dml (9696679bf7ac) provokes a mutex
> lock [2] on the amdgpu driver. Re-arrange the dcn10 code to avoid
> locking the mutex by placing the DC_FP_* wrappers around the proper
> functions.
>
> This fixes the following WARN/stacktrace:
>
> BUG: sleeping function called from invalid context at kernel/locking/mutex.c:283
> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 227, name: systemd-udevd
> preempt_count: 1, expected: 0
> CPU: 4 PID: 227 Comm: systemd-udevd Not tainted 6.0.0-rc6-qtec-standard #2
> Hardware name: Qtechnology A/S QT5222/QT5221, BIOS v1.0.1 06/07/2021
> Call Trace:
> <TASK>
> dump_stack_lvl+0x33/0x42
> __might_resched.cold.172+0xa5/0xb3
> mutex_lock+0x1a/0x40
> amdgpu_dpm_get_clock_by_type_with_voltage+0x38/0x70 [amdgpu]
> dm_pp_get_clock_levels_by_type_with_voltage+0x64/0xa0 [amdgpu]
> dcn_bw_update_from_pplib+0x70/0x340 [amdgpu]
> dcn10_create_resource_pool+0x8c8/0xd20 [amdgpu]
> ? __kmalloc+0x1c7/0x4a0
> dc_create_resource_pool+0xe7/0x190 [amdgpu]
> dc_create+0x212/0x5d0 [amdgpu]
> amdgpu_dm_init+0x246/0x370 [amdgpu]
> ? schedule_hrtimeout_range_clock+0x93/0x120
> ? phm_wait_for_register_unequal.part.1+0x4a/0x80 [amdgpu]
> dm_hw_init+0xe/0x20 [amdgpu]
> amdgpu_device_init.cold.56+0x1324/0x1653 [amdgpu]
> ? pci_bus_read_config_word+0x43/0x80
> amdgpu_driver_load_kms+0x15/0x120 [amdgpu]
> amdgpu_pci_probe+0x116/0x320 [amdgpu]
> pci_device_probe+0x97/0x110
> really_probe+0xdd/0x340
> __driver_probe_device+0x80/0x170
> driver_probe_device+0x1f/0x90
> __driver_attach+0xdc/0x180
> ? __device_attach_driver+0x100/0x100
> ? __device_attach_driver+0x100/0x100
> bus_for_each_dev+0x74/0xc0
> bus_add_driver+0x19e/0x210
> ? kset_find_obj+0x30/0xa0
> ? 0xffffffffa0a5b000
> driver_register+0x6b/0xc0
> ? 0xffffffffa0a5b000
> do_one_initcall+0x4a/0x1f0
> ? __vunmap+0x28e/0x2f0
> ? __cond_resched+0x15/0x30
> ? kmem_cache_alloc_trace+0x3d/0x440
> do_init_module+0x4a/0x1e0
> load_module+0x1cba/0x1e10
> ? __do_sys_finit_module+0xb7/0x120
> __do_sys_finit_module+0xb7/0x120
> do_syscall_64+0x3c/0x80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> RIP: 0033:0x7ff2b5f5422d
> Code: 5d c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48
> 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48>
> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 ab 0e 00 f7 d8 64 89 01 48
> RSP: 002b:00007ffc44ab28e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
> RAX: ffffffffffffffda RBX: 0000555c566a9240 RCX: 00007ff2b5f5422d
> RDX: 0000000000000000 RSI: 00007ff2b60bb353 RDI: 0000000000000019
> RBP: 00007ff2b60bb353 R08: 0000000000000000 R09: 0000555c566a9240
> R10: 0000000000000019 R11: 0000000000000246 R12: 0000000000000000
> R13: 0000000000020000 R14: 0000000000000000 R15: 0000000000000000
> </TASK>
>
> Fixes: 9696679bf7ac ("drm/amd/display: remove DC_FP_* wrapper from
> dml folder")
> Signed-off-by: Daniel Gomez <[email protected]>
> ---
> .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 12 +-
> .../drm/amd/display/dc/dcn10/dcn10_resource.c | 66 +++++++++-
> .../drm/amd/display/dc/dml/calcs/dcn_calcs.c | 118 ++++++++----------
> .../gpu/drm/amd/display/dc/inc/dcn_calcs.h | 19 ++-
> 4 files changed, 138 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> index 5b5d952b2b8c..cb1e06d62841 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> @@ -2994,6 +2994,7 @@ void dcn10_prepare_bandwidth(
> {
> struct dce_hwseq *hws = dc->hwseq;
> struct hubbub *hubbub = dc->res_pool->hubbub;
> + int min_fclk_khz, min_dcfclk_khz, socclk_khz;
>
> if (dc->debug.sanity_checks)
> hws->funcs.verify_allow_pstate_change_high(dc);
> @@ -3016,8 +3017,11 @@ void dcn10_prepare_bandwidth(
>
> if (dc->debug.pplib_wm_report_mode == WM_REPORT_OVERRIDE) {
> DC_FP_START();
> - dcn_bw_notify_pplib_of_wm_ranges(dc);
> + dcn_get_soc_clks(
> + dc, &min_fclk_khz, &min_dcfclk_khz, &socclk_khz);
> DC_FP_END();
> + dcn_bw_notify_pplib_of_wm_ranges(
> + dc, min_fclk_khz, min_dcfclk_khz, socclk_khz);
> }
>
> if (dc->debug.sanity_checks)
> @@ -3030,6 +3034,7 @@ void dcn10_optimize_bandwidth(
> {
> struct dce_hwseq *hws = dc->hwseq;
> struct hubbub *hubbub = dc->res_pool->hubbub;
> + int min_fclk_khz, min_dcfclk_khz, socclk_khz;
>
> if (dc->debug.sanity_checks)
> hws->funcs.verify_allow_pstate_change_high(dc);
> @@ -3053,8 +3058,11 @@ void dcn10_optimize_bandwidth(
>
> if (dc->debug.pplib_wm_report_mode == WM_REPORT_OVERRIDE) {
> DC_FP_START();
> - dcn_bw_notify_pplib_of_wm_ranges(dc);
> + dcn_get_soc_clks(
> + dc, &min_fclk_khz, &min_dcfclk_khz, &socclk_khz);
> DC_FP_END();
> + dcn_bw_notify_pplib_of_wm_ranges(
> + dc, min_fclk_khz, min_dcfclk_khz, socclk_khz);
> }
>
> if (dc->debug.sanity_checks)
> 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 174eebbe8b4f..a18a5b56ca7d 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
> @@ -1336,6 +1336,21 @@ static noinline void dcn10_resource_construct_fp(
> }
> }
>
> +static bool verify_clock_values(struct dm_pp_clock_levels_with_voltage *clks)
> +{
> + int i;
> +
> + if (clks->num_levels == 0)
> + return false;
> +
> + for (i = 0; i < clks->num_levels; i++)
> + /* Ensure that the result is sane */
> + if (clks->data[i].clocks_in_khz == 0)
> + return false;
> +
> + return true;
> +}
> +
> static bool dcn10_resource_construct(
> uint8_t num_virtual_links,
> struct dc *dc,
> @@ -1345,6 +1360,9 @@ static bool dcn10_resource_construct(
> int j;
> struct dc_context *ctx = dc->ctx;
> uint32_t pipe_fuses = read_pipe_fuses(ctx);
> + struct dm_pp_clock_levels_with_voltage fclks = {0}, dcfclks = {0};
> + int min_fclk_khz, min_dcfclk_khz, socclk_khz;
> + bool res;
>
> ctx->dc_bios->regs = &bios_regs;
>
> @@ -1505,15 +1523,53 @@ static bool dcn10_resource_construct(
> && pool->base.pp_smu->rv_funcs.set_pme_wa_enable != NULL)
> dc->debug.az_endpoint_mute_only = false;
>
> - DC_FP_START();
> - if (!dc->debug.disable_pplib_clock_request)
> - dcn_bw_update_from_pplib(dc);
> +
> + if (!dc->debug.disable_pplib_clock_request) {
> + /*
> + * TODO: This is not the proper way to obtain
> + * fabric_and_dram_bandwidth, should be min(fclk, memclk).
> + */
> + res = dm_pp_get_clock_levels_by_type_with_voltage(
> + ctx, DM_PP_CLOCK_TYPE_FCLK, &fclks);
> +
> + DC_FP_START();
> +
> + if (res)
> + res = verify_clock_values(&fclks);
> +
> + if (res)
> + dcn_bw_update_from_pplib_fclks(dc, &fclks);
> + else
> + BREAK_TO_DEBUGGER();
> +
> + DC_FP_END();
> +
> + res = dm_pp_get_clock_levels_by_type_with_voltage(
> + ctx, DM_PP_CLOCK_TYPE_DCFCLK, &dcfclks);
> +
> + DC_FP_START();
> +
> + if (res)
> + res = verify_clock_values(&dcfclks);
> +
> + if (res)
> + dcn_bw_update_from_pplib_dcfclks(dc, &dcfclks);
> + else
> + BREAK_TO_DEBUGGER();
> +
> + DC_FP_END();
> + }
> +
> dcn_bw_sync_calcs_and_dml(dc);
> if (!dc->debug.disable_pplib_wm_range) {
> dc->res_pool = &pool->base;
> - dcn_bw_notify_pplib_of_wm_ranges(dc);
> + DC_FP_START();
> + dcn_get_soc_clks(
> + dc, &min_fclk_khz, &min_dcfclk_khz, &socclk_khz);
> + DC_FP_END();
> + dcn_bw_notify_pplib_of_wm_ranges(
> + dc, min_fclk_khz, min_dcfclk_khz, socclk_khz);
> }
> - DC_FP_END();
>
> {
> struct irq_service_init_data init_data;
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
> index db3b16b77034..7d3394470352 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c
> @@ -1464,81 +1464,67 @@ unsigned int dcn_find_dcfclk_suits_all(
> return dcf_clk;
> }
>
> -static bool verify_clock_values(struct dm_pp_clock_levels_with_voltage *clks)
> +void dcn_bw_update_from_pplib_fclks(
> + struct dc *dc,
> + struct dm_pp_clock_levels_with_voltage *fclks)
> {
> - int i;
> -
> - if (clks->num_levels == 0)
> - return false;
> -
> - for (i = 0; i < clks->num_levels; i++)
> - /* Ensure that the result is sane */
> - if (clks->data[i].clocks_in_khz == 0)
> - return false;
> + unsigned vmin0p65_idx, vmid0p72_idx, vnom0p8_idx, vmax0p9_idx;
>
> - return true;
> + ASSERT(fclks->num_levels);
> +
> + vmin0p65_idx = 0;
> + vmid0p72_idx = fclks->num_levels -
> + (fclks->num_levels > 2 ? 3 : (fclks->num_levels > 1 ? 2 : 1));
> + vnom0p8_idx = fclks->num_levels - (fclks->num_levels > 1 ? 2 : 1);
> + vmax0p9_idx = fclks->num_levels - 1;
> +
> + dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 =
> + 32 * (fclks->data[vmin0p65_idx].clocks_in_khz / 1000.0) / 1000.0;
> + dc->dcn_soc->fabric_and_dram_bandwidth_vmid0p72 =
> + dc->dcn_soc->number_of_channels *
> + (fclks->data[vmid0p72_idx].clocks_in_khz / 1000.0)
> + * ddr4_dram_factor_single_Channel / 1000.0;
> + dc->dcn_soc->fabric_and_dram_bandwidth_vnom0p8 =
> + dc->dcn_soc->number_of_channels *
> + (fclks->data[vnom0p8_idx].clocks_in_khz / 1000.0)
> + * ddr4_dram_factor_single_Channel / 1000.0;
> + dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9 =
> + dc->dcn_soc->number_of_channels *
> + (fclks->data[vmax0p9_idx].clocks_in_khz / 1000.0)
> + * ddr4_dram_factor_single_Channel / 1000.0;
> }
>
> -void dcn_bw_update_from_pplib(struct dc *dc)
> +void dcn_bw_update_from_pplib_dcfclks(
> + struct dc *dc,
> + struct dm_pp_clock_levels_with_voltage *dcfclks)
> {
> - struct dc_context *ctx = dc->ctx;
> - struct dm_pp_clock_levels_with_voltage fclks = {0}, dcfclks = {0};
> - bool res;
> - unsigned vmin0p65_idx, vmid0p72_idx, vnom0p8_idx, vmax0p9_idx;
> -
> - /* TODO: This is not the proper way to obtain fabric_and_dram_bandwidth, should be min(fclk, memclk) */
> - res = dm_pp_get_clock_levels_by_type_with_voltage(
> - ctx, DM_PP_CLOCK_TYPE_FCLK, &fclks);
> -
> - if (res)
> - res = verify_clock_values(&fclks);
> -
> - if (res) {
> - ASSERT(fclks.num_levels);
> -
> - vmin0p65_idx = 0;
> - vmid0p72_idx = fclks.num_levels -
> - (fclks.num_levels > 2 ? 3 : (fclks.num_levels > 1 ? 2 : 1));
> - vnom0p8_idx = fclks.num_levels - (fclks.num_levels > 1 ? 2 : 1);
> - vmax0p9_idx = fclks.num_levels - 1;
> -
> - dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 =
> - 32 * (fclks.data[vmin0p65_idx].clocks_in_khz / 1000.0) / 1000.0;
> - dc->dcn_soc->fabric_and_dram_bandwidth_vmid0p72 =
> - dc->dcn_soc->number_of_channels *
> - (fclks.data[vmid0p72_idx].clocks_in_khz / 1000.0)
> - * ddr4_dram_factor_single_Channel / 1000.0;
> - dc->dcn_soc->fabric_and_dram_bandwidth_vnom0p8 =
> - dc->dcn_soc->number_of_channels *
> - (fclks.data[vnom0p8_idx].clocks_in_khz / 1000.0)
> - * ddr4_dram_factor_single_Channel / 1000.0;
> - dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9 =
> - dc->dcn_soc->number_of_channels *
> - (fclks.data[vmax0p9_idx].clocks_in_khz / 1000.0)
> - * ddr4_dram_factor_single_Channel / 1000.0;
> - } else
> - BREAK_TO_DEBUGGER();
> -
> - res = dm_pp_get_clock_levels_by_type_with_voltage(
> - ctx, DM_PP_CLOCK_TYPE_DCFCLK, &dcfclks);
> -
> - if (res)
> - res = verify_clock_values(&dcfclks);
> + if (dcfclks->num_levels >= 3) {
> + dc->dcn_soc->dcfclkv_min0p65 = dcfclks->data[0].clocks_in_khz / 1000.0;
> + dc->dcn_soc->dcfclkv_mid0p72 = dcfclks->data[dcfclks->num_levels - 3].clocks_in_khz / 1000.0;
> + dc->dcn_soc->dcfclkv_nom0p8 = dcfclks->data[dcfclks->num_levels - 2].clocks_in_khz / 1000.0;
> + dc->dcn_soc->dcfclkv_max0p9 = dcfclks->data[dcfclks->num_levels - 1].clocks_in_khz / 1000.0;
> + }
> +}
>
> - if (res && dcfclks.num_levels >= 3) {
> - dc->dcn_soc->dcfclkv_min0p65 = dcfclks.data[0].clocks_in_khz / 1000.0;
> - dc->dcn_soc->dcfclkv_mid0p72 = dcfclks.data[dcfclks.num_levels - 3].clocks_in_khz / 1000.0;
> - dc->dcn_soc->dcfclkv_nom0p8 = dcfclks.data[dcfclks.num_levels - 2].clocks_in_khz / 1000.0;
> - dc->dcn_soc->dcfclkv_max0p9 = dcfclks.data[dcfclks.num_levels - 1].clocks_in_khz / 1000.0;
> - } else
> - BREAK_TO_DEBUGGER();
> +void dcn_get_soc_clks(
> + struct dc *dc,
> + int *min_fclk_khz,
> + int *min_dcfclk_khz,
> + int *socclk_khz)
> +{
> + *min_fclk_khz = dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 * 1000000 / 32;
> + *min_dcfclk_khz = dc->dcn_soc->dcfclkv_min0p65 * 1000;
> + *socclk_khz = dc->dcn_soc->socclk * 1000;
> }
>
> -void dcn_bw_notify_pplib_of_wm_ranges(struct dc *dc)
> +void dcn_bw_notify_pplib_of_wm_ranges(
> + struct dc *dc,
> + int min_fclk_khz,
> + int min_dcfclk_khz,
> + int socclk_khz)
> {
> struct pp_smu_funcs_rv *pp = NULL;
> struct pp_smu_wm_range_sets ranges = {0};
> - int min_fclk_khz, min_dcfclk_khz, socclk_khz;
> const int overdrive = 5000000; /* 5 GHz to cover Overdrive */
>
> if (dc->res_pool->pp_smu)
> @@ -1546,10 +1532,6 @@ void dcn_bw_notify_pplib_of_wm_ranges(struct dc *dc)
> if (!pp || !pp->set_wm_ranges)
> return;
>
> - min_fclk_khz = dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 * 1000000 / 32;
> - min_dcfclk_khz = dc->dcn_soc->dcfclkv_min0p65 * 1000;
> - socclk_khz = dc->dcn_soc->socclk * 1000;
> -
> /* Now notify PPLib/SMU about which Watermarks sets they should select
> * depending on DPM state they are in. And update BW MGR GFX Engine and
> * Memory clock member variables for Watermarks calculations for each
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h b/drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h
> index 806f3041db14..9e4ddc985240 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h
> @@ -628,8 +628,23 @@ unsigned int dcn_find_dcfclk_suits_all(
> const struct dc *dc,
> struct dc_clocks *clocks);
>
> -void dcn_bw_update_from_pplib(struct dc *dc);
> -void dcn_bw_notify_pplib_of_wm_ranges(struct dc *dc);
> +void dcn_get_soc_clks(
> + struct dc *dc,
> + int *min_fclk_khz,
> + int *min_dcfclk_khz,
> + int *socclk_khz);
> +
> +void dcn_bw_update_from_pplib_fclks(
> + struct dc *dc,
> + struct dm_pp_clock_levels_with_voltage *fclks);
> +void dcn_bw_update_from_pplib_dcfclks(
> + struct dc *dc,
> + struct dm_pp_clock_levels_with_voltage *dcfclks);
> +void dcn_bw_notify_pplib_of_wm_ranges(
> + struct dc *dc,
> + int min_fclk_khz,
> + int min_dcfclk_khz,
> + int socclk_khz);
> void dcn_bw_sync_calcs_and_dml(struct dc *dc);
>
> enum source_macro_tile_size swizzle_mode_to_macro_tile_size(enum swizzle_mode_values sw_mode);

--
Hamza