[abonislawski]

It introduces two significant optimizations:

Separate the memory (heaps, stack) by cores so if we will turn off a specific core we can also turn off all memory of this core.
Remove fixed defines and gaps, previously stack was at fixed position - end of memory and buffer heap was everything between stack and fw. Now everything will be packed at sof_fw memory segment and takes only this memory which is really needed, everything else we can turn off.

BSS section now includes heaps and stack
Removed many heap fixed position defines
Removed gap before fw text section in CNL
memmap init is using real linker positions for heaps
Temporary stack for boot_ldr placed in heap_hp_buffer (safe)
use linker stack position in boot entry
use linker heap position in sanity check
sof fw max_size limited to memory configured by memory banks count

Using new macro to stack init per core
Stack is no longer a one big block for all cores (core_count*stack_size)
System_runtime_heap and system_heap are no longer a one big block for all cores

Memory layout for every slave core aligned to hw memory bank:

--align to memory bank--
system_heap
system_runtime_heap
stack
--align to memory bank--
next slave core..

So if we will turn off a specific core then we are also able to turn off memory "connected" to this core

Similar layout applies to master core

Signed-off-by: Adrian Bonislawski adrian.bonislawski@linux.intel.com

[dbaluta]

@abonislawski none of the commits explain why do we need a new memory layout and what's wrong with the existing one. Please fix this.

[lgirdwood]

@abonislawski also this breaks CI.

[tlauda]

Core 0 doesn't need to be master core - that's why we have PLATFORM_MASTER_CORE_ID.

[abonislawski]

is this a real scenario or just a theoretical option? I can branch in this macro if core is not a PLATFORM_MASTER_CORE_ID but it will work correctly only if it is 0 (this define), just like in memmap in memory.c - it is designed for master at 0 (without my changes)

[tlauda]

Real scenario as there is already HW with master core != 0 (although currently not supported by SOF). I know it probably won't work due to some historical assumptions, but better not to limit the new code.

[mmaka1]

Yes, we should not assume master == 0 in the generic xtensa arch code since the ID of the core must be unique and that is it, the actual value depends on how the cores are integrated.

We may assume it temporarily (better not, however safe atm) in the cavs platform code since we know how the cores are integrated in there.

[jajanusz]

Doesn't it break git bisect? I didn't look into details, but by looking at commit titles commits like new memory layout should be squashed with minimal commit that changes what layout is needed. Otherwise it will not build for other platforms, for few commits, making git bisect not so useful.

[abonislawski]

@jajanusz all memory layout commits squashed.

One more thing, please check intel licence note in xtos-internal.h, it should be added like this? Should I add author note? And it should be also added to boot_entry.S and crt1-boards.S If I have changed only 1 line in these files?

[abonislawski]

Because of recent changes on master this PR will work again when #1522 will be merged

[lgirdwood]

No idea why you are making these changes are there are no commit messages and the PR description fails to explain why each change is needed.

[abonislawski]

No idea why you are making these changes are there are no commit messages and the PR description fails to explain why each change is needed.

"This will optimize memory usage, remove fixed position defines and allow to
power down all memory of a specified core" + changelog

I will add more info to commit messages if thats not enough

It introduces two significant optimizations:

1. Separate the memory (heaps, stack) by cores so if we will turn off a specific core we can also turn off all memory of this core.
2. Remove fixed defines and gaps, previously stack was at fixed position - end of memory and buffer heap was everything between stack and fw. Now everything will be packed at sof_fw memory segment and takes only this memory which is really needed, everything else we can turn off.

[lgirdwood]

These are overall a good new feature, but can someone explain the programming differences (if any) between CAVS platforms and other platforms wrt buffer and other memory allocations.

[lgirdwood]

@abonislawski thanks, I see what you are doing now. It also looks like these changes need to be reflected in memory.h where we have ASCII descriptions of the memory map.
I think the baytrail/haswell linker script changes will also need applied to imx8 linker script ?
@dbaluta fyi

[abonislawski]

@lgirdwood sure, good point. I have updated ASCII descriptions of the memory map.

Also added one commit for imx8 linker so all of them will get fix

[abonislawski]

Force pushed because of QB issue, no changes today. Please finish your reviews @tlauda @jajanusz @mmaka1 @dbaluta

[lyakh]

@jajanusz @abonislawski @lgirdwood this broke Sue Creek.

[jajanusz]

@lyakh Sorry we have no suecreek here and we just try to not cause CI regression.
@lgirdwood Can we add something more than just building for suecreek in CI?

[abonislawski]

@lyakh I cannot test it without platforms but PR #1567 will probably fix it, please let me know