Re: [PATCH v7.2-rc6] drm/amd/display: fix usage of DC_FPU_{BEGIN,END} with PREEMPT_RT

[Bert Karwatzki <spasswolf@web.de>]

> 
> > > 
> > > Could the FPU regions with disabled preemption be limited to where we
> > > have actually have FPU usage in way that you don't have to worry when it
> > > is needed to use DC_RUN_WITH_PREEMPTION_ENABLED() and when not? Also the
> > > dc_fpu_begin()/ end() can nest and if they do the usage of
> > > DC_RUN_WITH_PREEMPTION_ENABLED() is futile, isn't it?
> > > 
> > 
> > I'm not very familiar with the amd display engine, and it's a lot of code,
> > but there I think there's some room for improvement, e.g. dml2_destroy():
> > 
> > dml2_destroy is called from dc_state_free() from within DC_FP_{START,END}()
> > and then basically just calls vfree with DC_RUN_WITH_PREEMPTION_ENABLED().
> > (directly or through dml21_destroy()). Here both the DC_FP_*() tags and
> > DC_RUN_WITH_PREEMPTION_ENABLED() could be dropped I think (not tested, yet)
> > 

Now, I've tested dropping some DC_FP_START,END() and DC_RUN_WITH_PREEMPTION_ENABLED().
(Patch for next-20260828+ with the fix above applied)
This works without error so far.
The function dml2_create() and dml2_create_copy() are moved to a non-FPU file so they
do not accidently use FPU instruction (they do not use FPU instruction on x86_64 when
compiled with gcc-16 but other architectures and compilers could probably use them).

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 666212cac105..709c37d2072f 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
@@ -209,9 +209,7 @@ struct dc_state *dc_state_create(struct dc *dc, struct dc_state_create_params *p
 	bool status;
 
 	if (dc->debug.using_dml2) {
-		DC_FP_START();
 		status = dml2_create(dc, &dc->dml2_options, &state->bw_ctx.dml2);
-		DC_FP_END();
 
 		if (!status) {
 			dc_state_release(state);
@@ -221,9 +219,7 @@ struct dc_state *dc_state_create(struct dc *dc, struct dc_state_create_params *p
 		if (dc->caps.dcmode_power_limits_present) {
 			bool dc_power_status;
 
-			DC_FP_START();
 			dc_power_status = dml2_create(dc, &dc->dml2_dc_power_options, &state->bw_ctx.dml2_dc_power_source);
-			DC_FP_END();
 
 			if (!dc_power_status) {
 				dc_state_release(state);
@@ -251,17 +247,13 @@ void dc_state_copy(struct dc_state *dst_state, struct dc_state *src_state)
 #ifdef CONFIG_DRM_AMD_DC_FP
 	dst_state->bw_ctx.dml2 = dst_dml2;
 	if (src_state->bw_ctx.dml2) {
-		DC_FP_START();
 		dml2_copy(dst_state->bw_ctx.dml2, src_state->bw_ctx.dml2);
-		DC_FP_END();
 	}
 
 	dst_state->bw_ctx.dml2_dc_power_source = dst_dml2_dc_power_source;
 
 	if (src_state->bw_ctx.dml2_dc_power_source) {
-		DC_FP_START();
 		dml2_copy(dst_state->bw_ctx.dml2_dc_power_source, src_state->bw_ctx.dml2_dc_power_source);
-		DC_FP_END();
 	}
 #endif // CONFIG_DRM_AMD_DC_FP
 	/* context refcount should not be overridden */
@@ -285,9 +277,7 @@ struct dc_state *dc_state_create_copy(struct dc_state *src_state)
 	new_state->bw_ctx.dml2_dc_power_source = NULL;
 
 	if (src_state->bw_ctx.dml2) {
-		DC_FP_START();
 		status = dml2_create_copy(&new_state->bw_ctx.dml2, src_state->bw_ctx.dml2);
-		DC_FP_END();
 
 		if (!status) {
 			dc_state_release(new_state);
@@ -297,10 +287,8 @@ struct dc_state *dc_state_create_copy(struct dc_state *src_state)
 
 
 	if (src_state->bw_ctx.dml2_dc_power_source) {
-		DC_FP_START();
 		status = dml2_create_copy(&new_state->bw_ctx.dml2_dc_power_source,
 					  src_state->bw_ctx.dml2_dc_power_source);
-		DC_FP_END();
 
 		if (!status) {
 			dc_state_release(new_state);
@@ -389,13 +377,11 @@ static void dc_state_free(struct kref *kref)
 	dc_state_destruct(state);
 
 #ifdef CONFIG_DRM_AMD_DC_FP
-	DC_FP_START();
 	dml2_destroy(state->bw_ctx.dml2);
 	state->bw_ctx.dml2 = 0;
 
 	dml2_destroy(state->bw_ctx.dml2_dc_power_source);
 	state->bw_ctx.dml2_dc_power_source = 0;
-	DC_FP_END();
 #endif
 
 	kvfree(state);
diff --git a/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/dml21_wrapper.c b/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/dml21_wrapper.c
index 8bed59e976d1..29e5cce51b99 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/dml21_wrapper.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/dml21_wrapper.c
@@ -23,11 +23,11 @@
 
 static bool dml21_allocate_memory(struct dml2_context **dml_ctx)
 {
-	DC_RUN_WITH_PREEMPTION_ENABLED(*dml_ctx = vzalloc(sizeof(struct dml2_context)));
+	*dml_ctx = vzalloc(sizeof(struct dml2_context));
 	if (!(*dml_ctx))
 		return false;
 
-	DC_RUN_WITH_PREEMPTION_ENABLED((*dml_ctx)->v21.dml_init.dml2_instance = vzalloc(sizeof(struct dml2_instance)));
+	(*dml_ctx)->v21.dml_init.dml2_instance = vzalloc(sizeof(struct dml2_instance));
 	if (!((*dml_ctx)->v21.dml_init.dml2_instance))
 		return false;
 
@@ -37,7 +37,7 @@ static bool dml21_allocate_memory(struct dml2_context **dml_ctx)
 	(*dml_ctx)->v21.mode_support.display_config = &(*dml_ctx)->v21.display_config;
 	(*dml_ctx)->v21.mode_programming.display_config = (*dml_ctx)->v21.mode_support.display_config;
 
-	DC_RUN_WITH_PREEMPTION_ENABLED((*dml_ctx)->v21.mode_programming.programming = vzalloc(sizeof(struct dml2_display_cfg_programming)));
+	(*dml_ctx)->v21.mode_programming.programming = vzalloc(sizeof(struct dml2_display_cfg_programming));
 
 	if (!((*dml_ctx)->v21.mode_programming.programming))
 		return false;
@@ -51,15 +51,17 @@ bool dml21_create(const struct dc *in_dc, struct dml2_context **dml_ctx, const s
 	if (!dml21_allocate_memory(dml_ctx))
 		return false;
 
+	DC_FP_START();
 	dml21_init(in_dc, *dml_ctx, config);
+	DC_FP_END();
 
 	return true;
 }
 
 void dml21_destroy(struct dml2_context *dml2)
 {
-	DC_RUN_WITH_PREEMPTION_ENABLED(vfree(dml2->v21.dml_init.dml2_instance));
-	DC_RUN_WITH_PREEMPTION_ENABLED(vfree(dml2->v21.mode_programming.programming));
+	vfree(dml2->v21.dml_init.dml2_instance);
+	vfree(dml2->v21.mode_programming.programming);
 }
 
 void dml21_copy(struct dml2_context *dst_dml_ctx,
@@ -88,7 +90,9 @@ void dml21_copy(struct dml2_context *dst_dml_ctx,
 	dst_dml_ctx->v21.mode_programming.programming = dst_dml2_programming;
 
 	/* need to initialize copied instance for internal references to be correct */
+	DC_FP_START();
 	dml2_initialize_instance(&dst_dml_ctx->v21.dml_init);
+	DC_FP_END();
 }
 
 bool dml21_create_copy(struct dml2_context **dst_dml_ctx,
diff --git a/drivers/gpu/drm/amd/display/dc/dml2_0/dml2_wrapper.c b/drivers/gpu/drm/amd/display/dc/dml2_0/dml2_wrapper.c
index 1772e74349c7..570da14fb1a7 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2_0/dml2_wrapper.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2_0/dml2_wrapper.c
@@ -21,7 +21,7 @@ struct dml2_context *dml2_allocate_memory(void)
 {
 	struct dml2_context *dml2;
 
-	DC_RUN_WITH_PREEMPTION_ENABLED(dml2 = vzalloc(sizeof(struct dml2_context)));
+	dml2 = vzalloc(sizeof(struct dml2_context));
 	return dml2;
 }
 bool dml2_validate(const struct dc *in_dc, struct dc_state *context, struct dml2_context *dml2,
@@ -51,7 +51,9 @@ bool dml2_validate(const struct dc *in_dc, struct dc_state *context, struct dml2
 static void dml2_init(const struct dc *in_dc, const struct dml2_configuration_options *config, struct dml2_context **dml2)
 {
 	if ((in_dc->debug.using_dml21) && (in_dc->ctx->dce_version >= DCN_VERSION_4_01)) {
+		DC_FP_START();
 		dml21_reinit(in_dc, *dml2, config);
+		DC_FP_END();
 		return;
 	}
 
@@ -82,11 +84,13 @@ static void dml2_init(const struct dc *in_dc, const struct dml2_configuration_op
 		break;
 	}
 
+	DC_FP_START();
 	initialize_dml2_ip_params(*dml2, in_dc, &(*dml2)->v20.dml_core_ctx.ip);
 
 	initialize_dml2_soc_bbox(*dml2, in_dc, &(*dml2)->v20.dml_core_ctx.soc);
 
 	initialize_dml2_soc_states(*dml2, in_dc, &(*dml2)->v20.dml_core_ctx.soc, &(*dml2)->v20.dml_core_ctx.states);
+	DC_FP_END();
 
 }
 
@@ -116,17 +120,52 @@ void dml2_destroy(struct dml2_context *dml2)
 	if (dml2->architecture == dml2_architecture_21)
 		dml21_destroy(dml2);
 
-	DC_RUN_WITH_PREEMPTION_ENABLED(vfree(dml2));
+	vfree(dml2);
 }
 
 void dml2_reinit(const struct dc *in_dc,
 				 const struct dml2_configuration_options *config,
 				 struct dml2_context **dml2)
 {
+	/*
 	if ((in_dc->debug.using_dml21) && (in_dc->ctx->dce_version >= DCN_VERSION_4_01)) {
+		DC_FP_START();
 		dml21_reinit(in_dc, *dml2, config);
+		DC_FP_END();
 		return;
 	}
+	*/
 
 	dml2_init(in_dc, config, dml2);
 }
+
+/* Moved here from dml2_wrapper_fpu.c */
+void dml2_copy(struct dml2_context *dst_dml2,
+	struct dml2_context *src_dml2)
+{
+	if (src_dml2->architecture == dml2_architecture_21) {
+		dml21_copy(dst_dml2, src_dml2);
+		return;
+	}
+	/* copy Mode Lib Ctx */
+	memcpy(dst_dml2, src_dml2, sizeof(struct dml2_context));
+}
+
+/* Moved here from dml2_wrapper_fpu.c */
+bool dml2_create_copy(struct dml2_context **dst_dml2,
+	struct dml2_context *src_dml2)
+{
+	if (src_dml2->architecture == dml2_architecture_21)
+		return dml21_create_copy(dst_dml2, src_dml2);
+	/* Allocate Mode Lib Ctx */
+	*dst_dml2 = dml2_allocate_memory();
+
+	if (!(*dst_dml2))
+		return false;
+
+	/* copy Mode Lib Ctx */
+	dml2_copy(*dst_dml2, src_dml2);
+
+	return true;
+}
+
diff --git a/drivers/gpu/drm/amd/display/dc/dml2_0/dml2_wrapper_fpu.c b/drivers/gpu/drm/amd/display/dc/dml2_0/dml2_wrapper_fpu.c
index a14e3004a7b7..b590d58ad3a9 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2_0/dml2_wrapper_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2_0/dml2_wrapper_fpu.c
@@ -561,31 +561,3 @@ void dml2_prepare_mcache_programming(struct dc *in_dc, struct dc_state *context,
 		dml21_prepare_mcache_programming(in_dc, context, dml2);
 }
 
-void dml2_copy(struct dml2_context *dst_dml2,
-	struct dml2_context *src_dml2)
-{
-	if (src_dml2->architecture == dml2_architecture_21) {
-		dml21_copy(dst_dml2, src_dml2);
-		return;
-	}
-	/* copy Mode Lib Ctx */
-	memcpy(dst_dml2, src_dml2, sizeof(struct dml2_context));
-}
-
-bool dml2_create_copy(struct dml2_context **dst_dml2,
-	struct dml2_context *src_dml2)
-{
-	if (src_dml2->architecture == dml2_architecture_21)
-		return dml21_create_copy(dst_dml2, src_dml2);
-	/* Allocate Mode Lib Ctx */
-	*dst_dml2 = dml2_allocate_memory();
-
-	if (!(*dst_dml2))
-		return false;
-
-	/* copy Mode Lib Ctx */
-	dml2_copy(*dst_dml2, src_dml2);
-
-	return true;
-}
-


Bert Karwatzki