2024-05-30 14:59:01

by Palmer Dabbelt

[permalink] [raw]
Subject: [PATCH] drm/amd/display: Increase frame-larger-than warning limit

From: Palmer Dabbelt <[email protected]>

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)
^

as of 6.10-rc1.

Signed-off-by: Palmer Dabbelt <[email protected]>
---
drivers/gpu/drm/amd/display/dc/dml/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile
index c4a5efd2dda5..b2bd72e63734 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
@@ -62,9 +62,9 @@ endif

ifneq ($(CONFIG_FRAME_WARN),0)
ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y)
-frame_warn_flag := -Wframe-larger-than=3072
+frame_warn_flag := -Wframe-larger-than=4096
else
-frame_warn_flag := -Wframe-larger-than=2048
+frame_warn_flag := -Wframe-larger-than=3072
endif
endif

--
2.45.1



2024-06-03 22:29:58

by Nathan Chancellor

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

Hi Palmer,

On Thu, May 30, 2024 at 07:57:42AM -0700, Palmer Dabbelt wrote:
> From: Palmer Dabbelt <[email protected]>
>
> 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? 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. Is this allmodconfig or some other configuration?

> as of 6.10-rc1.
>
> Signed-off-by: Palmer Dabbelt <[email protected]>
> ---
> drivers/gpu/drm/amd/display/dc/dml/Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile
> index c4a5efd2dda5..b2bd72e63734 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
> @@ -62,9 +62,9 @@ endif
>
> ifneq ($(CONFIG_FRAME_WARN),0)
> ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y)
> -frame_warn_flag := -Wframe-larger-than=3072
> +frame_warn_flag := -Wframe-larger-than=4096
> else
> -frame_warn_flag := -Wframe-larger-than=2048
> +frame_warn_flag := -Wframe-larger-than=3072
> endif
> endif
>
> --
> 2.45.1
>

2024-06-04 16:34:48

by Palmer Dabbelt

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

On Mon, 03 Jun 2024 15:29:48 PDT (-0700), [email protected] wrote:
> Hi Palmer,
>
> On Thu, May 30, 2024 at 07:57:42AM -0700, Palmer Dabbelt wrote:
>> From: Palmer Dabbelt <[email protected]>
>>
>> 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.

> 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.

> 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.

>> as of 6.10-rc1.
>>
>> Signed-off-by: Palmer Dabbelt <[email protected]>
>> ---
>> drivers/gpu/drm/amd/display/dc/dml/Makefile | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile
>> index c4a5efd2dda5..b2bd72e63734 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
>> @@ -62,9 +62,9 @@ endif
>>
>> ifneq ($(CONFIG_FRAME_WARN),0)
>> ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y)
>> -frame_warn_flag := -Wframe-larger-than=3072
>> +frame_warn_flag := -Wframe-larger-than=4096
>> else
>> -frame_warn_flag := -Wframe-larger-than=2048
>> +frame_warn_flag := -Wframe-larger-than=3072
>> endif
>> endif
>>
>> --
>> 2.45.1
>>

2024-06-13 22:22:51

by Nathan Chancellor

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

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), [email protected] wrote:
> > On Thu, May 30, 2024 at 07:57:42AM -0700, Palmer Dabbelt wrote:
> > > From: Palmer Dabbelt <[email protected]>
> > >
> > > 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

[1]: https://lore.kernel.org/[email protected]/
[2]: https://lore.kernel.org/[email protected]/

Cheers,
Nathan


Attachments:
(No filename) (4.36 kB)
display_mode_vba_32.i (106.88 kB)
Download all attachments

2024-06-14 15:56:11

by Harry Wentland

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



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), [email protected] wrote:
>>> On Thu, May 30, 2024 at 07:57:42AM -0700, Palmer Dabbelt wrote:
>>>> From: Palmer Dabbelt <[email protected]>
>>>>
>>>> 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/[email protected]/
> [2]: https://lore.kernel.org/[email protected]/
>
> Cheers,
> Nathan