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@xxxxxxxxxx wrote:
>>> On Thu, May 30, 2024 at 07:57:42AM -0700, Palmer Dabbelt wrote:
>>>> From: Palmer Dabbelt <palmer@xxxxxxxxxxxx>
>>>>
>>>> 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@xxxxxxxxxx/
>
> Cheers,
> Nathan
