thesofproject
Tools: Topology2: Add other rates support for DMIC and HDA
There was a mistake in previous PR version with sof-mtl-max98360a-rt5682 PCM IDs. DMIC0 should be 99 (it was 100) and I decided to set DMIC1 PCM ID to 100. The PCM name in all cases is "DMIC Raw2nd".
With sof-mtl-sdw-cs42l42-l0-max98363-l2 and HDA generic 2ch or 4ch the added DMIC1 capture PCM ID is default 11. It's the same as with wake-on-voice, but ultrasound and WoV should be mutually exclusive for DMIC1.
@jsarha @plbossart @RanderWang Do these names and IDs loook OK?
There was a mistake in previous PR version with sof-mtl-max98360a-rt5682 PCM IDs. DMIC0 should be 99 (it was 100) and I decided to set DMIC1 PCM ID to 100. The PCM name in all cases is "DMIC Raw2nd".
With sof-mtl-sdw-cs42l42-l0-max98363-l2 and HDA generic 2ch or 4ch the added DMIC1 capture PCM ID is default 11. It's the same as with wake-on-voice, but ultrasound and WoV should be mutually exclusive for DMIC1.
@jsarha @plbossart @RanderWang Do these names and IDs loook OK?
Name is good for me. It is for pass-through pipeline, we can rename it for real case.
for SoundWire, best to follow the list I came up with a couple of years ago...
Device 11 is "just" for 16khz, for KPB/WOV it's a different device really...
for SoundWire, best to follow the list I came up with a couple of years ago...
Device 11 is "just" for 16khz, for KPB/WOV it's a different device really...
Thanks, I'll change it to PCM22 for 96 kHz capture. That's very useful plan for other applications too.
@singalsu sorry if I am perceived as the painful reviewer, but I have to ask a background technical question:
In a recent thread (https://github.com/thesofproject/sof/issues/8834), @perexg mentioned that "The limitation (on some hardware) is that the speaker is usually connected to one DAC with a fixed rate (48000Hz) and the headphones to other (more rates up to 192kHz are available). "
So my question is "do we know for sure if the 96kHz rate is supported on the 'analog' endpoint on all HDAudio hardware platforms"? If not, then we would need some sort of hardware discovery at the kernel level to filter/supersede topology definitions.
I don't mind if we add 96kHz for 'development' topologies, I just wonder if this will remain an experimental capability for the foreseeable future or if we can also add it to mainstream HDaudio topologies at some point.
@wszypelt @lrudyX good to merge ? I dont think these files are tested by internal CI today ?
@singalsu sorry if I am perceived as the painful reviewer, but I have to ask a background technical question:
In a recent thread (#8834), @perexg mentioned that "The limitation (on some hardware) is that the speaker is usually connected to one DAC with a fixed rate (48000Hz) and the headphones to other (more rates up to 192kHz are available). "
So my question is "do we know for sure if the 96kHz rate is supported on the 'analog' endpoint on all HDAudio hardware platforms"? If not, then we would need some sort of hardware discovery at the kernel level to filter/supersede topology definitions.
I don't mind if we add 96kHz for 'development' topologies, I just wonder if this will remain an experimental capability for the foreseeable future or if we can also add it to mainstream HDaudio topologies at some point.
I don't know. I can't find from this specification that 96 kHz would be a mandatory feature of all HD audio codecs. It only describes how 96 kHz data is transmitted. Does someome know? https://www.intel.com/content/dam/www/public/us/en/documents/product-specifications/high-definition-audio-specification.pdf
But I'd assume that the the driver would error if 96 kHz would be attempted on a non-96 kHz capable system. Also this patch enables non-48 kHz only for test topologies in development folder. They are not part of FW release.
@lgirdwood @singalsu Unfortunately, Intel Internal CI shows an error in the tools step: make tools cmake
I don't know. I can't find from this specification that 96 kHz would be a mandatory feature of all HD audio codecs. It only describes how 96 kHz data is transmitted. Does someome know? https://www.intel.com/content/dam/www/public/us/en/documents/product-specifications/high-definition-audio-specification.pdf
48kHz seems to be a mandatory feature, since it's the baseline for the transmission based on 20.83us.
Other frequencies are derived with MULT/DIV bitfields in SDnDMT registers
There's wording in section 5.4.1 "Table 56 itemizes all the samples rates defined in (but not necessarily required by) in the HDaudio specification", so clearly not all devices will support all MULT/DIV values.
Section 7.3.4.7 tells us the 'Supported Rates' parameters returns a bitfield with the information. R7 (48kHz) must be supported by all codecs. All other bits can be left unset.
So the answer is: this is platform/codec dependent.
Thinking a bit more on this, maybe it's ok to have a topology which allows for additional rates beyond 48kHz. We could filter this at the kernel level by leaving the topology with all desired rates, and then at the codec driver level prevent non-supported streams from being configured at the hw_params stage.
The point is that we MUST always support 48kHz on all PCM devices.
@kv2019i @ujfalusi should double-check that what I am saying is correct.
But I'd assume that the the driver would error if 96 kHz would be attempted on a non-96 kHz capable system. Also this patch enables non-48 kHz only for test topologies in development folder. They are not part of FW release.
Just to clarify, is the topology going to support both 48+96kHz, or just 96kHz? The latter case is problematic, I have no real issues with the former.
@lgirdwood @singalsu Unfortunately, Intel Internal CI shows an error in the tools step: make tools cmake
Is the alsa-utils up to date there? It needs to have recent changes, commit 906a56f9ff136211e53db201383a47d9b0ee445d.
Just to clarify, is the topology going to support both 48+96kHz, or just 96kHz? The latter case is problematic, I have no real issues with the former.
The changes common to all (HDA) topology definitions cause no issues to build topologies for 48 kHz like we today build. The rate is just set explicitly in formats in addition to word lengths to enable changing the rate for PCM0 and in DMIC case the PCMs for it (DMIC0 and DMIC1). For simplicity there is no SRC placeholder and macros for multiple rates. So when build for 96 kHz is defined, I'm afraid this is " just 96kHz". I was asked to make this example for HDA and DMIC. I didn't want to make anything complicated, just something that allowed colleagues at Intel to further define this for customer.
Can you draw a picture of how these ultrasound development enabler topologies should look like ("both 48+96kHz").
Just to clarify, is the topology going to support both 48+96kHz, or just 96kHz? The latter case is problematic, I have no real issues with the former.
The changes common to all (HDA) topology definitions cause no issues to build topologies for 48 kHz like we today build. The rate is just set explicitly in formats in addition to word lengths to enable changing the rate for PCM0 and in DMIC case the PCMs for it (DMIC0 and DMIC1). For simplicity there is no SRC placeholder and macros for multiple rates. So when build for 96 kHz is defined, I'm afraid this is " just 96kHz". I was asked to make this example for HDA and DMIC. I didn't want to make anything complicated, just something that allowed colleagues at Intel to further define this for customer.
Can you draw a picture of how these ultrasound development enabler topologies should look like ("both 48+96kHz").
Now that I think of it, we can only support multiple rates if we remove the mixer. When we have a mixer in place we have to pick ONE sample rate, otherwise it's a mess if different streams with different rates get mixed.
But assuming we remove the deep buffer for playback (which no one uses), then we can make the entire pipeline support multiples of 48kHz, as done for SSP/BT. If the hardware doesn't support a rate, that would be rejected by the codec DAI so there's a clear error sent by to apps.
Just to clarify, is the topology going to support both 48+96kHz, or just 96kHz? The latter case is problematic, I have no real issues with the former.
The changes common to all (HDA) topology definitions cause no issues to build topologies for 48 kHz like we today build. The rate is just set explicitly in formats in addition to word lengths to enable changing the rate for PCM0 and in DMIC case the PCMs for it (DMIC0 and DMIC1). For simplicity there is no SRC placeholder and macros for multiple rates. So when build for 96 kHz is defined, I'm afraid this is " just 96kHz". I was asked to make this example for HDA and DMIC. I didn't want to make anything complicated, just something that allowed colleagues at Intel to further define this for customer. Can you draw a picture of how these ultrasound development enabler topologies should look like ("both 48+96kHz").
Now that I think of it, we can only support multiple rates if we remove the mixer. When we have a mixer in place we have to pick ONE sample rate, otherwise it's a mess if different streams with different rates get mixed.
But assuming we remove the deep buffer for playback (which no one uses), then we can make the entire pipeline support multiples of 48kHz, as done for SSP/BT. If the hardware doesn't support a rate, that would be rejected by the codec DAI so there's a clear error sent by to apps.
Removing the deep buffer from these 96 kHz topologies builds should be possible, I'll check it and update.
Removing the deep buffer from these 96 kHz topologies builds should be possible, I'll check it and update.
Actually the deep buffer is already disabled with DEEPBUFFER_FW_DMA_MS=false. I just forgot about it, sorry.
DEEPBUFFER_FW_DMA_MS=false
Does this actually disable the deep buffer path or just default to the regular buffer size for the deep-buffer path?
that's pretty bad...
DEEPBUFFER_FW_DMA_MS=false
Does this actually disable the deep buffer path or just default to the regular buffer size for the deep-buffer path?
that's pretty bad...
The first one, everything related to deep buffer is removed. See here: https://github.com/thesofproject/sof/blob/6f33aee2709ac619c4913a231f40e5b27d4390a5/tools/topology/topology2/cavs-mixin-mixout-hda.conf#L16
DEEPBUFFER_FW_DMA_MS=false
Does this actually disable the deep buffer path or just default to the regular buffer size for the deep-buffer path? that's pretty bad...
The first one, everything related to deep buffer is removed. See here:
https://github.com/thesofproject/sof/blob/6f33aee2709ac619c4913a231f40e5b27d4390a5/tools/topology/topology2/cavs-mixin-mixout-hda.conf#L16
wow. I thought IncludeByKey was using a true/false variable. I didn't know it could be used for integers. Oh well, you live and learn.
Ping @singalsu -- this pending ack from @plbossart and CI check is failing.
Ping @singalsu -- this pending ack from @plbossart and CI check is failing.
@kv2019i Thanks for following this. The fail is because is CI at IPG has not updated their alsa-utils. I've reported it and they are working on it ( @wszypelt ).
I rebased and pushed this again to see if it works now. There's no changes to "code".
@wszypelt I assume this is good for internal CI as AFAIK none of the changes here are tested by Internal CI ?
@lgirdwood @singalsu we managed to fix alsa-utils in CI, everything works :)