Skip to content
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.

Sign up for GitHub

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

Jump to bottom

src_lite: add module #8334

Merged
merged 1 commit into from
Oct 26, 2023
Merged

src_lite: add module #8334

merged 1 commit into from
Oct 26, 2023

Conversation

fkwasowi
Copy link
Contributor

@fkwasowi fkwasowi commented Oct 17, 2023 •
edited
Loading

Addition of SRC Lite module,
which only supports a subset of conversions
supported by the SRC module.

Purpose of SRC Lite module is memory optimization. Code of SRC Lite is drastically reduced and requires significantly less DSP RAM memory.
When needed one of defined conversions, driver can choose SRC Lite module instead of SRC module to optimize memory utilization.

48 -> 16kHz
44.1 -> 16 kHz
32 -> 16 kHz
44.1 -> 48

@fkwasowi fkwasowi force-pushed the prv-fkwasowi_src_lite branch from 9014f76 to faf7ffc Compare October 17, 2023 11:17
singalsu
singalsu approved these changes Oct 17, 2023
View reviewed changes
Copy link
Collaborator

@singalsu singalsu left a comment

Choose a reason for hiding this comment

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

Nice work, thanks!

@fkwasowi fkwasowi force-pushed the prv-fkwasowi_src_lite branch from faf7ffc to 7a4e75a Compare October 17, 2023 11:33
@fkwasowi fkwasowi force-pushed the prv-fkwasowi_src_lite branch 2 times, most recently from 44fb455 to 0c412b8 Compare October 17, 2023 11:42
lgirdwood
lgirdwood reviewed Oct 17, 2023
View reviewed changes
src/audio/src/src.c Outdated Show resolved Hide resolved
btian1
btian1 reviewed Oct 18, 2023
View reviewed changes
Copy link
Contributor

@btian1 btian1 left a comment

Choose a reason for hiding this comment

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

so original SRC module capability does not support listed 4 convertor? if so, compared with add a new module, possible to add more src parameters to make src module support this?

src/audio/src/coef/src_lite_ipc4_int32_define.h Show resolved Hide resolved
src/audio/src/coef/src_lite_ipc4_int32_table.h Outdated Show resolved Hide resolved
tools/rimage/config/mtl.toml Show resolved Hide resolved
@fkwasowi fkwasowi force-pushed the prv-fkwasowi_src_lite branch 6 times, most recently from 4032103 to c4694b1 Compare October 18, 2023 09:15
lgirdwood
lgirdwood reviewed Oct 18, 2023
View reviewed changes
app/boards/intel_adsp_ace15_mtpm.conf Show resolved Hide resolved
src/audio/src/coef/src_lite_ipc4_int32_define.h
#define STAGE1_TIMES_MAX 32
#define STAGE2_TIMES_MAX 32
#define STAGE_BUF_SIZE 672
#define NUM_ALL_COEFFICIENTS 69224
Copy link
Member

Choose a reason for hiding this comment

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

I would prefix these with SRC_ as some names are quite common, but I think these are added before this commit so we should revisit this @singalsu ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes you are right, I can do it after this PR. These files are automatically generated so better to do the change to Matlab code also.

Copy link
Member

Choose a reason for hiding this comment

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

ok, thaks @singalsu let add the prefix after this PR is merged.

kv2019i
kv2019i reviewed Oct 19, 2023
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

I'm good with the change, looks very nicely done! So I think only think missing is some note on why one would choose this instead of full SRC. Doesn't have to be very detailed, but some note to explain why to choose one over the other.

@lgirdwood
Copy link
Member

@fkwasowi ping, just some minor things and good to merge.

@fkwasowi fkwasowi force-pushed the prv-fkwasowi_src_lite branch from c4694b1 to df50371 Compare October 22, 2023 08:40
@fkwasowi fkwasowi requested review from singalsu and kv2019i October 22, 2023 08:41
@fkwasowi fkwasowi force-pushed the prv-fkwasowi_src_lite branch 7 times, most recently from ce4d4d4 to 032b7d0 Compare October 23, 2023 08:17
kv2019i
kv2019i reviewed Oct 23, 2023
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Thanks, my earlier comments addresses. One issue with "src_fir_one" initialization, and mergnig the build fix to one commit, and otherwise looks good to go.

src/audio/src/coef/src_ipc4_int32_10_21_2500_5000.h Show resolved Hide resolved
@fkwasowi fkwasowi force-pushed the prv-fkwasowi_src_lite branch from 032b7d0 to c6a60ce Compare October 24, 2023 07:16
@fkwasowi
src_lite: add module
f8ddb6f 
Addition of SRC Lite module,
which only supports a subset of conversions
supported by the SRC module.

Purpose of SRC Lite module is memory optimization.
Code of SRC Lite is drastically reduced and requires
significantly less memory. When needed one of
defined conversions, driver can choose SRC Lite
module instead of SRC module to optimize memory utilization.

48 -> 16kHz
44.1 -> 16 kHz
32 -> 16 kHz
44.1 -> 48

Signed-off-by: Fabiola Kwasowiec <[email protected]>
@fkwasowi fkwasowi force-pushed the prv-fkwasowi_src_lite branch from c6a60ce to f8ddb6f Compare October 24, 2023 07:35
@fkwasowi fkwasowi requested review from lgirdwood and kv2019i October 24, 2023 07:38
kv2019i
kv2019i approved these changes Oct 24, 2023
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Thanks @fkwasowi !

btian1
btian1 reviewed Oct 25, 2023
View reviewed changes
Copy link
Contributor

@btian1 btian1 left a comment

Choose a reason for hiding this comment

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

@fkwasowi , as commit message mentioned SRC_LITE is saving memory when specific converter happened, could you choose one type and list the memory saving with data:
previous code size with patch code size previous ram usage with patch ram usage, we can use 48 --> 16 as a example.

@kv2019i kv2019i merged commit 90fef5a into thesofproject:main Oct 26, 2023
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@btian1 btian1 btian1 left review comments

@marcinszkudlinski marcinszkudlinski marcinszkudlinski approved these changes

@kv2019i kv2019i kv2019i approved these changes

@lgirdwood lgirdwood lgirdwood approved these changes

@marc-hb marc-hb Awaiting requested review from marc-hb

@abonislawski abonislawski Awaiting requested review from abonislawski abonislawski is a code owner

@plbossart plbossart Awaiting requested review from plbossart plbossart is a code owner

@mmaka1 mmaka1 Awaiting requested review from mmaka1 mmaka1 is a code owner

@lbetlej lbetlej Awaiting requested review from lbetlej lbetlej is a code owner

@dbaluta dbaluta Awaiting requested review from dbaluta dbaluta is a code owner

@singalsu singalsu Awaiting requested review from singalsu singalsu 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.
7 participants
@fkwasowi @lgirdwood @dbaluta @btian1 @singalsu @kv2019i @marcinszkudlinski