On 5/3/2022 11:42 PM, Pierre-Louis Bossart wrote:
On 5/3/22 15:31, Mark Brown wrote:
On Tue, May 03, 2022 at 01:59:54PM -0500, Pierre-Louis Bossart wrote:
This means we get back to the assumption we started off with - what are
we gaining by partitioning things into cards when that's not really
what's going on with the hardware?
The main benefit is reuse of 'generic' cards.
Take for example HDMI support, it's typically exactly the same from one
board to the other - it's completely standard. And yet, for every card,
we have to add a path in the topology and the machine driver to
represent exactly the same information multiple times. see below how
many times we add the same 'late_probe' callback for HDMI support in
machine drivers.
BT audio is a similar case, the interface configuration and settings are
defined by profiles, so there's almost no variation from one board to
the other except for which I2S number is used.
A peripheral directly attached to an SOC or chipset, such as digital
microphones, is a similar case.
The point is really to try to split what will be variable from board to
board due to different choices of headset codecs, amplifiers,
microphones, from what is generic and reusable.
There's a reusability thing here which does seem somewhat valid, but it
does feel to me like separate cards is a bit of a case of having a
hammer so everything looks like a nail kind of solution to the issue.
The issue you've got is not that these are disconnected bits of hardware
which operate independently, it's that you don't have a good way of
dropping the board variations into a template machine driver or
otherwise customising one.
Off the top of my head you could probably get some way with this if
there were a way to tell the core to skip over some links during
instantiation, that might help keep a common core of these bits you wan
to split out into cards more static. Perhaps also being able to add
multiple sets of DAIs to a component rather than having to compose the
full list and then add everything in one shot?
Indeed, if we could tell that a BE does not exist on a specific board
without having to create a topology without said BE that would help
immensely.
Assuming you want one monolithic topology I guess we could unregister
FEs exposed by topology when there is no full route to BE? This will be
bit weird as first you need to load full topology to decide if routing
is possible and then either keep or remove FEs and associated widgets, etc.
The reason why we split boards in AVS driver to be per codec is that we
observed that it is mostly copy and paste code between multiple boards
in various permutations, where some auxiliary codecs are present and
some are not. This allows us to provide per codec topology.
See more comments below, if we could provide to the topology loading
sequence that a BE is not present, or remap it to use a different
hardware interface (e.g. use SSP2 instead of default SSP0, or number of
microphones in hardware) that would address most of the concerns we face
today.
I'll just note that it isn't impossible to do with current topologies
and is in fact what we do allow in AVS driver for topologies describing
connection to single i2s codec:
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/tree/sound/soc/intel/avs/topology.c#n375
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/tree/sound/soc/intel/avs/topology.c#n1354
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/tree/sound/soc/intel/avs/topology.c#n1386
Of course this still requires driver to decide which machine board to
use and topology to load based on ACPI information, as we use
snd_soc_acpi_mach struct as data source.
Do note that in case of machine board and topology describing board with
multiple i2s endpoints on one card we require hard coding the values as
we can't make a guess how endpoints are connected.
The logic can be pushed further, as done in the AVS patch series, to
split the card by codec type. This would avoid having to deal with the
permutations that we have to handle in machine drivers. See e.g. how
complicated the initially generic sof-rt5682 machine driver has become,
it now supports rt5682s, rt1011, rt1015, rt1015p, max98373 and
max98360a. I will accept this comes from ACPI limitations, but if we
could split cards it would also reduce the machine driver complexity.
If you want to split the cards up more as things stand there's nothing
really standing in your way there. As you say though I do think this is
just that your firmware doesn't describe most of the hardware and so you
end up with a large element of bikeshedding the deckchairs on the Titanic
which limits how good things can get. I do wonder what happens if we
split things into cards and then get better at letting userspace
dynamically manage what the DSP is doing rather than relying so much on
fixed topologies...
Parts of the problem is that the topology as currently defined is across
two 'domains' and doesn't do a great job in either case:
a) the DSP processing parts which should be programmable/reconfigurable
at will depending on the needs of userspace. Other OSes do not rely on
fixed topologies indeed and will have higher-level logic to deal with
pipeline creation. One example here is that the fixed topology forces us
to make worst case assumptions of concurrency between usages. There
could be more dynamic decisions with more intelligence on routing and
resource management in userspace.
I would say that currently this can be solved by providing custom per
device topologies, not sure if anything better can be done here. Overall
I suspect that it would require exposing some kind of new API allowing
end user to decide what kind of modules they want.
b) the hardware/board level parts. That is a weak part of the topology
today, it should come from a hardware description instead of being
hard-coded in each topology. We must have dozens of identical topology
files that differ only because of the SSP index or SoundWire link ID
used on specific board. There should be a way to 'remap' BEs for
specific boards.
It doesn't mean we should toss the topology and redo everything, the
latter part can be addressed by providing the 'real' hardware
information to the topology and dynamically modify defaults.
I already showed how we tried to solve this for i2s use cases in links
above.
In terms of functionality, I don't think there will be any performance
or power improvement coming from a multi-card solution, it's mostly a
maintenance simplification: less duplicated code, more reuse.
One key point is "who defines the split". That's really one of the main
problems and different people could have different opinions - Cezary and
I have a shared goal of enabling multiple cards but different takes on
what the 'optimal' split might be.
My take is that the integrator for a given hardware is responsible for
making the decision - not the provider of a DSP driver. In case you have
coupling between interfaces, playback or capture, it can become really
difficult to define a split that will work for all cases, or conversely
if you don't have 'self-contained' cards that can be tested
independently then splitting the functionality was probably a really bad
idea. If one needs to add dependencies and quirks for a local device,
Looks like something got dropped here. It does sound a bit concerning
that the split into machine drivers could be done using quirks.
that's not what I meant. I was more concerned about quirks that would be
required by the hardware or board, but cannot be added because of the
split in independent cards.
In other words, I think we need to agree on the means to create and
expose multiple cards, and agree not to debate on what the functionality
of individual cards might be.
Hope this helps clarify the ask?
It's still not clear to me if the problem you're facing couldn't be
addressed as well with better interfaces for adding dai_links to the
card, that (mainly around BEs) seems to be the main thing you're having
trouble with as far as I can see?
You are not wrong indeed. Splitting a monolithic card is a mechanism to
work-around the hassle of coupling BE and topology definitions, which in
practice results in duplication of machine drivers and topologies.
To be clearer, we currently describe BEs in the machine driver and
topology, and all the definitions have to match exactly. That's really
wrong, and goes boink every time an index or stream name is wrong on
either side, or if the topology makes a reference to a non-existent BE
in the machine driver. We should have a single definition that is used
by the topology and machine driver, and hopefully some day have that
definition come from ACPI (hope springs eternal).
If we had a better mechanism at the ASoC level to expose what the
hardware actually is, and ways to remap the BE and topology definitions
that would indeed remove most of the motivations for exposing multiple
cards.
Well, in AVS we solved this partially by just using snd_soc_acpi_mach in
topology parser - using it as configuration, with topology being a
template (once again see links above ;). Of course this still assumes
that machine board matches the topology, but it is a lot easier to
achieve when there is one machine board handling a codec type, instead
of multiple of them with slight differences. If you look at patchset
where we add machine boards you will probably notice that, we don't
hardcode i2s port in any of i2s boards - assumption being that it is
configured on probe by using passed port id.
https://lore.kernel.org/alsa-devel/20220427081902.3525183-1-cezary.rojewski@xxxxxxxxx/T/#mad2c35c57d016b7223650dae55b379b81d4607b7
I won't claim that we solved everything you mention above in AVS driver,
but most of it looks like things we already have some solutions for.
Good feedback indeed, much appreciated!
