Re: [PATCH] drm/amd/display: Increase frame-larger-than warning limit

[Harry Wentland <harry.wentland@amd.com>]

On 2024-06-13 18:22, Nathan Chancellor wrote:
> Hi Palmer (and AMD folks),
> 
> On Tue, Jun 04, 2024 at 09:04:23AM -0700, Palmer Dabbelt wrote:
>> On Mon, 03 Jun 2024 15:29:48 PDT (-0700), nathan@kernel.org wrote:
>>> On Thu, May 30, 2024 at 07:57:42AM -0700, Palmer Dabbelt wrote:
>>>> From: Palmer Dabbelt <palmer@rivosinc.com>
>>>>
>>>> I get a handful of build errors along the lines of
>>>>
>>>>     linux/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:58:13: error: stack frame size (2352) exceeds limit (2048) in 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Werror,-Wframe-larger-than]
>>>>     static void DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation(
>>>>                 ^
>>>>     linux/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1724:6: error: stack frame size (2096) exceeds limit (2048) in 'dml32_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
>>>>     void dml32_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib)
>>>>          ^
>>>
>>> Judging from the message, this is clang/LLVM? What version?
>>
>> Yes, LLVM.  Looks like I'm on 16.0.6.  Probably time for an update, so I'll
>> give it a shot.
> 
> FWIW, I can reproduce this with tip of tree, I was just curious in case
> that ended up mattering.
> 
>>> I assume
>>> this showed up in 6.10-rc1 because of commit 77acc6b55ae4 ("riscv: add
>>> support for kernel-mode FPU"), which allows this driver to be built for
>>> RISC-V.
>>
>> Seems reasonable.  This didn't show up until post-merge, not 100% sure why.
>> I didn't really dig any farther.
> 
> Perhaps you fast forwarded your tree to include that commit?
> 
>>> Is this allmodconfig or some other configuration?
>>
>> IIRC both "allmodconfig" and "allyesconfig" show it, but I don't have a
>> build tree sitting around to be 100% sure.
> 
> Yeah, allmodconfig triggers it.
> 
> I was able to come up with a "trivial" reproducer using cvise (attached
> to this mail if you are curious) that has worse stack usage by a rough
> factor of 2:
> 
>   $ clang --target=riscv64-linux-gnu -O2 -Wall -Wframe-larger-than=512 -c -o /dev/null display_mode_vba_32.i
>   display_mode_vba_32.i:598:6: warning: stack frame size (1264) exceeds limit (512) in 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Wframe-larger-than]
>     598 | void DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation() {
>         |      ^
>   1 warning generated.
> 
>   $ riscv64-linux-gcc -O2 -Wall -Wframe-larger-than=512 -c -o /dev/null display_mode_vba_32.i
>   display_mode_vba_32.i: In function 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation':
>   display_mode_vba_32.i:1729:1: warning: the frame size of 528 bytes is larger than 512 bytes [-Wframe-larger-than=]
>    1729 | }
>         | ^
> 
> I have not done too much further investigation but this is almost
> certainly the same issue that has come up before [1][2] with the AMD
> display code using functions with a large number of parameters, such
> that they have to passed on the stack, coupled with inlining (if I
> remember correctly, LLVM gives more of an inlining discount the less a
> function is used in a file).
> 
> While clang does poorly with that code, I am not interested in
> continuing to fix this code new hardware revision after new hardware
> revision. We could just avoid this code like we do for arm64 for a
> similar reason:
> 
> diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig
> index 5fcd4f778dc3..64df713df878 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 && (!(ARM64 || RISCV) || !CC_IS_CLANG)
>  	help
>  	  Choose this option if you want to use the new display engine
>  	  support for AMDGPU. This adds required support for Vega and
> 

This makes sense to me. I'll be happy to provide an RB if you send a patch.

Harry

> [1]: https://lore.kernel.org/20231019205117.GA839902@dev-arch.thelio-3990X/
> [2]: https://lore.kernel.org/20220830203409.3491379-1-nathan@kernel.org/
> 
> Cheers,
> Nathan