[RFC SOF 0/2] topology: Try to add sof-cht-nau8824 topology file

[RFC SOF 0/2] topology: Try to add sof-cht-nau8824 topology file

[Hans de Goede]

Hi All,

I have veen working on making the cht_bsw_nau8824 machine
driver work together with the sof-audio-acpi driver.

The sof topology files are missing a .tplg file for this setup.
Simply copying over the standard sof-byt-codec.tplg file does
the trick, but then some mixer setting changes are necessary
to fix the right speaker/headphones channel not working.
Specifically I need to change the DACR channel setting from CH1 to
CH0. But that mixer setting breaks the right channel when used with
the sst-acpi driver.

The problem seems to be that the codec and ucm-mixer settings
expect DSP_B TDM 4 channel 24 bit format, but the standard
sof-byt-codec.tplg file specifies I2S 2 channel 24 bit format.

I tried changing the kernel to program the codec and SSP2
CPU DAIs both at I2S 2 channel 24 bit format to match what
most other machine drivers are doing. This causes the no sound
on right speaker channel to reproduce with the SST driver too.
And like with the SOF driver it can be fixed by changing the
mixer setting. With this kernel changes the SOF driver also
still needs the mixer-setting fix.

So I can get things to work, and make SOF/SST behave consistent
by switching both DAIs to I2S 2 channel 24 bit format and
then changing the mixer settings.

The problem is that this will cause regressions unless users
update their kernel and UCM in sync...

So I tried to instead change the tplg file so that SOF will
use DSP_B TDM 4 channel 24 bit format on the SSP2 DAI side,
so that it matches what SST is doing and we don't need to
change the mixer settings.

This RFC series reflects my attempts at switching the SSP2
DAI to DSP_B TDM 4 channel 24 bit format when using SOF,
unfortunately it does not work, the resulting tplg file
causes the following errors:

[   21.537432] sof-audio-acpi 808622A8:00: Firmware info: version 1:6:0-18fab
[   21.537447] sof-audio-acpi 808622A8:00: Firmware: ABI 3:17:0 Kernel ABI 3:18:0
[   25.003836] sof-audio-acpi 808622A8:00: Topology: ABI 3:17:0 Kernel ABI 3:18:0
[   25.216003] sof-audio-acpi 808622A8:00: error: ipc error for 0x80010000 size 12
[   25.216060] sof-audio-acpi 808622A8:00: error: failed to set DAI config for SSP2-Codec index 2
[   25.216070] sof-audio-acpi 808622A8:00: error: failed to save DAI config for SSP2
[   25.216080] sof-audio-acpi 808622A8:00: ASoC: physical link loading failed
[   25.216089] sof-audio-acpi 808622A8:00: ASoC: topology: could not load header: -22
[   25.217459] sof-audio-acpi 808622A8:00: error: tplg component load failed -22
[   25.217643] sof-audio-acpi 808622A8:00: error: failed to load DSP topology -22

Which I'm not getting when using the default sof-byt-codec.tplg file.

Possible ways to fix this which I can think of are:

1. Fix SOF and/or the tplg file so that it can do
DSP_B TDM 4 channel 24 bit on BYT/CHT.

2. Switch the machine driver over the I2S 2 channel 24 bit, to align it
with other machine drivers; and update the UCM files to match. Preferably
together with some string in the components string advertised by the kernel
so that the UCM files can be written so that they also keep working with
older kernels.

3. Keep using DSP_B TDM 4 channel 24 bit with sst-acpi and switch to
I2S 2 channel 24 bit with sof-audio-acpi; and update the UCM file to
set the mixer settings accordingly depending on which driver is in use.

I'm not sure what the best way to proceed is. To go with solution 1
I'm going to need some help, since I'm stuck there. I find solution 2
tempting because I like the idea of standardising all BYT/CHT machine
drivers on using I2S 2 channel 24 bit format between the SSP DAI and
the codec DAI. But this means that people upgrading their kernel
might see a regression until they update their UCM files too.

Regards,

Hans

[RFC SOF 1/2] topology: Add sof-byt-codec-tdm4.m4 file

[Hans de Goede]

Some BYT/CHT boards (mostly Cherry Trail) use TDM 4 slots 24 bit as
wire format to the codec, rather then standard I2S 2 channel 24 bit.

Add a new m4 file for this. This is a copy of sof-byt-codec.m4 with
the following changes:

@@ -1,4 +1,4 @@
-`# Topology for generic' PLATFORM `board with' CODEC `on SSP' SSP_NUM
+`# Topology for generic' PLATFORM `board with' CODEC `on SSP' SSP_NUM `using TDM 4 slots 24 bit'

 # Include topology builder
 include(`utils.m4')
@@ -97,8 +97,8 @@
 # BE configurations - overrides config in ACPI if present
 #
 DAI_CONFIG(SSP, SSP_NUM, 0, SSP2-Codec,
-	   SSP_CONFIG(I2S, SSP_CLOCK(mclk, 19200000, codec_mclk_in),
-		      SSP_CLOCK(bclk, 2400000, codec_slave),
+	   SSP_CONFIG(DSP_B, SSP_CLOCK(mclk, 19200000, codec_mclk_in),
+		      SSP_CLOCK(bclk, 4800000, codec_slave),
 		      SSP_CLOCK(fsync, 48000, codec_slave),
-		      SSP_TDM(2, 25, 3, 3),
+		      SSP_TDM(4, 25, 3, 3),
 		      SSP_CONFIG_DATA(SSP, SSP_NUM, 24)))

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 tools/topology/sof-byt-codec-tdm4.m4 | 104 +++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)
 create mode 100644 tools/topology/sof-byt-codec-tdm4.m4

diff --git a/tools/topology/sof-byt-codec-tdm4.m4 b/tools/topology/sof-byt-codec-tdm4.m4
new file mode 100644
index 000000000..dabf8342b
--- /dev/null
+++ b/tools/topology/sof-byt-codec-tdm4.m4
@@ -0,0 +1,104 @@
+`# Topology for generic' PLATFORM `board with' CODEC `on SSP' SSP_NUM `using TDM 4 slots 24 bit'
+
+# Include topology builder
+include(`utils.m4')
+include(`dai.m4')
+include(`pipeline.m4')
+include(`ssp.m4')
+
+# Include TLV library
+include(`common/tlv.m4')
+
+# Include Token library
+include(`sof/tokens.m4')
+
+# Include DSP configuration
+include(`platform/intel/'PLATFORM`.m4')
+
+#
+# Define the pipelines
+#
+# PCM0 -----> volume -------v
+#                            low latency mixer ----> volume ---->  SSP2
+# PCM1 -----> volume -------^
+# PCM0 <---- Volume <---- SSP2
+#
+
+# Low Latency capture pipeline 2 on PCM 0 using max 2 channels of s32le.
+# 1000us deadline on core 0 with priority 0
+PIPELINE_PCM_ADD(sof/pipe-low-latency-capture.m4,
+	2, 0, 2, s32le,
+	1000, 0, 0,
+	48000, 48000, 48000)
+
+#
+# DAI configuration
+#
+# SSP port 2 is our only pipeline DAI
+#
+
+# playback DAI is SSP2 using 2 periods
+# Buffers use s24le format, 1000us deadline on core 0 with priority 1
+# this defines pipeline 1. The 'NOT_USED_IGNORED' is due to dependencies
+# and is adjusted later with an explicit dapm line.
+DAI_ADD(sof/pipe-mixer-dai-playback.m4,
+	1, SSP, SSP_NUM, SSP2-Codec,
+	NOT_USED_IGNORED, 2, s24le,
+	1000, 1, 0, SCHEDULE_TIME_DOMAIN_DMA,
+	2, 48000)
+
+# PCM Playback pipeline 3 on PCM 0 using max 2 channels of s32le.
+# 1000us deadline on core 0 with priority 0
+# this is connected to pipeline DAI 1
+PIPELINE_PCM_ADD(sof/pipe-host-volume-playback.m4,
+	3, 0, 2, s32le,
+	1000, 0, 0,
+	48000, 48000, 48000,
+	SCHEDULE_TIME_DOMAIN_DMA,
+	PIPELINE_PLAYBACK_SCHED_COMP_1)
+
+# PCM Playback pipeline 4 on PCM 1 using max 2 channels of s32le.
+# 10ms deadline on core 0 with priority 0
+# this is connected to pipeline DAI 1
+PIPELINE_PCM_ADD(sof/pipe-host-volume-playback.m4,
+	4, 1, 2, s32le,
+	5000, 0, 0,
+	48000, 48000, 48000,
+	SCHEDULE_TIME_DOMAIN_DMA,
+	PIPELINE_PLAYBACK_SCHED_COMP_1)
+
+# Connect pipelines together
+SectionGraph."PIPE_NAME" {
+	index "0"
+
+	lines [
+		# PCM pipeline 3 to DAI pipeline 1
+		dapm(PIPELINE_MIXER_1, PIPELINE_SOURCE_3)
+		# PCM pipeline 4 to DAI pipeline 1
+		dapm(PIPELINE_MIXER_1, PIPELINE_SOURCE_4)
+
+	]
+}
+
+# capture DAI is SSP2 using 2 periods
+# Buffers use s24le format, 1000us deadline on core 0 with priority 0
+# this is part of pipeline 2
+DAI_ADD(sof/pipe-dai-capture.m4,
+	2, SSP, SSP_NUM, SSP2-Codec,
+	PIPELINE_SINK_2, 2, s24le,
+	1000, 0, 0, SCHEDULE_TIME_DOMAIN_DMA)
+
+
+# PCM definitions
+PCM_DUPLEX_ADD(PCM, 0, PIPELINE_PCM_3, PIPELINE_PCM_2)
+PCM_PLAYBACK_ADD(PCM Deep Buffer, 1, PIPELINE_PCM_4)
+
+#
+# BE configurations - overrides config in ACPI if present
+#
+DAI_CONFIG(SSP, SSP_NUM, 0, SSP2-Codec,
+	   SSP_CONFIG(DSP_B, SSP_CLOCK(mclk, 19200000, codec_mclk_in),
+		      SSP_CLOCK(bclk, 4800000, codec_slave),
+		      SSP_CLOCK(fsync, 48000, codec_slave),
+		      SSP_TDM(4, 25, 3, 3),
+		      SSP_CONFIG_DATA(SSP, SSP_NUM, 24)))
-- 
2.28.0

[RFC SOF 2/2] topology: Add sof-cht-nau8824 topology file

[Hans de Goede]

This uses the new sof-byt-codec-tdm4.m4 file, as the cht_bsw_nau8824
machine driver programs the link to TDM 4 slots 24 bit.

At first I thought it would be easier to just change the kernel to
use the standard I2S 2 channel 24 bit format as other codecs do,
but this causes problems with the right speaker/headphone channel
unless the chan mixer-control for this is changed from CH1 to CH0,
which requires UCM changes, so changing this at the kernel level will
break setups with older UCM files (and updating UCM will break older
kernels).

So we are stuck with the TDM 4 slots 24 bit format for these boards,
as we cannot change it without causing regressions.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 tools/topology/CMakeLists.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/topology/CMakeLists.txt b/tools/topology/CMakeLists.txt
index 57f908302..72f1c6ef4 100644
--- a/tools/topology/CMakeLists.txt
+++ b/tools/topology/CMakeLists.txt
@@ -63,6 +63,7 @@ set(TPLGS
 	"sof-byt-codec\;sof-cht-da7213\;-DCODEC=DA7213\;-DPLATFORM=cht-codec\;-DSSP_NUM=2"
 	"sof-byt-codec\;sof-cht-cx2072x\;-DCODEC=CX2072X\;-DPLATFORM=cht-codec\;-DSSP_NUM=2"
 	"sof-byt-codec\;sof-cht-es8316\;-DCODEC=ES8316\;-DPLATFORM=cht-codec\;-DSSP_NUM=2"
+	"sof-byt-codec-tdm4\;sof-cht-nau8824\;-DCODEC=NAU8824\;-DPLATFORM=cht-codec\;-DSSP_NUM=2"
 	"sof-cht-max98090\;sof-cht-max98090\;-DCODEC=MAX98090\;-DPLATFORM=cht-codec\;-DSSP_NUM=2"
 	"sof-cnl-rt274\;sof-cnl-rt274"
 	"sof-apl-tdf8532\;sof-apl-tdf8532"
-- 
2.28.0

Re: [RFC SOF 1/2] topology: Add sof-byt-codec-tdm4.m4 file

[Pierre-Louis Bossart]

Hi Hans,

> Some BYT/CHT boards (mostly Cherry Trail) use TDM 4 slots 24 bit as
> wire format to the codec, rather then standard I2S 2 channel 24 bit.
> 
> Add a new m4 file for this. This is a copy of sof-byt-codec.m4 with
> the following changes:
> 
> @@ -1,4 +1,4 @@
> -`# Topology for generic' PLATFORM `board with' CODEC `on SSP' SSP_NUM
> +`# Topology for generic' PLATFORM `board with' CODEC `on SSP' SSP_NUM `using TDM 4 slots 24 bit'
> 
>   # Include topology builder
>   include(`utils.m4')
> @@ -97,8 +97,8 @@
>   # BE configurations - overrides config in ACPI if present
>   #
>   DAI_CONFIG(SSP, SSP_NUM, 0, SSP2-Codec,
> -	   SSP_CONFIG(I2S, SSP_CLOCK(mclk, 19200000, codec_mclk_in),
> -		      SSP_CLOCK(bclk, 2400000, codec_slave),
> +	   SSP_CONFIG(DSP_B, SSP_CLOCK(mclk, 19200000, codec_mclk_in),
> +		      SSP_CLOCK(bclk, 4800000, codec_slave),
>   		      SSP_CLOCK(fsync, 48000, codec_slave),
> -		      SSP_TDM(2, 25, 3, 3),
> +		      SSP_TDM(4, 25, 3, 3),
>   		      SSP_CONFIG_DATA(SSP, SSP_NUM, 24)))

I don't think this change is necessary, the problem is probably that we 
still rely on the hard-coded 4 slots in the machine driver which was the 
default for the Atom/SST driver.

If I look at the cht_bsw_nau8824.c driver, the only place where there is 
a reference to 4 slots is in the cht_codec_init() function.

https://elixir.bootlin.com/linux/latest/source/sound/soc/intel/boards/cht_bsw_nau8824.c#L104

In other drivers, this was moved to the codec_fixup routine which is 
overridden/ignored when SOF is used.

see e.g.

https://elixir.bootlin.com/linux/latest/source/sound/soc/intel/boards/cht_bsw_max98090_ti.c

https://elixir.bootlin.com/linux/latest/source/sound/soc/intel/boards/cht_bsw_rt5645.c

If you follow the same pattern that should make this problem go away. 
When SOF is used we typically don't have any hard-coded format and 
whatever is defined in the topology is propagated to the codec driver.

Hope this helps
-Pierre

Re: [RFC SOF 1/2] topology: Add sof-byt-codec-tdm4.m4 file

[Hans de Goede]

Hi,

On 12/7/20 4:11 PM, Pierre-Louis Bossart wrote:
> Hi Hans,
> 
>> Some BYT/CHT boards (mostly Cherry Trail) use TDM 4 slots 24 bit as
>> wire format to the codec, rather then standard I2S 2 channel 24 bit.
>>
>> Add a new m4 file for this. This is a copy of sof-byt-codec.m4 with
>> the following changes:
>>
>> @@ -1,4 +1,4 @@
>> -`# Topology for generic' PLATFORM `board with' CODEC `on SSP' SSP_NUM
>> +`# Topology for generic' PLATFORM `board with' CODEC `on SSP' SSP_NUM `using TDM 4 slots 24 bit'
>>
>>   # Include topology builder
>>   include(`utils.m4')
>> @@ -97,8 +97,8 @@
>>   # BE configurations - overrides config in ACPI if present
>>   #
>>   DAI_CONFIG(SSP, SSP_NUM, 0, SSP2-Codec,
>> -       SSP_CONFIG(I2S, SSP_CLOCK(mclk, 19200000, codec_mclk_in),
>> -              SSP_CLOCK(bclk, 2400000, codec_slave),
>> +       SSP_CONFIG(DSP_B, SSP_CLOCK(mclk, 19200000, codec_mclk_in),
>> +              SSP_CLOCK(bclk, 4800000, codec_slave),
>>                 SSP_CLOCK(fsync, 48000, codec_slave),
>> -              SSP_TDM(2, 25, 3, 3),
>> +              SSP_TDM(4, 25, 3, 3),
>>                 SSP_CONFIG_DATA(SSP, SSP_NUM, 24)))
> 
> I don't think this change is necessary, the problem is probably that we still rely on the hard-coded 4 slots in the machine driver which was the default for the Atom/SST driver.
> 
> If I look at the cht_bsw_nau8824.c driver, the only place where there is a reference to 4 slots is in the cht_codec_init() function.
> 
> https://elixir.bootlin.com/linux/latest/source/sound/soc/intel/boards/cht_bsw_nau8824.c#L104
> 
> In other drivers, this was moved to the codec_fixup routine which is overridden/ignored when SOF is used.
> 
> see e.g.
> 
> https://elixir.bootlin.com/linux/latest/source/sound/soc/intel/boards/cht_bsw_max98090_ti.c
> 
> https://elixir.bootlin.com/linux/latest/source/sound/soc/intel/boards/cht_bsw_rt5645.c
> 
> If you follow the same pattern that should make this problem go away. When SOF is used we typically don't have any hard-coded format and whatever is defined in the topology is propagated to the codec driver.

As I explained in the cover, this effectively switches to I2S 2 channel 24 bit format,
which requires mixer-setting / UCM changes in case of the nau8824 codec. The codec
driver has this comment:

 * Configures a DAI for TDM operation. Only support 4 slots TDM.
 */
static int nau8824_set_tdm_slot(struct snd_soc_dai *dai,
        unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)

Which is not entirely true I did get playback to work with I2S 2 channel 24 bit
(I did not test recording yet) but playback only works after mixer setting
changes, which in turn break things under SST.

Please read the cover letter, which should explain the issue.

As mentioned there we could use I2S 2 channel 24 bit as we do everywhere
(I like standardizing on that) but then the UCM profile would need to
differentiate between the SST and SOF setups and adjust the mixer
accordingly to not regress SST setups.

Regards,

Hans