On 9/29/2021 8:22 PM, Pierre-Louis Bossart wrote:
But in addition we'd need to agree on what an 'active BE' is. Why can't
we connect a second stream while the first one is already in HW_PARAMS
or PAUSED or STOP? It's perfectly legal in ALSA/ASoC to have multiple
HW_PARAMS calls, and when we reach STOP we have to do a prepare again.
And more fundamentally, the ability to add a second FE on a 'active' BE
in START state is a basic requirement for a mixer, e.g. to play a
notification on one FE while listening to music on another. What needs
to happen is only to make sure that the FE and BE are compatible in
terms of HW_PARAMS and not sending a second TRIGGER_STOP, only checking
the BE NEW or CLOSE state is way too restrictive.
Sorry for the trouble to your system.
Idea was to avoid reconfiguration of the same BE DAI again, but not to
stop the provision to add a subsequent FE. In fact I had tested mixing
of streams coming from 10 different FEs.
Can you describe the sequence that you used to start them? That may be
useful to understand the criteria you used?
I have something like this:
FE1 --> Crossbar -> Mixer Input1 |
FE2 --> Crossbar -> Mixer Input2 |
... | --> Mixer Output -->
... |
FE10 --> Crossbar -> Mixer Input10 |
All these FEs are started one after the other. This is an example of
10x1. Similarly we can have 2x1, 3x1 etc.,
In our system, the crossbar [0] and mixer [1] are separate ASoC
components. Basically audio paths consist of a group of ASoC components
which are connected back to back.
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/tree/sound/soc/tegra/tegra210_ahub.c
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/tree/sound/soc/tegra/tegra210_mixer.c
[...]
I don't fully understand the notion of mixer input DAI, in our case we
have a bunch of PCM devices connected to a mixer. The mixer is not
directly connected to a DAI.
Please see above mixer example. Since mixer is a separate ASoC
component, it exposes 10 inputs (or DAIs) and outputs. Originally what I
wanted to do was, for subsequent FE runs (FE2, FE3 ...) mixer output
need not be configured again and again.
The problem as I see is that with this patch one can not connect a new
FE to a BE which is _not_ in NEW or CLOSE state.
The FE and BE needs to be connected to have DPCM working and this patch
makes the code to skip the dpcm_be_connect().
Consider this simple setup:
FE1 -->|
| --> BE -->
FE2- ->|
First we start FE1, dpcm_be_connect(FE1, BE, stream) is made.
Later FE2 is started but dpcm_be_connect(FE2, BE, stream) would be not
made because BE is no longer in NEW/CLOSE state.
I share Peter's analysis, there cannot be any restrictions on
connections - at any time. A mixer input might become active and be
added to the mix. We might have a temporary lock to delay new
connections but cannot not reject them outright based on BE state.
Yes, I understand how this affects a system like yours. As per mixer
example above, in our case subsequent FEs always find BE from Crossbar.
That is why I don't see similar error.
I am just
curious to know, if originally you were reconfiguring the BE DAI again
with same parameters (for a second FE) or some additional configuration
is done?
That's a different question - and a good one.
In the case of a mixer, the propagation of hw_params is a broken
concept. It leads to the first FE configuring the BE to define its
preferred parameters, e.g. S16_LE format. If later on a second FE is
started which could play S24_LE, the mixer and BE are already configured
to a lower resolution. A mixer should really have its own parameters and
be the start of a new 'domain' - as Lars described it several years ago
at the audio miniconference.
Propagation is one of the problems we want to address and require help
from DPCM experts. But the scenario you mentioned is a special case
which need not be supported, because mixer can operate in one
configuration at a given time and subsequent FEs should agree to the
already running configuration. However mixer should support both S16_LE
and S24_LE (whenever possible), but not simultaneously. At least this is
the expecation from our systems. Yes mixer may require fixup of a
specific config (we earlier had proposed mixer controls to configure
mixer parameters, but the idea was disliked), but propagation may help
avoid fixing up everywhere in the audio path where it is not really
required. But I don't know how this can be done at the moment.
For now, the only restriction that we could enforce is that the BE
cannot be reconfigured after the prepare step.
Note that our DAIs tolerate multiple calls to hw_params. If you have a
case where the hw_params allocates resources, maybe you should consider
moving that allocation to the prepare step, or free them if you detect a
reconfiguration. That would be something needed even outside of the DPCM
scope. Similarly you need to support the case where the DAI hw_free is
called without hw_params ever being called, it's a known sequence that
can happen if the FE hw-params fails.
Currently this does not seem to be a problem for us. Patch was to avoid
reconfiguration which was felt to be redundant for our system.
I can send a revert with the explanations in the commit message if there
is a consensus that this patch needs to be revisited.
May be this can be revisited since it appears to be a critical problem
for your system. But I hope this discussion can be alive on following
points for a better fix.
1. The original issue at my end was not just a configuration redundancy.
I realize now that with more stream addition following error print is seen.
"ASoC: too many users playback at open 4"
This is because the max DPCM users is capped at 8. Increasing this
may help (need to see what number is better), but does not address the
redundancy problem.
we haven't used more than 2 users, but it's already broken at 2 with
race conditions left and right. I am really surprised you managed to
have more than 2 without hitting inconsistent states - our automated
play/stop/pause monkey tests reliably break DPCM in less than 20s.
I am not sure what is the exact difference, may be DPCM usage in our
case is different from what you have. I have mixer tests for different
combinations (2x1, 3x1 ...), which seem to pass. In general, we want to
have path like this.
FE -> BE1 -> BE2 -> ... -> BEx
Each BEx could be a mixer, resampler etc., Currently DPCM core sees this
as multiple BEs for a given FE and that is why multiple "users" are
reported.
In the interim, may be we can have following patch to keep both systems
working and keep the discussion going to address the oustanding
requirements/issues?
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index ab25f99..0fbab50 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1395,7 +1395,13 @@ static int dpcm_add_paths(struct
snd_soc_pcm_runtime *fe, int stream,
if (!fe->dpcm[stream].runtime && !fe->fe_compr)
continue;
- if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_NEW) &&
+ /*
+ * Filter for systems with 'component_chaining' enabled.
+ * This helps to avoid unnecessary re-configuration of an
+ * already active BE on such systems.
+ */
+ if (fe->card->component_chaining &&
+ (be->dpcm[stream].state != SND_SOC_DPCM_STATE_NEW) &&
(be->dpcm[stream].state != SND_SOC_DPCM_STATE_CLOSE))
continue;
2. If reconfiguration of the same BE is not necessary for a subsequent
FE run, shouldn't we avoid the reconfig itself and somehow avoid FE
failure?
I'm not sure, but it might be possible to just skip the
dpcm_set_be_update_state(be, stream, SND_SOC_DPCM_UPDATE_BE);
call at the end of the loop, but the question is under which condition?
Can a BE asked to be reconfigured when STOP/OPEN/HW_PARAMS?
Skipping the connect does not sound right for a new FE-BE connection.
The reconfiguration is one problem, but what also happens is that the BE
dailink will see multiple triggers. I've been playing with refcounts to
force consistency and make sure there is only one TRIGGER_START send to
the dailink, and conversely there are cases where the TRIGGER_STOP is
never sent...
Just a thought. FE links have dummy codec DAI and core wants to find a
real BE when FE is started. May be don't fail a FE when no back end DAI
is found (and/or find if the same BE is already connected to some FE)
and the above problem becomes simpler?
