Audio: Mixin_mixout: Add HiFi5 implementation. #8793
Conversation
|
here is the xtensa simulator test result: |
andrula-song
force-pushed
the
mixin_hifi5
branch
from
d07aece to
61dfc3c
Compare
There was a problem hiding this comment.
LGTM. @singalsu ?
@andrula-song whats still pending for non draft ?
andrula-song
requested review from
marc-hb,
plbossart,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
There was a problem hiding this comment.
Looks great @andrula-song ! You can update to the new-style HIFI ifdef @marc-hb and @btian1 added, but I'm ok to add this in a follow-up PR as well.
yeah, I would like to use a follow-up PR after #8791 merged. |
|
@andrula-song can you check CI. Thanks |
|
@andrula-song #8791 now merged, pls rebase. |
Add HiFi5 implementation of mix functions, compared with HiFi3 version, can reduce about 27% cycles. Signed-off-by: Andrula Song <[email protected]>
andrula-song
force-pushed
the
mixin_hifi5
branch
from
61dfc3c to
06ce478
Compare
|
@andrula-song ready ? |
|
hi @wszypelt , can you help to trigger the internal CI again? Seems failed on build. |
| AE_LA32X2X2_IP(in_sample, in_sample1, inu, in); | ||
| AE_LA32X2X2_IP(out_sample, out_sample1, outu1, out); | ||
| out--; | ||
| out_sample = AE_ADD24S(in_sample, out_sample); |
There was a problem hiding this comment.
Has this mix code been been verified? The ALSA S24_LE is aligned to least significant bits while ae_f24 in Xtensa is most significant bits aligned. In the past at least SOF was using the ALSA S24_LE. We used to have a mess with this and I'm no more sure if we have discarded the 24 bit ALSA format to be compatible with Windows OS's 24 bit format. Is the previous generic or HiFi3 mixing like this, if yes this would be OK. Changing 24 bit assumptions is another discussion and apparently we may support both in parts of code (cc @kv2019i ). But we should reject in prepare() unsupported 24 bit flavor.
There was a problem hiding this comment.
ok - having a reject message would be fine here until support for both 24bit formats is upstream. This message and 24 bit support can be added incrementally though since this is good for testing now and passing CI.
There was a problem hiding this comment.
I hope CI is testing the 24 bit case. Proposing to make another test patch that breaks mix_s24() function and see if it fails the CI run.
Add HiFi5 implementation of mix functions, compared with HiFi3 version, can reduce about 27% cycles.