Skip to content

Conversation

@lyakh
Copy link
Collaborator

@lyakh lyakh commented Jul 13, 2021

A draft for IMX / cAVS splitting. In fact the difference is very small, maybe we can keep a large portion of this common. This is just a quick RFC, suggestions?

lyakh added 2 commits July 13, 2021 18:42
Create alloc.c for all heap management code.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
IMX and cAVS have different heap configurations, split the file to
eliminate ifdefs.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
static uint8_t __aligned(PLATFORM_DCACHE_ALIGN)heapmem[HEAPMEM_SIZE];
static uint8_t __aligned(PLATFORM_DCACHE_ALIGN)heapmem_shared[HEAPMEM_SHARED_SIZE];
#endif

Copy link
Contributor

@iuliana-prodan iuliana-prodan Jul 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lyakh I don't think we should split this into 2 files, for imx and cavs - there is a lot of duplicated code.

I was thinking of keeping the #ifdef CONFIG_IMX at line 34, since we, for imx, need the heapmem variable in a different section.

The next part, the #else (starting with line 41) I believe it should be split into SMP and non-SMP.
For imx we only need non-SMP, APL has also 1 core, so there is no need for heapmem and heapmem_shared or sof_heap and sof_heap_shared- no need to split heap into shared and unshared.
The shared variables should be declared only for SMP. Please, correct me if I'm wrong.

I've tested only with sof_heap on imx and also @marc-hb verified on APL - see comment here.

In the rest of the file, where we use sof_heap_shared, should be under #ifdef CONFIG_SMP - in functions like statics_init(), rmalloc(), rfree().

@marc-hb
Copy link
Collaborator

marc-hb commented Jul 14, 2021

@lyakh I don't think we should split this into 2 files, for imx and cavs - there is a lot of duplicated code.

Agreed right now there's way too much duplication, copy/paste/diverge is unmaintainable. The alloc_platform.c files (if really needed) should be much smaller and all the rest in a new alloc_common.c. Large #ifdef suck because they require reading two different flows merged into one which is cognitive strain and error-prone. On the other hand, when there is only a small #ifdef then it's better to keep 90% identical functions in alloc_common.c instead of duplicating these 90%.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants