[BUG][MTL] SSP File is completely silent 24in32bit #8086
Description
Describe the bug
Issue was created after merging 87856a7
Request for revert and repair
Issue with I2S with config 24in32bit
ERROR:SSP File is completely silent (5s_i2sssp_1_24000_4_24_32_buffers_1_1_1_1.wav). Average samples amplitude: 0.25%.
To Reproduce
pytest -s "tests/ace/fw_113_ssp/test_02_ssp_internal_loop.py::TestSspInternalLoop::test_113_02_simple_internal_loopback[24000Hz_24in32bit_4ch-ssp_1_to_ssp_1]"
Reproduction Rate
100%
Expected behavior
File not silent
Impact
all I2S tests with config 24in32bit
Environment
-
- SOF: 87856a7
- Platform: MTL
Topology
"""
OBJECTIVE:
SSP (I2S mode) internal loopback test.
TEST STEPS:
1. Load FW.
2. Create topology with audio format and SSP instance as in title:
- Host Output Copier->I2S TX Copier (internal loopback enabled)
- I2S RX Copier->Host Input Copier (internal loopback enabled)
3. Run pipelines and streams (5 seconds).
4. Verify amount of unhandled notification - proper value is 0.
5. Perform SNR comparison verification method of played and
recorded file and verify it meets quality criteria.
PASS CRITERIA:
All steps passed successfully
PIPELINES TOPOLOGY::
+--------------------------------+
| COPIER_HOST < -- COPIER_SSP-+ |
| | |
| | |
| COPIER_HOST -- > COPIER_SSP-+ |
+--------------------- ----------+
"""
Screenshots or console output
Logs:
i2s_1_24000_4_24_32_buffers_1_1_1_1.zip
grabbed_fw_logs_0.txt
Activity
mengdonglin commentedon Aug 24, 2023
@RanderWang Can you help give a check?
wszypelt commentedon Aug 24, 2023
@mengdonglin @RanderWang CI is not working properly because of this issue, please revert, then fix the errors
[-][BUG][MTL] SSP File is completely silent 24in32bit[/-][+][BUG][MTL] SSP File is completely silent 24in32bit [/+]Revert "pcm:_converter: simplify the pcm conversion algorithm"
wszypelt commentedon Aug 25, 2023
after the revert, the error does not occur
RanderWang commentedon Aug 28, 2023
@lgirdwood @marcinszkudlinski @kv2019i @wszypelt I made great effort and finally found the root reason.
The pipelines are showed in the following graph. All the 4 copiers use 24/32 MSB sample type.
The original code choose conversion function : convert_s32_to_s24, copy_stream, copy_stream, convert_s24_to_s32. (this conversion is incorrect for single playback and only work for loop back test but it was not detected by CI test)
My PR change it to : copy_stream, copy_stream, copy_stream, copy_stream. This works on Linux tests but is failed with the CI test. Why ?
l did many tests then found it was caused by SSP setting
SOF:
ffffff10 c1d0077f d0700004 00000000
02200000 00000003 00000003 00004002
00000000 07070000 00000020 00110001
00000fff
CI test:
00000000 81D00F37 C0700004 00000000
02990000 00000103 00000103 00004002
00000000 07070F00 00000020 00000000
00000000
SOF use 32bits frame width for SSP port but CI use 25 bit frame width with 1 dummy bits. So if converting stream with copy_stream function the MSB 8bits will be clamped by CI blob, but it works with SOF blob. The original code is very lucky to workaround this CI setting.
SOF use 32bits frame width which can deal with different formats and this makes Linux kernel driver easy to set I2S codec to use 32bits format for all cases. How to deal with this CI blob setting ? The real usage on Windows, Linux and chrome don't use this blob, do we need to hack FW to support it ?
lgirdwood commentedon Aug 28, 2023
@wszypelt @mwasko fyi, need to align SSP blobs with CI. Can CI use upstream blobs ? i.e. we upstream good blobs and align on BKC.
kv2019i commentedon Aug 28, 2023
Great investigation @RanderWang ! So it sounds like the CI copier configuration should use "32 bit, 24 valid, LSB" for the DAI copiers and then the 25bit SSP blob is correct. Now the copier config and SSP blob mismatch. I assume this passes with reference firmware as it ignores requested copier config, and override the value. But this isn't really good, host should send correct copier configuration.
mwasko commentedon Aug 28, 2023
@dnikodem FYI
mwasko commentedon Aug 30, 2023
@lgirdwood after internal discussion we decided that both SSP blob configurations need to be supported - there are production use-cases. However, to support 25bit SSP blob (24 bit + 1 dummy) the Copier format config must be set correctly (LSB bit) in CI tests.
@wszypelt is coordinating CI tests update.
wszypelt commentedon Sep 4, 2023
@RanderWang @lgirdwood
We changed the tests and I checked them on FW with changes from #7959,
The tests pass correctly :) However, I have a lot of problems to make changes to the Internal Intel CI, I don't know how long it will take, in this case I will withdraw the 24in32bit I2S test from the Internal Intel CI, and when we deal with the service, I will add the corrected one.
I believe that you can already merge the changes that were in the above pull request (7959).
RanderWang commentedon Sep 5, 2023
@wszypelt thanks very much !
mengdonglin commentedon Sep 8, 2023
Closing as PR #8151 has been merged. @RanderWang @wszypelt