[bkokoszx]

This commit adds CHANNELS define in sof-tgl-rt711-i2s-rt1308.m4
topology in order to properly set pipeline pcm channels and dmic
configuration (STEREO_PDM0 or FOUR_CH_PDM0_PDM1 mode).

Fixes #2445.

Signed-off-by: Bartosz Kokoszko bartoszx.kokoszko@linux.intel.com

[lgirdwood]

CI jenkins result is unrelated and looks like CI issue. @fredoh9 @xiulipan fyi

[xiulipan]

@lgirdwood Indeed, someone installed another version sof-logger to the DUT and we used wrong one.

[plbossart]

This doesn't seem very consistent, see e.g. cml_rt700.m4:

# dmic16k (ID: 2)
DAI_CONFIG(DMIC, 1, 3, dmic16k,
	   DMIC_CONFIG(1, 500000, 4800000, 40, 60, 16000,
		DMIC_WORD_LENGTH(s16le), 400, DMIC, 1,
		PDM_CONFIG(DMIC, 1, STEREO_PDM0)))

Edit: it's the same on ICL RVP., tgl with RT1308 w/ I2S, etc.

I'd rather get an agreement on what we do with the dmic16k input and apply a consistent settting.

[lgirdwood]

@plbossart do you know if the use case for 4 channel (requirement ?), or is demand being artificially created by validation testing ?

[plbossart]

@plbossart do you know if the use case for 4 channel (requirement ?), or is demand being artificially created by validation testing ?

I have no idea @lgirdwood. usually 16khz data is mono or stereo, if we have to provide the 4channels it would mean 4ch pre-processing is applied on the host?

@singalsu what do you think?

[singalsu]

I don't know what's the requirement. Sure 4ch gives better test coverage for platform but if the target device is with two microphones four is unnecessary. And if there are devices with four microphones using stereo topology would be wrong. Is the microphones count in the BIOS so the machine driver could get the correct 2ch or 4ch topology?

Unrelated to this patch it would be better to use topologies with additional IIR high-pass for DMIC paths, not the volume capture pipeline. It's because of long time not fixed shortcoming in the HW DC blocker filter. The HW can cancel only a low amount of DC (individual cheap type microphones with higher DC can exceed the capability) and the settling time is very slow due to another HW issue. Also please setup DMIC into 32 bit mode, not 16 bit. When there's IIR and/or volume control in the pipeline the end user quality is much better than with 16 bit DMIC FIFO. There's e.g. 40 Hz high-pass for 48 kHz and 16 kHz pipeline with 0 dB or 20 dB gain available.

(I experimented in PR #2434 passing the filter blob name from topology top level via pipeline macro. If people like the idea I could make a generic PR.)

Also why is there no volume control in 16 kHz capture pipe?

[plbossart]

I don't know what's the requirement. Sure 4ch gives better test coverage for platform but if the target device is with two microphones four is unnecessary. And if there are devices with four microphones using stereo topology would be wrong. Is the microphones count in the BIOS so the machine driver could get the correct 2ch or 4ch topology?

@sathya-nujella @naveen-manohar do you guys know if the TGL firmware contains any NHLT information? That's the only thing we can use to automatically detect the number of microphones.

[plbossart]

Unrelated to this patch it would be better to use topologies with additional IIR high-pass for DMIC paths, not the volume capture pipeline. It's because of long time not fixed shortcoming in the HW DC blocker filter. The HW can cancel only a low amount of DC (individual cheap type microphones with higher DC can exceed the capability) and the settling time is very slow due to another HW issue. Also please setup DMIC into 32 bit mode, not 16 bit. When there's IIR and/or volume control in the pipeline the end user quality is much better than with 16 bit DMIC FIFO. There's e.g. 40 Hz high-pass for 48 kHz and 16 kHz pipeline with 0 dB or 20 dB gain available.

(I experimented in PR #2434 passing the filter blob name from topology top level via pipeline macro. If people like the idea I could make a generic PR.)

Also why is there no volume control in 16 kHz capture pipe?

@singalsu you are the man on all things DMIC. Feel free to change things as needed, be it filters or volume controls. We'll just merge blindly :-)

[lgirdwood]

@singalsu please also capture these items as feature requests.

[sathya-nujella]

@sathya-nujella @naveen-manohar do you guys know if the TGL firmware contains any NHLT information? That's the only thing we can use to automatically detect the number of microphones.

Sorry for late..
1)
In Chromium, CRAS introduced CaptureChanelMap.
So, what we are doing is: Independent of DUT having 1/2/4-DMIC's..in kernel/topology, always configuring DMIC to 4_channel. As UCM specific to DUT, in UCM Capture Channel map, we are specifying what channels are valid.
An example, a DUT has two DMIC's:
SectionDevice."Internal Mic".0 {
Value {
CapturePCM "hw:"
CaptureChannelMap "0 1 -1 -1 -1 -1 -1 -1 -1 -1 -1"
}
}CaptureChannelMap
Chanel map definition..
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/common/cras_audio_format.h#50
2)
After moving to SOF, Corbeoot stopped configuring NHLT data.

[sathya-nujella]

@plbossart do you know if the use case for 4 channel (requirement ?), or is demand being artificially created by validation testing ?

I have no idea @lgirdwood. usually 16khz data is mono or stereo, if we have to provide the 4channels it would mean 4ch pre-processing is applied on the host?

@singalsu what do you think?

I don't know in certain why they are changing here, but what to add some discussion what I am aware.. there was an ask to support stereo WoV/hotword from customers. If both DMIC's are on PDM_0, then dmic16 with STEREO_PDM0 configuration in tplg will work. If both DMIC's are on PDM_1, then dmic16 with STEREO_PDM1 configuration will work.
Some ODM came back and asked .. can I connect one DMIC to PDM_0 and one DMIC to PDM_1.. can your stereo hotword/WoV be supported? At that time we told NO to ODM/OEM/Hardware teams.. and we told power may be a concern and they fixed in HW.
They thought it is a SW limitation.. So, may be here we need guidance ..

[singalsu]

@lgirdwood I've created two related issues and I can create also one for mandatory FW DCblock or IIR for DMIC capture. It's a significant effort task to fix all topologies (the testing effort) so better to have an issue created.

[singalsu]

@sathya-nujella Did they have a reason to connect one microphone to PDM0 and other to PDM1 or was it just some not well thought decision? We have the channel selector component and a minor enhance into it would make it work with more than 2ch.

The coming beamformer component also will have capability to select any 1-X channels from X channel stream for processing, mix output to any Y channels and ignore input channels and duplicate outputs if configured so.

[sathya-nujella]

@sathya-nujella Did they have a reason to connect one microphone to PDM0 and other to PDM1 or was it just some not well thought decision?

Some ODM's want to put 2-DMIC's: one near Camera on top of display panel and the other one at bottom of display panel.. at that time: they will pick one on DMIC_CLK1 and the other one on DMIC_CLK2 so that easy to wire it. But now, after we told not possible on stereo WOV if they do like that, they fixed it on H/W side.

We have the channel selector component and a minor enhance into it would make it work with >more than 2ch.
Please share any PR or details for us to read through/subscribe.

The coming beamformer component also will have capability to select any 1-X channels from X channel stream for processing, mix output to any Y channels and ignore input channels and duplicate outputs if configured so.

Good to know. Any pointers to read through for this and for subscribe to.

[lgirdwood]

@bkokoszx the long term fix is the multi DAI config, but for short term could we have a channel argument passed into the topology generation that can build in either 2 chan or 4 chan DMIC support depending on the end user requirement.
@plbossart @sathya-nujella any objections for short term fix above ?

[plbossart]

@bkokoszx the long term fix is the multi DAI config, but for short term could we have a channel argument passed into the topology generation that can build in either 2 chan or 4 chan DMIC support depending on the end user requirement.
@plbossart @sathya-nujella any objections for short term fix above ?

That's be fine, this is what we do for HDaudio - we have no dmic, 2 dmics and 4 dmic topology files, and select one dynamically. The dynamic part is TBD but if we have the files it's a kernel issue we can deal with later.

[jajanusz]

@bkokoszx the long term fix is the multi DAI config, but for short term could we have a channel argument passed into the topology generation that can build in either 2 chan or 4 chan DMIC support depending on the end user requirement.
@plbossart @sathya-nujella any objections for short term fix above ?

You can go with solution like this:

sof/tools/topology/CMakeLists.txt

Line 35 in b4e8b1e

"sof-hda-generic-idisp\;sof-hda-generic-idisp-4ch\;-DCHANNELS=4"

[sathya-nujella]

@bkokoszx the long term fix is the multi DAI config, but for short term could we have a channel argument passed into the topology generation that can build in either 2 chan or 4 chan DMIC support depending on the end user requirement.
@plbossart @sathya-nujella any objections for short term fix above ?

You can go with solution like this:

sof/tools/topology/CMakeLists.txt

Line 35 in b4e8b1e

"sof-hda-generic-idisp\;sof-hda-generic-idisp-4ch\;-DCHANNELS=4"

Yes @jajanusz , got it. Thanks for reference.

[monocodus]

This is autogenerated code-style review, all good, approving

[monocodus]

This is autogenerated code-style review, all good, approving

[monocodus]

This is autogenerated code-style review, all good, approving

[bkokoszx]

@lgirdwood @jajanusz
I've updated this PR by adding CHANNEL define. If you approve, please wait for test results under issue #2445 before merging.

[lgirdwood]

@bkokoszx will do, can you remind me when the test passes.

[bkokoszx]

@lgirdwood
As I saw under #2445 that test passed and there is no issues. If you do not have other remarks, it could be merged.

[marc-hb]

This caused the following regression, discuss in PR #2954

[ 8%] Generating sof-tgl-rt711-i2s-rt1308-nohdmi.conf

m4:tools/topology/sof/pipe-volume-capture-16khz.m4:54: bad expression
in eval (bad input): 2 * 2 * CHANNELS * 16
m4:tools/topology/sof/pipe-volume-capture-16khz.m4:57: bad expression
in eval (bad input): 2 * 2 * CHANNELS * 16