[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0f6b54fa-720f-43b7-ad74-91c2c5700dad@amd.com>
Date: Mon, 8 Sep 2025 14:18:46 -0600
From: Alex Hung <alex.hung@....com>
To: Xi Ruoyao <xry111@...111.site>, Alex Deucher <alexander.deucher@....com>,
Christian König <christian.koenig@....com>
Cc: amd-gfx@...ts.freedesktop.org, Huacai Chen <chenhuacai@...nel.org>,
WANG Xuerui <kernel@...0n.name>, Mingcong Bai <jeffbai@...c.io>,
dri-devel@...ts.freedesktop.org, loongarch@...ts.linux.dev,
linux-kernel@...r.kernel.org, Asiacn <710187964@...com>,
Austin Zheng <Austin.Zheng@....com>, Harry Wentland
<Harry.Wentland@....com>, Wenjing Liu <wenjing.liu@....com>
Subject: Re: [PATCH] drm/amd/display/dml2: Guard
dml21_map_dc_state_into_dml_display_cfg with DC_FP_START
On 8/25/25 02:52, Xi Ruoyao wrote:
> dml21_map_dc_state_into_dml_display_cfg calls (the call is usually
> inlined by the compiler) populate_dml21_surface_config_from_plane_state
> and populate_dml21_plane_config_from_plane_state which may use FPU. In
> a x86-64 build:
>
> $ objdump --disassemble=dml21_map_dc_state_into_dml_display_cfg \
> > drivers/gpu/drm/amd/display/dc/dml2/dml21/dml21_translation_helper.o |
> > grep %xmm -c
> 63
>
> Thus it needs to be guarded with DC_FP_START. But we must note that the
> current code quality of the in-kernel FPU use in AMD dml2 is very much
> problematic: we are actually calling DC_FP_START in dml21_wrapper.c
> here, and this translation unit is built with CC_FLAGS_FPU. Strictly
> speaking this does not make any sense: with CC_FLAGS_FPU the compiler is
> allowed to generate FPU uses anywhere in the translated code, perhaps
> out of the DC_FP_START guard. This problematic pattern also occurs in
> at least dml2_wrapper.c, dcn35_fpu.c, and dcn351_fpu.c. Thus we really
Let me share Austin's comments below:
"
Both CC_FLAGS_FPU and DC_FP_START are required for FPU usage.
CC_FLAGS_FPU allows the compiler to generate FPU code whereas
DC_FP_START ensures that the FPU registers don't get tainted during runtime.
The change itself looks fine to me.
"
> need a careful audit and refactor for the in-kernel FPU uses, and this
> patch is simply whacking a mole. However per the reporter, whacking
> this mole is enough to make a 9060XT "just work."
>
> Reported-by: Asiacn <710187964@...com>
> Link: https://github.com/loongson-community/discussions/issues/102
"Link" to "BugLink"?
The link is in Chinese and it would be helpful if you can spell out the
problem in commit messages to something like like "this fixes 9060XT
fails to output to HDMI on xa61200" or other meaningful descriptions.
> Tested-by: Asiacn <710187964@...com>
> Signed-off-by: Xi Ruoyao <xry111@...111.site>
> ---
> drivers/gpu/drm/amd/display/dc/dml2/dml21/dml21_wrapper.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dml2/dml21/dml21_wrapper.c b/drivers/gpu/drm/amd/display/dc/dml2/dml21/dml21_wrapper.c
> index 03de3cf06ae5..059ede6ff256 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml2/dml21/dml21_wrapper.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml2/dml21/dml21_wrapper.c
> @@ -224,7 +224,9 @@ static bool dml21_mode_check_and_programming(const struct dc *in_dc, struct dc_s
> dml_ctx->config.svp_pstate.callbacks.release_phantom_streams_and_planes(in_dc, context);
>
> /* Populate stream, plane mappings and other fields in display config. */
> + DC_FP_START();
> result = dml21_map_dc_state_into_dml_display_cfg(in_dc, context, dml_ctx);
> + DC_FP_END();
> if (!result)
> return false;
>
> @@ -279,7 +281,9 @@ static bool dml21_check_mode_support(const struct dc *in_dc, struct dc_state *co
> dml_ctx->config.svp_pstate.callbacks.release_phantom_streams_and_planes(in_dc, context);
>
> mode_support->dml2_instance = dml_init->dml2_instance;
> + DC_FP_START();
> dml21_map_dc_state_into_dml_display_cfg(in_dc, context, dml_ctx);
> + DC_FP_END();
> dml_ctx->v21.mode_programming.dml2_instance->scratch.build_mode_programming_locals.mode_programming_params.programming = dml_ctx->v21.mode_programming.programming;
> DC_FP_START();
> is_supported = dml2_check_mode_supported(mode_support);
I will send this patch to promotion test.
Powered by blists - more mailing lists