Skip to content

audio: volume: optimize volume component with HIFI3 - #5169

Merged
lgirdwood merged 1 commit into
thesofproject:mainfrom
andrula-song:vol-opt
Jan 27, 2022
Merged

audio: volume: optimize volume component with HIFI3#5169
lgirdwood merged 1 commit into
thesofproject:mainfrom
andrula-song:vol-opt

Conversation

@andrula-song

Copy link
Copy Markdown
Contributor

There are 2 methods to optimize volume component in HiFi3

1.Method 1 : Processing data per channel instead of per sample. Skipping address reading and
writing, only load volume gain once for one channel. This method can
reduce about 20% MCPS than the original one

  1. Method 2: Processing continuous data. Asking a 8-byte aligned memory
    to store volume gain 4 times, using 2-way or 4-way pipeline operations
    of xtensa intrinsics. This method can reduce about 40% MCPS than the
    original one.

Signed-off-by: Andurla Song xiaoyuan.song@intel.com

@aiChaoSONG
aiChaoSONG requested review from bkokoszx, cujomalainey and singalsu and removed request for cujomalainey and singalsuJanuary 10, 2022 07:06
@aiChaoSONGaiChaoSONG changed the title Audio: volume component optimization: 2 new methods in HiFi3audio: volume: optimize volume component with HIFI3Jan 10, 2022

@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.

wouldn't it be easier to create two additional files for the new optimisation options and link one of them in CMakeLists.txt?

Comment threadsrc/audio/volume/volume.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated

@lgirdwoodlgirdwood left a comment

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.

@singalsu any comments ?

Comment threadsrc/audio/volume/volume.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated

@singalsusingalsu 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.

Good work! Some comments:

Comment threadsrc/audio/volume/volume.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated
Comment threadtest/cmocka/src/audio/volume/volume_process.c Outdated
Comment threadsrc/audio/volume/volume.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated
Comment threadsrc/audio/volume/volume.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated
@cujomalainey

Copy link
Copy Markdown
Contributor

@andrula-song if you have fixed comments please mark them as resolved

@lgirdwoodlgirdwood left a comment

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.

Just one minor thing in the regular C code, I'll let @singalsu and others review the intrinsics.
Btw, can you check the formatting in the commit message (the lines look very short). It could be your git config.

Comment threadsrc/audio/volume/volume.c Outdated
Comment threadsrc/audio/volume/volume.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated
@andrula-song

Copy link
Copy Markdown
ContributorAuthor

becuse there is still a failed case about check-alsabat-headset-playback.sh on ADLP_RVP_NOCODEC device, I am trying to fix it and may need many submission, so i will creat a new draft to test it.
sorry for disturbing you, and thanks for your comment.

@lgirdwood

Copy link
Copy Markdown
Member

becuse there is still a failed case about check-alsabat-headset-playback.sh on ADLP_RVP_NOCODEC device, I am trying to fix it and may need many submission, so i will creat a new draft to test it. sorry for disturbing you, and thanks for your comment.

Please use the same PR and just force push your updates. This keeps the history and context for your changes.

@andrula-song

Copy link
Copy Markdown
ContributorAuthor

@lgirdwood It seems like this commit passed all sof-ci test cases except heck-suspend-resume-with-playback-5.sh(other PRs also failed this case). But there is still tgl failed cases of Internal Intel CI System/merge/build, if these are unrelated cases, maybe the latest commit is ready for review.

@lgirdwood

Copy link
Copy Markdown
Member

@andrula-song there is an issues with CI that @kkarask is fixing. We should be able to retest later today.

Comment threadsrc/audio/volume/volume_hifi3.c Outdated
@andrula-song

Copy link
Copy Markdown
ContributorAuthor

@kkarask can you provid wav file of failed cases in Internal Intel CI System/merge/build?

@andrula-song

Copy link
Copy Markdown
ContributorAuthor

hi @lgirdwood ,
It seems this draft is ready to review now. Can you help to review it?
Thanks.

Comment threadsrc/audio/volume/volume.c Outdated
Comment threadsrc/audio/volume/volume.c Outdated
Comment threadsrc/audio/volume/volume.c Outdated

@singalsusingalsu 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.

Looks very good, minor things to change!

Comment threadsrc/audio/volume/volume.c Outdated
Comment threadsrc/audio/volume/volume.c Outdated
Comment threadsrc/audio/volume/volume.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated
Comment threadsrc/audio/volume/volume_hifi3.c Outdated

@lgirdwoodlgirdwood left a comment

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.

@andrula-song looks like we are almost done, but we do need that alignment API as I would imagine other will be doing the same thing. @singalsu any comment ?

Comment threadsrc/audio/volume/volume.c Outdated
Comment threadsrc/audio/volume/volume.c Outdated

@lgirdwoodlgirdwood left a comment

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.

Just need to remove the alignment check and we are good.

Comment threadsrc/audio/volume/volume.c Outdated
Processing continuous data. Asking a 8-byte aligned memory
to store volume gain 4 times, using 2-way or 4-way pipeline
operations of xtensa intrinsics. This method can reduce about
40% MCPS than the original one.
Signed-off-by: Andrula Song <xiaoyuan.song@intel.com>
@lgirdwood
lgirdwood merged commit 3eaf974 into thesofproject:mainJan 27, 2022
@andrula-song
andrula-song deleted the vol-opt branch March 23, 2022 09:05
Sign up for freeto 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.

5 participants

@andrula-song@cujomalainey@lgirdwood@lyakh@singalsu