AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy()
@ 2025-03-18 11:17 Huacai Chen
  2025-03-18 11:17 ` [PATCH 2/2] drm/amd/display: Protect dml2_validate() from its callers Huacai Chen
  2025-03-20  2:16 ` [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy() Alex Hung
  0 siblings, 2 replies; 10+ messages in thread
From: Huacai Chen @ 2025-03-18 11:17 UTC (permalink / raw)
  To: David Airlie, Simona Vetter, Alex Deucher, Christian König,
	Huacai Chen
  Cc: amd-gfx, dri-devel, Huacai Chen, stable

Commit 7da55c27e76749b9 ("drm/amd/display: Remove incorrect FP context
start") removes the FP context protection of dml2_create(), and it said
"All the DC_FP_START/END should be used before call anything from DML2".

However, dml2_create()/dml2_copy()/dml2_create_copy() are not protected
from their callers, causing such errors:

 do_fpu invoked from kernel context![#1]:
 CPU: 0 UID: 0 PID: 239 Comm: kworker/0:5 Not tainted 6.14.0-rc6+ #1
 Workqueue: events work_for_cpu_fn
 pc ffff80000319de80 ra ffff80000319de5c tp 900000010575c000 sp 900000010575f840
 a0 0000000000000000 a1 900000012f210130 a2 900000012f000000 a3 ffff80000357e268
 a4 ffff80000357e260 a5 900000012ea52cf0 a6 0000000400000004 a7 0000012c00001388
 t0 00001900000015e0 t1 ffff80000379d000 t2 0000000010624dd3 t3 0000006400000014
 t4 00000000000003e8 t5 0000005000000018 t6 0000000000000020 t7 0000000f00000064
 t8 000000000000002f u0 5f5e9200f8901912 s9 900000012d380010 s0 900000012ea51fd8
 s1 900000012f000000 s2 9000000109296000 s3 0000000000000001 s4 0000000000001fd8
 s5 0000000000000001 s6 ffff800003415000 s7 900000012d390000 s8 ffff800003211f80
    ra: ffff80000319de5c dml21_apply_soc_bb_overrides+0x3c/0x960 [amdgpu]
   ERA: ffff80000319de80 dml21_apply_soc_bb_overrides+0x60/0x960 [amdgpu]
  CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
  PRMD: 00000004 (PPLV0 +PIE -PWE)
  EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
  ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
 ESTAT: 000f0000 [FPD] (IS= ECode=15 EsubCode=0)
  PRID: 0014d010 (Loongson-64bit, Loongson-3C6000/S)
 Process kworker/0:5 (pid: 239, threadinfo=00000000927eadc6, task=000000008fd31682)
 Stack : 00040dc000003164 0000000000000001 900000012f210130 900000012eabeeb8
         900000012f000000 ffff80000319fe48 900000012f210000 900000012f210130
         900000012f000000 900000012eabeeb8 0000000000000001 ffff8000031a0064
         900000010575f9f0 900000012f210130 900000012eac0000 900000012ea80000
         900000012f000000 ffff8000031cefc4 900000010575f9f0 ffff8000035859c0
         ffff800003414000 900000010575fa78 900000012f000000 ffff8000031b4c50
         0000000000000000 9000000101c9d700 9000000109c40000 5f5e9200f8901912
         900000012d3c4bd0 900000012d3c5000 ffff8000034aed18 900000012d380010
         900000012d3c4bd0 ffff800003414000 900000012d380000 ffff800002ea49dc
         0000000000000001 900000012d3c6000 00000000ffffe423 0000000000010000
         ...
 Call Trace:
 [<ffff80000319de80>] dml21_apply_soc_bb_overrides+0x60/0x960 [amdgpu]
 [<ffff80000319fe44>] dml21_init+0xa4/0x280 [amdgpu]
 [<ffff8000031a0060>] dml21_create+0x40/0x80 [amdgpu]
 [<ffff8000031cefc0>] dc_state_create+0x100/0x160 [amdgpu]
 [<ffff8000031b4c4c>] dc_create+0x44c/0x640 [amdgpu]
 [<ffff800002ea49d8>] amdgpu_dm_init+0x3f8/0x2060 [amdgpu]
 [<ffff800002ea6658>] dm_hw_init+0x18/0x60 [amdgpu]
 [<ffff800002b16738>] amdgpu_device_init+0x1938/0x27e0 [amdgpu]
 [<ffff800002b18e80>] amdgpu_driver_load_kms+0x20/0xa0 [amdgpu]
 [<ffff800002b0c8f0>] amdgpu_pci_probe+0x1b0/0x580 [amdgpu]
 [<900000000448eae4>] local_pci_probe+0x44/0xc0
 [<9000000003b02b18>] work_for_cpu_fn+0x18/0x40
 [<9000000003b05da0>] process_one_work+0x160/0x300
 [<9000000003b06718>] worker_thread+0x318/0x440
 [<9000000003b11b8c>] kthread+0x12c/0x220
 [<9000000003ac1484>] ret_from_kernel_thread+0x8/0xa4

So protect dml2_create()/dml2_copy()/dml2_create_copy() with DC_FP_START
and DC_FP_END.

Cc: stable@vger.kernel.org
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
 drivers/gpu/drm/amd/display/dc/core/dc_state.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
index 1b2cce127981..6e2cac08002d 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
@@ -210,17 +210,23 @@ struct dc_state *dc_state_create(struct dc *dc, struct dc_state_create_params *p
 
 #ifdef CONFIG_DRM_AMD_DC_FP
 	if (dc->debug.using_dml2) {
+		DC_FP_START();
+
 		dml2_opt->use_clock_dc_limits = false;
 		if (!dml2_create(dc, dml2_opt, &state->bw_ctx.dml2)) {
+			DC_FP_END();
 			dc_state_release(state);
 			return NULL;
 		}
 
 		dml2_opt->use_clock_dc_limits = true;
 		if (!dml2_create(dc, dml2_opt, &state->bw_ctx.dml2_dc_power_source)) {
+			DC_FP_END();
 			dc_state_release(state);
 			return NULL;
 		}
+
+		DC_FP_END();
 	}
 #endif
 
@@ -240,6 +246,8 @@ void dc_state_copy(struct dc_state *dst_state, struct dc_state *src_state)
 	dc_state_copy_internal(dst_state, src_state);
 
 #ifdef CONFIG_DRM_AMD_DC_FP
+	DC_FP_START();
+
 	dst_state->bw_ctx.dml2 = dst_dml2;
 	if (src_state->bw_ctx.dml2)
 		dml2_copy(dst_state->bw_ctx.dml2, src_state->bw_ctx.dml2);
@@ -247,6 +255,8 @@ void dc_state_copy(struct dc_state *dst_state, struct dc_state *src_state)
 	dst_state->bw_ctx.dml2_dc_power_source = dst_dml2_dc_power_source;
 	if (src_state->bw_ctx.dml2_dc_power_source)
 		dml2_copy(dst_state->bw_ctx.dml2_dc_power_source, src_state->bw_ctx.dml2_dc_power_source);
+
+	DC_FP_END();
 #endif
 
 	/* context refcount should not be overridden */
@@ -268,17 +278,23 @@ struct dc_state *dc_state_create_copy(struct dc_state *src_state)
 	new_state->bw_ctx.dml2 = NULL;
 	new_state->bw_ctx.dml2_dc_power_source = NULL;
 
+	DC_FP_START();
+
 	if (src_state->bw_ctx.dml2 &&
 			!dml2_create_copy(&new_state->bw_ctx.dml2, src_state->bw_ctx.dml2)) {
+		DC_FP_END();
 		dc_state_release(new_state);
 		return NULL;
 	}
 
 	if (src_state->bw_ctx.dml2_dc_power_source &&
 			!dml2_create_copy(&new_state->bw_ctx.dml2_dc_power_source, src_state->bw_ctx.dml2_dc_power_source)) {
+		DC_FP_END();
 		dc_state_release(new_state);
 		return NULL;
 	}
+
+	DC_FP_END();
 #endif
 
 	kref_init(&new_state->refcount);
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] drm/amd/display: Protect dml2_validate() from its callers
  2025-03-18 11:17 [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy() Huacai Chen
@ 2025-03-18 11:17 ` Huacai Chen
  2025-03-20  2:17   ` Alex Hung
  2025-03-20  2:16 ` [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy() Alex Hung
  1 sibling, 1 reply; 10+ messages in thread
From: Huacai Chen @ 2025-03-18 11:17 UTC (permalink / raw)
  To: David Airlie, Simona Vetter, Alex Deucher, Christian König,
	Huacai Chen
  Cc: amd-gfx, dri-devel, Huacai Chen, stable

Similar to dml2_create()/dml2_copy()/dml2_create_copy(), dml2_validate()
should also be protected from its callers because "All the DC_FP_START/END
should be used before call anything from DML2".

So protect dml2_validate() with DC_FP_START() and DC_FP_END(), otherwise
it causes such errors:

 do_fpu invoked from kernel context![#1]:
 CPU: 10 UID: 0 PID: 331 Comm: kworker/10:1H Not tainted 6.14.0-rc6+ #4
 Workqueue: events_highpri dm_irq_work_func [amdgpu]
 pc ffff800003191eb0 ra ffff800003191e60 tp 9000000107a94000 sp 9000000107a975b0
 a0 9000000140ce4910 a1 0000000000000000 a2 9000000140ce49b0 a3 9000000140ce49a8
 a4 9000000140ce49a8 a5 0000000100000000 a6 0000000000000001 a7 9000000107a97660
 t0 ffff800003790000 t1 9000000140ce5000 t2 0000000000000001 t3 0000000000000000
 t4 0000000000000004 t5 0000000000000000 t6 0000000000000000 t7 0000000000000000
 t8 0000000100000000 u0 ffff8000031a3b9c s9 9000000130bc0000 s0 9000000132400000
 s1 9000000140ec0000 s2 9000000132400000 s3 9000000140ce0000 s4 90000000057f8b88
 s5 9000000140ec0000 s6 9000000140ce4910 s7 0000000000000001 s8 9000000130d45010
 ra: ffff800003191e60 dml21_map_dc_state_into_dml_display_cfg+0x40/0x1140 [amdgpu]
   ERA: ffff800003191eb0 dml21_map_dc_state_into_dml_display_cfg+0x90/0x1140 [amdgpu]
  CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
  PRMD: 00000004 (PPLV0 +PIE -PWE)
  EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
  ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
 ESTAT: 000f0000 [FPD] (IS= ECode=15 EsubCode=0)
  PRID: 0014d010 (Loongson-64bit, Loongson-3C6000/S)
 Process kworker/10:1H (pid: 331, threadinfo=000000007bf9ddb0, task=00000000cc4ab9f3)
 Stack : 0000000100000000 0000043800000780 0000000100000001 0000000100000001
         0000000000000000 0000078000000000 0000000000000438 0000078000000000
         0000000000000438 0000078000000000 0000000000000438 0000000100000000
         0000000100000000 0000000100000000 0000000100000000 0000000100000000
         0000000000000001 9000000140ec0000 9000000132400000 9000000132400000
         ffff800003408000 ffff800003408000 9000000132400000 9000000140ce0000
         9000000140ce0000 ffff800003193850 0000000000000001 9000000140ec0000
         9000000132400000 9000000140ec0860 9000000140ec0738 0000000000000001
         90000001405e8000 9000000130bc0000 9000000140ec02a8 ffff8000031b5db8
         0000000000000000 0000043800000780 0000000000000003 ffff8000031b79cc
         ...
 Call Trace:
 [<ffff800003191eb0>] dml21_map_dc_state_into_dml_display_cfg+0x90/0x1140 [amdgpu]
 [<ffff80000319384c>] dml21_validate+0xcc/0x520 [amdgpu]
 [<ffff8000031b8948>] dc_validate_global_state+0x2e8/0x460 [amdgpu]
 [<ffff800002e94034>] create_validate_stream_for_sink+0x3d4/0x420 [amdgpu]
 [<ffff800002e940e4>] amdgpu_dm_connector_mode_valid+0x64/0x240 [amdgpu]
 [<900000000441d6b8>] drm_connector_mode_valid+0x38/0x80
 [<900000000441d824>] __drm_helper_update_and_validate+0x124/0x3e0
 [<900000000441ddc0>] drm_helper_probe_single_connector_modes+0x2e0/0x620
 [<90000000044050dc>] drm_client_modeset_probe+0x23c/0x1780
 [<9000000004420384>] __drm_fb_helper_initial_config_and_unlock+0x44/0x5a0
 [<9000000004403acc>] drm_client_dev_hotplug+0xcc/0x140
 [<ffff800002e9ab50>] handle_hpd_irq_helper+0x1b0/0x1e0 [amdgpu]
 [<90000000038f5da0>] process_one_work+0x160/0x300
 [<90000000038f6718>] worker_thread+0x318/0x440
 [<9000000003901b8c>] kthread+0x12c/0x220
 [<90000000038b1484>] ret_from_kernel_thread+0x8/0xa4

Cc: stable@vger.kernel.org
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
 .../gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c   | 5 ++++-
 .../gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c   | 2 ++
 .../gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c | 2 ++
 .../gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c | 5 ++++-
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
index 664302876019..63ef8629d7ed 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
@@ -1811,10 +1811,13 @@ bool dcn32_validate_bandwidth(struct dc *dc,
 {
 	bool out = false;
 
-	if (dc->debug.using_dml2)
+	if (dc->debug.using_dml2) {
+		DC_FP_START();
 		out = dml2_validate(dc, context,
 				context->power_source == DC_POWER_SOURCE_DC ? context->bw_ctx.dml2_dc_power_source : context->bw_ctx.dml2,
 				fast_validate);
+		DC_FP_END();
+	}
 	else
 		out = dml1_validate(dc, context, fast_validate);
 	return out;
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c
index 8ee3d99ea2aa..0495c8dbcf1e 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c
@@ -1738,9 +1738,11 @@ static bool dcn35_validate_bandwidth(struct dc *dc,
 {
 	bool out = false;
 
+	DC_FP_START();
 	out = dml2_validate(dc, context,
 			context->power_source == DC_POWER_SOURCE_DC ? context->bw_ctx.dml2_dc_power_source : context->bw_ctx.dml2,
 			fast_validate);
+	DC_FP_END();
 
 	if (fast_validate)
 		return out;
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c
index 14f7c3acdc96..aea5acf22b26 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c
@@ -1718,9 +1718,11 @@ static bool dcn351_validate_bandwidth(struct dc *dc,
 {
 	bool out = false;
 
+	DC_FP_START();
 	out = dml2_validate(dc, context,
 			context->power_source == DC_POWER_SOURCE_DC ? context->bw_ctx.dml2_dc_power_source : context->bw_ctx.dml2,
 			fast_validate);
+	DC_FP_END();
 
 	if (fast_validate)
 		return out;
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
index c1ebc6b1c937..a1815fa59b02 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
@@ -1650,10 +1650,13 @@ bool dcn401_validate_bandwidth(struct dc *dc,
 		bool fast_validate)
 {
 	bool out = false;
-	if (dc->debug.using_dml2)
+	if (dc->debug.using_dml2) {
+		DC_FP_START();
 		out = dml2_validate(dc, context,
 				context->power_source == DC_POWER_SOURCE_DC ? context->bw_ctx.dml2_dc_power_source : context->bw_ctx.dml2,
 				fast_validate);
+		DC_FP_END();
+	}
 	return out;
 }
 
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy()
  2025-03-18 11:17 [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy() Huacai Chen
  2025-03-18 11:17 ` [PATCH 2/2] drm/amd/display: Protect dml2_validate() from its callers Huacai Chen
@ 2025-03-20  2:16 ` Alex Hung
  2025-03-20  4:23   ` Huacai Chen
  1 sibling, 1 reply; 10+ messages in thread
From: Alex Hung @ 2025-03-20  2:16 UTC (permalink / raw)
  To: Huacai Chen, David Airlie, Simona Vetter, Alex Deucher,
	Christian König, Huacai Chen
  Cc: amd-gfx, dri-devel, stable, Austin.Zheng



On 3/18/25 05:17, Huacai Chen wrote:
> Commit 7da55c27e76749b9 ("drm/amd/display: Remove incorrect FP context
> start") removes the FP context protection of dml2_create(), and it said
> "All the DC_FP_START/END should be used before call anything from DML2".
> 
> However, dml2_create()/dml2_copy()/dml2_create_copy() are not protected
> from their callers, causing such errors:
> 
>   do_fpu invoked from kernel context![#1]:
>   CPU: 0 UID: 0 PID: 239 Comm: kworker/0:5 Not tainted 6.14.0-rc6+ #1
>   Workqueue: events work_for_cpu_fn
>   pc ffff80000319de80 ra ffff80000319de5c tp 900000010575c000 sp 900000010575f840
>   a0 0000000000000000 a1 900000012f210130 a2 900000012f000000 a3 ffff80000357e268
>   a4 ffff80000357e260 a5 900000012ea52cf0 a6 0000000400000004 a7 0000012c00001388
>   t0 00001900000015e0 t1 ffff80000379d000 t2 0000000010624dd3 t3 0000006400000014
>   t4 00000000000003e8 t5 0000005000000018 t6 0000000000000020 t7 0000000f00000064
>   t8 000000000000002f u0 5f5e9200f8901912 s9 900000012d380010 s0 900000012ea51fd8
>   s1 900000012f000000 s2 9000000109296000 s3 0000000000000001 s4 0000000000001fd8
>   s5 0000000000000001 s6 ffff800003415000 s7 900000012d390000 s8 ffff800003211f80
>      ra: ffff80000319de5c dml21_apply_soc_bb_overrides+0x3c/0x960 [amdgpu]
>     ERA: ffff80000319de80 dml21_apply_soc_bb_overrides+0x60/0x960 [amdgpu]
>    CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
>    PRMD: 00000004 (PPLV0 +PIE -PWE)
>    EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
>    ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
>   ESTAT: 000f0000 [FPD] (IS= ECode=15 EsubCode=0)
>    PRID: 0014d010 (Loongson-64bit, Loongson-3C6000/S)
>   Process kworker/0:5 (pid: 239, threadinfo=00000000927eadc6, task=000000008fd31682)
>   Stack : 00040dc000003164 0000000000000001 900000012f210130 900000012eabeeb8
>           900000012f000000 ffff80000319fe48 900000012f210000 900000012f210130
>           900000012f000000 900000012eabeeb8 0000000000000001 ffff8000031a0064
>           900000010575f9f0 900000012f210130 900000012eac0000 900000012ea80000
>           900000012f000000 ffff8000031cefc4 900000010575f9f0 ffff8000035859c0
>           ffff800003414000 900000010575fa78 900000012f000000 ffff8000031b4c50
>           0000000000000000 9000000101c9d700 9000000109c40000 5f5e9200f8901912
>           900000012d3c4bd0 900000012d3c5000 ffff8000034aed18 900000012d380010
>           900000012d3c4bd0 ffff800003414000 900000012d380000 ffff800002ea49dc
>           0000000000000001 900000012d3c6000 00000000ffffe423 0000000000010000
>           ...
>   Call Trace:
>   [<ffff80000319de80>] dml21_apply_soc_bb_overrides+0x60/0x960 [amdgpu]
>   [<ffff80000319fe44>] dml21_init+0xa4/0x280 [amdgpu]
>   [<ffff8000031a0060>] dml21_create+0x40/0x80 [amdgpu]
>   [<ffff8000031cefc0>] dc_state_create+0x100/0x160 [amdgpu]
>   [<ffff8000031b4c4c>] dc_create+0x44c/0x640 [amdgpu]
>   [<ffff800002ea49d8>] amdgpu_dm_init+0x3f8/0x2060 [amdgpu]
>   [<ffff800002ea6658>] dm_hw_init+0x18/0x60 [amdgpu]
>   [<ffff800002b16738>] amdgpu_device_init+0x1938/0x27e0 [amdgpu]
>   [<ffff800002b18e80>] amdgpu_driver_load_kms+0x20/0xa0 [amdgpu]
>   [<ffff800002b0c8f0>] amdgpu_pci_probe+0x1b0/0x580 [amdgpu]
>   [<900000000448eae4>] local_pci_probe+0x44/0xc0
>   [<9000000003b02b18>] work_for_cpu_fn+0x18/0x40
>   [<9000000003b05da0>] process_one_work+0x160/0x300
>   [<9000000003b06718>] worker_thread+0x318/0x440
>   [<9000000003b11b8c>] kthread+0x12c/0x220
>   [<9000000003ac1484>] ret_from_kernel_thread+0x8/0xa4
> 
> So protect dml2_create()/dml2_copy()/dml2_create_copy() with DC_FP_START
> and DC_FP_END.

Hi Huacai,

Can you try to put DC_FP_START DC_FP_END in the 
dml2_create()/dml2_copy()/dml2_create_copy()/dml2_validate() instead? 
The code will be cleaner and less error-prone to future changes.

Thanks.


> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
>   drivers/gpu/drm/amd/display/dc/core/dc_state.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> index 1b2cce127981..6e2cac08002d 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> @@ -210,17 +210,23 @@ struct dc_state *dc_state_create(struct dc *dc, struct dc_state_create_params *p
>   
>   #ifdef CONFIG_DRM_AMD_DC_FP
>   	if (dc->debug.using_dml2) {
> +		DC_FP_START();
> +
>   		dml2_opt->use_clock_dc_limits = false;
>   		if (!dml2_create(dc, dml2_opt, &state->bw_ctx.dml2)) {
> +			DC_FP_END();
>   			dc_state_release(state);
>   			return NULL;
>   		}
>   
>   		dml2_opt->use_clock_dc_limits = true;
>   		if (!dml2_create(dc, dml2_opt, &state->bw_ctx.dml2_dc_power_source)) {
> +			DC_FP_END();
>   			dc_state_release(state);
>   			return NULL;
>   		}
> +
> +		DC_FP_END();
>   	}
>   #endif
>   
> @@ -240,6 +246,8 @@ void dc_state_copy(struct dc_state *dst_state, struct dc_state *src_state)
>   	dc_state_copy_internal(dst_state, src_state);
>   
>   #ifdef CONFIG_DRM_AMD_DC_FP
> +	DC_FP_START();
> +
>   	dst_state->bw_ctx.dml2 = dst_dml2;
>   	if (src_state->bw_ctx.dml2)
>   		dml2_copy(dst_state->bw_ctx.dml2, src_state->bw_ctx.dml2);
> @@ -247,6 +255,8 @@ void dc_state_copy(struct dc_state *dst_state, struct dc_state *src_state)
>   	dst_state->bw_ctx.dml2_dc_power_source = dst_dml2_dc_power_source;
>   	if (src_state->bw_ctx.dml2_dc_power_source)
>   		dml2_copy(dst_state->bw_ctx.dml2_dc_power_source, src_state->bw_ctx.dml2_dc_power_source);
> +
> +	DC_FP_END();
>   #endif
>   
>   	/* context refcount should not be overridden */
> @@ -268,17 +278,23 @@ struct dc_state *dc_state_create_copy(struct dc_state *src_state)
>   	new_state->bw_ctx.dml2 = NULL;
>   	new_state->bw_ctx.dml2_dc_power_source = NULL;
>   
> +	DC_FP_START();
> +
>   	if (src_state->bw_ctx.dml2 &&
>   			!dml2_create_copy(&new_state->bw_ctx.dml2, src_state->bw_ctx.dml2)) {
> +		DC_FP_END();
>   		dc_state_release(new_state);
>   		return NULL;
>   	}
>   
>   	if (src_state->bw_ctx.dml2_dc_power_source &&
>   			!dml2_create_copy(&new_state->bw_ctx.dml2_dc_power_source, src_state->bw_ctx.dml2_dc_power_source)) {
> +		DC_FP_END();
>   		dc_state_release(new_state);
>   		return NULL;
>   	}
> +
> +	DC_FP_END();
>   #endif
>   
>   	kref_init(&new_state->refcount);


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/amd/display: Protect dml2_validate() from its callers
  2025-03-18 11:17 ` [PATCH 2/2] drm/amd/display: Protect dml2_validate() from its callers Huacai Chen
@ 2025-03-20  2:17   ` Alex Hung
  0 siblings, 0 replies; 10+ messages in thread
From: Alex Hung @ 2025-03-20  2:17 UTC (permalink / raw)
  To: Huacai Chen, David Airlie, Simona Vetter, Alex Deucher,
	Christian König, Huacai Chen
  Cc: amd-gfx, dri-devel, stable, Austin.Zheng

Please see comments for patch 1.

On 3/18/25 05:17, Huacai Chen wrote:
> Similar to dml2_create()/dml2_copy()/dml2_create_copy(), dml2_validate()
> should also be protected from its callers because "All the DC_FP_START/END
> should be used before call anything from DML2".
> 
> So protect dml2_validate() with DC_FP_START() and DC_FP_END(), otherwise
> it causes such errors:
> 
>   do_fpu invoked from kernel context![#1]:
>   CPU: 10 UID: 0 PID: 331 Comm: kworker/10:1H Not tainted 6.14.0-rc6+ #4
>   Workqueue: events_highpri dm_irq_work_func [amdgpu]
>   pc ffff800003191eb0 ra ffff800003191e60 tp 9000000107a94000 sp 9000000107a975b0
>   a0 9000000140ce4910 a1 0000000000000000 a2 9000000140ce49b0 a3 9000000140ce49a8
>   a4 9000000140ce49a8 a5 0000000100000000 a6 0000000000000001 a7 9000000107a97660
>   t0 ffff800003790000 t1 9000000140ce5000 t2 0000000000000001 t3 0000000000000000
>   t4 0000000000000004 t5 0000000000000000 t6 0000000000000000 t7 0000000000000000
>   t8 0000000100000000 u0 ffff8000031a3b9c s9 9000000130bc0000 s0 9000000132400000
>   s1 9000000140ec0000 s2 9000000132400000 s3 9000000140ce0000 s4 90000000057f8b88
>   s5 9000000140ec0000 s6 9000000140ce4910 s7 0000000000000001 s8 9000000130d45010
>   ra: ffff800003191e60 dml21_map_dc_state_into_dml_display_cfg+0x40/0x1140 [amdgpu]
>     ERA: ffff800003191eb0 dml21_map_dc_state_into_dml_display_cfg+0x90/0x1140 [amdgpu]
>    CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
>    PRMD: 00000004 (PPLV0 +PIE -PWE)
>    EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
>    ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
>   ESTAT: 000f0000 [FPD] (IS= ECode=15 EsubCode=0)
>    PRID: 0014d010 (Loongson-64bit, Loongson-3C6000/S)
>   Process kworker/10:1H (pid: 331, threadinfo=000000007bf9ddb0, task=00000000cc4ab9f3)
>   Stack : 0000000100000000 0000043800000780 0000000100000001 0000000100000001
>           0000000000000000 0000078000000000 0000000000000438 0000078000000000
>           0000000000000438 0000078000000000 0000000000000438 0000000100000000
>           0000000100000000 0000000100000000 0000000100000000 0000000100000000
>           0000000000000001 9000000140ec0000 9000000132400000 9000000132400000
>           ffff800003408000 ffff800003408000 9000000132400000 9000000140ce0000
>           9000000140ce0000 ffff800003193850 0000000000000001 9000000140ec0000
>           9000000132400000 9000000140ec0860 9000000140ec0738 0000000000000001
>           90000001405e8000 9000000130bc0000 9000000140ec02a8 ffff8000031b5db8
>           0000000000000000 0000043800000780 0000000000000003 ffff8000031b79cc
>           ...
>   Call Trace:
>   [<ffff800003191eb0>] dml21_map_dc_state_into_dml_display_cfg+0x90/0x1140 [amdgpu]
>   [<ffff80000319384c>] dml21_validate+0xcc/0x520 [amdgpu]
>   [<ffff8000031b8948>] dc_validate_global_state+0x2e8/0x460 [amdgpu]
>   [<ffff800002e94034>] create_validate_stream_for_sink+0x3d4/0x420 [amdgpu]
>   [<ffff800002e940e4>] amdgpu_dm_connector_mode_valid+0x64/0x240 [amdgpu]
>   [<900000000441d6b8>] drm_connector_mode_valid+0x38/0x80
>   [<900000000441d824>] __drm_helper_update_and_validate+0x124/0x3e0
>   [<900000000441ddc0>] drm_helper_probe_single_connector_modes+0x2e0/0x620
>   [<90000000044050dc>] drm_client_modeset_probe+0x23c/0x1780
>   [<9000000004420384>] __drm_fb_helper_initial_config_and_unlock+0x44/0x5a0
>   [<9000000004403acc>] drm_client_dev_hotplug+0xcc/0x140
>   [<ffff800002e9ab50>] handle_hpd_irq_helper+0x1b0/0x1e0 [amdgpu]
>   [<90000000038f5da0>] process_one_work+0x160/0x300
>   [<90000000038f6718>] worker_thread+0x318/0x440
>   [<9000000003901b8c>] kthread+0x12c/0x220
>   [<90000000038b1484>] ret_from_kernel_thread+0x8/0xa4
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
>   .../gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c   | 5 ++++-
>   .../gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c   | 2 ++
>   .../gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c | 2 ++
>   .../gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c | 5 ++++-
>   4 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
> index 664302876019..63ef8629d7ed 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
> @@ -1811,10 +1811,13 @@ bool dcn32_validate_bandwidth(struct dc *dc,
>   {
>   	bool out = false;
>   
> -	if (dc->debug.using_dml2)
> +	if (dc->debug.using_dml2) {
> +		DC_FP_START();
>   		out = dml2_validate(dc, context,
>   				context->power_source == DC_POWER_SOURCE_DC ? context->bw_ctx.dml2_dc_power_source : context->bw_ctx.dml2,
>   				fast_validate);
> +		DC_FP_END();
> +	}
>   	else
>   		out = dml1_validate(dc, context, fast_validate);
>   	return out;
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c
> index 8ee3d99ea2aa..0495c8dbcf1e 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c
> @@ -1738,9 +1738,11 @@ static bool dcn35_validate_bandwidth(struct dc *dc,
>   {
>   	bool out = false;
>   
> +	DC_FP_START();
>   	out = dml2_validate(dc, context,
>   			context->power_source == DC_POWER_SOURCE_DC ? context->bw_ctx.dml2_dc_power_source : context->bw_ctx.dml2,
>   			fast_validate);
> +	DC_FP_END();
>   
>   	if (fast_validate)
>   		return out;
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c
> index 14f7c3acdc96..aea5acf22b26 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c
> @@ -1718,9 +1718,11 @@ static bool dcn351_validate_bandwidth(struct dc *dc,
>   {
>   	bool out = false;
>   
> +	DC_FP_START();
>   	out = dml2_validate(dc, context,
>   			context->power_source == DC_POWER_SOURCE_DC ? context->bw_ctx.dml2_dc_power_source : context->bw_ctx.dml2,
>   			fast_validate);
> +	DC_FP_END();
>   
>   	if (fast_validate)
>   		return out;
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
> index c1ebc6b1c937..a1815fa59b02 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
> @@ -1650,10 +1650,13 @@ bool dcn401_validate_bandwidth(struct dc *dc,
>   		bool fast_validate)
>   {
>   	bool out = false;
> -	if (dc->debug.using_dml2)
> +	if (dc->debug.using_dml2) {
> +		DC_FP_START();
>   		out = dml2_validate(dc, context,
>   				context->power_source == DC_POWER_SOURCE_DC ? context->bw_ctx.dml2_dc_power_source : context->bw_ctx.dml2,
>   				fast_validate);
> +		DC_FP_END();
> +	}
>   	return out;
>   }
>   


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy()
  2025-03-20  2:16 ` [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy() Alex Hung
@ 2025-03-20  4:23   ` Huacai Chen
  2025-03-26  0:02     ` Alex Hung
  0 siblings, 1 reply; 10+ messages in thread
From: Huacai Chen @ 2025-03-20  4:23 UTC (permalink / raw)
  To: Alex Hung
  Cc: Huacai Chen, David Airlie, Simona Vetter, Alex Deucher,
	Christian König, amd-gfx, dri-devel, stable, Austin.Zheng

Hi, Alex,

On Thu, Mar 20, 2025 at 10:16 AM Alex Hung <alex.hung@amd.com> wrote:
>
>
>
> On 3/18/25 05:17, Huacai Chen wrote:
> > Commit 7da55c27e76749b9 ("drm/amd/display: Remove incorrect FP context
> > start") removes the FP context protection of dml2_create(), and it said
> > "All the DC_FP_START/END should be used before call anything from DML2".
> >
> > However, dml2_create()/dml2_copy()/dml2_create_copy() are not protected
> > from their callers, causing such errors:
> >
> >   do_fpu invoked from kernel context![#1]:
> >   CPU: 0 UID: 0 PID: 239 Comm: kworker/0:5 Not tainted 6.14.0-rc6+ #1
> >   Workqueue: events work_for_cpu_fn
> >   pc ffff80000319de80 ra ffff80000319de5c tp 900000010575c000 sp 900000010575f840
> >   a0 0000000000000000 a1 900000012f210130 a2 900000012f000000 a3 ffff80000357e268
> >   a4 ffff80000357e260 a5 900000012ea52cf0 a6 0000000400000004 a7 0000012c00001388
> >   t0 00001900000015e0 t1 ffff80000379d000 t2 0000000010624dd3 t3 0000006400000014
> >   t4 00000000000003e8 t5 0000005000000018 t6 0000000000000020 t7 0000000f00000064
> >   t8 000000000000002f u0 5f5e9200f8901912 s9 900000012d380010 s0 900000012ea51fd8
> >   s1 900000012f000000 s2 9000000109296000 s3 0000000000000001 s4 0000000000001fd8
> >   s5 0000000000000001 s6 ffff800003415000 s7 900000012d390000 s8 ffff800003211f80
> >      ra: ffff80000319de5c dml21_apply_soc_bb_overrides+0x3c/0x960 [amdgpu]
> >     ERA: ffff80000319de80 dml21_apply_soc_bb_overrides+0x60/0x960 [amdgpu]
> >    CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
> >    PRMD: 00000004 (PPLV0 +PIE -PWE)
> >    EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
> >    ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
> >   ESTAT: 000f0000 [FPD] (IS= ECode=15 EsubCode=0)
> >    PRID: 0014d010 (Loongson-64bit, Loongson-3C6000/S)
> >   Process kworker/0:5 (pid: 239, threadinfo=00000000927eadc6, task=000000008fd31682)
> >   Stack : 00040dc000003164 0000000000000001 900000012f210130 900000012eabeeb8
> >           900000012f000000 ffff80000319fe48 900000012f210000 900000012f210130
> >           900000012f000000 900000012eabeeb8 0000000000000001 ffff8000031a0064
> >           900000010575f9f0 900000012f210130 900000012eac0000 900000012ea80000
> >           900000012f000000 ffff8000031cefc4 900000010575f9f0 ffff8000035859c0
> >           ffff800003414000 900000010575fa78 900000012f000000 ffff8000031b4c50
> >           0000000000000000 9000000101c9d700 9000000109c40000 5f5e9200f8901912
> >           900000012d3c4bd0 900000012d3c5000 ffff8000034aed18 900000012d380010
> >           900000012d3c4bd0 ffff800003414000 900000012d380000 ffff800002ea49dc
> >           0000000000000001 900000012d3c6000 00000000ffffe423 0000000000010000
> >           ...
> >   Call Trace:
> >   [<ffff80000319de80>] dml21_apply_soc_bb_overrides+0x60/0x960 [amdgpu]
> >   [<ffff80000319fe44>] dml21_init+0xa4/0x280 [amdgpu]
> >   [<ffff8000031a0060>] dml21_create+0x40/0x80 [amdgpu]
> >   [<ffff8000031cefc0>] dc_state_create+0x100/0x160 [amdgpu]
> >   [<ffff8000031b4c4c>] dc_create+0x44c/0x640 [amdgpu]
> >   [<ffff800002ea49d8>] amdgpu_dm_init+0x3f8/0x2060 [amdgpu]
> >   [<ffff800002ea6658>] dm_hw_init+0x18/0x60 [amdgpu]
> >   [<ffff800002b16738>] amdgpu_device_init+0x1938/0x27e0 [amdgpu]
> >   [<ffff800002b18e80>] amdgpu_driver_load_kms+0x20/0xa0 [amdgpu]
> >   [<ffff800002b0c8f0>] amdgpu_pci_probe+0x1b0/0x580 [amdgpu]
> >   [<900000000448eae4>] local_pci_probe+0x44/0xc0
> >   [<9000000003b02b18>] work_for_cpu_fn+0x18/0x40
> >   [<9000000003b05da0>] process_one_work+0x160/0x300
> >   [<9000000003b06718>] worker_thread+0x318/0x440
> >   [<9000000003b11b8c>] kthread+0x12c/0x220
> >   [<9000000003ac1484>] ret_from_kernel_thread+0x8/0xa4
> >
> > So protect dml2_create()/dml2_copy()/dml2_create_copy() with DC_FP_START
> > and DC_FP_END.
>
> Hi Huacai,
>
> Can you try to put DC_FP_START DC_FP_END in the
> dml2_create()/dml2_copy()/dml2_create_copy()/dml2_validate() instead?
> The code will be cleaner and less error-prone to future changes.
At first I want to add them in
dml2_create()/dml2_copy()/dml2_create_copy()/dml2_validate(), but
commit 7da55c27e76749b9 ("drm/amd/display: Remove incorrect FP context
start")  said
that "All the DC_FP_START/END should be used before call anything from DML2".

Huacai

>
> Thanks.
>
>
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> >   drivers/gpu/drm/amd/display/dc/core/dc_state.c | 16 ++++++++++++++++
> >   1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> > index 1b2cce127981..6e2cac08002d 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> > @@ -210,17 +210,23 @@ struct dc_state *dc_state_create(struct dc *dc, struct dc_state_create_params *p
> >
> >   #ifdef CONFIG_DRM_AMD_DC_FP
> >       if (dc->debug.using_dml2) {
> > +             DC_FP_START();
> > +
> >               dml2_opt->use_clock_dc_limits = false;
> >               if (!dml2_create(dc, dml2_opt, &state->bw_ctx.dml2)) {
> > +                     DC_FP_END();
> >                       dc_state_release(state);
> >                       return NULL;
> >               }
> >
> >               dml2_opt->use_clock_dc_limits = true;
> >               if (!dml2_create(dc, dml2_opt, &state->bw_ctx.dml2_dc_power_source)) {
> > +                     DC_FP_END();
> >                       dc_state_release(state);
> >                       return NULL;
> >               }
> > +
> > +             DC_FP_END();
> >       }
> >   #endif
> >
> > @@ -240,6 +246,8 @@ void dc_state_copy(struct dc_state *dst_state, struct dc_state *src_state)
> >       dc_state_copy_internal(dst_state, src_state);
> >
> >   #ifdef CONFIG_DRM_AMD_DC_FP
> > +     DC_FP_START();
> > +
> >       dst_state->bw_ctx.dml2 = dst_dml2;
> >       if (src_state->bw_ctx.dml2)
> >               dml2_copy(dst_state->bw_ctx.dml2, src_state->bw_ctx.dml2);
> > @@ -247,6 +255,8 @@ void dc_state_copy(struct dc_state *dst_state, struct dc_state *src_state)
> >       dst_state->bw_ctx.dml2_dc_power_source = dst_dml2_dc_power_source;
> >       if (src_state->bw_ctx.dml2_dc_power_source)
> >               dml2_copy(dst_state->bw_ctx.dml2_dc_power_source, src_state->bw_ctx.dml2_dc_power_source);
> > +
> > +     DC_FP_END();
> >   #endif
> >
> >       /* context refcount should not be overridden */
> > @@ -268,17 +278,23 @@ struct dc_state *dc_state_create_copy(struct dc_state *src_state)
> >       new_state->bw_ctx.dml2 = NULL;
> >       new_state->bw_ctx.dml2_dc_power_source = NULL;
> >
> > +     DC_FP_START();
> > +
> >       if (src_state->bw_ctx.dml2 &&
> >                       !dml2_create_copy(&new_state->bw_ctx.dml2, src_state->bw_ctx.dml2)) {
> > +             DC_FP_END();
> >               dc_state_release(new_state);
> >               return NULL;
> >       }
> >
> >       if (src_state->bw_ctx.dml2_dc_power_source &&
> >                       !dml2_create_copy(&new_state->bw_ctx.dml2_dc_power_source, src_state->bw_ctx.dml2_dc_power_source)) {
> > +             DC_FP_END();
> >               dc_state_release(new_state);
> >               return NULL;
> >       }
> > +
> > +     DC_FP_END();
> >   #endif
> >
> >       kref_init(&new_state->refcount);
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy()
  2025-03-20  4:23   ` Huacai Chen
@ 2025-03-26  0:02     ` Alex Hung
  2025-03-27  0:10       ` Alex Hung
  0 siblings, 1 reply; 10+ messages in thread
From: Alex Hung @ 2025-03-26  0:02 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Huacai Chen, David Airlie, Simona Vetter, Alex Deucher,
	Christian König, amd-gfx, dri-devel, stable, Austin.Zheng



On 3/19/25 22:23, Huacai Chen wrote:
> Hi, Alex,
> 
> On Thu, Mar 20, 2025 at 10:16 AM Alex Hung <alex.hung@amd.com> wrote:
>>
>>
>>
>> On 3/18/25 05:17, Huacai Chen wrote:
>>> Commit 7da55c27e76749b9 ("drm/amd/display: Remove incorrect FP context
>>> start") removes the FP context protection of dml2_create(), and it said
>>> "All the DC_FP_START/END should be used before call anything from DML2".
>>>
>>> However, dml2_create()/dml2_copy()/dml2_create_copy() are not protected
>>> from their callers, causing such errors:
>>>
>>>    do_fpu invoked from kernel context![#1]:
>>>    CPU: 0 UID: 0 PID: 239 Comm: kworker/0:5 Not tainted 6.14.0-rc6+ #1
>>>    Workqueue: events work_for_cpu_fn
>>>    pc ffff80000319de80 ra ffff80000319de5c tp 900000010575c000 sp 900000010575f840
>>>    a0 0000000000000000 a1 900000012f210130 a2 900000012f000000 a3 ffff80000357e268
>>>    a4 ffff80000357e260 a5 900000012ea52cf0 a6 0000000400000004 a7 0000012c00001388
>>>    t0 00001900000015e0 t1 ffff80000379d000 t2 0000000010624dd3 t3 0000006400000014
>>>    t4 00000000000003e8 t5 0000005000000018 t6 0000000000000020 t7 0000000f00000064
>>>    t8 000000000000002f u0 5f5e9200f8901912 s9 900000012d380010 s0 900000012ea51fd8
>>>    s1 900000012f000000 s2 9000000109296000 s3 0000000000000001 s4 0000000000001fd8
>>>    s5 0000000000000001 s6 ffff800003415000 s7 900000012d390000 s8 ffff800003211f80
>>>       ra: ffff80000319de5c dml21_apply_soc_bb_overrides+0x3c/0x960 [amdgpu]
>>>      ERA: ffff80000319de80 dml21_apply_soc_bb_overrides+0x60/0x960 [amdgpu]
>>>     CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
>>>     PRMD: 00000004 (PPLV0 +PIE -PWE)
>>>     EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
>>>     ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
>>>    ESTAT: 000f0000 [FPD] (IS= ECode=15 EsubCode=0)
>>>     PRID: 0014d010 (Loongson-64bit, Loongson-3C6000/S)
>>>    Process kworker/0:5 (pid: 239, threadinfo=00000000927eadc6, task=000000008fd31682)
>>>    Stack : 00040dc000003164 0000000000000001 900000012f210130 900000012eabeeb8
>>>            900000012f000000 ffff80000319fe48 900000012f210000 900000012f210130
>>>            900000012f000000 900000012eabeeb8 0000000000000001 ffff8000031a0064
>>>            900000010575f9f0 900000012f210130 900000012eac0000 900000012ea80000
>>>            900000012f000000 ffff8000031cefc4 900000010575f9f0 ffff8000035859c0
>>>            ffff800003414000 900000010575fa78 900000012f000000 ffff8000031b4c50
>>>            0000000000000000 9000000101c9d700 9000000109c40000 5f5e9200f8901912
>>>            900000012d3c4bd0 900000012d3c5000 ffff8000034aed18 900000012d380010
>>>            900000012d3c4bd0 ffff800003414000 900000012d380000 ffff800002ea49dc
>>>            0000000000000001 900000012d3c6000 00000000ffffe423 0000000000010000
>>>            ...
>>>    Call Trace:
>>>    [<ffff80000319de80>] dml21_apply_soc_bb_overrides+0x60/0x960 [amdgpu]
>>>    [<ffff80000319fe44>] dml21_init+0xa4/0x280 [amdgpu]
>>>    [<ffff8000031a0060>] dml21_create+0x40/0x80 [amdgpu]
>>>    [<ffff8000031cefc0>] dc_state_create+0x100/0x160 [amdgpu]
>>>    [<ffff8000031b4c4c>] dc_create+0x44c/0x640 [amdgpu]
>>>    [<ffff800002ea49d8>] amdgpu_dm_init+0x3f8/0x2060 [amdgpu]
>>>    [<ffff800002ea6658>] dm_hw_init+0x18/0x60 [amdgpu]
>>>    [<ffff800002b16738>] amdgpu_device_init+0x1938/0x27e0 [amdgpu]
>>>    [<ffff800002b18e80>] amdgpu_driver_load_kms+0x20/0xa0 [amdgpu]
>>>    [<ffff800002b0c8f0>] amdgpu_pci_probe+0x1b0/0x580 [amdgpu]
>>>    [<900000000448eae4>] local_pci_probe+0x44/0xc0
>>>    [<9000000003b02b18>] work_for_cpu_fn+0x18/0x40
>>>    [<9000000003b05da0>] process_one_work+0x160/0x300
>>>    [<9000000003b06718>] worker_thread+0x318/0x440
>>>    [<9000000003b11b8c>] kthread+0x12c/0x220
>>>    [<9000000003ac1484>] ret_from_kernel_thread+0x8/0xa4
>>>
>>> So protect dml2_create()/dml2_copy()/dml2_create_copy() with DC_FP_START
>>> and DC_FP_END.
>>
>> Hi Huacai,
>>
>> Can you try to put DC_FP_START DC_FP_END in the
>> dml2_create()/dml2_copy()/dml2_create_copy()/dml2_validate() instead?
>> The code will be cleaner and less error-prone to future changes.
> At first I want to add them in
> dml2_create()/dml2_copy()/dml2_create_copy()/dml2_validate(), but
> commit 7da55c27e76749b9 ("drm/amd/display: Remove incorrect FP context
> start")  said
> that "All the DC_FP_START/END should be used before call anything from DML2".
> 
> Huacai

Thanks. I sent them to testing, and will share results later.

> 
>>
>> Thanks.
>>
>>
>>>
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
>>> ---
>>>    drivers/gpu/drm/amd/display/dc/core/dc_state.c | 16 ++++++++++++++++
>>>    1 file changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
>>> index 1b2cce127981..6e2cac08002d 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
>>> @@ -210,17 +210,23 @@ struct dc_state *dc_state_create(struct dc *dc, struct dc_state_create_params *p
>>>
>>>    #ifdef CONFIG_DRM_AMD_DC_FP
>>>        if (dc->debug.using_dml2) {
>>> +             DC_FP_START();
>>> +
>>>                dml2_opt->use_clock_dc_limits = false;
>>>                if (!dml2_create(dc, dml2_opt, &state->bw_ctx.dml2)) {
>>> +                     DC_FP_END();
>>>                        dc_state_release(state);
>>>                        return NULL;
>>>                }
>>>
>>>                dml2_opt->use_clock_dc_limits = true;
>>>                if (!dml2_create(dc, dml2_opt, &state->bw_ctx.dml2_dc_power_source)) {
>>> +                     DC_FP_END();
>>>                        dc_state_release(state);
>>>                        return NULL;
>>>                }
>>> +
>>> +             DC_FP_END();
>>>        }
>>>    #endif
>>>
>>> @@ -240,6 +246,8 @@ void dc_state_copy(struct dc_state *dst_state, struct dc_state *src_state)
>>>        dc_state_copy_internal(dst_state, src_state);
>>>
>>>    #ifdef CONFIG_DRM_AMD_DC_FP
>>> +     DC_FP_START();
>>> +
>>>        dst_state->bw_ctx.dml2 = dst_dml2;
>>>        if (src_state->bw_ctx.dml2)
>>>                dml2_copy(dst_state->bw_ctx.dml2, src_state->bw_ctx.dml2);
>>> @@ -247,6 +255,8 @@ void dc_state_copy(struct dc_state *dst_state, struct dc_state *src_state)
>>>        dst_state->bw_ctx.dml2_dc_power_source = dst_dml2_dc_power_source;
>>>        if (src_state->bw_ctx.dml2_dc_power_source)
>>>                dml2_copy(dst_state->bw_ctx.dml2_dc_power_source, src_state->bw_ctx.dml2_dc_power_source);
>>> +
>>> +     DC_FP_END();
>>>    #endif
>>>
>>>        /* context refcount should not be overridden */
>>> @@ -268,17 +278,23 @@ struct dc_state *dc_state_create_copy(struct dc_state *src_state)
>>>        new_state->bw_ctx.dml2 = NULL;
>>>        new_state->bw_ctx.dml2_dc_power_source = NULL;
>>>
>>> +     DC_FP_START();
>>> +
>>>        if (src_state->bw_ctx.dml2 &&
>>>                        !dml2_create_copy(&new_state->bw_ctx.dml2, src_state->bw_ctx.dml2)) {
>>> +             DC_FP_END();
>>>                dc_state_release(new_state);
>>>                return NULL;
>>>        }
>>>
>>>        if (src_state->bw_ctx.dml2_dc_power_source &&
>>>                        !dml2_create_copy(&new_state->bw_ctx.dml2_dc_power_source, src_state->bw_ctx.dml2_dc_power_source)) {
>>> +             DC_FP_END();
>>>                dc_state_release(new_state);
>>>                return NULL;
>>>        }
>>> +
>>> +     DC_FP_END();
>>>    #endif
>>>
>>>        kref_init(&new_state->refcount);
>>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy()
  2025-03-26  0:02     ` Alex Hung
@ 2025-03-27  0:10       ` Alex Hung
  2025-03-27  1:40         ` Huacai Chen
  0 siblings, 1 reply; 10+ messages in thread
From: Alex Hung @ 2025-03-27  0:10 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Huacai Chen, David Airlie, Simona Vetter, Alex Deucher,
	Christian König, amd-gfx, dri-devel, stable, Austin.Zheng

The following error messages showed up on an APU and a dGPU during testing.

<3> [100.231411] BUG: sleeping function called from invalid context at 
include/linux/sched/mm.h:321
<3> [100.231414] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 
1711, name: kms_color
<3> [100.231416] preempt_count: 2, expected: 0
<3> [100.231417] RCU nest depth: 0, expected: 0
<3> [100.231418] Preemption disabled at:
<3> [100.231419] [<ffffffffc0c2843b>] dc_fpu_begin+0x2b/0xc0 [amdgpu]
<4> [100.231626] CPU: 4 UID: 0 PID: 1711 Comm: kms_color Tainted: G 
   W          6.12.0+ #1
<4> [100.231629] Tainted: [W]=WARN
<4> [100.231631] Call Trace:
<4> [100.231632]  <TASK>
<4> [100.231633]  dump_stack_lvl+0x5b/0x70
<4> [100.231638]  dump_stack+0x10/0x20
<4> [100.231639]  __might_resched+0x170/0x1d0
<4> [100.231643]  __might_sleep+0x44/0x70
<4> [100.231645]  __alloc_pages_noprof+0x22f/0x370
<4> [100.231649]  ___kmalloc_large_node+0x95/0x150
<4> [100.231651]  ? preempt_count_add+0x4e/0xc0
<4> [100.231653]  __kmalloc_large_noprof+0x1d/0xb0
<4> [100.231655]  dml2_create_copy+0x27/0x60 [amdgpu]
<4> [100.231827]  dc_state_create_copy+0x7e/0x170 [amdgpu]
<4> [100.231995]  update_planes_and_stream_state+0x23c/0x600 [amdgpu]
<4> [100.232189]  update_planes_and_stream_v2+0x22b/0x530 [amdgpu]
<4> [100.232366]  ? amdgpu_dm_atomic_commit_tail+0x1310/0x4100 [amdgpu]
<4> [100.232569]  ? commit_tail+0x96/0x140 [drm_kms_helper]
<4> [100.232577]  dc_update_planes_and_stream+0x5b/0xe0 [amdgpu]
<4> [100.232730]  amdgpu_dm_atomic_commit_tail+0x1fa7/0x4100 [amdgpu]
<4> [100.232908]  ? stack_depot_save_flags+0x2c/0x730
<4> [100.232915]  ? wait_for_completion_timeout+0x1d/0x30
<4> [100.232917]  commit_tail+0x96/0x140 [drm_kms_helper]
<4> [100.232923]  drm_atomic_helper_commit+0x12b/0x150 [drm_kms_helper]
<4> [100.232927]  drm_atomic_commit+0xad/0xe0 [drm]
<4> [100.232939]  ? __pfx___drm_printfn_info+0x10/0x10 [drm]
<4> [100.232956]  drm_atomic_helper_set_config+0x80/0xc0 [drm_kms_helper]
<4> [100.232961]  drm_mode_setcrtc+0x22e/0x910 [drm]
<4> [100.232975]  ? kfree+0x18f/0x350
<4> [100.232977]  ? __pfx_drm_mode_setcrtc+0x10/0x10 [drm]
<4> [100.232987]  drm_ioctl_kernel+0xa7/0x100 [drm]
<4> [100.233004]  drm_ioctl+0x29d/0x500 [drm]
<4> [100.233015]  ? __pfx_drm_mode_setcrtc+0x10/0x10 [drm]
<4> [100.233026]  ? _raw_spin_unlock_irqrestore+0x1f/0x40
<4> [100.233029]  amdgpu_drm_ioctl+0x4b/0x80 [amdgpu]
<4> [100.233131]  __x64_sys_ioctl+0x92/0xd0
<4> [100.233133]  x64_sys_call+0x1205/0x20d0
<4> [100.233136]  do_syscall_64+0x50/0x110
<4> [100.233138]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
<4> [100.233142] RIP: 0033:0x7fb21e71a94f
<4> [100.233144] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 
10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 
0f 05 <41> 89 c0 3d 00 f0 ff ff 77 1f 48 8b 44 24 18 64 48 2b 04 25 28 00
<4> [100.233145] RSP: 002b:00007ffdd9a52e50 EFLAGS: 00000246 ORIG_RAX: 
0000000000000010
<4> [100.233148] RAX: ffffffffffffffda RBX: 00007ffdd9a52ee0 RCX: 
00007fb21e71a94f
<4> [100.233149] RDX: 00007ffdd9a52ee0 RSI: 00000000c06864a2 RDI: 
0000000000000005
<4> [100.233149] RBP: 00000000c06864a2 R08: 0000000000000000 R09: 
00005609537f7b08
<4> [100.233150] R10: 0000000000000000 R11: 0000000000000246 R12: 
0000000000000000
<4> [100.233151] R13: 0000000000000005 R14: 0000000000000000 R15: 
00005609537e2848
<4> [100.233152]  </TASK>

On 3/25/25 18:02, Alex Hung wrote:
> 
> 
> On 3/19/25 22:23, Huacai Chen wrote:
>> Hi, Alex,
>>
>> On Thu, Mar 20, 2025 at 10:16 AM Alex Hung <alex.hung@amd.com> wrote:
>>>
>>>
>>>
>>> On 3/18/25 05:17, Huacai Chen wrote:
>>>> Commit 7da55c27e76749b9 ("drm/amd/display: Remove incorrect FP context
>>>> start") removes the FP context protection of dml2_create(), and it said
>>>> "All the DC_FP_START/END should be used before call anything from 
>>>> DML2".
>>>>
>>>> However, dml2_create()/dml2_copy()/dml2_create_copy() are not protected
>>>> from their callers, causing such errors:
>>>>
>>>>    do_fpu invoked from kernel context![#1]:
>>>>    CPU: 0 UID: 0 PID: 239 Comm: kworker/0:5 Not tainted 6.14.0-rc6+ #1
>>>>    Workqueue: events work_for_cpu_fn
>>>>    pc ffff80000319de80 ra ffff80000319de5c tp 900000010575c000 sp 
>>>> 900000010575f840
>>>>    a0 0000000000000000 a1 900000012f210130 a2 900000012f000000 a3 
>>>> ffff80000357e268
>>>>    a4 ffff80000357e260 a5 900000012ea52cf0 a6 0000000400000004 a7 
>>>> 0000012c00001388
>>>>    t0 00001900000015e0 t1 ffff80000379d000 t2 0000000010624dd3 t3 
>>>> 0000006400000014
>>>>    t4 00000000000003e8 t5 0000005000000018 t6 0000000000000020 t7 
>>>> 0000000f00000064
>>>>    t8 000000000000002f u0 5f5e9200f8901912 s9 900000012d380010 s0 
>>>> 900000012ea51fd8
>>>>    s1 900000012f000000 s2 9000000109296000 s3 0000000000000001 s4 
>>>> 0000000000001fd8
>>>>    s5 0000000000000001 s6 ffff800003415000 s7 900000012d390000 s8 
>>>> ffff800003211f80
>>>>       ra: ffff80000319de5c dml21_apply_soc_bb_overrides+0x3c/0x960 
>>>> [amdgpu]
>>>>      ERA: ffff80000319de80 dml21_apply_soc_bb_overrides+0x60/0x960 
>>>> [amdgpu]
>>>>     CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
>>>>     PRMD: 00000004 (PPLV0 +PIE -PWE)
>>>>     EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
>>>>     ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
>>>>    ESTAT: 000f0000 [FPD] (IS= ECode=15 EsubCode=0)
>>>>     PRID: 0014d010 (Loongson-64bit, Loongson-3C6000/S)
>>>>    Process kworker/0:5 (pid: 239, threadinfo=00000000927eadc6, 
>>>> task=000000008fd31682)
>>>>    Stack : 00040dc000003164 0000000000000001 900000012f210130 
>>>> 900000012eabeeb8
>>>>            900000012f000000 ffff80000319fe48 900000012f210000 
>>>> 900000012f210130
>>>>            900000012f000000 900000012eabeeb8 0000000000000001 
>>>> ffff8000031a0064
>>>>            900000010575f9f0 900000012f210130 900000012eac0000 
>>>> 900000012ea80000
>>>>            900000012f000000 ffff8000031cefc4 900000010575f9f0 
>>>> ffff8000035859c0
>>>>            ffff800003414000 900000010575fa78 900000012f000000 
>>>> ffff8000031b4c50
>>>>            0000000000000000 9000000101c9d700 9000000109c40000 
>>>> 5f5e9200f8901912
>>>>            900000012d3c4bd0 900000012d3c5000 ffff8000034aed18 
>>>> 900000012d380010
>>>>            900000012d3c4bd0 ffff800003414000 900000012d380000 
>>>> ffff800002ea49dc
>>>>            0000000000000001 900000012d3c6000 00000000ffffe423 
>>>> 0000000000010000
>>>>            ...
>>>>    Call Trace:
>>>>    [<ffff80000319de80>] dml21_apply_soc_bb_overrides+0x60/0x960 
>>>> [amdgpu]
>>>>    [<ffff80000319fe44>] dml21_init+0xa4/0x280 [amdgpu]
>>>>    [<ffff8000031a0060>] dml21_create+0x40/0x80 [amdgpu]
>>>>    [<ffff8000031cefc0>] dc_state_create+0x100/0x160 [amdgpu]
>>>>    [<ffff8000031b4c4c>] dc_create+0x44c/0x640 [amdgpu]
>>>>    [<ffff800002ea49d8>] amdgpu_dm_init+0x3f8/0x2060 [amdgpu]
>>>>    [<ffff800002ea6658>] dm_hw_init+0x18/0x60 [amdgpu]
>>>>    [<ffff800002b16738>] amdgpu_device_init+0x1938/0x27e0 [amdgpu]
>>>>    [<ffff800002b18e80>] amdgpu_driver_load_kms+0x20/0xa0 [amdgpu]
>>>>    [<ffff800002b0c8f0>] amdgpu_pci_probe+0x1b0/0x580 [amdgpu]
>>>>    [<900000000448eae4>] local_pci_probe+0x44/0xc0
>>>>    [<9000000003b02b18>] work_for_cpu_fn+0x18/0x40
>>>>    [<9000000003b05da0>] process_one_work+0x160/0x300
>>>>    [<9000000003b06718>] worker_thread+0x318/0x440
>>>>    [<9000000003b11b8c>] kthread+0x12c/0x220
>>>>    [<9000000003ac1484>] ret_from_kernel_thread+0x8/0xa4
>>>>
>>>> So protect dml2_create()/dml2_copy()/dml2_create_copy() with 
>>>> DC_FP_START
>>>> and DC_FP_END.
>>>
>>> Hi Huacai,
>>>
>>> Can you try to put DC_FP_START DC_FP_END in the
>>> dml2_create()/dml2_copy()/dml2_create_copy()/dml2_validate() instead?
>>> The code will be cleaner and less error-prone to future changes.
>> At first I want to add them in
>> dml2_create()/dml2_copy()/dml2_create_copy()/dml2_validate(), but
>> commit 7da55c27e76749b9 ("drm/amd/display: Remove incorrect FP context
>> start")  said
>> that "All the DC_FP_START/END should be used before call anything from 
>> DML2".
>>
>> Huacai
> 
> Thanks. I sent them to testing, and will share results later.
> 
>>
>>>
>>> Thanks.
>>>
>>>
>>>>
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
>>>> ---
>>>>    drivers/gpu/drm/amd/display/dc/core/dc_state.c | 16 ++++++++++++++++
>>>>    1 file changed, 16 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c b/ 
>>>> drivers/gpu/drm/amd/display/dc/core/dc_state.c
>>>> index 1b2cce127981..6e2cac08002d 100644
>>>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
>>>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
>>>> @@ -210,17 +210,23 @@ struct dc_state *dc_state_create(struct dc 
>>>> *dc, struct dc_state_create_params *p
>>>>
>>>>    #ifdef CONFIG_DRM_AMD_DC_FP
>>>>        if (dc->debug.using_dml2) {
>>>> +             DC_FP_START();
>>>> +
>>>>                dml2_opt->use_clock_dc_limits = false;
>>>>                if (!dml2_create(dc, dml2_opt, &state->bw_ctx.dml2)) {
>>>> +                     DC_FP_END();
>>>>                        dc_state_release(state);
>>>>                        return NULL;
>>>>                }
>>>>
>>>>                dml2_opt->use_clock_dc_limits = true;
>>>>                if (!dml2_create(dc, dml2_opt, &state- 
>>>> >bw_ctx.dml2_dc_power_source)) {
>>>> +                     DC_FP_END();
>>>>                        dc_state_release(state);
>>>>                        return NULL;
>>>>                }
>>>> +
>>>> +             DC_FP_END();
>>>>        }
>>>>    #endif
>>>>
>>>> @@ -240,6 +246,8 @@ void dc_state_copy(struct dc_state *dst_state, 
>>>> struct dc_state *src_state)
>>>>        dc_state_copy_internal(dst_state, src_state);
>>>>
>>>>    #ifdef CONFIG_DRM_AMD_DC_FP
>>>> +     DC_FP_START();
>>>> +
>>>>        dst_state->bw_ctx.dml2 = dst_dml2;
>>>>        if (src_state->bw_ctx.dml2)
>>>>                dml2_copy(dst_state->bw_ctx.dml2, src_state- 
>>>> >bw_ctx.dml2);
>>>> @@ -247,6 +255,8 @@ void dc_state_copy(struct dc_state *dst_state, 
>>>> struct dc_state *src_state)
>>>>        dst_state->bw_ctx.dml2_dc_power_source = 
>>>> dst_dml2_dc_power_source;
>>>>        if (src_state->bw_ctx.dml2_dc_power_source)
>>>>                dml2_copy(dst_state->bw_ctx.dml2_dc_power_source, 
>>>> src_state->bw_ctx.dml2_dc_power_source);
>>>> +
>>>> +     DC_FP_END();
>>>>    #endif
>>>>
>>>>        /* context refcount should not be overridden */
>>>> @@ -268,17 +278,23 @@ struct dc_state *dc_state_create_copy(struct 
>>>> dc_state *src_state)
>>>>        new_state->bw_ctx.dml2 = NULL;
>>>>        new_state->bw_ctx.dml2_dc_power_source = NULL;
>>>>
>>>> +     DC_FP_START();
>>>> +
>>>>        if (src_state->bw_ctx.dml2 &&
>>>>                        !dml2_create_copy(&new_state->bw_ctx.dml2, 
>>>> src_state->bw_ctx.dml2)) {
>>>> +             DC_FP_END();
>>>>                dc_state_release(new_state);
>>>>                return NULL;
>>>>        }
>>>>
>>>>        if (src_state->bw_ctx.dml2_dc_power_source &&
>>>>                        !dml2_create_copy(&new_state- 
>>>> >bw_ctx.dml2_dc_power_source, src_state- 
>>>> >bw_ctx.dml2_dc_power_source)) {
>>>> +             DC_FP_END();
>>>>                dc_state_release(new_state);
>>>>                return NULL;
>>>>        }
>>>> +
>>>> +     DC_FP_END();
>>>>    #endif
>>>>
>>>>        kref_init(&new_state->refcount);
>>>
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy()
  2025-03-27  0:10       ` Alex Hung
@ 2025-03-27  1:40         ` Huacai Chen
  2025-03-28 18:27           ` Aurabindo Pillai
  0 siblings, 1 reply; 10+ messages in thread
From: Huacai Chen @ 2025-03-27  1:40 UTC (permalink / raw)
  To: Alex Hung, aurabindo.pillai
  Cc: Huacai Chen, David Airlie, Simona Vetter, Alex Deucher,
	Christian König, amd-gfx, dri-devel, stable, Austin.Zheng

Hi, Alex,

On Thu, Mar 27, 2025 at 8:10 AM Alex Hung <alex.hung@amd.com> wrote:
>
> The following error messages showed up on an APU and a dGPU during testing.
>
> <3> [100.231411] BUG: sleeping function called from invalid context at
> include/linux/sched/mm.h:321
> <3> [100.231414] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid:
> 1711, name: kms_color
> <3> [100.231416] preempt_count: 2, expected: 0
> <3> [100.231417] RCU nest depth: 0, expected: 0
> <3> [100.231418] Preemption disabled at:
> <3> [100.231419] [<ffffffffc0c2843b>] dc_fpu_begin+0x2b/0xc0 [amdgpu]
> <4> [100.231626] CPU: 4 UID: 0 PID: 1711 Comm: kms_color Tainted: G
>    W          6.12.0+ #1
> <4> [100.231629] Tainted: [W]=WARN
> <4> [100.231631] Call Trace:
> <4> [100.231632]  <TASK>
> <4> [100.231633]  dump_stack_lvl+0x5b/0x70
> <4> [100.231638]  dump_stack+0x10/0x20
> <4> [100.231639]  __might_resched+0x170/0x1d0
> <4> [100.231643]  __might_sleep+0x44/0x70
> <4> [100.231645]  __alloc_pages_noprof+0x22f/0x370
> <4> [100.231649]  ___kmalloc_large_node+0x95/0x150
> <4> [100.231651]  ? preempt_count_add+0x4e/0xc0
> <4> [100.231653]  __kmalloc_large_noprof+0x1d/0xb0
> <4> [100.231655]  dml2_create_copy+0x27/0x60 [amdgpu]
> <4> [100.231827]  dc_state_create_copy+0x7e/0x170 [amdgpu]
> <4> [100.231995]  update_planes_and_stream_state+0x23c/0x600 [amdgpu]
> <4> [100.232189]  update_planes_and_stream_v2+0x22b/0x530 [amdgpu]
> <4> [100.232366]  ? amdgpu_dm_atomic_commit_tail+0x1310/0x4100 [amdgpu]
> <4> [100.232569]  ? commit_tail+0x96/0x140 [drm_kms_helper]
> <4> [100.232577]  dc_update_planes_and_stream+0x5b/0xe0 [amdgpu]
> <4> [100.232730]  amdgpu_dm_atomic_commit_tail+0x1fa7/0x4100 [amdgpu]
> <4> [100.232908]  ? stack_depot_save_flags+0x2c/0x730
> <4> [100.232915]  ? wait_for_completion_timeout+0x1d/0x30
> <4> [100.232917]  commit_tail+0x96/0x140 [drm_kms_helper]
> <4> [100.232923]  drm_atomic_helper_commit+0x12b/0x150 [drm_kms_helper]
> <4> [100.232927]  drm_atomic_commit+0xad/0xe0 [drm]
> <4> [100.232939]  ? __pfx___drm_printfn_info+0x10/0x10 [drm]
> <4> [100.232956]  drm_atomic_helper_set_config+0x80/0xc0 [drm_kms_helper]
> <4> [100.232961]  drm_mode_setcrtc+0x22e/0x910 [drm]
> <4> [100.232975]  ? kfree+0x18f/0x350
> <4> [100.232977]  ? __pfx_drm_mode_setcrtc+0x10/0x10 [drm]
> <4> [100.232987]  drm_ioctl_kernel+0xa7/0x100 [drm]
> <4> [100.233004]  drm_ioctl+0x29d/0x500 [drm]
> <4> [100.233015]  ? __pfx_drm_mode_setcrtc+0x10/0x10 [drm]
> <4> [100.233026]  ? _raw_spin_unlock_irqrestore+0x1f/0x40
> <4> [100.233029]  amdgpu_drm_ioctl+0x4b/0x80 [amdgpu]
> <4> [100.233131]  __x64_sys_ioctl+0x92/0xd0
> <4> [100.233133]  x64_sys_call+0x1205/0x20d0
> <4> [100.233136]  do_syscall_64+0x50/0x110
> <4> [100.233138]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> <4> [100.233142] RIP: 0033:0x7fb21e71a94f
> <4> [100.233144] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24
> 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00
> 0f 05 <41> 89 c0 3d 00 f0 ff ff 77 1f 48 8b 44 24 18 64 48 2b 04 25 28 00
> <4> [100.233145] RSP: 002b:00007ffdd9a52e50 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000010
> <4> [100.233148] RAX: ffffffffffffffda RBX: 00007ffdd9a52ee0 RCX:
> 00007fb21e71a94f
> <4> [100.233149] RDX: 00007ffdd9a52ee0 RSI: 00000000c06864a2 RDI:
> 0000000000000005
> <4> [100.233149] RBP: 00000000c06864a2 R08: 0000000000000000 R09:
> 00005609537f7b08
> <4> [100.233150] R10: 0000000000000000 R11: 0000000000000246 R12:
> 0000000000000000
> <4> [100.233151] R13: 0000000000000005 R14: 0000000000000000 R15:
> 00005609537e2848
> <4> [100.233152]  </TASK>
This seems caused by dml2_allocate_memory(), to fix this we can only
protect FPU in DML2, I can do it in the new version, but I want to
listen Aurabindo's opinion.


Huacai

>
> On 3/25/25 18:02, Alex Hung wrote:
> >
> >
> > On 3/19/25 22:23, Huacai Chen wrote:
> >> Hi, Alex,
> >>
> >> On Thu, Mar 20, 2025 at 10:16 AM Alex Hung <alex.hung@amd.com> wrote:
> >>>
> >>>
> >>>
> >>> On 3/18/25 05:17, Huacai Chen wrote:
> >>>> Commit 7da55c27e76749b9 ("drm/amd/display: Remove incorrect FP context
> >>>> start") removes the FP context protection of dml2_create(), and it said
> >>>> "All the DC_FP_START/END should be used before call anything from
> >>>> DML2".
> >>>>
> >>>> However, dml2_create()/dml2_copy()/dml2_create_copy() are not protected
> >>>> from their callers, causing such errors:
> >>>>
> >>>>    do_fpu invoked from kernel context![#1]:
> >>>>    CPU: 0 UID: 0 PID: 239 Comm: kworker/0:5 Not tainted 6.14.0-rc6+ #1
> >>>>    Workqueue: events work_for_cpu_fn
> >>>>    pc ffff80000319de80 ra ffff80000319de5c tp 900000010575c000 sp
> >>>> 900000010575f840
> >>>>    a0 0000000000000000 a1 900000012f210130 a2 900000012f000000 a3
> >>>> ffff80000357e268
> >>>>    a4 ffff80000357e260 a5 900000012ea52cf0 a6 0000000400000004 a7
> >>>> 0000012c00001388
> >>>>    t0 00001900000015e0 t1 ffff80000379d000 t2 0000000010624dd3 t3
> >>>> 0000006400000014
> >>>>    t4 00000000000003e8 t5 0000005000000018 t6 0000000000000020 t7
> >>>> 0000000f00000064
> >>>>    t8 000000000000002f u0 5f5e9200f8901912 s9 900000012d380010 s0
> >>>> 900000012ea51fd8
> >>>>    s1 900000012f000000 s2 9000000109296000 s3 0000000000000001 s4
> >>>> 0000000000001fd8
> >>>>    s5 0000000000000001 s6 ffff800003415000 s7 900000012d390000 s8
> >>>> ffff800003211f80
> >>>>       ra: ffff80000319de5c dml21_apply_soc_bb_overrides+0x3c/0x960
> >>>> [amdgpu]
> >>>>      ERA: ffff80000319de80 dml21_apply_soc_bb_overrides+0x60/0x960
> >>>> [amdgpu]
> >>>>     CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
> >>>>     PRMD: 00000004 (PPLV0 +PIE -PWE)
> >>>>     EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
> >>>>     ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
> >>>>    ESTAT: 000f0000 [FPD] (IS= ECode=15 EsubCode=0)
> >>>>     PRID: 0014d010 (Loongson-64bit, Loongson-3C6000/S)
> >>>>    Process kworker/0:5 (pid: 239, threadinfo=00000000927eadc6,
> >>>> task=000000008fd31682)
> >>>>    Stack : 00040dc000003164 0000000000000001 900000012f210130
> >>>> 900000012eabeeb8
> >>>>            900000012f000000 ffff80000319fe48 900000012f210000
> >>>> 900000012f210130
> >>>>            900000012f000000 900000012eabeeb8 0000000000000001
> >>>> ffff8000031a0064
> >>>>            900000010575f9f0 900000012f210130 900000012eac0000
> >>>> 900000012ea80000
> >>>>            900000012f000000 ffff8000031cefc4 900000010575f9f0
> >>>> ffff8000035859c0
> >>>>            ffff800003414000 900000010575fa78 900000012f000000
> >>>> ffff8000031b4c50
> >>>>            0000000000000000 9000000101c9d700 9000000109c40000
> >>>> 5f5e9200f8901912
> >>>>            900000012d3c4bd0 900000012d3c5000 ffff8000034aed18
> >>>> 900000012d380010
> >>>>            900000012d3c4bd0 ffff800003414000 900000012d380000
> >>>> ffff800002ea49dc
> >>>>            0000000000000001 900000012d3c6000 00000000ffffe423
> >>>> 0000000000010000
> >>>>            ...
> >>>>    Call Trace:
> >>>>    [<ffff80000319de80>] dml21_apply_soc_bb_overrides+0x60/0x960
> >>>> [amdgpu]
> >>>>    [<ffff80000319fe44>] dml21_init+0xa4/0x280 [amdgpu]
> >>>>    [<ffff8000031a0060>] dml21_create+0x40/0x80 [amdgpu]
> >>>>    [<ffff8000031cefc0>] dc_state_create+0x100/0x160 [amdgpu]
> >>>>    [<ffff8000031b4c4c>] dc_create+0x44c/0x640 [amdgpu]
> >>>>    [<ffff800002ea49d8>] amdgpu_dm_init+0x3f8/0x2060 [amdgpu]
> >>>>    [<ffff800002ea6658>] dm_hw_init+0x18/0x60 [amdgpu]
> >>>>    [<ffff800002b16738>] amdgpu_device_init+0x1938/0x27e0 [amdgpu]
> >>>>    [<ffff800002b18e80>] amdgpu_driver_load_kms+0x20/0xa0 [amdgpu]
> >>>>    [<ffff800002b0c8f0>] amdgpu_pci_probe+0x1b0/0x580 [amdgpu]
> >>>>    [<900000000448eae4>] local_pci_probe+0x44/0xc0
> >>>>    [<9000000003b02b18>] work_for_cpu_fn+0x18/0x40
> >>>>    [<9000000003b05da0>] process_one_work+0x160/0x300
> >>>>    [<9000000003b06718>] worker_thread+0x318/0x440
> >>>>    [<9000000003b11b8c>] kthread+0x12c/0x220
> >>>>    [<9000000003ac1484>] ret_from_kernel_thread+0x8/0xa4
> >>>>
> >>>> So protect dml2_create()/dml2_copy()/dml2_create_copy() with
> >>>> DC_FP_START
> >>>> and DC_FP_END.
> >>>
> >>> Hi Huacai,
> >>>
> >>> Can you try to put DC_FP_START DC_FP_END in the
> >>> dml2_create()/dml2_copy()/dml2_create_copy()/dml2_validate() instead?
> >>> The code will be cleaner and less error-prone to future changes.
> >> At first I want to add them in
> >> dml2_create()/dml2_copy()/dml2_create_copy()/dml2_validate(), but
> >> commit 7da55c27e76749b9 ("drm/amd/display: Remove incorrect FP context
> >> start")  said
> >> that "All the DC_FP_START/END should be used before call anything from
> >> DML2".
> >>
> >> Huacai
> >
> > Thanks. I sent them to testing, and will share results later.
> >
> >>
> >>>
> >>> Thanks.
> >>>
> >>>
> >>>>
> >>>> Cc: stable@vger.kernel.org
> >>>> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> >>>> ---
> >>>>    drivers/gpu/drm/amd/display/dc/core/dc_state.c | 16 ++++++++++++++++
> >>>>    1 file changed, 16 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c b/
> >>>> drivers/gpu/drm/amd/display/dc/core/dc_state.c
> >>>> index 1b2cce127981..6e2cac08002d 100644
> >>>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> >>>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> >>>> @@ -210,17 +210,23 @@ struct dc_state *dc_state_create(struct dc
> >>>> *dc, struct dc_state_create_params *p
> >>>>
> >>>>    #ifdef CONFIG_DRM_AMD_DC_FP
> >>>>        if (dc->debug.using_dml2) {
> >>>> +             DC_FP_START();
> >>>> +
> >>>>                dml2_opt->use_clock_dc_limits = false;
> >>>>                if (!dml2_create(dc, dml2_opt, &state->bw_ctx.dml2)) {
> >>>> +                     DC_FP_END();
> >>>>                        dc_state_release(state);
> >>>>                        return NULL;
> >>>>                }
> >>>>
> >>>>                dml2_opt->use_clock_dc_limits = true;
> >>>>                if (!dml2_create(dc, dml2_opt, &state-
> >>>> >bw_ctx.dml2_dc_power_source)) {
> >>>> +                     DC_FP_END();
> >>>>                        dc_state_release(state);
> >>>>                        return NULL;
> >>>>                }
> >>>> +
> >>>> +             DC_FP_END();
> >>>>        }
> >>>>    #endif
> >>>>
> >>>> @@ -240,6 +246,8 @@ void dc_state_copy(struct dc_state *dst_state,
> >>>> struct dc_state *src_state)
> >>>>        dc_state_copy_internal(dst_state, src_state);
> >>>>
> >>>>    #ifdef CONFIG_DRM_AMD_DC_FP
> >>>> +     DC_FP_START();
> >>>> +
> >>>>        dst_state->bw_ctx.dml2 = dst_dml2;
> >>>>        if (src_state->bw_ctx.dml2)
> >>>>                dml2_copy(dst_state->bw_ctx.dml2, src_state-
> >>>> >bw_ctx.dml2);
> >>>> @@ -247,6 +255,8 @@ void dc_state_copy(struct dc_state *dst_state,
> >>>> struct dc_state *src_state)
> >>>>        dst_state->bw_ctx.dml2_dc_power_source =
> >>>> dst_dml2_dc_power_source;
> >>>>        if (src_state->bw_ctx.dml2_dc_power_source)
> >>>>                dml2_copy(dst_state->bw_ctx.dml2_dc_power_source,
> >>>> src_state->bw_ctx.dml2_dc_power_source);
> >>>> +
> >>>> +     DC_FP_END();
> >>>>    #endif
> >>>>
> >>>>        /* context refcount should not be overridden */
> >>>> @@ -268,17 +278,23 @@ struct dc_state *dc_state_create_copy(struct
> >>>> dc_state *src_state)
> >>>>        new_state->bw_ctx.dml2 = NULL;
> >>>>        new_state->bw_ctx.dml2_dc_power_source = NULL;
> >>>>
> >>>> +     DC_FP_START();
> >>>> +
> >>>>        if (src_state->bw_ctx.dml2 &&
> >>>>                        !dml2_create_copy(&new_state->bw_ctx.dml2,
> >>>> src_state->bw_ctx.dml2)) {
> >>>> +             DC_FP_END();
> >>>>                dc_state_release(new_state);
> >>>>                return NULL;
> >>>>        }
> >>>>
> >>>>        if (src_state->bw_ctx.dml2_dc_power_source &&
> >>>>                        !dml2_create_copy(&new_state-
> >>>> >bw_ctx.dml2_dc_power_source, src_state-
> >>>> >bw_ctx.dml2_dc_power_source)) {
> >>>> +             DC_FP_END();
> >>>>                dc_state_release(new_state);
> >>>>                return NULL;
> >>>>        }
> >>>> +
> >>>> +     DC_FP_END();
> >>>>    #endif
> >>>>
> >>>>        kref_init(&new_state->refcount);
> >>>
> >
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy()
  2025-03-27  1:40         ` Huacai Chen
@ 2025-03-28 18:27           ` Aurabindo Pillai
  2025-03-29  8:47             ` Huacai Chen
  0 siblings, 1 reply; 10+ messages in thread
From: Aurabindo Pillai @ 2025-03-28 18:27 UTC (permalink / raw)
  To: Huacai Chen, Alex Hung
  Cc: Huacai Chen, David Airlie, Simona Vetter, Alex Deucher,
	Christian König, amd-gfx, dri-devel, stable, Austin.Zheng



On 2025-03-26 21:40, Huacai Chen wrote:
> Hi, Alex,
> 
> On Thu, Mar 27, 2025 at 8:10 AM Alex Hung <alex.hung@amd.com> wrote:
>>
>> The following error messages showed up on an APU and a dGPU during testing.
>>
>> <3> [100.231411] BUG: sleeping function called from invalid context at
>> include/linux/sched/mm.h:321
>> <3> [100.231414] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid:
>> 1711, name: kms_color
>> <3> [100.231416] preempt_count: 2, expected: 0
>> <3> [100.231417] RCU nest depth: 0, expected: 0
>> <3> [100.231418] Preemption disabled at:
>> <3> [100.231419] [<ffffffffc0c2843b>] dc_fpu_begin+0x2b/0xc0 [amdgpu]
>> <4> [100.231626] CPU: 4 UID: 0 PID: 1711 Comm: kms_color Tainted: G
>>     W          6.12.0+ #1
>> <4> [100.231629] Tainted: [W]=WARN
>> <4> [100.231631] Call Trace:
>> <4> [100.231632]  <TASK>
>> <4> [100.231633]  dump_stack_lvl+0x5b/0x70
>> <4> [100.231638]  dump_stack+0x10/0x20
>> <4> [100.231639]  __might_resched+0x170/0x1d0
>> <4> [100.231643]  __might_sleep+0x44/0x70
>> <4> [100.231645]  __alloc_pages_noprof+0x22f/0x370
>> <4> [100.231649]  ___kmalloc_large_node+0x95/0x150
>> <4> [100.231651]  ? preempt_count_add+0x4e/0xc0
>> <4> [100.231653]  __kmalloc_large_noprof+0x1d/0xb0
>> <4> [100.231655]  dml2_create_copy+0x27/0x60 [amdgpu]
>> <4> [100.231827]  dc_state_create_copy+0x7e/0x170 [amdgpu]
>> <4> [100.231995]  update_planes_and_stream_state+0x23c/0x600 [amdgpu]
>> <4> [100.232189]  update_planes_and_stream_v2+0x22b/0x530 [amdgpu]
>> <4> [100.232366]  ? amdgpu_dm_atomic_commit_tail+0x1310/0x4100 [amdgpu]
>> <4> [100.232569]  ? commit_tail+0x96/0x140 [drm_kms_helper]
>> <4> [100.232577]  dc_update_planes_and_stream+0x5b/0xe0 [amdgpu]
>> <4> [100.232730]  amdgpu_dm_atomic_commit_tail+0x1fa7/0x4100 [amdgpu]
>> <4> [100.232908]  ? stack_depot_save_flags+0x2c/0x730
>> <4> [100.232915]  ? wait_for_completion_timeout+0x1d/0x30
>> <4> [100.232917]  commit_tail+0x96/0x140 [drm_kms_helper]
>> <4> [100.232923]  drm_atomic_helper_commit+0x12b/0x150 [drm_kms_helper]
>> <4> [100.232927]  drm_atomic_commit+0xad/0xe0 [drm]
>> <4> [100.232939]  ? __pfx___drm_printfn_info+0x10/0x10 [drm]
>> <4> [100.232956]  drm_atomic_helper_set_config+0x80/0xc0 [drm_kms_helper]
>> <4> [100.232961]  drm_mode_setcrtc+0x22e/0x910 [drm]
>> <4> [100.232975]  ? kfree+0x18f/0x350
>> <4> [100.232977]  ? __pfx_drm_mode_setcrtc+0x10/0x10 [drm]
>> <4> [100.232987]  drm_ioctl_kernel+0xa7/0x100 [drm]
>> <4> [100.233004]  drm_ioctl+0x29d/0x500 [drm]
>> <4> [100.233015]  ? __pfx_drm_mode_setcrtc+0x10/0x10 [drm]
>> <4> [100.233026]  ? _raw_spin_unlock_irqrestore+0x1f/0x40
>> <4> [100.233029]  amdgpu_drm_ioctl+0x4b/0x80 [amdgpu]
>> <4> [100.233131]  __x64_sys_ioctl+0x92/0xd0
>> <4> [100.233133]  x64_sys_call+0x1205/0x20d0
>> <4> [100.233136]  do_syscall_64+0x50/0x110
>> <4> [100.233138]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>> <4> [100.233142] RIP: 0033:0x7fb21e71a94f
>> <4> [100.233144] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24
>> 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00
>> 0f 05 <41> 89 c0 3d 00 f0 ff ff 77 1f 48 8b 44 24 18 64 48 2b 04 25 28 00
>> <4> [100.233145] RSP: 002b:00007ffdd9a52e50 EFLAGS: 00000246 ORIG_RAX:
>> 0000000000000010
>> <4> [100.233148] RAX: ffffffffffffffda RBX: 00007ffdd9a52ee0 RCX:
>> 00007fb21e71a94f
>> <4> [100.233149] RDX: 00007ffdd9a52ee0 RSI: 00000000c06864a2 RDI:
>> 0000000000000005
>> <4> [100.233149] RBP: 00000000c06864a2 R08: 0000000000000000 R09:
>> 00005609537f7b08
>> <4> [100.233150] R10: 0000000000000000 R11: 0000000000000246 R12:
>> 0000000000000000
>> <4> [100.233151] R13: 0000000000000005 R14: 0000000000000000 R15:
>> 00005609537e2848
>> <4> [100.233152]  </TASK>
> This seems caused by dml2_allocate_memory(), to fix this we can only
> protect FPU in DML2, I can do it in the new version, but I want to
> listen Aurabindo's opinion.
> 
>

It looks like dml21_apply_soc_bb_overrides() does have some division on 
double variables. I'm curious why we dont see this on our side. Was this 
seen on x86 or Loongson?

I think your approach is correct. Thanks for taking time to fix this. We 
can add it to weekly testing if you send us a patch.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy()
  2025-03-28 18:27           ` Aurabindo Pillai
@ 2025-03-29  8:47             ` Huacai Chen
  0 siblings, 0 replies; 10+ messages in thread
From: Huacai Chen @ 2025-03-29  8:47 UTC (permalink / raw)
  To: Aurabindo Pillai
  Cc: Alex Hung, Huacai Chen, David Airlie, Simona Vetter, Alex Deucher,
	Christian König, amd-gfx, dri-devel, stable, Austin.Zheng

Hi, Aurabindo,

On Sat, Mar 29, 2025 at 2:27 AM Aurabindo Pillai
<aurabindo.pillai@amd.com> wrote:
>
>
>
> On 2025-03-26 21:40, Huacai Chen wrote:
> > Hi, Alex,
> >
> > On Thu, Mar 27, 2025 at 8:10 AM Alex Hung <alex.hung@amd.com> wrote:
> >>
> >> The following error messages showed up on an APU and a dGPU during testing.
> >>
> >> <3> [100.231411] BUG: sleeping function called from invalid context at
> >> include/linux/sched/mm.h:321
> >> <3> [100.231414] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid:
> >> 1711, name: kms_color
> >> <3> [100.231416] preempt_count: 2, expected: 0
> >> <3> [100.231417] RCU nest depth: 0, expected: 0
> >> <3> [100.231418] Preemption disabled at:
> >> <3> [100.231419] [<ffffffffc0c2843b>] dc_fpu_begin+0x2b/0xc0 [amdgpu]
> >> <4> [100.231626] CPU: 4 UID: 0 PID: 1711 Comm: kms_color Tainted: G
> >>     W          6.12.0+ #1
> >> <4> [100.231629] Tainted: [W]=WARN
> >> <4> [100.231631] Call Trace:
> >> <4> [100.231632]  <TASK>
> >> <4> [100.231633]  dump_stack_lvl+0x5b/0x70
> >> <4> [100.231638]  dump_stack+0x10/0x20
> >> <4> [100.231639]  __might_resched+0x170/0x1d0
> >> <4> [100.231643]  __might_sleep+0x44/0x70
> >> <4> [100.231645]  __alloc_pages_noprof+0x22f/0x370
> >> <4> [100.231649]  ___kmalloc_large_node+0x95/0x150
> >> <4> [100.231651]  ? preempt_count_add+0x4e/0xc0
> >> <4> [100.231653]  __kmalloc_large_noprof+0x1d/0xb0
> >> <4> [100.231655]  dml2_create_copy+0x27/0x60 [amdgpu]
> >> <4> [100.231827]  dc_state_create_copy+0x7e/0x170 [amdgpu]
> >> <4> [100.231995]  update_planes_and_stream_state+0x23c/0x600 [amdgpu]
> >> <4> [100.232189]  update_planes_and_stream_v2+0x22b/0x530 [amdgpu]
> >> <4> [100.232366]  ? amdgpu_dm_atomic_commit_tail+0x1310/0x4100 [amdgpu]
> >> <4> [100.232569]  ? commit_tail+0x96/0x140 [drm_kms_helper]
> >> <4> [100.232577]  dc_update_planes_and_stream+0x5b/0xe0 [amdgpu]
> >> <4> [100.232730]  amdgpu_dm_atomic_commit_tail+0x1fa7/0x4100 [amdgpu]
> >> <4> [100.232908]  ? stack_depot_save_flags+0x2c/0x730
> >> <4> [100.232915]  ? wait_for_completion_timeout+0x1d/0x30
> >> <4> [100.232917]  commit_tail+0x96/0x140 [drm_kms_helper]
> >> <4> [100.232923]  drm_atomic_helper_commit+0x12b/0x150 [drm_kms_helper]
> >> <4> [100.232927]  drm_atomic_commit+0xad/0xe0 [drm]
> >> <4> [100.232939]  ? __pfx___drm_printfn_info+0x10/0x10 [drm]
> >> <4> [100.232956]  drm_atomic_helper_set_config+0x80/0xc0 [drm_kms_helper]
> >> <4> [100.232961]  drm_mode_setcrtc+0x22e/0x910 [drm]
> >> <4> [100.232975]  ? kfree+0x18f/0x350
> >> <4> [100.232977]  ? __pfx_drm_mode_setcrtc+0x10/0x10 [drm]
> >> <4> [100.232987]  drm_ioctl_kernel+0xa7/0x100 [drm]
> >> <4> [100.233004]  drm_ioctl+0x29d/0x500 [drm]
> >> <4> [100.233015]  ? __pfx_drm_mode_setcrtc+0x10/0x10 [drm]
> >> <4> [100.233026]  ? _raw_spin_unlock_irqrestore+0x1f/0x40
> >> <4> [100.233029]  amdgpu_drm_ioctl+0x4b/0x80 [amdgpu]
> >> <4> [100.233131]  __x64_sys_ioctl+0x92/0xd0
> >> <4> [100.233133]  x64_sys_call+0x1205/0x20d0
> >> <4> [100.233136]  do_syscall_64+0x50/0x110
> >> <4> [100.233138]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> >> <4> [100.233142] RIP: 0033:0x7fb21e71a94f
> >> <4> [100.233144] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24
> >> 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00
> >> 0f 05 <41> 89 c0 3d 00 f0 ff ff 77 1f 48 8b 44 24 18 64 48 2b 04 25 28 00
> >> <4> [100.233145] RSP: 002b:00007ffdd9a52e50 EFLAGS: 00000246 ORIG_RAX:
> >> 0000000000000010
> >> <4> [100.233148] RAX: ffffffffffffffda RBX: 00007ffdd9a52ee0 RCX:
> >> 00007fb21e71a94f
> >> <4> [100.233149] RDX: 00007ffdd9a52ee0 RSI: 00000000c06864a2 RDI:
> >> 0000000000000005
> >> <4> [100.233149] RBP: 00000000c06864a2 R08: 0000000000000000 R09:
> >> 00005609537f7b08
> >> <4> [100.233150] R10: 0000000000000000 R11: 0000000000000246 R12:
> >> 0000000000000000
> >> <4> [100.233151] R13: 0000000000000005 R14: 0000000000000000 R15:
> >> 00005609537e2848
> >> <4> [100.233152]  </TASK>
> > This seems caused by dml2_allocate_memory(), to fix this we can only
> > protect FPU in DML2, I can do it in the new version, but I want to
> > listen Aurabindo's opinion.
> >
> >
>
> It looks like dml21_apply_soc_bb_overrides() does have some division on
> double variables. I'm curious why we dont see this on our side. Was this
> seen on x86 or Loongson?
It is seen on Loongson.

>
> I think your approach is correct. Thanks for taking time to fix this. We
> can add it to weekly testing if you send us a patch.
V2 is sent, please take a look.
https://lore.kernel.org/dri-devel/20250327095334.3327111-1-chenhuacai@loongson.cn/T/#t

Huacai

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-03-29  8:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-18 11:17 [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy() Huacai Chen
2025-03-18 11:17 ` [PATCH 2/2] drm/amd/display: Protect dml2_validate() from its callers Huacai Chen
2025-03-20  2:17   ` Alex Hung
2025-03-20  2:16 ` [PATCH 1/2] drm/amd/display: Protect dml2_create()/dml2_copy()/dml2_create_copy() Alex Hung
2025-03-20  4:23   ` Huacai Chen
2025-03-26  0:02     ` Alex Hung
2025-03-27  0:10       ` Alex Hung
2025-03-27  1:40         ` Huacai Chen
2025-03-28 18:27           ` Aurabindo Pillai
2025-03-29  8:47             ` Huacai Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox