Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG][BYT][CHT] Output channel left/right swap on various BYT/CHT devices with various codecs #3699

Closed
jwrdegoede opened this issue Dec 18, 2020 · 17 comments · Fixed by #3777
Assignees
Labels
bug Something isn't working as expected BYT Applies to Baytrail platform channel swap L/R channel swap is observed for stereo output or input CHT Applies to Cherry Trail platform
Milestone

Comments

@jwrdegoede
Copy link
Contributor

Describe the bug
Because of the plan to drop the old SST Linux driver for BYT/CHT devices, replacing it with the SOF driver, I have been testing the Linux SOF driver on various BYT and CHT devices.

Things mostly work well, but there is one bug when using the SOF driver which is not present when using the SST driver. Sometimes the left and right playback channels are swapped. This happens with both stereo speakers and headphones (as is to be expected).

To Reproduce
Run the GNOME control-panel speaker test (which allows testing the left/right speakers separately) on a Cherry Trail device. I've seen this on the following devices:

  • HP Pavilion X2 10-p002nd, Cherry Trail x5-Z8350 SoC with a rt5640 codec
  • Cube iWork8 Air, Cherry Trail x5-Z8300 SoC with a nuvoton nau88l24i codec (swap only noticeable with headphones as this device has only 1 speaker)

Reproduction Rate
50/50

Expected behavior
Hitting the "Left speaker" button in the GNOME control-panel speaker test produces sound on the left speaker,
(and the same for the right button + speaker.

Impact
I'm planning on submitting an official Fedora change for flipping the default sound driver on BYT/CHT platforms to the SOF driver to help with testing the switch to SOF so that the SST driver can be dropped (lowering the maintenance burden) sometime during 2021. The deadline for submitting a Fedora change for this is Jan 19th 2021. For me to be able to move ahead with this, this issue needs to be resolved in at least the SOF master branch before the deadline.

Environment

  1. Branch name and commit hash of the 2 repositories: sof (firmware/topology) and linux (kernel driver).
    • Kernel: 5.10 + asoc/for-5.11 branch merged into it with snd_intel_dspcfg.dsp_driver=3
    • SOF: v1.6 (Nov 19) version
  2. Name of the topology file
    • Topology: Seen on multiple CHT devices, all using the standard sof-byt-codec.m4 based tplg files
  3. Name of the platform(s) on which the bug is observed.
    • Platform: CHT

Observations

  1. If I let the "soundcard" powerdown by not using it for at least 10 seconds and then try again this may fix the swap, or it may introduce it. IOW the coinflip deciding if the swap is there or not happens every time the card is powered-on.
  2. I have a feeling that this might not necessarily be CHT specific but rather SSP2 specific. To confirm or deny this I need to test SOF on a BYT device using SSP2. I will report back when I have done that.
@jwrdegoede jwrdegoede added the bug Something isn't working as expected label Dec 18, 2020
@kv2019i
Copy link
Collaborator

kv2019i commented Dec 18, 2020

@jwrdegoede Thanks for testing! We have one bug open for channel swapping for BYT:
#3520

@lgirdwood lgirdwood added this to the v1.7 milestone Dec 18, 2020
@lgirdwood
Copy link
Member

Fwiw, this could be some extra step that is needed in the SSP or DMA shutdown so that the DMA/SSP state is consistent on stream start. The DMA IP on CHT/BYT has is the same IP as on other DSPs (Designware) but it has a slightly different IP configuration so some programming flows may need to be different.

@keyonjie
Copy link
Contributor

@jwrdegoede can you confirm if the issue happen for S24/S32 only, and works fine with S16 format? if so, maybe remove the ssp_empty_rx_fifo() might help.

@lgirdwood
Copy link
Member

@keyonjie @plbossart I'm thinking this is a step or register programming order that we are missing in the SSP or DMA shutdown logic (but is being corrected by the subsequent stream start), hence why it works every second playback

@jwrdegoede
Copy link
Contributor Author

@keyonjie @plbossart I'm thinking this is a step or register programming order that we are missing in the SSP or DMA shutdown logic (but is being corrected by the subsequent stream start), hence why it works every second playback

Note AFAIK it does not work every second playback. It works about 50/50 but it is like flipping a coin not every other playback. Next time I'm testing this I will pay extra attention to this but I'm pretty sure it is NOT every other / second playback.

@jwrdegoede
Copy link
Contributor Author

@jwrdegoede can you confirm if the issue happen for S24/S32 only, and works fine with S16 format? if so, maybe remove the ssp_empty_rx_fifo() might help.

I'll try to make some time to test this soonish.

@keyonjie
Copy link
Contributor

@keyonjie @plbossart I'm thinking this is a step or register programming order that we are missing in the SSP or DMA shutdown logic (but is being corrected by the subsequent stream start), hence why it works every second playback

Note AFAIK it does not work every second playback. It works about 50/50 but it is like flipping a coin not every other playback. Next time I'm testing this I will pay extra attention to this but I'm pretty sure it is NOT every other / second playback.

thanks @jwrdegoede what you described are expected result to me. @lgirdwood it is not related with SSP/DMA shutdown logic, it is because of the flushing doesn't work on BYT/CHT as the capture/playback can't be stopped separately there.

@jwrdegoede
Copy link
Contributor Author

@jwrdegoede can you confirm if the issue happen for S24/S32 only, and works fine with S16 format? if so, maybe remove the ssp_empty_rx_fifo() might help.

I just hit this issue on a BYTCR device using SSP0 connected to AIF1 of a RT5640 codec. SSP0 always uses S16_LE format, so this means that:

  1. My hunch that this was related to using SSP2 was wrong, this happens with both SSP0 and SSP2
  2. This happens with both S24_SE and S16_LE formats
  3. This happens on both CHT and BYT platforms.

@jwrdegoede jwrdegoede changed the title [BUG][CHT] Output channel left/right swap on various CHT devices with various codecs [BUG][BYT][CHT] Output channel left/right swap on various BYT/CHT devices with various codecs Dec 23, 2020
@lyakh
Copy link
Collaborator

lyakh commented Jan 14, 2021

thanks @jwrdegoede what you described are expected result to me. @lgirdwood it is not related with SSP/DMA shutdown logic, it is because of the flushing doesn't work on BYT/CHT as the capture/playback can't be stopped separately there.

@keyonjie can you expand a bit on that? Do you know which flushing is insufficient? SSP or DMA?

@lgirdwood
Copy link
Member

@lyakh it seems that the flipping is random and if so, could be a race between enabling DMA and SSP e.g. enabling certain SSP bits wrt DMA channel enable (like wise on disable flow too).

@plbossart
Copy link
Member

@lyakh it seems that the flipping is random and if so, could be a race between enabling DMA and SSP e.g. enabling certain SSP bits wrt DMA channel enable (like wise on disable flow too).

one would hope that the DMA is armed first, and the FIFO full when the SSP is enabled?

@lyakh
Copy link
Collaborator

lyakh commented Jan 15, 2021

@lgirdwood @plbossart @keyonjie BYT is one of several (legacy) platforms, where CONFIG_HW_LLI isn't available. So can it be that the clean up in this case isn't complete?

@lgirdwood
Copy link
Member

@lyakh check the reference I sent you as the correct flow.

@lyakh
Copy link
Collaborator

lyakh commented Jan 18, 2021

@jwrdegoede Hi Hans! Interesting that you're saying, that it's only happening after 10 seconds, since SOF on CHT and BYT doesn't support runtime-PM... I can reproduce the problem but with a much lower success rate than 50 / 50. But what I did hear a couple of times: I use speaker-test -c 2 -t sine -D hw:0,0. It always begins playing with the left channel. If I interrupt it when it's playing the right channel, sometimes when I then restart it, I hear a short tone in the right channel. That probably happens when there is a large enough and even number of samples "stuck" from the previous run. And I was able to reproduce it when restarting quickly, so it doesn't seem to be related to any timeouts, delayed work, etc.

@jwrdegoede
Copy link
Contributor Author

@lyakh I'm testing with a full-blown GNOME desktop using pulseaudio, it takes 10 seconds for pulseaudio to close the audio-device after the last app stops using it.

So it seems to me that the coin-toss for swapping gets done on every opening of a playback-stream. This also seems to match with your experience when repeatedly running speaker-test.

@lyakh
Copy link
Collaborator

lyakh commented Jan 18, 2021

@jwrdegoede same here - Ubuntu with PA. And the speaker-test uses PA too. But I don't see PA holding sound devices open after speaker-test terminates. Maybe it's the settings application that does that?

@lyakh
Copy link
Collaborator

lyakh commented Jan 18, 2021

@jwrdegoede could you attach your amixer contents output here, please?

@mengdonglin mengdonglin added BYT Applies to Baytrail platform channel swap L/R channel swap is observed for stereo output or input CHT Applies to Cherry Trail platform labels Jan 19, 2021
lyakh added a commit to lyakh/sof that referenced this issue Jan 22, 2021
Commit 05e1c26 ("byt-ssp: fixes for DSP modes") removed a
work-around for known hardware bugs, causing channel swapping
in I2S mode. Restore the work-around but make sure it's only
enabled in I2S and LEFT_J modes.

Fixes: thesofproject#3699, thesofproject#3520

Signed-off-by: Guennadi Liakhovetski <[email protected]>
keyonjie pushed a commit that referenced this issue Jan 25, 2021
Commit 05e1c26 ("byt-ssp: fixes for DSP modes") removed a
work-around for known hardware bugs, causing channel swapping
in I2S mode. Restore the work-around but make sure it's only
enabled in I2S and LEFT_J modes.

Fixes: #3699, #3520

Signed-off-by: Guennadi Liakhovetski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected BYT Applies to Baytrail platform channel swap L/R channel swap is observed for stereo output or input CHT Applies to Cherry Trail platform
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants