-
Notifications
You must be signed in to change notification settings - Fork 349
cAVS heap map refinement #4445
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cAVS heap map refinement #4445
Changes from all commits
eda92ab
68fdce1
1f30806
22bb0da
be74e9c
9ada8d2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| +1 −1 | config/apl.toml | |
| +1 −1 | config/cnl.toml | |
| +1 −1 | config/icl.toml | |
| +0 −1 | config/jsl.toml | |
| +54 −0 | config/jsl.toml | |
| +0 −1 | config/tgl-h.toml | |
| +49 −0 | config/tgl-h.toml | |
| +1 −1 | config/tgl.toml |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -135,47 +135,47 @@ | |
|
|
||
| /* | ||
| * The HP SRAM Region on Tigerlake is organised like this :- | ||
| * +--------------------------------------------------------------------------+ | ||
| * | Offset | Region | Size | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_SW_REG_BASE | SW Registers W0 | SRAM_SW_REG_SIZE | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_OUTBOX_BASE | Outbox W0 | SRAM_MAILBOX_SIZE | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_INBOX_BASE | Inbox W1 | SRAM_INBOX_SIZE | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_DEBUG_BASE | Debug data W2 | SRAM_DEBUG_SIZE | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_EXCEPT_BASE | Debug data W2 | SRAM_EXCEPT_SIZE | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_STREAM_BASE | Stream data W2 | SRAM_STREAM_SIZE | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_TRACE_BASE | Trace Buffer W3 | SRAM_TRACE_SIZE | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | HP_SRAM_BASE | DMA | HEAP_HP_BUFFER_SIZE | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | SOF_FW_START | text | | | ||
| * | | data | | | ||
| * | | BSS | | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | | Runtime Heap | HEAP_RUNTIME_SIZE | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | | Runtime shared Heap | HEAP_RUNTIME_SHARED_SIZE | | ||
| * | |-------------------------+-----------------------------+ | ||
| * | | System shared Heap | HEAP_SYSTEM_SHARED_SIZE | | ||
| * | |-------------------------+-----------------------------+ | ||
| * | | Module Buffers | HEAP_BUFFER_SIZE | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | | Primary core Sys Heap | HEAP_SYSTEM_M_SIZE | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | | Pri. Sys Runtime Heap | HEAP_SYS_RUNTIME_M_SIZE | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | | Primary core Stack | SOF_STACK_SIZE | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * | | Sec. core Sys Heap | SOF_CORE_S_T_SIZE | | ||
| * | | Sec. Sys Runtime Heap | | | ||
| * | | Secondary core Stack | | | ||
| * +------------------+-------------------------+-----------------------------+ | ||
| * +----------------------------------------------------------------------------+ | ||
| * | Offset | Region | Size | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_SW_REG_BASE | SW Registers W0 | SRAM_SW_REG_SIZE | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_OUTBOX_BASE | Outbox W0 | SRAM_OUTBOX_SIZE | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_INBOX_BASE | Inbox W1 | SRAM_INBOX_SIZE | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_DEBUG_BASE | Debug data W2 | SRAM_DEBUG_SIZE | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_EXCEPT_BASE | Debug data W2 | SRAM_EXCEPT_SIZE | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_STREAM_BASE | Stream data W2 | SRAM_STREAM_SIZE | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | SRAM_TRACE_BASE | Trace Buffer W3 | SRAM_TRACE_SIZE | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | HEAP_HP_BUFFER_BASE| DMA | HEAP_HP_BUFFER_SIZE | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | SOF_FW_START | text | | | ||
| * | | data | | | ||
| * | | BSS | | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | | Runtime Heap | HEAP_RUNTIME_SIZE | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | | Runtime shared Heap | HEAP_RUNTIME_SHARED_SIZE | | ||
| * | |-------------------------+-----------------------------+ | ||
| * | | System shared Heap | HEAP_SYSTEM_SHARED_SIZE | | ||
| * | |-------------------------+-----------------------------+ | ||
| * | | Module Buffers | HEAP_BUFFER_SIZE | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | | Primary core Sys Heap | HEAP_SYSTEM_M_SIZE | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | | Pri. Sys Runtime Heap | HEAP_SYS_RUNTIME_M_SIZE | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | | Primary core Stack | SOF_STACK_SIZE | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| * | | Sec. core Sys Heap | SOF_CORE_S_T_SIZE | | ||
| * | | Sec. Sys Runtime Heap | | | ||
| * | | Secondary core Stack | | | ||
| * +--------------------+-------------------------+-----------------------------+ | ||
| */ | ||
|
|
||
| /* HP SRAM */ | ||
|
|
@@ -249,14 +249,31 @@ | |
| #define HEAP_SYS_RT_X_COUNT512 8 | ||
| #define HEAP_SYS_RT_X_COUNT1024 4 | ||
|
|
||
| /* Heap section counts base */ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I trust your calculations but for the sake of others like me, can you please add an explanation for how this makes full use of the TGL memory and that it is correct?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the 'Heap section counts base' is only some use experience based summarize of how should we arrange percentage of different size sections, e.g. make 128B and 256B counts as larger as possible. This is only only a base with assumption that we have 64KB size for the specific type. E.g. TGL and ADL share the share memory.h but with different HPSRAM size, we want to keep the percentage of different size section consistent on TGL and ADL, with only changing the RT_TIMES, e.g. 3 for ADL and 8 for TGL. @ranj063 all those values might still need to be fine-tuned, I only did draft tuning so it is only makes more (not 'full') use of our memory. Better solution could be do accurate calculation and put all not used memory to our pool, I think we are using too many types of zone today, and our .text/.data size are varying so this .bss section total max size varies also. It's a good topic about how to manage this well and it is not a short task IMHO.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most of these comments (including any TODO) belong to the source code. You already put a lot of effort there, no one should have to make the exact same effort again.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@keyonjie lets not make short work of this. This seems like a critical change that could make or break multi-core and dynamic platforms on our platforms. I'd prefer taking the time to do this right instead of changing it now only to realize it doesnt work as intended shortly after.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Well, I can imagine we still have quite a lot of opens with respect to the heap block memory map (struct mm), how many types are needed, how to make sure the .share_data section is in uncached region so can be really shared among DSP COREs, how to allocate memory to make the SRAM power gating more efficient add more power saving we can benefit from it, ... The goal of this relative small PR is not to do drive this to far, it is only to rescue those missed memory banks and help for 'out of memory' scenarios, for other (non-OOM related) multi-core + dynamic pipeline cases, it won't help and won't introduce breakage neither.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
As commented to Ranjani, most of them are still open and not suitable to be added to the source, to avoid misleading.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's even more important to mention opens in the source than explain the current status because by default readers assume the work is done and because there's no code that opens can be "reverse-engineered" from. Comments are misleading only if they're not correct.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No, the open is not to this specific part of code, though it could be a topic in sof-developers.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @keyonjie I read through the comments and still have no idea on how you determined what values are necessary/required? Is this based on an experiment using e.g. daily test devices or commercial ones? Put differently, how do we know how much is left for each bank and the current usage on a per bank basis? If we don't have this information things will continue to break, won't they?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @plbossart it is based on our daily test and will be showed in sof-logger if you enable this logging information. |
||
| #define HEAP_COUNT64 128 | ||
| #define HEAP_COUNT128 128 | ||
| #define HEAP_COUNT256 96 | ||
| #define HEAP_COUNT512 8 | ||
| #define HEAP_COUNT1024 4 | ||
| #define HEAP_COUNT2048 2 | ||
| #define HEAP_COUNT4096 1 | ||
|
|
||
| #if HP_SRAM_SIZE < 0x200000 | ||
| #define RT_TIMES 3 | ||
| #define RT_SHARED_TIMES 6 | ||
| #else | ||
| #define RT_TIMES 8 | ||
| #define RT_SHARED_TIMES 16 | ||
| #endif | ||
|
|
||
| /* Heap section sizes for module pool */ | ||
| #define HEAP_RT_COUNT64 128 | ||
| #define HEAP_RT_COUNT128 64 | ||
| #define HEAP_RT_COUNT256 128 | ||
| #define HEAP_RT_COUNT512 8 | ||
| #define HEAP_RT_COUNT1024 4 | ||
| #define HEAP_RT_COUNT2048 1 | ||
| #define HEAP_RT_COUNT4096 1 | ||
| #define HEAP_RT_COUNT64 (HEAP_COUNT64 * RT_TIMES) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @keyonjie for my education, what does HEAP_COUNT64 (HEap section counts base) mean? |
||
| #define HEAP_RT_COUNT128 (HEAP_COUNT128 * RT_TIMES) | ||
| #define HEAP_RT_COUNT256 (HEAP_COUNT256 * RT_TIMES) | ||
| #define HEAP_RT_COUNT512 (HEAP_COUNT512 * RT_TIMES) | ||
| #define HEAP_RT_COUNT1024 (HEAP_COUNT1024 * RT_TIMES) | ||
| #define HEAP_RT_COUNT2048 (HEAP_COUNT2048 * RT_TIMES) | ||
| #define HEAP_RT_COUNT4096 (HEAP_COUNT4096 * RT_TIMES) | ||
|
|
||
| /* Heap configuration */ | ||
| #define HEAP_RUNTIME_SIZE \ | ||
|
|
@@ -266,11 +283,11 @@ | |
| HEAP_RT_COUNT4096 * 4096) | ||
|
|
||
| /* Heap section sizes for runtime shared heap */ | ||
| #define HEAP_RUNTIME_SHARED_COUNT64 (64 + 32 * CONFIG_CORE_COUNT) | ||
| #define HEAP_RUNTIME_SHARED_COUNT128 64 | ||
| #define HEAP_RUNTIME_SHARED_COUNT256 4 | ||
| #define HEAP_RUNTIME_SHARED_COUNT512 16 | ||
| #define HEAP_RUNTIME_SHARED_COUNT1024 (4 + CONFIG_CORE_COUNT) | ||
| #define HEAP_RUNTIME_SHARED_COUNT64 (HEAP_COUNT64 * RT_SHARED_TIMES) | ||
| #define HEAP_RUNTIME_SHARED_COUNT128 (HEAP_COUNT128 * RT_SHARED_TIMES) | ||
| #define HEAP_RUNTIME_SHARED_COUNT256 (HEAP_COUNT256 * RT_SHARED_TIMES) | ||
| #define HEAP_RUNTIME_SHARED_COUNT512 (HEAP_COUNT512 * RT_SHARED_TIMES) | ||
| #define HEAP_RUNTIME_SHARED_COUNT1024 (HEAP_COUNT1024 * RT_SHARED_TIMES) | ||
|
|
||
| #define HEAP_RUNTIME_SHARED_SIZE \ | ||
| (HEAP_RUNTIME_SHARED_COUNT64 * 64 + HEAP_RUNTIME_SHARED_COUNT128 * 128 + \ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to address these large block sizes later. I dont think we have any users ?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes the 2KB and 4KB blocks have no users yet per my observation, maybe we can leave it for 256Bs and 512Bs for advanced fine tuning.