Repository navigation
[BUG] Byt low latency pipeline produces distortion with S24/S32 playback #1487
Description
Describe the bug
byt low latency playback pipe he:0,0 produces distortion with s24/s32 playback
To Reproduce
play s32 or s24 audio to hw:0,0
for e.g. like this in script file (play.sh 48000 test16bit.wav):
#!/bin/sh
FS=$1
BITS=32
ENDIAN=little
NCH=2
RAW=_tmp.raw
DEV=hw:0,0
sox "$2" -c $NCH -r $FS -b $BITS --endian $ENDIAN $RAW
time -p aplay -v -D$DEV -t raw -c $NCH -f S32_LE -r $FS $RAW --dump-hw-params
rm $RAW
Reproduction Rate
Always
Expected behavior
Clean sound
Impact
showstopper
Environment
- latest kernel, latest firmware
- topology:sof-byt-rt5682.tplg
- platform: byt
- always
Screenshots or console output
we have gaps around every 30ms
Activity
juimonen commentedon May 29, 2019
so obviously we have some buffer wrap issue somewhere...
juimonen commentedon May 29, 2019
btw the strange thing here is that I can play s24/s32 48khz just fine to media pipe hw:0,1...
lgirdwood commentedon May 29, 2019
@juimonen what does aplay -v show for both pipelines as this may relate to host period size.
juimonen commentedon May 31, 2019
@tlauda also this looks like some buffer wrap issue, but I think this is different to #1482? #1482 we are leaving stuff out and "catenating" buffer's together (so actually playback is shorter than it supposed to be). In this we are playing as long as it should be, but missing pieces from the waveform.
tlauda commentedon May 31, 2019
@juimonen Could you verify if the mixer uses the right processing function (mix_n_s32)?
juimonen commentedon Jun 3, 2019
@tlauda s32/s24 playback to hw:0,0 uses s32 mixer function (and produces distortion). "normal" aplay to hw:0,0 uses s16 mixer function. s32/s24 playback to hw:0,1 seems to use also s32 mixer function (but doesn't produce distortion).
juimonen commentedon Jun 3, 2019
@tlauda if I "rewind" fw code to 9be2694 before dw-dma overhaul and apply your a720164cd5ae4c4e656e0cb7d74b199cd9f3ee1a on top of it ->s32 playback to hw:0,0 works...
juimonen commentedon Jun 3, 2019
@tlauda sorry meant your (I cherry picked it): 5d554c5
tlauda commentedon Jun 3, 2019
@juimonen Can you bisect the faulty commit?
juimonen commentedon Jun 3, 2019
@tlauda attached verbose logs of s32 playback to hw00 (distortion) and hw01 (clean)
test_play_s32_00.txt
test_play_s32_01.txt
juimonen commentedon Jun 3, 2019
@tlauda bisected and the commit to break s24/s32 is e2b994e
juimonen commentedon Jun 3, 2019
@tlauda there's something really dodgy in the platform_dw_dma_set_transfer_size macro.... the old one before cleanup produces sane buffer size values... the new macro produces something like overflowed pointer values... I'm not totally sure I understand how this should work. If I change it the "old way": (lli->ctrl_hi = DW_CTLH_CLASS(class) | (size & DW_CTLH_BLOCK_TS_MASK)) things are working...
juimonen commentedon Jun 3, 2019
@tlauda so I mean I don't think you can replace a = b | c with a |= b and a |= c
tlauda commentedon Jun 3, 2019
Fixed by #1487.