Re: [PATCH 1/2] drm/amd/display: use pre-allocated temp structure for bounding box

["Arnd Bergmann" <arnd@arndb.de>]

On Tue, Jun 4, 2024, at 20:06, Alex Deucher wrote:
> This mirrors what the driver does for older DCN generations.
>
> Should fix:
>
> 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
>
> Fixes: 88c61827cedc ("drm/amd/display: dynamically allocate 
> dml2_configuration_options structures")
> Tested-by: George Zhang <George.zhang@amd.com>
> Suggested-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: George Zhang <george.zhang@amd.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: harry.wentland@amd.com
> Cc: sunpeng.li@amd.com
> Cc: Rodrigo.Siqueira@amd.com

That looks nicer than all the other suggestions, thanks!

Acked-by: Arnd Bergmann <arnd@arndb.de>

One part sticks out though:

> @@ -2027,7 +2025,7 @@ static void dcn32_update_bw_bounding_box(struct 
> dc *dc, struct clk_bw_params *bw
> 
>  	DC_FP_END();
> 
> -	kfree(dml2_opt);
> +	memcpy(&dc->dml2_options, dml2_opt, sizeof(dc->dml2_options));
>  }

The driver did not copy the data back before, so this is a
change in behavior. Is that intentional or a mistake?

If the intention is to have the data copied back into
dc->dml2_options in the end, wouldn't it be easier to
just pass a pointer as in the old version before commit
e779f4587f61 ("drm/amd/display: Add handling for DC power
mode")?

     Arnd