[PATCH] Revert "drm/amd/display: dynamically allocate dml2_configuration_options structures"

[Alex Deucher]

On Mon, Jun 3, 2024 at 5:07 PM George Zhang <george.zhang at amd.com> wrote:
>
> This reverts commit 416b5c5eec9e708b31c68f00cb79130f2cfaf7ed.
>
> This patch caused a regression on DCN 3.2 on the IGT test assr-links-suspend, with
> the dmesg warning:
>
> BUG: sleeping function called from invalid context at include/linux/sched/mm.h:306
> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 449, name: kworker/u64:8
> preempt_count: 2, expected: 0
> RCU nest depth: 0, expected: 0
> Preemption disabled at:
> ffffffffc0ce1580>] dc_fpu_begin+0x30/0xd0 [amdgpu]
> CPU: 5 PID: 449 Comm: kworker/u64:8 Tainted: G        W          6.8.0+ #35
> Hardware name: System manufacturer System Product Name/ROG STRIX X570-E GAMING WIFI II, BIOS 4204 02/24/2022
> Workqueue: events_unbound async_run_entry_fn
>
> Reverting this patch will re-introduce stack size warnings.
>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Signed-off-by: George Zhang <george.zhang at amd.com>
> ---
>  .../display/dc/resource/dcn32/dcn32_resource.c   | 16 +++++-----------
>  .../display/dc/resource/dcn321/dcn321_resource.c | 16 +++++-----------
>  2 files changed, 10 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
> index 0f11d7c8791c..022d320be1d5 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
> @@ -2007,27 +2007,21 @@ void dcn32_calculate_wm_and_dlg(struct dc *dc, struct dc_state *context,
>
>  static void dcn32_update_bw_bounding_box(struct dc *dc, struct clk_bw_params *bw_params)
>  {
> -       struct dml2_configuration_options *dml2_opt;
> -
> -       dml2_opt = kmemdup(&dc->dml2_options, sizeof(dc->dml2_options), GFP_KERNEL);

Instead of reverting, you could change this from GFP_KERNEL to
GFP_ATOMIC.  This aligns with similar allocations in the other
resource files.  Same comment on the allocation below.

That said, to Arnd's point, I'm not sure about FP in atomic contexts.

Alex

> -       if (!dml2_opt)
> -               return;
> +       struct dml2_configuration_options dml2_opt = dc->dml2_options;
>
>         DC_FP_START();
>
>         dcn32_update_bw_bounding_box_fpu(dc, bw_params);
>
> -       dml2_opt->use_clock_dc_limits = false;
> +       dml2_opt.use_clock_dc_limits = false;
>         if (dc->debug.using_dml2 && dc->current_state && dc->current_state->bw_ctx.dml2)
> -               dml2_reinit(dc, dml2_opt, &dc->current_state->bw_ctx.dml2);
> +               dml2_reinit(dc, &dml2_opt, &dc->current_state->bw_ctx.dml2);
>
> -       dml2_opt->use_clock_dc_limits = true;
> +       dml2_opt.use_clock_dc_limits = true;
>         if (dc->debug.using_dml2 && dc->current_state && dc->current_state->bw_ctx.dml2_dc_power_source)
> -               dml2_reinit(dc, dml2_opt, &dc->current_state->bw_ctx.dml2_dc_power_source);
> +               dml2_reinit(dc, &dml2_opt, &dc->current_state->bw_ctx.dml2_dc_power_source);
>
>         DC_FP_END();
> -
> -       kfree(dml2_opt);
>  }
>
>  static struct resource_funcs dcn32_res_pool_funcs = {
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
> index 07ca6f58447d..e4b360d89b3b 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
> @@ -1581,27 +1581,21 @@ static struct dc_cap_funcs cap_funcs = {
>
>  static void dcn321_update_bw_bounding_box(struct dc *dc, struct clk_bw_params *bw_params)
>  {
> -       struct dml2_configuration_options *dml2_opt;
> -
> -       dml2_opt = kmemdup(&dc->dml2_options, sizeof(dc->dml2_options), GFP_KERNEL);
> -       if (!dml2_opt)
> -               return;
> +       struct dml2_configuration_options dml2_opt = dc->dml2_options;
>
>         DC_FP_START();
>
>         dcn321_update_bw_bounding_box_fpu(dc, bw_params);
>
> -       dml2_opt->use_clock_dc_limits = false;
> +       dml2_opt.use_clock_dc_limits = false;
>         if (dc->debug.using_dml2 && dc->current_state && dc->current_state->bw_ctx.dml2)
> -               dml2_reinit(dc, dml2_opt, &dc->current_state->bw_ctx.dml2);
> +               dml2_reinit(dc, &dml2_opt, &dc->current_state->bw_ctx.dml2);
>
> -       dml2_opt->use_clock_dc_limits = true;
> +       dml2_opt.use_clock_dc_limits = true;
>         if (dc->debug.using_dml2 && dc->current_state && dc->current_state->bw_ctx.dml2_dc_power_source)
> -               dml2_reinit(dc, dml2_opt, &dc->current_state->bw_ctx.dml2_dc_power_source);
> +               dml2_reinit(dc, &dml2_opt, &dc->current_state->bw_ctx.dml2_dc_power_source);
>
>         DC_FP_END();
> -
> -       kfree(dml2_opt);
>  }
>
>  static struct resource_funcs dcn321_res_pool_funcs = {
> --
> 2.34.1
>