Enable SOF on Broadwell/Samus (bdw-rt5677) #1659

plbossart · 2020-01-04T02:15:40Z

before we even look at SOF support on Broadwell, the current mailine code is broken with the legacy driver (w/ legacy BIOS and Ubuntu installed with chrx)

I was able to restore functionality with the patches in this PR, but there's clearly a problem registering the SPI component.

Without the last patch which essentially removes the SPI/Hotword support, I see errors such as

[   73.454937] bdw-rt5677 bdw-rt5677: ASoC: no sink widget found for DSP Capture
[   73.454940] bdw-rt5677 bdw-rt5677: ASoC: Failed to add route DSP Buffer -> direct -> DSP Capture

[   21.791961] bdw-rt5677 bdw-rt5677: ASoC: CPU DAI spi-RT5677AA:00 not registered

[   15.467040] bdw-rt5677 bdw-rt5677: snd_soc_add_dai_link start
[   15.467042] bdw-rt5677 bdw-rt5677: ASoC: binding System PCM
[   15.467044] bdw-rt5677 bdw-rt5677: ASoC: sanity check done System PCM
[   15.467057] bdw-rt5677 bdw-rt5677: snd_soc_add_dai_link end
[   15.467058] bdw-rt5677 bdw-rt5677: snd_soc_add_dai_link start
[   15.467059] bdw-rt5677 bdw-rt5677: ASoC: binding Codec DSP
[   15.467061] bdw-rt5677 bdw-rt5677: ASoC: platform component spi-RT5677AA:00 not found for Codec DSP
[   15.467064] bdw-rt5677 bdw-rt5677: ASoC: sanity check failed Codec DSP, ret -517

@cujomalainey what is needed to probe this SPI thingy? Could this be a legacy BIOS issue?

@xiulipan @fredoh9 @ranj063 FYI.

cujomalainey · 2020-01-04T02:44:45Z

@plbossart I cc'ed you on a patch a few hours ago to alsa that should fix the issue with regards to the SPI links.

plbossart · 2020-01-06T17:23:24Z

@plbossart I cc'ed you on a patch a few hours ago to alsa that should fix the issue with regards to the SPI links.

The patch doesn't help, I have the Kconfig selected but somehow the SPI probe fails. There is something missing either in my configuration or the SPI probe stuff.

This is with the seabios/legacy mode installed with chrx.

[   19.365810] bdw-rt5677 bdw-rt5677: bdw_rt5677_probe entry
[   19.365814] bdw-rt5677 bdw-rt5677: bdw_rt5677_probe end
[   19.365817] bdw-rt5677 bdw-rt5677: snd_soc_add_dai_link start
[   19.365818] bdw-rt5677 bdw-rt5677: ASoC: binding System PCM
[   19.365820] bdw-rt5677 bdw-rt5677: ASoC: sanity check done System PCM
[   19.365833] bdw-rt5677 bdw-rt5677: snd_soc_add_dai_link end
[   19.365834] bdw-rt5677 bdw-rt5677: snd_soc_add_dai_link start
[   19.365836] bdw-rt5677 bdw-rt5677: ASoC: binding Codec DSP
[   19.365837] bdw-rt5677 bdw-rt5677: ASoC: platform component spi-RT5677AA:00 not found for Codec DSP
[   19.365839] bdw-rt5677 bdw-rt5677: ASoC: sanity check failed Codec DSP, ret -517

cujomalainey · 2020-01-06T20:26:28Z

Is your bios defining the SPI port? ID RT5677AA?

plbossart · 2020-01-06T20:53:59Z

Is your bios defining the SPI port? ID RT5677AA?

Yes it's defined in the DSDT and I just checked it's copy/pasted from the Coreboot parts.
I must be missing a CONFIG, likely for the SPI controller. We don't have any dependency for SPI as we do for I2C_DESIGNWARE, so something must be missing.

cujomalainey · 2020-01-06T21:00:08Z

Looks related to ID INT3430 which is found in these drivers, which reminds me of when I had to fix the bug in the spi bus after suspend/resume issues.

drivers/acpi/acpi_lpss.c
drivers/spi/spi-pxa2xx.c

plbossart · 2020-01-06T21:47:20Z

Looks related to ID INT3430 which is found in these drivers, which reminds me of when I had to fix the bug in the spi bus after suspend/resume issues.

drivers/acpi/acpi_lpss.c
drivers/spi/spi-pxa2xx.c

I got it, it's indeed a miss with the SPI_MASTER and SPI_PXA2XX dependencies not modeled. Fixed now with the update.

plbossart · 2020-01-06T21:52:19Z

well not quite, I saw one error with DMAs now:

[   53.154287] haswell-pcm-audio haswell-pcm-audio: initialising Audio DSP IPC
[   53.154302] haswell-pcm-audio haswell-pcm-audio: initialising audio DSP id 0x3438
[   53.164474] haswell-pcm-audio haswell-pcm-audio: error: DMA device register failed
[   53.164478] haswell-pcm-audio haswell-pcm-audio: sst_dma_new failed -22
[   53.165332] bdw-rt5677 bdw-rt5677: bdw_rt5677_probe entry
[   53.165335] bdw-rt5677 bdw-rt5677: bdw_rt5677_probe end
[   53.165338] bdw-rt5677 bdw-rt5677: snd_soc_add_dai_link start
[   53.165340] bdw-rt5677 bdw-rt5677: ASoC: binding System PCM
[   53.165342] bdw-rt5677 bdw-rt5677: ASoC: platform component haswell-pcm-audio not found for System PCM
[   53.165344] bdw-rt5677 bdw-rt5677: ASoC: sanity check failed System PCM, ret -517
ro

@cujomalainey this reminds me of an error reported by others on the Chrome team?

cujomalainey · 2020-01-06T22:06:37Z

Ah yes I remember those errors with the uprev team. That appears to be slightly different iirc. It was a transient failure on samus, it was on buddy that it was consistent. Does rebooting solve it? Also the only solution to date afaik is still to revert that really old sst D3 crash prevention patch.

plbossart · 2020-01-06T22:11:11Z

Ah yes I remember those errors with the uprev team. That appears to be slightly different iirc. It was a transient failure on samus, it was on buddy that it was consistent. Does rebooting solve it? Also the only solution to date afaik is still to revert that really old sst D3 crash prevention patch.

I can't reproduce it so far on Samus after rebooting so it does look like a transient. I guess now that I've seen it I would be more inclined to revert this code that no one is able to explain anyways.

cujomalainey · 2020-01-06T22:17:21Z

Looked up the old bug, that actually is the exact error message we were seeing on samus, so that is definitely the bug

The existing machine driver depends on SPI Master capabilities, but the Kconfig does not model this dependency and the SPI controller needs to be selected as well. Without this patch the machine driver probe would fail with the spi-RT5677AA:00 component never registered by the ACPI/LPSS subsystem. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The legacy driver uses dummy cpu_dai and platform, SOF requires actual values to bind. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

plbossart · 2020-01-06T23:02:52Z

@cujomalainey @fredoh9 @xiulipan with these patches I get nice sound on Samus :-)

sound/soc/intel/boards/bdw-rt5677.c

ranj063 · 2020-01-06T23:13:06Z

sound/soc/intel/boards/bdw-rt5677.c

@plbossart are you possibly missing the virtual widgets for the other three "AIF1 Capture", "AIF1 Playback" and "DSP Buffer"?

AIF1 playback/capture are on the codec side, so are defined by the codec driver.

I have no idea what the 'DSP Buffer' is

DSP Buffer is for the codecs internal DSP power graph.

@plbossart That was strange, I redo the test without this patch on samus with the latest tplg from https://github.com/thesofproject/sof/blob/c68605403c52ac43ac1b97dec780b9407026ec7f/tools/topology/sof-bdw-codec.m4 and it works without error.

dnl CODEC is defined and will be expanded, need to undefine it before use undefine(`CODEC') VIRTUAL_WIDGET(SSP0 CODEC OUT, output, 0) VIRTUAL_WIDGET(SSP0 CODEC IN, input, 1) VIRTUAL_WIDGET(DSP Capture, input, 2)

@xiulipan thanks for testing, indeed this is not needed, I was using an old topology file :-(

Fixed now

sound/soc/intel/boards/bdw-rt5677.c

plbossart · 2020-01-06T23:55:30Z

Instructions to reproduce my setup:

install Ubuntu with chrx
copy firmware and topology files over to /lib/firmware
copy UCM file from https://github.com/plbossart/UCM/tree/master/sof-bdw-rt5677 to /usr/share/alsa/ucm

For now playback works fine in PulseAudio with jack detection and switching between speakers and headphones.

I haven't tested the capture at all.

plbossart · 2020-01-06T23:58:44Z

plbossart · 2020-01-07T00:17:26Z

@cujomalainey @juimonen can you take a look at the UCM file at https://github.com/plbossart/UCM/tree/master/sof-bdw-rt5677?

I can get capture to work fine but the switch between internal mic and headset mic is not automatic. I wonder if this is supported by PulseAudio or if there is a way to express an action to be taken on headset plug? Thanks!

cujomalainey · 2020-01-07T00:23:45Z

evtest reports the jack name as "bdw-rt5677 Mic Jack" try changing that

plbossart · 2020-01-07T00:42:20Z

evtest reports the jack name as "bdw-rt5677 Mic Jack" try changing that

doesn't help. evtest does dectest sof-bdw-rt5677 Headphone and Mic Jack, but if I use 'JackControl "sdw-bdw-rt5677 Headphone Jack"' then the automatic switch is broken for playback, and the value of the string has no impact for capture. Must be a PulseAudio restriction maybe?

cujomalainey · 2020-01-07T00:53:12Z

is the jack being detected in amixer? if it is then I would leave it with pulse

xiulipan · 2020-01-07T02:49:22Z

@plbossart I will update #1484 to cover only sound/soc/intel/boards/broadwell.c with the same change style here.

lgirdwood

I've no other comments to add other than those already discussed. I'm assuming this works for XPS13 too ?

plbossart · 2020-01-07T15:23:52Z

I think we are good to merge, the only issues I see for now are

left/right inversion. This could be an SSP issue or a codec issue.
headset automatic detection in PulseAudio

Considering that Samus never worked before, this is small potatoes to fix.

lgirdwood · 2020-01-07T15:53:43Z

I think we are good to merge, the only issues I see for now are

Ack

left/right inversion. This could be an SSP issue or a codec issue.

I'm assuming this is consistent rather than random ?

Fwiw, could be the FRAME_POLARITY logic needs negated, I do remeber running into this in the last few years but I cant remember the platform.

plbossart · 2020-01-07T17:01:16Z

left/right inversion. This could be an SSP issue or a codec issue.

I'm assuming this is consistent rather than random ?

yes, it's not the result of an underflow, I have a consistent inversion both on speakers and headphones.

But I am not 100% it's firmware, the UCM file I took from Chrome folks has weird settings such as

		cset "name='Stereo DAC MIXL ST L Switch' off"
		cset "name='Stereo DAC MIXL DAC1 L Switch' off"
		cset "name='Stereo DAC MIXL DAC2 L Switch' off"
		cset "name='Stereo DAC MIXL DAC1 R Switch' on" <<< ???

		cset "name='Stereo DAC MIXR ST R Switch' off"
		cset "name='Stereo DAC MIXR DAC1 R Switch' off"
		cset "name='Stereo DAC MIXR DAC2 R Switch' off"
		cset "name='Stereo DAC MIXR DAC1 L Switch' on" <<<???

Fwiw, could be the FRAME_POLARITY logic needs negated, I do remeber running into this in the last few years but I cant remember the platform.

yes, that'd be my next check. I have a Samus device wired with the SSP pins blue-wired so I'll try to capture the signals with my LogicPro when I have a bit of time.

cujomalainey · 2020-01-07T22:41:26Z

sound/soc/intel/boards/bdw-rt5677.c

+	    DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
+#else
+SND_SOC_DAILINK_DEF(ssp0_port,
+	    DAILINK_COMP_ARRAY(COMP_DUMMY())));


One too many brackets there. Fails to compile

@xiulipan @fredoh9 I thought we compiled in SST mode? We have a kconfig-sst.sh, isn't this used?

Travis did catch the error
https://travis-ci.org/thesofproject/linux/jobs/633817565#L2610

plbossart requested a review from lgirdwood as a code owner January 4, 2020 02:15

plbossart force-pushed the fix/bdw-rt5677 branch from 5e18bbb to d16a77f Compare January 6, 2020 21:46

plbossart added 2 commits January 6, 2020 17:01

ASoC: Intel: bdw-rt5677: change cpu_dai and platform components for SOF

fb03cd4

The legacy driver uses dummy cpu_dai and platform, SOF requires actual values to bind. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

plbossart force-pushed the fix/bdw-rt5677 branch from c8c1e62 to 7b822a2 Compare January 6, 2020 23:01

plbossart changed the title ~~[TEST] Fix bdw rt5677 in legacy mode~~ Enable SOF on Broadwell/Samus (bdw-rt5677) Jan 6, 2020

cujomalainey reviewed Jan 6, 2020

View reviewed changes

sound/soc/intel/boards/bdw-rt5677.c Outdated Show resolved Hide resolved

ranj063 reviewed Jan 6, 2020

View reviewed changes

sound/soc/intel/boards/bdw-rt5677.c Show resolved Hide resolved

lgirdwood previously approved these changes Jan 7, 2020

View reviewed changes

plbossart dismissed lgirdwood’s stale review via fb03cd4 January 7, 2020 15:20

plbossart force-pushed the fix/bdw-rt5677 branch from 7b822a2 to fb03cd4 Compare January 7, 2020 15:20

plbossart requested review from cujomalainey, lgirdwood, ranj063 and xiulipan January 7, 2020 15:22

ranj063 approved these changes Jan 7, 2020

View reviewed changes

cujomalainey approved these changes Jan 7, 2020

View reviewed changes

plbossart merged commit 8d23d03 into thesofproject:topic/sof-dev Jan 7, 2020

cujomalainey reviewed Jan 7, 2020

View reviewed changes

cujomalainey mentioned this pull request Feb 29, 2020

[RFC] Resolve DMA boot bug in BDW in SST driver #1842

Closed

Enable SOF on Broadwell/Samus (bdw-rt5677) #1659

Enable SOF on Broadwell/Samus (bdw-rt5677) #1659

Uh oh!

Conversation

plbossart commented Jan 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cujomalainey commented Jan 4, 2020

Uh oh!

plbossart commented Jan 6, 2020

Uh oh!

cujomalainey commented Jan 6, 2020

Uh oh!

plbossart commented Jan 6, 2020

Uh oh!

cujomalainey commented Jan 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

plbossart commented Jan 6, 2020

Uh oh!

plbossart commented Jan 6, 2020

Uh oh!

cujomalainey commented Jan 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

plbossart commented Jan 6, 2020

Uh oh!

cujomalainey commented Jan 6, 2020

Uh oh!

plbossart commented Jan 6, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

plbossart commented Jan 6, 2020

Uh oh!

plbossart commented Jan 6, 2020

Uh oh!

plbossart commented Jan 7, 2020

Uh oh!

cujomalainey commented Jan 7, 2020

Uh oh!

plbossart commented Jan 7, 2020

Uh oh!

cujomalainey commented Jan 7, 2020

Uh oh!

xiulipan commented Jan 7, 2020

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart commented Jan 7, 2020

Uh oh!

lgirdwood commented Jan 7, 2020

Uh oh!

plbossart commented Jan 7, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

plbossart commented Jan 4, 2020 •

edited

Loading

cujomalainey commented Jan 6, 2020 •

edited

Loading

cujomalainey commented Jan 6, 2020 •

edited

Loading