amd-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>,
	llvm@lists.linux.dev, Roman Li <roman.li@amd.com>,
	amd-gfx@lists.freedesktop.org, linux-next@vger.kernel.org,
	Alex Deucher <alexander.deucher@amd.com>,
	Chaitanya Dhere <chaitanya.dhere@amd.com>
Subject: Re: [PATCH 0/2] Reduce stack size for DML2
Date: Tue, 17 Oct 2023 10:22:31 -0700	[thread overview]
Message-ID: <20231017172231.GA2348194@dev-arch.thelio-3990X> (raw)
In-Reply-To: <20231016142031.241912-1-Rodrigo.Siqueira@amd.com>

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

[1]: https://mirrors.edge.kernel.org/pub/tools/llvm/

Cheers,
Nathan

  parent reply	other threads:[~2023-10-17 17:22 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-16 14:19 [PATCH 0/2] Reduce stack size for DML2 Rodrigo Siqueira
2023-10-16 14:19 ` [PATCH 1/2] drm/amd/display: Reduce stack size by splitting function Rodrigo Siqueira
2023-10-16 14:19 ` [PATCH 2/2] drm/amd/display: Fix stack size issue on DML2 Rodrigo Siqueira
2023-10-16 17:35   ` Alex Deucher
2023-11-05 17:55   ` Guenter Roeck
2023-11-06 12:13     ` Hamza Mahfooz
2023-10-17 17:22 ` Nathan Chancellor [this message]
2023-10-17 17:45   ` [PATCH 0/2] Reduce stack size for DML2 Rodrigo Siqueira Jordao
2023-10-17 21:19     ` Nathan Chancellor
2023-10-17 18:02   ` Alex Deucher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231017172231.GA2348194@dev-arch.thelio-3990X \
    --to=nathan@kernel.org \
    --cc=Rodrigo.Siqueira@amd.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=chaitanya.dhere@amd.com \
    --cc=linux-next@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=roman.li@amd.com \
    --cc=sfr@canb.auug.org.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).