Re: [PATCH 1/3] drm/amd/display/dc/calcs/dce_calcs: Break-out a stack-heavy chunk of code

[Arnd Bergmann]

On Fri, Nov 25, 2022, at 10:25, Lee Jones wrote:
> bw_calcs() presently blows the stack-frame limit by calling functions
> inside a argument list which return quite a bit of data to be passed
> onto sub-functions.  Simply breaking out this hunk reduces the
> stack-frame use by 500 Bytes, preventing the following compiler
> warning:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/calcs/dce_calcs.c:3285:6:
>   warning: stack frame size (1384) exceeds limit (1024)
> in 'bw_calcs' [-Wframe-larger-than]
> bool bw_calcs(struct dc_context *ctx,
>  ^
> 1 warning generated.
>
> This resolves the issue and takes us one step closer towards a
> successful allmodconfig WERROR build.
>
> Signed-off-by: Lee Jones 

Is this still needed with the patch to turn off the display engine
on most architectures? On which architecture and with which compiler
do you still observe the problem?

Note that this probably doesn't actually solve the potential stack
overflow by itself, since the function that is now split out
is still called with the parent stack active. Splitting out multiple
smaller bits however would solve it since then the stack frames
could overlap.

Arnd

[PATCH 1/3] drm/amd/display/dc/calcs/dce_calcs: Break-out a stack-heavy chunk of code

[Lee Jones]

bw_calcs() presently blows the stack-frame limit by calling functions
inside a argument list which return quite a bit of data to be passed
onto sub-functions.  Simply breaking out this hunk reduces the
stack-frame use by 500 Bytes, preventing the following compiler
warning:

drivers/gpu/drm/amd/amdgpu/../display/dc/dml/calcs/dce_calcs.c:3285:6:
  warning: stack frame size (1384) exceeds limit (1024)
in 'bw_calcs' [-Wframe-larger-than]
bool bw_calcs(struct dc_context *ctx,
 ^
1 warning generated.

This resolves the issue and takes us one step closer towards a
successful allmodconfig WERROR build.

Signed-off-by: Lee Jones 
---
 .../drm/amd/display/dc/dml/calcs/dce_calcs.c  | 483 +-
 1 file changed, 245 insertions(+), 238 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/calcs/dce_calcs.c 
b/drivers/gpu/drm/amd/display/dc/dml/calcs/dce_calcs.c
index 0100a6053ab6b..ce5918830c030 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/calcs/dce_calcs.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/calcs/dce_calcs.c
@@ -3034,6 +3034,248 @@ static bool all_displays_in_sync(const struct pipe_ctx 
pipe[],
return true;
 }
 
+void bw_calcs_mid_phase(struct dc_context *ctx, const struct bw_calcs_dceip 
*dceip,
+   const struct bw_calcs_vbios *vbios, struct 
dce_bw_output *calcs_output,
+   struct bw_fixed low_sclk, struct bw_fixed mid1_sclk,
+   struct bw_fixed mid2_sclk, struct bw_fixed mid3_sclk,
+   struct bw_fixed mid_yclk, struct bw_calcs_data *data)
+{
+   ((struct bw_calcs_vbios *)vbios)->low_sclk = mid3_sclk;
+   ((struct bw_calcs_vbios *)vbios)->mid1_sclk = mid3_sclk;
+   ((struct bw_calcs_vbios *)vbios)->mid2_sclk = mid3_sclk;
+   calculate_bandwidth(dceip, vbios, data);
+
+   calcs_output->nbp_state_change_wm_ns[0].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  
nbp_state_change_watermark[4],bw_int_to_fixed(1000)));
+   calcs_output->nbp_state_change_wm_ns[1].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  nbp_state_change_watermark[5], 
bw_int_to_fixed(1000)));
+   calcs_output->nbp_state_change_wm_ns[2].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  nbp_state_change_watermark[6], 
bw_int_to_fixed(1000)));
+
+   if (ctx->dc->caps.max_slave_planes) {
+   calcs_output->nbp_state_change_wm_ns[3].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  nbp_state_change_watermark[0], 
bw_int_to_fixed(1000)));
+   calcs_output->nbp_state_change_wm_ns[4].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  nbp_state_change_watermark[1], 
bw_int_to_fixed(1000)));
+   } else {
+   calcs_output->nbp_state_change_wm_ns[3].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  nbp_state_change_watermark[7], 
bw_int_to_fixed(1000)));
+   calcs_output->nbp_state_change_wm_ns[4].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  nbp_state_change_watermark[8], 
bw_int_to_fixed(1000)));
+   }
+   calcs_output->nbp_state_change_wm_ns[5].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  nbp_state_change_watermark[9], 
bw_int_to_fixed(1000)));
+
+   calcs_output->stutter_exit_wm_ns[0].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  stutter_exit_watermark[4], 
bw_int_to_fixed(1000)));
+   calcs_output->stutter_exit_wm_ns[1].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  stutter_exit_watermark[5], 
bw_int_to_fixed(1000)));
+   calcs_output->stutter_exit_wm_ns[2].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  stutter_exit_watermark[6], 
bw_int_to_fixed(1000)));
+   if (ctx->dc->caps.max_slave_planes) {
+   calcs_output->stutter_exit_wm_ns[3].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  stutter_exit_watermark[0], 
bw_int_to_fixed(1000)));
+   calcs_output->stutter_exit_wm_ns[4].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  stutter_exit_watermark[1], 
bw_int_to_fixed(1000)));
+   } else {
+   calcs_output->stutter_exit_wm_ns[3].b_mark =
+   bw_fixed_to_int(bw_mul(data->
+  stutter_exit_watermark[7], 
bw_int_to_fixed(1000)));
+   calcs_output->stutter_exit_wm_ns[4].b_mark =
+