Skip to content

Loadable up down mixer - #8667

Closed
pjdobrowolski wants to merge 2 commits into
thesofproject:mainfrom
pjdobrowolski:loadable_up_down_mixer
Closed

Loadable up down mixer#8667
pjdobrowolski wants to merge 2 commits into
thesofproject:mainfrom
pjdobrowolski:loadable_up_down_mixer

Conversation

@pjdobrowolski

@pjdobrowolskipjdobrowolski commented Dec 21, 2023

Copy link
Copy Markdown
Contributor

This is an module_example of loadable up_down_mixer with lmdk.

lyakh
lyakh requested changes Jan 2, 2024
View reviewed changes
Comment threadsrc/audio/up_down_mixer/up_down_mixer.toml
load_offset = "0x40000"

[module]
count = 1

@lyakhlyakhJan 2, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

no, all this must not be here.

@pjdobrowolskipjdobrowolskiJan 8, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Where can it be more suitable place to leave that toml?

@lyakhlyakhJan 8, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Where can it be more suitable place to leave that toml?

sorry, I meant that specific part that you're adding here. It's platform specific and it should be in a suitable .toml and they should be merged together by the C preprocessor similar to how https://github.com/thesofproject/sof/blob/main/tools/rimage/config/mtl.toml.h currently includes all module toml files

@pjdobrowolskipjdobrowolskiJan 8, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

good point

@pjdobrowolski
pjdobrowolski marked this pull request as ready for review January 10, 2024 14:04
@dbaluta

dbaluta commented Jan 10, 2024

Copy link
Copy Markdown
Collaborator

@pjdobrowolski are you using a 3rd party binary for the mixer OR just the code we have in the SOF repo? Or, does it matter?

dbaluta
dbaluta reviewed Jan 10, 2024
View reviewed changes
Comment threadposix/include/rtos/alloc.h
RanderWang
RanderWang reviewed Jan 11, 2024
View reviewed changes
Comment threadsrc/include/sof/audio/component.h Outdated
@pjdobrowolski

pjdobrowolski commented Jan 11, 2024

Copy link
Copy Markdown
ContributorAuthor

@pjdobrowolski are you using a 3rd party binary for the mixer OR just the code we have in the SOF repo? Or, does it matter?

This is the example how to convert up_down_mixer from SOF repo to loadable version. Only requirement for conversion is to set UPDWMIX to know in configuration to exclude up_down_mixer from build_in modules.

lyakh
lyakh requested changes Jan 11, 2024
View reviewed changes

@lyakhlyakh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is draft quality at best

Comment threadlmdk/modules/up_down_mixer/CMakeLists.txt Outdated
Comment threadsrc/audio/up_down_mixer/up_down_mixer.c
Comment threadsrc/audio/up_down_mixer/up_down_mixer.c Outdated
Comment threadsrc/audio/up_down_mixer/up_down_mixer.c
Comment threadlmdk/libraries/module_example/module_example_mtl.toml
@@ -0,0 +1,86 @@
version = [3, 0]

@lyakhlyakhJan 11, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why is this file needed, and particularly in this PR?

@lgirdwoodlgirdwoodJan 11, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

toml is modular now, so we can take the up/down mixer parts in the correct place.

Comment threadsrc/include/sof/trace/trace.h Outdated
Comment threadsrc/include/sof/audio/component.h Outdated
Comment threadsrc/include/sof/audio/component.h Outdated
Comment threadsrc/include/sof/audio/pipeline-trace.h Outdated
Comment threadzephyr/include/rtos/bit.h Outdated
@pjdobrowolski

pjdobrowolski commented Jan 11, 2024

Copy link
Copy Markdown
ContributorAuthor

this is draft quality at best

You are welcome.

lgirdwood
lgirdwood reviewed Jan 11, 2024
View reviewed changes
Comment threadlmdk/libraries/module_example/module_example_mtl.toml
@@ -0,0 +1,86 @@
version = [3, 0]

@lgirdwoodlgirdwoodJan 11, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

toml is modular now, so we can take the up/down mixer parts in the correct place.

Comment threadlmdk/modules/up_down_mixer/up_down_mixer_hifi3.c Outdated
@pjdobrowolski
pjdobrowolski marked this pull request as draft January 22, 2024 11:37
@pjdobrowolski

pjdobrowolski commented Jan 22, 2024

Copy link
Copy Markdown
ContributorAuthor

Changed to draft, module will be moved to https://github.com/thesofproject/converged-sof-modules

@lgirdwood

lgirdwood commented Jan 24, 2024

Copy link
Copy Markdown
Member

Changed to draft, module will be moved to https://github.com/thesofproject/converged-sof-modules

No, this repo was temporary and for staging only prior to module API, IPC4 etc being upstream in main. All the dependencies are now upstream so all library support should go upstream. We also cannot duplicate our modules in two places.

This repo will be moving to archived state, there is no need to keep it today.

@pjdobrowolski
pjdobrowolskiforce-pushed the loadable_up_down_mixer branch 2 times, most recently from d565c36 to 5ff7f2bCompareJanuary 25, 2024 14:35
@pjdobrowolski

pjdobrowolski commented Jan 25, 2024

Copy link
Copy Markdown
ContributorAuthor

Ok, I removed all unnecessary MODULE_PRIVAT tags and created exported headers from sof. Now up_down_mixer is building outside of sof. @softwarecki now we must create some kind of git/python verification script to check if these headers change or what happen with them for ABI verification.

@RanderWang

RanderWang commented Jan 29, 2024

Copy link
Copy Markdown
Collaborator

please check #8554. We don't need to copy headers to lmdk.

@pjdobrowolski

pjdobrowolski commented Jan 29, 2024

Copy link
Copy Markdown
ContributorAuthor

llext and native loadable modules are not the same.

@lyakh

lyakh commented Jan 29, 2024

Copy link
Copy Markdown
Collaborator

llext and native loadable modules are not the same.

@pjdobrowolski what @RanderWang is talking about isn't an LLEXT module, it's what you call a "native" system-service module, that he has submitted, you still don't need to copy headers for those

@pjdobrowolski
lmdk: undefining unnecessary code
9b1855c
Not all headers and funtions are used in loadable modules that is why we
need for smaller header packs reduce it to what is truely need.
Signed-off-by: Dobrowolski, PawelX <pawelx.dobrowolski@intel.com>
@pjdobrowolski
loadable_module: up_down_mixer loadable example
651dea1
Example of loadable module build using lmdk
Signed-off-by: Dobrowolski, PawelX <pawelx.dobrowolski@intel.com>
@pjdobrowolskipjdobrowolski mentioned this pull request Mar 11, 2024
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akloniexakloniexAwaiting requested review from akloniex
@kv2019ikv2019iAwaiting requested review from kv2019ikv2019i is a code owner
@iuliana-prodaniuliana-prodanAwaiting requested review from iuliana-prodan
@abonislawskiabonislawskiAwaiting requested review from abonislawskiabonislawski is a code owner
@marcinszkudlinskimarcinszkudlinskiAwaiting requested review from marcinszkudlinski
@pblaszkopblaszkoAwaiting requested review from pblaszko
@plbossartplbossartAwaiting requested review from plbossartplbossart is a code owner
@mmaka1mmaka1Awaiting requested review from mmaka1mmaka1 is a code owner
@lbetlejlbetlejAwaiting requested review from lbetlejlbetlej is a code owner
@lyakhlyakhAwaiting requested review from lyakh
@RanderWangRanderWangAwaiting requested review from RanderWang
@lgirdwoodlgirdwoodAwaiting requested review from lgirdwoodlgirdwood is a code owner
@dbalutadbalutaAwaiting requested review from dbalutadbaluta is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@pjdobrowolski@dbaluta@lgirdwood@RanderWang@lyakh@softwarecki