All of lore.kernel.org
 help / color / mirror / Atom feed
* ASoC component/card relationship
@ 2022-04-29 21:55 Pierre-Louis Bossart
  2022-04-29 22:32 ` Curtis Malainey
  2022-05-03 18:10 ` Mark Brown
  0 siblings, 2 replies; 13+ messages in thread
From: Pierre-Louis Bossart @ 2022-04-29 21:55 UTC (permalink / raw)
  To: ALSA Development Mailing List
  Cc: Cezary Rojewski, Kuninori Morimoto, Kai Vehmanen,
	Curtis Malainey, Péter Ujfalusi, Takashi Iwai,
	Liam Girdwood, Mark Brown, Amadeusz Sławiński,
	Bard Liao

Hi,
In the existing ASoC code, there is a fixed mapping between ASoC card and component. A component relies on a ->card pointer that is set during the probe. A component cannot be used by or "bound to" more than one card [1]

This has interesting impacts on how a codec or DSP driver need to be implemented.

In the AVS series posted this week, multiple components are registered by the DSP driver, following an interface-based split. There's in addition a second-level split, where the logic is pushed further: the DSP driver partitions the SSP DAIs in different set of 'dai_driver's used by different components, which are in turn used by different cards. What is done in these patches is not wrong, and is probably the only solution to support a real-world platform with the existing ASoC code, but are the framework assumptions correct? In this example, the board-level information on which interface is used for what functionality trickles down to the lowest level of the DSP driver implementation.

I believe this breaks to some extent the 'clean' split between platform and machine driver(s), and it's not quite aligned with the usual notion of register/probe used across frameworks, be it for drivers/clocks/you name it.

A similar case could happen in a codec driver, if independent functionality such as headset and amplifier support was exposed by separate cards, that would in turn mandate that the codec driver exposed N components, each handling different functionality but the same type of DAI.

An alternative approach would be that the DSP driver exposes all the possible DAIs that can be used, and the binding is refined to allow for more flexibility. I think it's really the individual DAI that cannot be used by more than one card.

I figured I would ask on this mailing list if

a) I am not mistaken on the component/card relationship and

b) if this is by design, or if we want to clarify what a component is and what its restrictions might be.

Thanks for your feedback/comments
-Pierre

[1] https://elixir.bootlin.com/linux/latest/source/sound/soc/soc-core.c#L1364

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: ASoC component/card relationship
  2022-04-29 21:55 ASoC component/card relationship Pierre-Louis Bossart
@ 2022-04-29 22:32 ` Curtis Malainey
  2022-05-02 15:06   ` Pierre-Louis Bossart
  2022-05-03 18:10 ` Mark Brown
  1 sibling, 1 reply; 13+ messages in thread
From: Curtis Malainey @ 2022-04-29 22:32 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: ALSA Development Mailing List, Kuninori Morimoto, Kai Vehmanen,
	Péter Ujfalusi, Cezary Rojewski, Takashi Iwai,
	Liam Girdwood, Mark Brown, Amadeusz Sławiński,
	Bard Liao

On Fri, Apr 29, 2022 at 2:55 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
> Hi,
> In the existing ASoC code, there is a fixed mapping between ASoC card and component. A component relies on a ->card pointer that is set during the probe. A component cannot be used by or "bound to" more than one card [1]
>
> This has interesting impacts on how a codec or DSP driver need to be implemented.
>
> In the AVS series posted this week, multiple components are registered by the DSP driver, following an interface-based split. There's in addition a second-level split, where the logic is pushed further: the DSP driver partitions the SSP DAIs in different set of 'dai_driver's used by different components, which are in turn used by different cards. What is done in these patches is not wrong, and is probably the only solution to support a real-world platform with the existing ASoC code, but are the framework assumptions correct? In this example, the board-level information on which interface is used for what functionality trickles down to the lowest level of the DSP driver implementation.
>
> I believe this breaks to some extent the 'clean' split between platform and machine driver(s), and it's not quite aligned with the usual notion of register/probe used across frameworks, be it for drivers/clocks/you name it.
>
> A similar case could happen in a codec driver, if independent functionality such as headset and amplifier support was exposed by separate cards, that would in turn mandate that the codec driver exposed N components, each handling different functionality but the same type of DAI.
>
> An alternative approach would be that the DSP driver exposes all the possible DAIs that can be used, and the binding is refined to allow for more flexibility. I think it's really the individual DAI that cannot be used by more than one card.

Would it also be logical to expose the DAIs on the codecs
independently or should this be validated on a case by case basis?

>
> I figured I would ask on this mailing list if
>
> a) I am not mistaken on the component/card relationship and
>

Just trying to think of a reason why this would not be true. Are we
aware of platforms that have configuration relationships across DAIs?
E.g. they use a single clock and must be configured together, so
splitting them might cause them to be in sync? Otherwise I agree, if
DAIs can be handled independently then I don't see why we should tie
them together.

Curtis

> b) if this is by design, or if we want to clarify what a component is and what its restrictions might be.
>
> Thanks for your feedback/comments
> -Pierre
>
> [1] https://elixir.bootlin.com/linux/latest/source/sound/soc/soc-core.c#L1364

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: ASoC component/card relationship
  2022-04-29 22:32 ` Curtis Malainey
@ 2022-05-02 15:06   ` Pierre-Louis Bossart
  2022-05-02 20:06     ` Curtis Malainey
  0 siblings, 1 reply; 13+ messages in thread
From: Pierre-Louis Bossart @ 2022-05-02 15:06 UTC (permalink / raw)
  To: Curtis Malainey
  Cc: ALSA Development Mailing List, Kuninori Morimoto, Kai Vehmanen,
	Péter Ujfalusi, Cezary Rojewski, Takashi Iwai,
	Liam Girdwood, Mark Brown, Amadeusz Sławiński,
	Bard Liao



On 4/29/22 17:32, Curtis Malainey wrote:
> On Fri, Apr 29, 2022 at 2:55 PM Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
>>
>> Hi,
>> In the existing ASoC code, there is a fixed mapping between ASoC card and component. A component relies on a ->card pointer that is set during the probe. A component cannot be used by or "bound to" more than one card [1]
>>
>> This has interesting impacts on how a codec or DSP driver need to be implemented.
>>
>> In the AVS series posted this week, multiple components are registered by the DSP driver, following an interface-based split. There's in addition a second-level split, where the logic is pushed further: the DSP driver partitions the SSP DAIs in different set of 'dai_driver's used by different components, which are in turn used by different cards. What is done in these patches is not wrong, and is probably the only solution to support a real-world platform with the existing ASoC code, but are the framework assumptions correct? In this example, the board-level information on which interface is used for what functionality trickles down to the lowest level of the DSP driver implementation.
>>
>> I believe this breaks to some extent the 'clean' split between platform and machine driver(s), and it's not quite aligned with the usual notion of register/probe used across frameworks, be it for drivers/clocks/you name it.
>>
>> A similar case could happen in a codec driver, if independent functionality such as headset and amplifier support was exposed by separate cards, that would in turn mandate that the codec driver exposed N components, each handling different functionality but the same type of DAI.
>>
>> An alternative approach would be that the DSP driver exposes all the possible DAIs that can be used, and the binding is refined to allow for more flexibility. I think it's really the individual DAI that cannot be used by more than one card.
> 
> Would it also be logical to expose the DAIs on the codecs
> independently or should this be validated on a case by case basis?

Not following the question, sorry.
 
>> I figured I would ask on this mailing list if
>>
>> a) I am not mistaken on the component/card relationship and
>>
> 
> Just trying to think of a reason why this would not be true. Are we
> aware of platforms that have configuration relationships across DAIs?
> E.g. they use a single clock and must be configured together, so
> splitting them might cause them to be in sync? Otherwise I agree, if
> DAIs can be handled independently then I don't see why we should tie
> them together.

There are restrictions on most platforms, but those restrictions should be expressed with modeling of clocks and serialization when accessing registers if required. Splitting the DAIs in different components to expose different cards to userspace without modeling such dependencies is a sure fail indeed. It's also an assured fail even if the DAIs are exposed in a single component and used in a single card. One example would be our very own Intel SSP, if you try to configure a shared MCLK with different settings that will quickly go South.

> 
> Curtis
> 
>> b) if this is by design, or if we want to clarify what a component is and what its restrictions might be.
>>
>> Thanks for your feedback/comments
>> -Pierre
>>
>> [1] https://elixir.bootlin.com/linux/latest/source/sound/soc/soc-core.c#L1364

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: ASoC component/card relationship
  2022-05-02 15:06   ` Pierre-Louis Bossart
@ 2022-05-02 20:06     ` Curtis Malainey
  0 siblings, 0 replies; 13+ messages in thread
From: Curtis Malainey @ 2022-05-02 20:06 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: ALSA Development Mailing List, Kuninori Morimoto, Kai Vehmanen,
	Péter Ujfalusi, Cezary Rojewski, Takashi Iwai,
	Liam Girdwood, Mark Brown, Amadeusz Sławiński,
	Bard Liao

On Mon, May 2, 2022 at 8:06 AM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
>
> On 4/29/22 17:32, Curtis Malainey wrote:
> > On Fri, Apr 29, 2022 at 2:55 PM Pierre-Louis Bossart
> > <pierre-louis.bossart@linux.intel.com> wrote:
> >>
> >> Hi,
> >> In the existing ASoC code, there is a fixed mapping between ASoC card and component. A component relies on a ->card pointer that is set during the probe. A component cannot be used by or "bound to" more than one card [1]
> >>
> >> This has interesting impacts on how a codec or DSP driver need to be implemented.
> >>
> >> In the AVS series posted this week, multiple components are registered by the DSP driver, following an interface-based split. There's in addition a second-level split, where the logic is pushed further: the DSP driver partitions the SSP DAIs in different set of 'dai_driver's used by different components, which are in turn used by different cards. What is done in these patches is not wrong, and is probably the only solution to support a real-world platform with the existing ASoC code, but are the framework assumptions correct? In this example, the board-level information on which interface is used for what functionality trickles down to the lowest level of the DSP driver implementation.
> >>
> >> I believe this breaks to some extent the 'clean' split between platform and machine driver(s), and it's not quite aligned with the usual notion of register/probe used across frameworks, be it for drivers/clocks/you name it.
> >>
> >> A similar case could happen in a codec driver, if independent functionality such as headset and amplifier support was exposed by separate cards, that would in turn mandate that the codec driver exposed N components, each handling different functionality but the same type of DAI.
> >>
> >> An alternative approach would be that the DSP driver exposes all the possible DAIs that can be used, and the binding is refined to allow for more flexibility. I think it's really the individual DAI that cannot be used by more than one card.
> >
> > Would it also be logical to expose the DAIs on the codecs
> > independently or should this be validated on a case by case basis?
>
> Not following the question, sorry.

If we are considering helping divide the boundary between DAIs and
components, just curious if there is any gain on codecs with more than
1 DAI.

E.g. rt5677 has 6 DAIs, just pondering if it's possible (or even
useful) to do this on the codec side as well. So in theory a single
codec could be part of 2 cards.

>
> >> I figured I would ask on this mailing list if
> >>
> >> a) I am not mistaken on the component/card relationship and
> >>
> >
> > Just trying to think of a reason why this would not be true. Are we
> > aware of platforms that have configuration relationships across DAIs?
> > E.g. they use a single clock and must be configured together, so
> > splitting them might cause them to be in sync? Otherwise I agree, if
> > DAIs can be handled independently then I don't see why we should tie
> > them together.
>
> There are restrictions on most platforms, but those restrictions should be expressed with modeling of clocks and serialization when accessing registers if required. Splitting the DAIs in different components to expose different cards to userspace without modeling such dependencies is a sure fail indeed. It's also an assured fail even if the DAIs are exposed in a single component and used in a single card. One example would be our very own Intel SSP, if you try to configure a shared MCLK with different settings that will quickly go South.
>
> >
> > Curtis
> >
> >> b) if this is by design, or if we want to clarify what a component is and what its restrictions might be.
> >>
> >> Thanks for your feedback/comments
> >> -Pierre
> >>
> >> [1] https://elixir.bootlin.com/linux/latest/source/sound/soc/soc-core.c#L1364

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: ASoC component/card relationship
  2022-04-29 21:55 ASoC component/card relationship Pierre-Louis Bossart
  2022-04-29 22:32 ` Curtis Malainey
@ 2022-05-03 18:10 ` Mark Brown
  2022-05-03 18:59   ` Pierre-Louis Bossart
  1 sibling, 1 reply; 13+ messages in thread
From: Mark Brown @ 2022-05-03 18:10 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: ALSA Development Mailing List, Kuninori Morimoto, Kai Vehmanen,
	Curtis Malainey, Péter Ujfalusi, Cezary Rojewski,
	Takashi Iwai, Liam Girdwood, Amadeusz Sławiński,
	Bard Liao

[-- Attachment #1: Type: text/plain, Size: 4122 bytes --]

On Fri, Apr 29, 2022 at 04:55:18PM -0500, Pierre-Louis Bossart wrote:

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

> In the existing ASoC code, there is a fixed mapping between ASoC card
> and component. A component relies on a ->card pointer that is set
> during the probe. A component cannot be used by or "bound to" more
> than one card [1]

> This has interesting impacts on how a codec or DSP driver need to be
> implemented.

> In the AVS series posted this week, multiple components are registered
> by the DSP driver, following an interface-based split. There's in
> addition a second-level split, where the logic is pushed further: the
> DSP driver partitions the SSP DAIs in different set of 'dai_driver's
> used by different components, which are in turn used by different
> cards. What is done in these patches is not wrong, and is probably the
> only solution to support a real-world platform with the existing ASoC
> code, but are the framework assumptions correct? In this example, the
> board-level information on which interface is used for what
> functionality trickles down to the lowest level of the DSP driver
> implementation.

I'm unclear as to why this is the only mechanism for supporting a
platform - it's the only way currently to achieve multiple cards with
the current code but there's an assumption there that we need to do so.
If we start from the assumption that we have to split a given bit of
hardware between cards then it currently follows that the driver for
that card is going to have to register multiple components, but that's a
bit of an assumption.

> I believe this breaks to some extent the 'clean' split between
> platform and machine driver(s), and it's not quite aligned with the
> usual notion of register/probe used across frameworks, be it for
> drivers/clocks/you name it.

This is something which does cause issues for these other frameworks,
and it's something which we were having some trouble with in ASoC when
components were added.  Where there's interlinks between parts of a
device something needs to know about them and coordinate to avoid or
resolve any conflicts in requirements.  This was causing issues for ASoC
when DAIs didn't know about things like shared clocking - drivers ended
up having to invent components for themselves.

> A similar case could happen in a codec driver, if independent
> functionality such as headset and amplifier support was exposed by
> separate cards, that would in turn mandate that the codec driver
> exposed N components, each handling different functionality but the
> same type of DAI.

If a device genuinely had a bunch of completely independent blocks that
just happened to be packaged together I think that would be a completely
sensible implementation TBH, it's just a MFD at that point so there's
very little reason for the different components to even be the same
Linux device other than a presumably shared control interface.

> An alternative approach would be that the DSP driver exposes all the
> possible DAIs that can be used, and the binding is refined to allow
> for more flexibility. I think it's really the individual DAI that
> cannot be used by more than one card.

There's a bit more going on than just that DAIs can't be shared (and
indeed one might ask questions about splitting bits of a DAI up, for
example do playback and capture *really* need to go to the same card?).
It's also that the clocking and routing within the component need to be
coordinated and if multiple cards are talking to the same component both
the machine drivers and DAPM are going to need to understand this and
handle it in some sensible fashion.  At some point you then end up with
something that's got the internals of a single card providing muliple
cards to userspace only with a more complicated implementation.

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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: ASoC component/card relationship
  2022-05-03 18:10 ` Mark Brown
@ 2022-05-03 18:59   ` Pierre-Louis Bossart
  2022-05-03 20:31     ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: Pierre-Louis Bossart @ 2022-05-03 18:59 UTC (permalink / raw)
  To: Mark Brown
  Cc: Cezary Rojewski, Kuninori Morimoto, Kai Vehmanen,
	Curtis Malainey, Bard Liao, ALSA Development Mailing List,
	Takashi Iwai, Liam Girdwood, Amadeusz Sławiński,
	Péter Ujfalusi


> 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.

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.

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,
the notion of split cards is probably not so good either.

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?
-Pierre

sound/soc/intel/boards$ git grep '\.late_probe'

bxt_da7219_max98357a.c: .late_probe = bxt_card_late_probe,

bxt_rt298.c:    .late_probe = bxt_card_late_probe,

bxt_rt298.c:    .late_probe = bxt_card_late_probe,

cml_rt1011_rt5682.c:    .late_probe = sof_card_late_probe,

ehl_rt5660.c:   .late_probe = card_late_probe,

glk_rt5682_max98357a.c: .late_probe = glk_card_late_probe,

kbl_da7219_max98357a.c: .late_probe = kabylake_card_late_probe,

kbl_da7219_max98927.c:  .late_probe = kabylake_card_late_probe,

kbl_da7219_max98927.c:  .late_probe = kabylake_card_late_probe,

kbl_da7219_max98927.c:  .late_probe = kabylake_card_late_probe,

kbl_da7219_max98927.c:  .late_probe = kabylake_card_late_probe,

kbl_rt5660.c:   .late_probe = kabylake_card_late_probe,

kbl_rt5663_max98927.c:  .late_probe = kabylake_card_late_probe,

kbl_rt5663_max98927.c:  .late_probe = kabylake_card_late_probe,

kbl_rt5663_rt5514_max98927.c:   .late_probe = kabylake_card_late_probe,

skl_hda_dsp_generic.c:  .late_probe = skl_hda_card_late_probe,

skl_nau88l25_max98357a.c:       .late_probe = skylake_card_late_probe,

skl_nau88l25_ssm4567.c: .late_probe = skylake_card_late_probe,

skl_rt286.c:    .late_probe = skylake_card_late_probe,

sof_cs42l42.c:  .late_probe = sof_card_late_probe,

sof_da7219_max98373.c:  .late_probe = card_late_probe,

sof_da7219_max98373.c:  .late_probe = card_late_probe,

sof_es8336.c:   .late_probe = sof_es8336_late_probe,

sof_nau8825.c:  .late_probe = sof_card_late_probe,

sof_pcm512x.c:  .late_probe = sof_card_late_probe,

sof_rt5682.c:   .late_probe = sof_card_late_probe,

sof_sdw.c:              if (!codec_info_list[i].late_probe)

sof_sdw.c:      .late_probe = sof_sdw_card_late_probe,

sof_ssp_amp.c:  .late_probe = sof_card_late_probe,


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: ASoC component/card relationship
  2022-05-03 18:59   ` Pierre-Louis Bossart
@ 2022-05-03 20:31     ` Mark Brown
  2022-05-03 21:42       ` Pierre-Louis Bossart
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2022-05-03 20:31 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: Cezary Rojewski, Kuninori Morimoto, Kai Vehmanen,
	Curtis Malainey, Bard Liao, ALSA Development Mailing List,
	Takashi Iwai, Liam Girdwood, Amadeusz Sławiński,
	Péter Ujfalusi

[-- Attachment #1: Type: text/plain, Size: 4596 bytes --]

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?

> 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...

> 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.

> 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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: ASoC component/card relationship
  2022-05-03 20:31     ` Mark Brown
@ 2022-05-03 21:42       ` Pierre-Louis Bossart
  2022-05-04  9:21         ` Amadeusz Sławiński
  0 siblings, 1 reply; 13+ messages in thread
From: Pierre-Louis Bossart @ 2022-05-03 21:42 UTC (permalink / raw)
  To: Mark Brown
  Cc: Cezary Rojewski, Kuninori Morimoto, Kai Vehmanen,
	Curtis Malainey, Bard Liao, ALSA Development Mailing List,
	Takashi Iwai, Liam Girdwood, Amadeusz Sławiński,
	Péter Ujfalusi



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.

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.

>> 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.

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.

>> 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.

Good feedback indeed, much appreciated!



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: ASoC component/card relationship
  2022-05-03 21:42       ` Pierre-Louis Bossart
@ 2022-05-04  9:21         ` Amadeusz Sławiński
  2022-05-04 15:26           ` Pierre-Louis Bossart
  0 siblings, 1 reply; 13+ messages in thread
From: Amadeusz Sławiński @ 2022-05-04  9:21 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Mark Brown
  Cc: Cezary Rojewski, Kuninori Morimoto, Kai Vehmanen,
	Curtis Malainey, Péter Ujfalusi,
	ALSA Development Mailing List, Takashi Iwai, Liam Girdwood,
	Bard Liao

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@intel.com/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!
> 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: ASoC component/card relationship
  2022-05-04  9:21         ` Amadeusz Sławiński
@ 2022-05-04 15:26           ` Pierre-Louis Bossart
  2022-05-04 15:38             ` Jaroslav Kysela
  2022-05-04 16:00             ` Cezary Rojewski
  0 siblings, 2 replies; 13+ messages in thread
From: Pierre-Louis Bossart @ 2022-05-04 15:26 UTC (permalink / raw)
  To: Amadeusz Sławiński, Mark Brown
  Cc: Cezary Rojewski, Kuninori Morimoto, Kai Vehmanen,
	Curtis Malainey, Bard Liao, ALSA Development Mailing List,
	Takashi Iwai, Liam Girdwood, Péter Ujfalusi



On 5/4/22 04:21, Amadeusz Sławiński wrote:
> 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.

That's right, there will be both a need to prune parts of the topology
that are not supported in hardware, or conversely mark them as
conditional and only load them if there is a clear hardware support for
the resources exposed.

But it's not just a filter or conditional addition, we have to be able
to remap an index and modify clock values. It's really important to
avoid maintaining different topologies for the same platform, just
because one hardware designer used SSP0 and another used SSP1.

> 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.

That's the point, the topology doesn't know so we will duplicate the
same stuff to cover all possible hardware variations.

The machine driver knows that information, when you create the BE
dailink you have to say, e.g. SSP #2 is connect to AIF #1.

With the current solution we have to create one topology for each
possible SSP connection, and modify the ACPI tables to specify a
different topology name. What we need instead is a new callback to tell
the topology that the 'cpu dai' is #2.

You have exactly the same problem with your ssp test cases, since we
have up to 6 SSPs you will need to maintain and load 6 different
topology files that differ only by the SSP index.

My take is that the topology should not make any hard-coded assumptions
on the hardware connection to the codecs but be given the information
before the topology is loaded.


>> 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.

You still have N identical topologies, completely identical with the
exception of 1 SSP index to maintain.

Just to make this point stronger, with the recent support of the ES8336
codecs, we had to generate the following topologies to account for all
permutations:

sof-apl-es8336-ssp0.tplg
sof-apl-es8336-ssp2.tplg
sof-cml-es8336-dmic2ch-ssp0.tplg
sof-cml-es8336-dmic2ch-ssp2.tplg
sof-cml-es8336-dmic4ch-ssp0.tplg
sof-cml-es8336-dmic4ch-ssp2.tplg
sof-cml-es8336-ssp0.tplg
sof-cml-es8336-ssp2.tplg
sof-glk-es8336-ssp0.tplg
sof-glk-es8336-ssp2.tplg
sof-jsl-es8336-dmic2ch-ssp0.tplg
sof-jsl-es8336-dmic2ch-ssp2.tplg
sof-jsl-es8336-dmic4ch-ssp0.tplg
sof-jsl-es8336-dmic4ch-ssp2.tplg
sof-jsl-es8336-ssp0.tplg
sof-jsl-es8336-ssp2.tplg
sof-tgl-es8336-dmic2ch-ssp0.tplg
sof-tgl-es8336-dmic2ch-ssp2.tplg
sof-tgl-es8336-dmic4ch-ssp0.tplg
sof-tgl-es8336-dmic4ch-ssp2.tplg
sof-tgl-es8336-ssp0.tplg
sof-tgl-es8336-ssp2.tplg

All these topologies come from the same file, and generated using
macros. That makes no sense to me, this should be the same topology that
is remapped at run-time.


>> 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@intel.com/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.

It's easier because you have limited numbers of Chromebooks to handle.
See above what happened with the same ES8336 codec used on multiple
platforms, with the same ACPI HID, the permutations quickly become
completely unmanageable.

I forget that it's not just the SSP index that should be passed at run
time, if we wanted to have more generic topologies the assumptions of
clocking should be provided at run time. That's the main reason why we
have different topologies for GLK, CML and TGL, even though it's the
same pipeline and capabilities provided in the rest of the topology.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: ASoC component/card relationship
  2022-05-04 15:26           ` Pierre-Louis Bossart
@ 2022-05-04 15:38             ` Jaroslav Kysela
  2022-05-04 16:00             ` Cezary Rojewski
  1 sibling, 0 replies; 13+ messages in thread
From: Jaroslav Kysela @ 2022-05-04 15:38 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Amadeusz Sławiński, Mark Brown
  Cc: Cezary Rojewski, Kuninori Morimoto, Kai Vehmanen,
	Curtis Malainey, Péter Ujfalusi,
	ALSA Development Mailing List, Takashi Iwai, Liam Girdwood,
	Bard Liao

On 04. 05. 22 17:26, Pierre-Louis Bossart wrote:

> My take is that the topology should not make any hard-coded assumptions
> on the hardware connection to the codecs but be given the information
> before the topology is loaded.

The information is already coded in the topology filename, so it should be 
easy to extend the topology format to refer to the machine variables instead 
using the fixed numbers or strings. Eventually, it may be easy to 
conditionally use a blocks from the topology on demand, if the referred 
variables do not exist.

> sof-tgl-es8336-ssp0.tplg
> sof-tgl-es8336-ssp2.tplg
> 
> All these topologies come from the same file, and generated using
> macros. That makes no sense to me, this should be the same topology that
> is remapped at run-time.

Yes, it should be handled more elegantly.

					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: ASoC component/card relationship
  2022-05-04 15:26           ` Pierre-Louis Bossart
  2022-05-04 15:38             ` Jaroslav Kysela
@ 2022-05-04 16:00             ` Cezary Rojewski
  2022-05-04 16:27               ` Pierre-Louis Bossart
  1 sibling, 1 reply; 13+ messages in thread
From: Cezary Rojewski @ 2022-05-04 16:00 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Amadeusz Sławiński, Mark Brown
  Cc: ALSA Development Mailing List, Kuninori Morimoto, Kai Vehmanen,
	Curtis Malainey, Bard Liao, Takashi Iwai, Liam Girdwood,
	Péter Ujfalusi

On 2022-05-04 5:26 PM, Pierre-Louis Bossart wrote:

>>> 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.
> 
> You still have N identical topologies, completely identical with the
> exception of 1 SSP index to maintain.
> 
> Just to make this point stronger, with the recent support of the ES8336
> codecs, we had to generate the following topologies to account for all
> permutations:
> 
> sof-apl-es8336-ssp0.tplg
> sof-apl-es8336-ssp2.tplg
> sof-cml-es8336-dmic2ch-ssp0.tplg
> sof-cml-es8336-dmic2ch-ssp2.tplg
> sof-cml-es8336-dmic4ch-ssp0.tplg
> sof-cml-es8336-dmic4ch-ssp2.tplg
> sof-cml-es8336-ssp0.tplg
> sof-cml-es8336-ssp2.tplg
> sof-glk-es8336-ssp0.tplg
> sof-glk-es8336-ssp2.tplg
> sof-jsl-es8336-dmic2ch-ssp0.tplg
> sof-jsl-es8336-dmic2ch-ssp2.tplg
> sof-jsl-es8336-dmic4ch-ssp0.tplg
> sof-jsl-es8336-dmic4ch-ssp2.tplg
> sof-jsl-es8336-ssp0.tplg
> sof-jsl-es8336-ssp2.tplg
> sof-tgl-es8336-dmic2ch-ssp0.tplg
> sof-tgl-es8336-dmic2ch-ssp2.tplg
> sof-tgl-es8336-dmic4ch-ssp0.tplg
> sof-tgl-es8336-dmic4ch-ssp2.tplg
> sof-tgl-es8336-ssp0.tplg
> sof-tgl-es8336-ssp2.tplg
> 
> All these topologies come from the same file, and generated using
> macros. That makes no sense to me, this should be the same topology that
> is remapped at run-time.


What Amadeo is explaining here is that AVS driver already addresses this 
too - at least in our opinion - see parse_link_formatted_string() in 
sound/soc/intel/avs/topology.c.

User is allowed to specify widget name as: ssp%d within the topology 
file, leaving kernel with responsibility to fill the missing index. And 
this is something I (perhaps we all) would like to see within the 
framework in the future.

In consequence, avs-driver user does NOT need to define N identical 
topologies. For example, SSP-test board [1] and its definition in 
board_selection.c [2] clearly show that all 6 SSP boards look at the 
exact same file. The same approach is used when speaking of other, 
simple i2s codecs, e.g.: rt274, rt286, rt298. Difference between rt298 
on APL and GML comes down to SSP port number. Here, board_selection.c 
shows different prefixes (apl- vs gml-) but the reality is that both 
files are symlinks looking at the exact same actual topology file with 
ssp%d specified as widget names.

At the same time, compound machine boards are still permitted and made 
use of, example being TDF8532/Dirana board for Automotive (not yet 
present on the list).

Really, flexibility is key here. As long as devices found on given 
platform are not connected or dependent on each other, there are no 
major objections preventing card split. Such code scales better and has 
good reuseability.


Regards,
Czarek


[1]: 
https://lore.kernel.org/alsa-devel/20220427081902.3525183-6-cezary.rojewski@intel.com/
[2]: 
https://lore.kernel.org/alsa-devel/20220426172346.3508411-11-cezary.rojewski@intel.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: ASoC component/card relationship
  2022-05-04 16:00             ` Cezary Rojewski
@ 2022-05-04 16:27               ` Pierre-Louis Bossart
  0 siblings, 0 replies; 13+ messages in thread
From: Pierre-Louis Bossart @ 2022-05-04 16:27 UTC (permalink / raw)
  To: Cezary Rojewski, Amadeusz Sławiński, Mark Brown
  Cc: ALSA Development Mailing List, Kuninori Morimoto, Kai Vehmanen,
	Curtis Malainey, Péter Ujfalusi, Takashi Iwai,
	Liam Girdwood, Bard Liao


>> Just to make this point stronger, with the recent support of the ES8336
>> codecs, we had to generate the following topologies to account for all
>> permutations:
>>
>> sof-apl-es8336-ssp0.tplg
>> sof-apl-es8336-ssp2.tplg
>> sof-cml-es8336-dmic2ch-ssp0.tplg
>> sof-cml-es8336-dmic2ch-ssp2.tplg
>> sof-cml-es8336-dmic4ch-ssp0.tplg
>> sof-cml-es8336-dmic4ch-ssp2.tplg
>> sof-cml-es8336-ssp0.tplg
>> sof-cml-es8336-ssp2.tplg
>> sof-glk-es8336-ssp0.tplg
>> sof-glk-es8336-ssp2.tplg
>> sof-jsl-es8336-dmic2ch-ssp0.tplg
>> sof-jsl-es8336-dmic2ch-ssp2.tplg
>> sof-jsl-es8336-dmic4ch-ssp0.tplg
>> sof-jsl-es8336-dmic4ch-ssp2.tplg
>> sof-jsl-es8336-ssp0.tplg
>> sof-jsl-es8336-ssp2.tplg
>> sof-tgl-es8336-dmic2ch-ssp0.tplg
>> sof-tgl-es8336-dmic2ch-ssp2.tplg
>> sof-tgl-es8336-dmic4ch-ssp0.tplg
>> sof-tgl-es8336-dmic4ch-ssp2.tplg
>> sof-tgl-es8336-ssp0.tplg
>> sof-tgl-es8336-ssp2.tplg
>>
>> All these topologies come from the same file, and generated using
>> macros. That makes no sense to me, this should be the same topology that
>> is remapped at run-time.
> 
> 
> What Amadeo is explaining here is that AVS driver already addresses this
> too - at least in our opinion - see parse_link_formatted_string() in
> sound/soc/intel/avs/topology.c.
> 
> User is allowed to specify widget name as: ssp%d within the topology
> file, leaving kernel with responsibility to fill the missing index. And
> this is something I (perhaps we all) would like to see within the
> framework in the future.

Wow. I don't think anyone saw that concept in the code or comments....

That's not a bad idea, but is this really what the definition of
SND_SOC_TPLG_TUPLE_TYPE_STRING meant?

That's a de-facto ABI or strong convention between kernel and topology,
we'd probably need framework extensions to be on the safe side and deal
with variability in more 'controlled' way.

In the AVS example, the 'i2s_link_mask' is used, but in practice there
are cases where the information is only known by checking a package in
the DSDT (baytrail), a DMI quirk, an NHLT information, etc. We should
really plan for extensions with a callback into the topology parser.

> In consequence, avs-driver user does NOT need to define N identical
> topologies. For example, SSP-test board [1] and its definition in
> board_selection.c [2] clearly show that all 6 SSP boards look at the
> exact same file. The same approach is used when speaking of other,
> simple i2s codecs, e.g.: rt274, rt286, rt298. Difference between rt298
> on APL and GML comes down to SSP port number. Here, board_selection.c
> shows different prefixes (apl- vs gml-) but the reality is that both
> files are symlinks looking at the exact same actual topology file with
> ssp%d specified as widget names.

ok, but why even consider symlinks? Why not using the same topology name?

We are making things too hard for package maintainers, e.g. for SOF we
released 198 topologies already, that's not sustainable.

> At the same time, compound machine boards are still permitted and made
> use of, example being TDF8532/Dirana board for Automotive (not yet
> present on the list).
> 
> Really, flexibility is key here. As long as devices found on given
> platform are not connected or dependent on each other, there are no
> major objections preventing card split. Such code scales better and has
> good reuseability.

I don't disagree here but you combined two capabilities in one. The
major simplification comes from the remapping or 'dynamic naming' to
quote your code, the split is a refinement.

Thanks for all the comments so far, good discussion indeed.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-05-04 16:28 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-29 21:55 ASoC component/card relationship Pierre-Louis Bossart
2022-04-29 22:32 ` Curtis Malainey
2022-05-02 15:06   ` Pierre-Louis Bossart
2022-05-02 20:06     ` Curtis Malainey
2022-05-03 18:10 ` Mark Brown
2022-05-03 18:59   ` Pierre-Louis Bossart
2022-05-03 20:31     ` Mark Brown
2022-05-03 21:42       ` Pierre-Louis Bossart
2022-05-04  9:21         ` Amadeusz Sławiński
2022-05-04 15:26           ` Pierre-Louis Bossart
2022-05-04 15:38             ` Jaroslav Kysela
2022-05-04 16:00             ` Cezary Rojewski
2022-05-04 16:27               ` Pierre-Louis Bossart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.