Unable to open hostless PCM device after introduction of commit - ASoC: Stop dummy from overriding hwparams
Hi Amadeusz,
On the product I am working on, a hostless PCM device is defined for purpose of activating CODEC driver to setup the path inside CODEC. So, CPU DAI and PCM Platform are defined to use dummy dai & DMA supplied by sound/soc/soc-utils.c.
After upgrading to newer kernel, hostless PCM device failed to open. After doing a bit of digging, the root cause is that dummy_dma_hardware is not set in dummy_dma_open() due to new conditional check logic introduced in this commit - 6c504663ba2ee2abeaf5622e27082819326c1bd4.
In order to fix problem I am encountering properly without regressing your scenario, I would like to get a better understanding of problem you were addressing. My understanding, from looking through other drivers under sound/soc, is that pcm hardware info is usually set by PCM platform/DMA drivers. For your scenario, do you have other component e.g CPU/CODEC DAI, set PCM hardware definition? I am not sure conditional check logic from 6c504663ba2ee2abeaf5622e27082819326c1bd4 guarantees that other component would be setting pcm hardware info. Appreciate if you can provide more insight to your scenario?
Thanks Patrick
On 11/18/2022 6:17 PM, Patrick Lai wrote:
Hi Amadeusz,
On the product I am working on, a hostless PCM device is defined for purpose of activating CODEC driver to setup the path inside CODEC. So, CPU DAI and PCM Platform are defined to use dummy dai & DMA supplied by sound/soc/soc-utils.c.
After upgrading to newer kernel, hostless PCM device failed to open. After doing a bit of digging, the root cause is that dummy_dma_hardware is not set in dummy_dma_open() due to new conditional check logic introduced in this commit - 6c504663ba2ee2abeaf5622e27082819326c1bd4.
In order to fix problem I am encountering properly without regressing your scenario, I would like to get a better understanding of problem you were addressing. My understanding, from looking through other drivers under sound/soc, is that pcm hardware info is usually set by PCM platform/DMA drivers. For your scenario, do you have other component e.g CPU/CODEC DAI, set PCM hardware definition? I am not sure conditional check logic from 6c504663ba2ee2abeaf5622e27082819326c1bd4 guarantees that other component would be setting pcm hardware info. Appreciate if you can provide more insight to your scenario?
Thanks Patrick
Hi Patrick,
Call path is: ... -> __soc_pcm_open() -> soc_pcm_components_open -> snd_soc_component_open -> open callback, where for dummy device open callback is dummy_dma_open.
Expanding on the issue in question which was cause of the patch.
With following debug log: diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c index e12f8244242b..b086ec05da25 100644 --- a/sound/soc/soc-component.c +++ b/sound/soc/soc-component.c @@ -290,6 +290,8 @@ int snd_soc_component_open(struct snd_soc_component *component, { int ret = 0;
+ pr_err("%s\n", component->name); + if (component->driver->open) ret = component->driver->open(component, substream);
that's what I get in dmesg on one of our test platforms: [ 95.522577] avs_rt274.1-platform [ 95.526019] i2c-INT34C2:00 [ 95.528837] snd_soc_core:dpcm_fe_dai_startup: audio: ASoC: open FE audio [ 95.528849] avs_rt274.1-platform [ 95.532249] snd-soc-dummy [ 95.534989] snd-soc-dummy [ 95.537800] snd_soc_avs:avs_dai_fe_startup: snd_soc_avs 0000:00:1f.3: avs_dai_fe_startup fe STARTUP tag 1 str 0000000064defd29
"avs_rt274.1-platform" component is handled in sound/soc/intel/avs/pcm.c it calls avs_component_open() which sets hwparams to generic set supported by i2s devices in AVS driver.
"i2c-INT34C2:00" is codec driver sound/soc/codecs/rt274.c it does not have open callback.
And finally "snd-soc-dummy" which as mentioned above calls dummy_dma_open which originally overridden hwparams set in avs_component_open() with its own limited ones.
(When topology is loaded it also creates FEs, which further limit allowed hwparams, they are a subset of the ones set above).
As mentioned in the patch: "Alternative approach would be to copy whole dummy handling and rename it to "snd-soc-null" or something similar. And remove hwparams assignment to make it really do nothing."
However, looking at it again, I would consider the existence of dummy_dma_open() to be scope creep. If component is really a dummy one it should not affect any other components in any way. And if any drivers depends on dummy setting parameters for it, I would consider it partially broken. And would say that issue should rather be fixed on driver side by making a dedicated component for it instead of using a dummy one.
I hope that I cleared up situation a bit.
Thanks, Amadeusz
On Mon, Nov 21, 2022 at 04:29:19PM +0100, Amadeusz Sławiński wrote:
And finally "snd-soc-dummy" which as mentioned above calls dummy_dma_open which originally overridden hwparams set in avs_component_open() with its own limited ones.
...
However, looking at it again, I would consider the existence of dummy_dma_open() to be scope creep. If component is really a dummy one it should not affect any other components in any way. And if any drivers depends on dummy setting parameters for it, I would consider it partially broken. And would say that issue should rather be fixed on driver side by making a dedicated component for it instead of using a dummy one.
That's not such a clear statement in this case. The thing with the hostless links Qualcomm are trying to use here is that we don't actually want to do any DMA ops at all, these paths never go to memory. Really they're CODEC to CODEC links but done in an older way. There's only DMA ops there because we need something to keep userspace happy, they don't actually mean anything and we'd never want them to be used if anything else is providing actual parameters.
It would be cleaner if these systems were to use a CODEC to CODEC link but I'm not sure how well that plays with DPCM - the whole thing is a bit of a house of cards there.
In any case, any news on a fix for this? The suggestion of a custom/variant version of the dummy component that explicitly flags that it's supposed to be used in a hostless configuration does seem like the least fragile thing.
participants (3)
-
Amadeusz Sławiński
-
Mark Brown
-
Patrick Lai