diff mbox series

drm/amd/display: Disable CONFIG_DRM_AMD_DC_FP for RISC-V with clang

Message ID 20240614-amdgpu-disable-drm-amd-dc-fp-riscv-clang-v1-1-a6d40617dc9b@kernel.org (mailing list archive)
State New, archived
Headers show
Series drm/amd/display: Disable CONFIG_DRM_AMD_DC_FP for RISC-V with clang | expand

Commit Message

Nathan Chancellor June 14, 2024, 7:54 p.m. UTC 
Commit 77acc6b55ae4 ("riscv: add support for kernel-mode FPU") and
commit a28e4b672f04 ("drm/amd/display: use ARCH_HAS_KERNEL_FPU_SUPPORT")
enabled support for CONFIG_DRM_AMD_DC_FP with RISC-V. Unfortunately,
this exposed -Wframe-larger-than warnings (which become fatal with
CONFIG_WERROR=y) when building ARCH=riscv allmodconfig with clang:

  drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:58:13: error: stack frame size (2448) exceeds limit (2048) in 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Werror,-Wframe-larger-than]
     58 | static void DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation(
        |             ^
  1 error generated.

Many functions in this file use a large number of parameters, which must
be passed on the stack at a certain pointer due to register exhaustion,
which can cause high stack usage when inlining and issues with stack
slot analysis get involved. While the compiler can and should do better
(as GCC uses less than half the amount of stack space for the same
function), it is not as simple as a fix as adjusting the functions not
to take a large number of parameters.

Unfortunately, modifying these files to avoid the problem is a difficult
to justify approach because any revisions to the files in the kernel
tree never make it back to the original source (so copies of the code
for newer hardware revisions just reintroduce the issue) and the files
are hard to read/modify due to being "gcc-parsable HW gospel, coming
straight from HW engineers".

Avoid building the problematic code for RISC-V by modifying the existing
condition for arm64 that exists for the same reason. Factor out the
logical not to make the condition a little more readable naturally.

Fixes: a28e4b672f04 ("drm/amd/display: use ARCH_HAS_KERNEL_FPU_SUPPORT")
Reported-by: Palmer Dabbelt <palmer@rivosinc.com>
Closes: https://lore.kernel.org/20240530145741.7506-2-palmer@rivosinc.com/
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 drivers/gpu/drm/amd/display/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


---
base-commit: c6c4dd54012551cce5cde408b35468f2c62b0cce
change-id: 20240614-amdgpu-disable-drm-amd-dc-fp-riscv-clang-31c84f6b990d

Best regards,

Comments

Harry Wentland June 18, 2024, 2:08 p.m. UTC | #1 
On 2024-06-14 15:54, Nathan Chancellor wrote:
> Commit 77acc6b55ae4 ("riscv: add support for kernel-mode FPU") and
> commit a28e4b672f04 ("drm/amd/display: use ARCH_HAS_KERNEL_FPU_SUPPORT")
> enabled support for CONFIG_DRM_AMD_DC_FP with RISC-V. Unfortunately,
> this exposed -Wframe-larger-than warnings (which become fatal with
> CONFIG_WERROR=y) when building ARCH=riscv allmodconfig with clang:
> 
>   drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:58:13: error: stack frame size (2448) exceeds limit (2048) in 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Werror,-Wframe-larger-than]
>      58 | static void DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation(
>         |             ^
>   1 error generated.
> 
> Many functions in this file use a large number of parameters, which must
> be passed on the stack at a certain pointer due to register exhaustion,
> which can cause high stack usage when inlining and issues with stack
> slot analysis get involved. While the compiler can and should do better
> (as GCC uses less than half the amount of stack space for the same
> function), it is not as simple as a fix as adjusting the functions not
> to take a large number of parameters.
> 
> Unfortunately, modifying these files to avoid the problem is a difficult
> to justify approach because any revisions to the files in the kernel
> tree never make it back to the original source (so copies of the code
> for newer hardware revisions just reintroduce the issue) and the files
> are hard to read/modify due to being "gcc-parsable HW gospel, coming
> straight from HW engineers".
> 
> Avoid building the problematic code for RISC-V by modifying the existing
> condition for arm64 that exists for the same reason. Factor out the
> logical not to make the condition a little more readable naturally.
> 
> Fixes: a28e4b672f04 ("drm/amd/display: use ARCH_HAS_KERNEL_FPU_SUPPORT")
> Reported-by: Palmer Dabbelt <palmer@rivosinc.com>
> Closes: https://lore.kernel.org/20240530145741.7506-2-palmer@rivosinc.com/
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  drivers/gpu/drm/amd/display/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig
> index 5fcd4f778dc3..47b8b49da8a7 100644
> --- a/drivers/gpu/drm/amd/display/Kconfig
> +++ b/drivers/gpu/drm/amd/display/Kconfig
> @@ -8,7 +8,7 @@ config DRM_AMD_DC
>  	depends on BROKEN || !CC_IS_CLANG || ARM64 || RISCV || SPARC64 || X86_64
>  	select SND_HDA_COMPONENT if SND_HDA_CORE
>  	# !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752
> -	select DRM_AMD_DC_FP if ARCH_HAS_KERNEL_FPU_SUPPORT && (!ARM64 || !CC_IS_CLANG)
> +	select DRM_AMD_DC_FP if ARCH_HAS_KERNEL_FPU_SUPPORT && !(CC_IS_CLANG && (ARM64 || RISCV))

Thanks for also making the logic easier to parse.

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

>  	help
>  	  Choose this option if you want to use the new display engine
>  	  support for AMDGPU. This adds required support for Vega and
> 
> ---
> base-commit: c6c4dd54012551cce5cde408b35468f2c62b0cce
> change-id: 20240614-amdgpu-disable-drm-amd-dc-fp-riscv-clang-31c84f6b990d
> 
> Best regards,
Alex Deucher June 18, 2024, 9:32 p.m. UTC | #2 
Applied.  Thanks!

Alex

On Tue, Jun 18, 2024 at 10:17 AM Harry Wentland <harry.wentland@amd.com> wrote:
>
>
>
> On 2024-06-14 15:54, Nathan Chancellor wrote:
> > Commit 77acc6b55ae4 ("riscv: add support for kernel-mode FPU") and
> > commit a28e4b672f04 ("drm/amd/display: use ARCH_HAS_KERNEL_FPU_SUPPORT")
> > enabled support for CONFIG_DRM_AMD_DC_FP with RISC-V. Unfortunately,
> > this exposed -Wframe-larger-than warnings (which become fatal with
> > CONFIG_WERROR=y) when building ARCH=riscv allmodconfig with clang:
> >
> >   drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:58:13: error: stack frame size (2448) exceeds limit (2048) in 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Werror,-Wframe-larger-than]
> >      58 | static void DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation(
> >         |             ^
> >   1 error generated.
> >
> > Many functions in this file use a large number of parameters, which must
> > be passed on the stack at a certain pointer due to register exhaustion,
> > which can cause high stack usage when inlining and issues with stack
> > slot analysis get involved. While the compiler can and should do better
> > (as GCC uses less than half the amount of stack space for the same
> > function), it is not as simple as a fix as adjusting the functions not
> > to take a large number of parameters.
> >
> > Unfortunately, modifying these files to avoid the problem is a difficult
> > to justify approach because any revisions to the files in the kernel
> > tree never make it back to the original source (so copies of the code
> > for newer hardware revisions just reintroduce the issue) and the files
> > are hard to read/modify due to being "gcc-parsable HW gospel, coming
> > straight from HW engineers".
> >
> > Avoid building the problematic code for RISC-V by modifying the existing
> > condition for arm64 that exists for the same reason. Factor out the
> > logical not to make the condition a little more readable naturally.
> >
> > Fixes: a28e4b672f04 ("drm/amd/display: use ARCH_HAS_KERNEL_FPU_SUPPORT")
> > Reported-by: Palmer Dabbelt <palmer@rivosinc.com>
> > Closes: https://lore.kernel.org/20240530145741.7506-2-palmer@rivosinc.com/
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> >  drivers/gpu/drm/amd/display/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig
> > index 5fcd4f778dc3..47b8b49da8a7 100644
> > --- a/drivers/gpu/drm/amd/display/Kconfig
> > +++ b/drivers/gpu/drm/amd/display/Kconfig
> > @@ -8,7 +8,7 @@ config DRM_AMD_DC
> >       depends on BROKEN || !CC_IS_CLANG || ARM64 || RISCV || SPARC64 || X86_64
> >       select SND_HDA_COMPONENT if SND_HDA_CORE
> >       # !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752
> > -     select DRM_AMD_DC_FP if ARCH_HAS_KERNEL_FPU_SUPPORT && (!ARM64 || !CC_IS_CLANG)
> > +     select DRM_AMD_DC_FP if ARCH_HAS_KERNEL_FPU_SUPPORT && !(CC_IS_CLANG && (ARM64 || RISCV))
>
> Thanks for also making the logic easier to parse.
>
> Reviewed-by: Harry Wentland <harry.wentland@amd.com>
>
> Harry
>
> >       help
> >         Choose this option if you want to use the new display engine
> >         support for AMDGPU. This adds required support for Vega and
> >
> > ---
> > base-commit: c6c4dd54012551cce5cde408b35468f2c62b0cce
> > change-id: 20240614-amdgpu-disable-drm-amd-dc-fp-riscv-clang-31c84f6b990d
> >
> > Best regards,
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig
index 5fcd4f778dc3..47b8b49da8a7 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -8,7 +8,7 @@  config DRM_AMD_DC
 	depends on BROKEN || !CC_IS_CLANG || ARM64 || RISCV || SPARC64 || X86_64
 	select SND_HDA_COMPONENT if SND_HDA_CORE
 	# !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752
-	select DRM_AMD_DC_FP if ARCH_HAS_KERNEL_FPU_SUPPORT && (!ARM64 || !CC_IS_CLANG)
+	select DRM_AMD_DC_FP if ARCH_HAS_KERNEL_FPU_SUPPORT && !(CC_IS_CLANG && (ARM64 || RISCV))
 	help
 	  Choose this option if you want to use the new display engine
 	  support for AMDGPU. This adds required support for Vega and