thesofproject / sof

Sound Open Firmware
Other
547 stars 313 forks source link

[BUG] SSP waveform error when fclk width > sample width #2356

Open zrombel opened 4 years ago

zrombel commented 4 years ago

Describe the bug Bug regards configuration of FCLK pulse with of SSP interface. When setting frame_pulse_width > sample bit depth (for example sample bit depth = 16 and frame_pulse_width = 17, or sample bit depth = 32 and frame_pulse_width = 33) SSP wave forms are generated incorrectly: FCLK frequency changes and data bits are shifted. Bug occurs only when channel count is greater then 1 and is sample rate independent.

Example waveforms:

Audio Format: 8kHz 16/16b 2ch Frame pulse width: 1 Bclk: 512kHz Fclk: 8kHz Waveform: Correct image

Audio Format: 8kHz 16/16b 2ch Frame pulse width: 16 Bclk: 512kHz Fclk: 8kHz Waveform: Correct image

Audio Format: 8kHz 16/16b 2ch Frame pulse width: 17 Bclk: 512kHz Fclk: 6.4kHz Waveform: Incorrect image

Audio Format: 8kHz 32/32b 2ch Frame pulse width: 1 Bclk: 1.2MHz Fclk: 8kHz Waveform: Correct image

Audio Format: 8kHz 32/32b 2ch Frame pulse width: 32 Bclk: 1.2MHz Fclk: 8kHz Waveform: Correct image

Audio Format: 8kHz 32/32b 2ch Frame pulse width: 33 Bclk: 1.2MHz Fclk: 6.593kHz Waveform: Incorrect image

To Reproduce Python tests 01_06_TestPcm1bWaveformVerification with frame_pulse_width set accordingly to tested audio format.

Reproduction Rate 100%

Environment FW branch: Master, commit: 0469a018ba856b0fd96b042c4011301e988d0f8f and earlier.

Logs 8kHz16b2ch1fpw.zip 8kHz16b2ch16fpw.zip 8kHz16b2ch17fpw.zip 8kHz32b2ch1fpw.zip 8kHz32b2ch32fpw.zip 8kHz32b2ch33fpw.zip

jajanusz commented 4 years ago

@zrombel I guess it's on current master, right?

zrombel commented 4 years ago

Right. Found on master branch commit 0469a018ba856b0fd96b042c4011301e988d0f8f but it seems it's present in FW since beginning.

plbossart commented 4 years ago

IIRC there was a hardware "feature" that the pulse has to be limited to the first slot in master mode. @lbetlej does this ring a bell? We had such requirements a long time ago from our automotive friends, I am not sure the SSP supports e.g. a TDM mode with a long pulse (or even half cycle)

lgirdwood commented 4 years ago

Pushed to v1.6 until further comments can be aligned.

lgirdwood commented 4 years ago

Ok, I guess we can live with this. Re-assign priority and milestone unless anyone objects.