Re: [PATCH 0/2] Reduce stack size for DML2

[Alex Deucher <alexdeucher@gmail.com>]

On Tue, Oct 17, 2023 at 1:22 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Hi Rodrigo,
>
> On Mon, Oct 16, 2023 at 08:19:16AM -0600, Rodrigo Siqueira wrote:
> > Stephen discovers a stack size issue when compiling the latest amdgpu
> > code with allmodconfig. This patchset addresses that issue by splitting
> > a large function into two smaller parts.
> >
> > Thanks
> > Siqueira
> >
> > Rodrigo Siqueira (2):
> >   drm/amd/display: Reduce stack size by splitting function
> >   drm/amd/display: Fix stack size issue on DML2
> >
> >  .../amd/display/dc/dml2/display_mode_core.c   | 3289 +++++++++--------
> >  1 file changed, 1653 insertions(+), 1636 deletions(-)
> >
> > --
> > 2.42.0
> >
>
> This series appears in -next as commit c587ee30f376 ("drm/amd/display:
> Reduce stack size by splitting function"); while it may help stack usage
> for GCC, clang still suffers. All clang versions that the kernel
> supports show a warning for dml_prefetch_check(), the following is with
> LLVM 17.0.2 from kernel.org [1].
>
>   drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6263:13: error: stack frame size (2520) exceeds limit (2048) in 'dml_prefetch_check' [-Werror,-Wframe-larger-than]
>    6263 | static void dml_prefetch_check(struct display_mode_lib_st *mode_lib)
>         |             ^
>
> With clang 18.0.0 (tip of tree) and 15.0.7, I see:
>
>   drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:8277:6: error: stack frame size (2056) exceeds limit (2048) in 'dml_core_mode_programming' [-Werror,-Wframe-larger-than]
>    8277 | void dml_core_mode_programming(struct display_mode_lib_st *mode_lib, const struct dml_clk_cfg_st *clk_cfg)
>         |      ^
>
> For what it's worth, building with GCC 13.2.0 with a slighly lower
> -Wframe-larger-than value reveals that dml_prefetch_check() is right at
> the current limit and the stack usage of dml_core_mode_programming()
> when built with GCC is not too far of clang's, so it seems like there
> should be a more robust set of fixes, such as the ones that I have
> already done for older generations of this code.
>
>   drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c: In function 'dml_prefetch_check':
>   drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6705:1: error: the frame size of 2048 bytes is larger than 1800 bytes [-Werror=frame-larger-than=]
>    6705 | }
>         | ^
>
>   drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c: In function 'dml_core_mode_programming':
>   drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:9893:1: error: the frame size of 1880 bytes is larger than 1800 bytes [-Werror=frame-larger-than=]
>    9893 | } // dml_core_mode_programming
>         | ^
>
> 41012d715d5d drm/amd/display: Mark dml30's UseMinimumDCFCLK() as noinline for stack usage
> 21485d3da659 drm/amd/display: Reduce number of arguments of dml31's CalculateFlipSchedule()
> 37934d4118e2 drm/amd/display: Reduce number of arguments of dml31's CalculateWatermarksAndDRAMSpeedChangeSupport()
> a3fef74b1d48 drm/amd/display: Reduce number of arguments of dml32_CalculatePrefetchSchedule()
> c4be0ac987f2 drm/amd/display: Reduce number of arguments of dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport()
> 25ea501ed85d drm/amd/display: Reduce number of arguments of dml314's CalculateFlipSchedule()
> ca07f4f5a98b drm/amd/display: Reduce number of arguments of dml314's CalculateWatermarksAndDRAMSpeedChangeSupport()
>
> It would be really nice if these would somehow make it back to the
> original sources so that we stop going through this every time a new
> version of this code shows up. I thought that AMD has started testing
> with clang, how were these warnings not caught before the code was
> merged? If you are unable to look into these warnings, I can try to
> double back to this once I look into the other fires in -next...

Our clang CI builds are passing just fine.  It seems to be specific to
certain kernel configs.  I guess it depends on how different functions
get inlined.  This issue was reported by Stephen originally which is
how we discovered it.

Alex


Alex