thesofproject / sof

Sound Open Firmware
Other
540 stars 309 forks source link

[BUG] THD value is larger than acceptable value #4180

Open wszypelt opened 3 years ago

wszypelt commented 3 years ago

Describe the bug Issue was found in: [ICL] 11_03_TestSrcSamplingRateChange16000Hz32b32b8ch Repro: 1/75 11_03_TestSrcSamplingRateChange48000Hz32b32b4ch Repro: 1/75 [APL] 11_03_TestSrcSamplingRateChange48000Hz32b32b4ch Repro: 1/75

Topology

                plb_ppl
                +--------------------------------+
    +------+    | +---+  +- -+  +---+  +-------+ |
    | Host |----->|Buf|->|Src|->|Buf|->|SSP Dai|---+
    +------+    | +---+  +-- +  +---+  +-------+ | |
                +--------------------------------+ |
                                                   |
                cap_ppl                            |
                +------------------+               |
    +------+    | +---+  +-------+ |               |
    | Host |<-----|Buf|<-|SSP Dai|<----------------+
    +------+    | +---+  +-------+ |
                +------------------+

To Reproduce Run tests with Diagnostic driver: 11_03_TestSrcSamplingRateChange16000Hz32b32b8ch 11_03_TestSrcSamplingRateChange48000Hz32b32b4ch With options --playback-iterations=75

Reproduction Rate 1/75

Environment 1) Branch name and commit hash of the 2 repositories: sof (firmware/topology) and linux (kernel driver).

Screenshots or console output ICL 11_03_TestSrcSamplingRateChange16000Hz32b32b8ch image

ICL 11_03_TestSrcSamplingRateChange48000Hz32b32b4ch image

APL 11_03_TestSrcSamplingRateChange48000Hz32b32b4ch image

ICL_11_03_16000Hz.zip ICL_11_03_48000Hz.zip APL_11_03.zip

lgirdwood commented 3 years ago

@wszypelt this looks like a DMA R/W race, but it's really difficult to tell which pipeline and whether it's related to ASRC or SRC. Can you try and isolate this further. I would also like to see if a volume on capture pipeline fixes this and bridges the DMAs.

wszypelt commented 3 years ago

@lgirdwood I edited the bug and left only the SRC pipeline. It should be noted that with these tests, we convert various input formats on one topology and there is 1 output format. Simultaneous playback and capture on external loopback (same SSP) with signal verification. Playback pipeline contains SRC component and plays audio formats with different frequencies. Subsequent input formats are run without destroying the current topology. I'm not sure this answers your question. If not, please ask.

lgirdwood commented 3 years ago

Duplicate of simpler https://github.com/thesofproject/sof/issues/4076

kkarask commented 3 years ago

Glitches are visible in the capture stream when SRC component is in playback and capture pipeline.

Tests which failed: 11_02_TestSrcSimpleCapture48000On16000Hz32b32b4ch and 11_01_TestSrcSimple16000On48000Hz32b32b4ch witch reproduction rate 1/75.

lgirdwood commented 3 years ago

@kkarask can you alos test this with #4471 locally. Thanks, we think they may all be related.

kkarask commented 3 years ago

@lgirdwood I run tests with SRC component with #4471 and found fails:

lgirdwood commented 3 years ago

Thanks again @kkarask , so for this topology it reduces the reproduction rate, but not full fix.

lgirdwood commented 3 years ago

@kkarask do you know the buffer/period size used by the diagnostic driver on the host side, if we were to see glitches in the first 2 - 5 ms this could be explained by FW (as it uses 1ms buffers), but these glitches could be related to the size of host side buffers.

kkarask commented 3 years ago

@lgirdwood Buffer size depends on audio format and is different in each test. For test 11_03_TestSrcSamplingRateChange16000Hz32b32b8ch buffer_size = 1024 11_03_TestSrcSamplingRateChange48000Hz32b32b4ch buffer_size = 1536

lgirdwood commented 3 years ago

@lgirdwood Buffer size depends on audio format and is different in each test. For test 11_03_TestSrcSamplingRateChange16000Hz32b32b8ch buffer_size = 1024

So this looks very small for a 32bit sample size * 8 channels host buffer. i.e. 1 frame is 256 bytes, so we only get 4 frames in the buffer or 240uS

Do you mean 1024 frames instead ?? This would give us 15.625 ms per buffer.

11_03_TestSrcSamplingRateChange48000Hz32b32b4ch buffer_size = 1536

Ditto as above, could this be in frames ?

Btw, do you also know how these buffer are sent via DMA, i.e. how many chunks/periods are the buffers split into when programming the DMA ?

kkarask commented 3 years ago

@lgirdwood Sorry, it was my mistake. I thought about buffer between components when I wrote above values.
I have checked host side buffer and it is equal to 1 MB in all tests.

lgirdwood commented 3 years ago

@lgirdwood Sorry, it was my mistake. I thought about buffer between components when I wrote above values. I have checked host side buffer and it is equal to 1 MB in all tests.

Thanks 1MB makes sense. Do you know how this 1MB is partitioned into DMA descriptors ? The DMA wont send this a single 1MB chunk, but will split up the buffer to smaller chunks. We need to know the DMA chunk size for each use case (or how it is worked out).

wszypelt commented 3 years ago

@lgirdwood As far as I know it works like this: Diagnostic buffer is big because we handle it in test code (in python) We do it by checking the buffer in time intervals. not in interrupts.

This is irrelevant for the operation of the FW as we configure the HDA DMA via the diagnostic driver. The FW creates the input and output buffers and also configures the buffer.

If we want to send something via HDA DMA. We have our buffer, divide it into 2 parts and fill it with data from files.

After that, the process is that the HDA DMA will successively fetch the data from the buffer and we have to ensure that this data is delivered there all the time.

By dividing the buffer in half. If half of the buffer has been copied and it starts working on the second part, new data can be written to the first part.

When there is a reading, we check HDA DMA pointers.

It looks like this. We read from the read pointer to the write pointer (the write pointer that was set by HDA DMA) then update the read pointer to where we read the data.

I hope this is the answer to your question If not, let me know please.