Re: [bug report] drm/amd/display: Simplify the per-CPU usage.

2024-02-01 Thread Sebastian Andrzej Siewior
On 2024-02-01 15:18:04 [+0300], Dan Carpenter wrote:
> Hello Sebastian Andrzej Siewior,
Hi Dan,

> The patch de5e73dc6baf: "drm/amd/display: Simplify the per-CPU
> usage." from Sep 21, 2023 (linux-next), leads to the following Smatch
> static checker warning:

Did I introduce that or has it been made visible?
That change adds preempt_disable() to DC_FP_START() but this was there
already, just hidden. For x86 it is done within kernel_fpu_begin().

> drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn30/dcn30_resource.c:2385 
> dcn30_resource_construct() warn: sleeping in atomic context
> drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn32/dcn32_resource.c:2136 
> dcn32_resource_construct() warn: sleeping in atomic context
> 
> drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn30/dcn30_resource.c
> 2263 static bool dcn30_resource_construct(
> 2264 uint8_t num_virtual_links,
> 2265 struct dc *dc,
> 2266 struct dcn30_resource_pool *pool)
> 2267 {
…
> 2281 
> 2282 DC_FP_START();
>  ^^
> Preempt disabled here.
…
> 2383 /* Clock Sources for Pixel Clock*/
> 2384 pool->base.clock_sources[DCN30_CLK_SRC_PLL0] =
> --> 2385 dcn30_clock_source_create(ctx, ctx->dc_bios,
>  ^^
> sleeping allocation here.
> 

Correct you are. But there is more. Later is also dccg30_create() and
dcn30_hubbub_create() and probably a few other, too.
Could we please restructure the init-phase so that the memory allocation
happen outside of DC_FP_START(). Is DC_FP_START() even needed here?

> regards,
> dan carpenter

Sebastian


Re: [bug report] drm/amd/display: Simplify the per-CPU usage.

2024-02-01 Thread Dan Carpenter
On Thu, Feb 01, 2024 at 02:53:42PM +0100, Sebastian Andrzej Siewior wrote:
> On 2024-02-01 15:18:04 [+0300], Dan Carpenter wrote:
> > Hello Sebastian Andrzej Siewior,
> Hi Dan,
> 
> > The patch de5e73dc6baf: "drm/amd/display: Simplify the per-CPU
> > usage." from Sep 21, 2023 (linux-next), leads to the following Smatch
> > static checker warning:
> 
> Did I introduce that or has it been made visible?
> That change adds preempt_disable() to DC_FP_START() but this was there
> already, just hidden. For x86 it is done within kernel_fpu_begin().
> 

Sorry, yeah, the bug was there before.  I don't know why this shows up
as a new warning.  Probably it's because AMD driver files were renamed...
Smatch parses kernel_fpu_begin() correctly and sees the preempt_disable()
but I didn't know it disables preemption so it's likely human error on
my part.

regards,
dan carpenter



[bug report] drm/amd/display: Simplify the per-CPU usage.

2024-02-01 Thread Dan Carpenter
Hello Sebastian Andrzej Siewior,

The patch de5e73dc6baf: "drm/amd/display: Simplify the per-CPU
usage." from Sep 21, 2023 (linux-next), leads to the following Smatch
static checker warning:

drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn30/dcn30_resource.c:2385 
dcn30_resource_construct() warn: sleeping in atomic context
drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn32/dcn32_resource.c:2136 
dcn32_resource_construct() warn: sleeping in atomic context

drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn30/dcn30_resource.c
2263 static bool dcn30_resource_construct(
2264 uint8_t num_virtual_links,
2265 struct dc *dc,
2266 struct dcn30_resource_pool *pool)
2267 {
2268 int i;
2269 struct dc_context *ctx = dc->ctx;
2270 struct irq_service_init_data init_data;
2271 struct ddc_service_init_data ddc_init_data = {0};
2272 uint32_t pipe_fuses = read_pipe_fuses(ctx);
2273 uint32_t num_pipes = 0;
2274 
2275 if (!(pipe_fuses == 0 || pipe_fuses == 0x3e)) {
2276 BREAK_TO_DEBUGGER();
2277 dm_error("DC: Unexpected fuse recipe for navi2x !\n");
2278 /* fault to single pipe */
2279 pipe_fuses = 0x3e;
2280 }
2281 
2282 DC_FP_START();
 ^^
Preempt disabled here.

2283 
2284 ctx->dc_bios->regs = _regs;
2285 
2286 pool->base.res_cap = _cap_dcn3;
2287 
2288 pool->base.funcs = _res_pool_funcs;
2289 
2290 /*
2291  *  Resource + asic cap harcoding*
2292  */
2293 pool->base.underlay_pipe_index = NO_UNDERLAY_PIPE;
2294 pool->base.pipe_count = 
pool->base.res_cap->num_timing_generator;
2295 pool->base.mpcc_count = 
pool->base.res_cap->num_timing_generator;
2296 dc->caps.max_downscale_ratio = 600;
2297 dc->caps.i2c_speed_in_khz = 100;
2298 dc->caps.i2c_speed_in_khz_hdcp = 100; /*1.4 w/a not applied by 
default*/
2299 dc->caps.max_cursor_size = 256;
2300 dc->caps.min_horizontal_blanking_period = 80;
2301 dc->caps.dmdata_alloc_size = 2048;
2302 dc->caps.mall_size_per_mem_channel = 8;
2303 /* total size = mall per channel * num channels * 1024 * 1024 
*/
2304 dc->caps.mall_size_total = dc->caps.mall_size_per_mem_channel 
* dc->ctx->dc_bios->vram_info.num_chans * 1048576;
2305 dc->caps.cursor_cache_size = dc->caps.max_cursor_size * 
dc->caps.max_cursor_size * 8;
2306 
2307 dc->caps.max_slave_planes = 2;
2308 dc->caps.max_slave_yuv_planes = 2;
2309 dc->caps.max_slave_rgb_planes = 2;
2310 dc->caps.post_blend_color_processing = true;
2311 dc->caps.force_dp_tps4_for_cp2520 = true;
2312 dc->caps.extended_aux_timeout_support = true;
2313 dc->caps.dmcub_support = true;
2314 
2315 /* Color pipeline capabilities */
2316 dc->caps.color.dpp.dcn_arch = 1;
2317 dc->caps.color.dpp.input_lut_shared = 0;
2318 dc->caps.color.dpp.icsc = 1;
2319 dc->caps.color.dpp.dgam_ram = 0; // must use gamma_corr
2320 dc->caps.color.dpp.dgam_rom_caps.srgb = 1;
2321 dc->caps.color.dpp.dgam_rom_caps.bt2020 = 1;
2322 dc->caps.color.dpp.dgam_rom_caps.gamma2_2 = 1;
2323 dc->caps.color.dpp.dgam_rom_caps.pq = 1;
2324 dc->caps.color.dpp.dgam_rom_caps.hlg = 1;
2325 dc->caps.color.dpp.post_csc = 1;
2326 dc->caps.color.dpp.gamma_corr = 1;
2327 dc->caps.color.dpp.dgam_rom_for_yuv = 0;
2328 
2329 dc->caps.color.dpp.hw_3d_lut = 1;
2330 dc->caps.color.dpp.ogam_ram = 1;
2331 // no OGAM ROM on DCN3
2332 dc->caps.color.dpp.ogam_rom_caps.srgb = 0;
2333 dc->caps.color.dpp.ogam_rom_caps.bt2020 = 0;
2334 dc->caps.color.dpp.ogam_rom_caps.gamma2_2 = 0;
2335 dc->caps.color.dpp.ogam_rom_caps.pq = 0;
2336 dc->caps.color.dpp.ogam_rom_caps.hlg = 0;
2337 dc->caps.color.dpp.ocsc = 0;
2338 
2339 dc->caps.color.mpc.gamut_remap = 1;
2340 dc->caps.color.mpc.num_3dluts = 
pool->base.res_cap->num_mpc_3dlut; //3
2341 dc->caps.color.mpc.ogam_ram = 1;
2342 dc->caps.color.mpc.ogam_rom_caps.srgb = 0;
2343 dc->caps.color.mpc.ogam_rom_caps.bt2020 = 0;
2344 dc->caps.color.mpc.ogam_rom_caps.gamma2_2 = 0;
2345 dc->caps.color.mpc.ogam_rom_caps.pq = 0;
2346 dc->caps.color.mpc.ogam_rom_caps.hlg = 0;
2347 dc->caps.color.mpc.ocsc =