Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-04-19 Thread Mark Brown
On Fri, Apr 16, 2021 at 02:39:25PM -0500, Pierre-Louis Bossart wrote:
> On 4/16/21 1:55 PM, Mark Brown wrote:

> > to the maximum supported bit width for internal operation so bit width
> > only matters on external interfaces) but I think for a first pass we can
> > get away with forcing everything other than what DPCM has as front ends
> > into static configurations.

> You lost me on the last sentence. did you mean "forcing everything into
> static configurations except for what DPCM has as front-ends"?

Yes...

> It may already be too late for static configurations, Intel, NXP and others
> have started to enable cases where the dailink configuration varies.

Well, they won't be able to use any new stuff until someone implements
support for dynamic configurations in the new stuff.


signature.asc
Description: PGP signature


Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-04-16 Thread Pierre-Louis Bossart




On 4/16/21 1:55 PM, Mark Brown wrote:

On Fri, Apr 16, 2021 at 11:47:01AM -0500, Pierre-Louis Bossart wrote:

On 4/16/21 11:31 AM, Mark Brown wrote:



Not really written down that I can think of.  I think the next steps
that I can think of right now are unfortunately bigger and harder ones,
mainly working out a way to represent digital configuration as a graph
that can be attached to/run in parallel with DAPM other people might
have some better ideas though.  Sorry, I appreciate that this isn't
super helpful :/



I see a need for this in our future SoundWire/SDCA work. So far I was
planning to model the entities as 'widgets' and lets DAPM propagate
activation information for power management, however there are also bits of
information in 'Clusters' (number of channels and spatial relationships)
that could change dynamically and would be interesting to propagate across
entities, so that when we get a stream of data on the bus we know what it
is.


Yes, I was thinking along similar lines last time I looked at it - I was
using the term digital domains.  You'd need some impedence matching
objects for things like SRCs, and the ability to flag which
configuration matters within a domain (eg, a lot of things will covert
to the maximum supported bit width for internal operation so bit width
only matters on external interfaces) but I think for a first pass we can
get away with forcing everything other than what DPCM has as front ends
into static configurations.


You lost me on the last sentence. did you mean "forcing everything into 
static configurations except for what DPCM has as front-ends"?


It may already be too late for static configurations, Intel, NXP and 
others have started to enable cases where the dailink configuration varies.


FWIW both the USB and SDCA class document are very careful with the 
notion of constraints and whether an entity is implemented in the analog 
or digital domains. There are 'clock sources' that may be used in 
specific terminals but no notion of explicit SRC in the graph to leave 
implementers a lot of freedom. There is a notion of 'Usage' that 
describes e.g. FullBand or WideBand without defining what the 
representation is. The bit width is also not described except where 
necessary (history buffers and external bus-facing interfaces). Like you 
said it's mostly the boundaries of the domains that matter.


Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-04-16 Thread Mark Brown
On Fri, Apr 16, 2021 at 11:47:01AM -0500, Pierre-Louis Bossart wrote:
> On 4/16/21 11:31 AM, Mark Brown wrote:

> > Not really written down that I can think of.  I think the next steps
> > that I can think of right now are unfortunately bigger and harder ones,
> > mainly working out a way to represent digital configuration as a graph
> > that can be attached to/run in parallel with DAPM other people might
> > have some better ideas though.  Sorry, I appreciate that this isn't
> > super helpful :/

> I see a need for this in our future SoundWire/SDCA work. So far I was
> planning to model the entities as 'widgets' and lets DAPM propagate
> activation information for power management, however there are also bits of
> information in 'Clusters' (number of channels and spatial relationships)
> that could change dynamically and would be interesting to propagate across
> entities, so that when we get a stream of data on the bus we know what it
> is.

Yes, I was thinking along similar lines last time I looked at it - I was
using the term digital domains.  You'd need some impedence matching
objects for things like SRCs, and the ability to flag which
configuration matters within a domain (eg, a lot of things will covert
to the maximum supported bit width for internal operation so bit width
only matters on external interfaces) but I think for a first pass we can
get away with forcing everything other than what DPCM has as front ends
into static configurations.


signature.asc
Description: PGP signature


Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-04-16 Thread Pierre-Louis Bossart




On 4/16/21 11:31 AM, Mark Brown wrote:

On Fri, Apr 16, 2021 at 04:03:05PM +, codrin.ciubota...@microchip.com wrote:


Thank you for the links! So basically the machine driver disappears and
all the components will be visible in user-space.


Not entirely - you still need something to say how they're wired
together but it'll be a *lot* simpler for anything that currently used
DPCM.


If there is a list with the 'steps' or tasks to achieve this? I can try
to pitch in.


Not really written down that I can think of.  I think the next steps
that I can think of right now are unfortunately bigger and harder ones,
mainly working out a way to represent digital configuration as a graph
that can be attached to/run in parallel with DAPM other people might
have some better ideas though.  Sorry, I appreciate that this isn't
super helpful :/


I see a need for this in our future SoundWire/SDCA work. So far I was 
planning to model the entities as 'widgets' and lets DAPM propagate 
activation information for power management, however there are also bits 
of information in 'Clusters' (number of channels and spatial 
relationships) that could change dynamically and would be interesting to 
propagate across entities, so that when we get a stream of data on the 
bus we know what it is.


when we discussed the multi-configuration support for BT offload, it 
also became apparent that we don't fully control the sample rate changes 
between FE and BE, we only control the start and ends. I fully agree 
that the division between front- and back-ends is becoming limiting and 
DPCM is not only complicated but difficult to stretch further.


Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-04-16 Thread Codrin.Ciubotariu
On 16.04.2021 19:31, Mark Brown wrote:
> On Fri, Apr 16, 2021 at 04:03:05PM +, codrin.ciubota...@microchip.com 
> wrote:
> 
>> Thank you for the links! So basically the machine driver disappears and
>> all the components will be visible in user-space.
> 
> Not entirely - you still need something to say how they're wired
> together but it'll be a *lot* simpler for anything that currently used
> DPCM.

Right, device-tree/acpi wiring is not enough...

> 
>> If there is a list with the 'steps' or tasks to achieve this? I can try
>> to pitch in.
> 
> Not really written down that I can think of.  I think the next steps
> that I can think of right now are unfortunately bigger and harder ones,
> mainly working out a way to represent digital configuration as a graph
> that can be attached to/run in parallel with DAPM other people might
> have some better ideas though.  Sorry, I appreciate that this isn't
> super helpful :/
> 

I think I have good picture, a more robust card->dai_link, not with just 
FEs and BEs ... I will try to monitor the alsa list more, see what other 
people are working on. Thank you for your time!

Best regards,
Codrin


Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-04-16 Thread Mark Brown
On Fri, Apr 16, 2021 at 04:03:05PM +, codrin.ciubota...@microchip.com wrote:

> Thank you for the links! So basically the machine driver disappears and 
> all the components will be visible in user-space.

Not entirely - you still need something to say how they're wired
together but it'll be a *lot* simpler for anything that currently used
DPCM.

> If there is a list with the 'steps' or tasks to achieve this? I can try 
> to pitch in.

Not really written down that I can think of.  I think the next steps
that I can think of right now are unfortunately bigger and harder ones,
mainly working out a way to represent digital configuration as a graph
that can be attached to/run in parallel with DAPM other people might
have some better ideas though.  Sorry, I appreciate that this isn't
super helpful :/


signature.asc
Description: PGP signature


Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-04-16 Thread Codrin.Ciubotariu
On 15.04.2021 20:25, Mark Brown wrote:
> On Thu, Apr 15, 2021 at 04:56:00PM +, codrin.ciubota...@microchip.com 
> wrote:
> 
>> Are there any plans for refactoring DPCM? any ideas ongoing? I also have
>> some changes for PCM dmaengine, in the same 'style', similar to what I
>> sent some time ago...
>> I can adjust to different ideas, if there are any, but, for a start, can
>> anyone confirm that the problem I am trying to fix is real?
> 
> Lars-Peter's presentation from ELC in 2016 (!) is probably the clearest
> summary of the ideas:
> 
> https://elinux.org/images/e/e7/Audio_on_Linux.pdf
> https://youtu.be/6oQF2TzCYtQ
> 
> Essentially the idea is to represent everything, including the DSPs, as
> ASoC components and be able to represent the digital links between
> components in the DAPM graph in the same way we currently represent
> analogue links.  This means we need a way to propagate digital
> configuration along the DAPM graph (or a parallel, cross linked digital
> one).  Sadly I'm not really aware of anyone actively working on the
> actual conversion at the minute, Morimoto-san has done a lot of great
> preparatory work to make everything a component which makes the whole
> thing more tractable.
> 

Thank you for the links! So basically the machine driver disappears and 
all the components will be visible in user-space.
If there is a list with the 'steps' or tasks to achieve this? I can try 
to pitch in.

Best regards,
Codrin


Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-04-15 Thread Mark Brown
On Thu, Apr 15, 2021 at 04:56:00PM +, codrin.ciubota...@microchip.com wrote:

> Are there any plans for refactoring DPCM? any ideas ongoing? I also have 
> some changes for PCM dmaengine, in the same 'style', similar to what I 
> sent some time ago...
> I can adjust to different ideas, if there are any, but, for a start, can 
> anyone confirm that the problem I am trying to fix is real?

Lars-Peter's presentation from ELC in 2016 (!) is probably the clearest
summary of the ideas:

   https://elinux.org/images/e/e7/Audio_on_Linux.pdf
   https://youtu.be/6oQF2TzCYtQ

Essentially the idea is to represent everything, including the DSPs, as
ASoC components and be able to represent the digital links between
components in the DAPM graph in the same way we currently represent
analogue links.  This means we need a way to propagate digital
configuration along the DAPM graph (or a parallel, cross linked digital
one).  Sadly I'm not really aware of anyone actively working on the
actual conversion at the minute, Morimoto-san has done a lot of great
preparatory work to make everything a component which makes the whole
thing more tractable.


signature.asc
Description: PGP signature


Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-04-15 Thread Codrin.Ciubotariu
On 15.04.2021 19:17, Mark Brown wrote:
> On Wed, Apr 14, 2021 at 02:58:10PM +, codrin.ciubota...@microchip.com 
> wrote:
> 
>> How about using a different API for ASoC only, since that's the place of
>> DPCM. Only drivers that do not involve DSPs would have to to be changed
>> to call the new snd_pcm_hw_rule_add() variant.
>> Another solution would be to have a different snd_soc_pcm_runtime for BE
>> interfaces (with a new hw_constraints member of course). What do you think?
> 
> I'm really not convinced we want to continue to pile stuff on top of
> DPCM, it is just fundamentally not up to modelling what modern systems
> are able to do - it's already making things more fragile than they
> should be and more special cases seems like it's going to end up making
> that worse.  That said I've not seen the code but...
> 

Are there any plans for refactoring DPCM? any ideas ongoing? I also have 
some changes for PCM dmaengine, in the same 'style', similar to what I 
sent some time ago...
I can adjust to different ideas, if there are any, but, for a start, can 
anyone confirm that the problem I am trying to fix is real?

Best regards,
Codrin


Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-04-15 Thread Mark Brown
On Wed, Apr 14, 2021 at 02:58:10PM +, codrin.ciubota...@microchip.com wrote:

> How about using a different API for ASoC only, since that's the place of 
> DPCM. Only drivers that do not involve DSPs would have to to be changed 
> to call the new snd_pcm_hw_rule_add() variant.
> Another solution would be to have a different snd_soc_pcm_runtime for BE 
> interfaces (with a new hw_constraints member of course). What do you think?

I'm really not convinced we want to continue to pile stuff on top of
DPCM, it is just fundamentally not up to modelling what modern systems
are able to do - it's already making things more fragile than they
should be and more special cases seems like it's going to end up making
that worse.  That said I've not seen the code but...


signature.asc
Description: PGP signature


Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-04-14 Thread Codrin.Ciubotariu
On 23.03.2021 16:18, codrin.ciubota...@microchip.com wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> On 23.03.2021 14:15, Jaroslav Kysela wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
>> content is safe
>>
>> Dne 23. 03. 21 v 12:43 Codrin Ciubotariu napsal(a):
>>
>>> To achieve this, the first thing needed is to detect whether a HW
>>> constraint rule is enforced by a FE or a BE DAI. This means that
>>> snd_pcm_hw_rule_add() needs to be able to differentiate between the two
>>> type of DAIs. For this, the runtime pointer to struct snd_pcm_runtime is
>>> replaced with a pointer to struct snd_pcm_substream, to be able to reach
>>> substream->pcm->internal to differentiate between FE and BE DAIs.
>>
>> Think about other not-so-invasive solution. What about to use
>> 'runtime->private_data' (struct snd_soc_pcm_runtime *) to determine FE / BE?
>>
> 
> I think struct snd_soc_pcm_runtime * is placed in
> substream->private_data, while runtime->private_data is used more by the
> platform drivers. runtime->trigger_master could be an idea, but it looks
> like it's initialized just before the trigger callback is called, way
> after the constraint rules are added and I am not sure it can be
> initialized earlier...
> 
> Best regards,
> Codrin
> 

How about using a different API for ASoC only, since that's the place of 
DPCM. Only drivers that do not involve DSPs would have to to be changed 
to call the new snd_pcm_hw_rule_add() variant.
Another solution would be to have a different snd_soc_pcm_runtime for BE 
interfaces (with a new hw_constraints member of course). What do you think?

Thanks!
Codrin


Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-03-24 Thread Codrin.Ciubotariu
On 24.03.2021 17:28, Pierre-Louis Bossart wrote:
>> I am using hw_params_fixup, but it's not enough. The first thing I do is
>> to not add the BE HW constraint rules in runtime->hw_constraints,
>> because this will potentially affect the FE HW params. If the FE does
>> sampling rate conversion, for example, applying the sampling rate
>> constrain rules from a BE codec on FE is useless. The second thing I do
>> is to apply these BE HW constraint rules to the BE HW params. It's true
>> that the BE HW params can be fine tuned via hw_params_fixup (topology,
>> device-tree or even static parameters) and set in such a way that avoid
>> the BE HW constraints, so we could ignore the constraint rules added by
>> their drivers. It's not every elegant and running the BE HW constraint
>> rules for the fixup BE HW params can be a sanity check. Also, I am
>> thinking that if the FE does no conversion (be_hw_params_fixup missing)
>> and more than one BEs are available, applying the HW constraint rules on
>> the same set of BE HW params could rule out the incompatible BE DAI
>> links and start the compatible ones only. I am not sure this is a real
>> usecase.
> 
> Even after a second cup of coffee I was not able to follow why the
> hw_params_fixup was not enough? That paragraph is rather dense.

Right, sorry about that. :)

> 
> And to be frank I don't fully understand the problem statement above:
> "separate the FE HW constraints from the BE HW constraints". We have
> existing solutions with a DSP-based SRC adjusting FE rates to what is
> required by the BE dailink.
> 
> Maybe it would help to show examples of what you can do today and what
> you cannot, so that we are on the same wavelength on what the
> limitations are and how to remove them?

For example, ad1934 driver sets a constraint to always have 32 sample 
bits [1] and this rule will be added in struct snd_pcm_runtime -> 
hw_constraints [2]. As you can see, this is added early and always. So 
this rule will affect the HW parameters used from the user-space [3] 
and, in our example, only audio streams that have formats of 4B 
containers will be used (S24_LE, S32_LE, etc). For playback, if the 
audio stream won't have this format, the stream will need to be 
converted in software, using plug ALSA plugin for example. This is OK 
for normal PCM:

 HW params
user  <--> CPU <---> AD1934
space ^DAI |
   ||
   -|
  sample bits constraint rule (32b)

For DPCM, we have the same behavior (unfortunately). ad1934, as a BE 
codec, will add it's rule in the same place, which means that it will 
again affect the HW parameters set by user-space. So user-space will 
have to convert all audio streams to have a 32b sample size. If the FE 
is capable of format conversing, this software conversion is useless.

 FE HW paramsBE HW params
user  <--> FE <--> BE CPU <> BE AD1934
space ^DAI   DAI|
   | |
   --|
  sample bits constraint rule (32b)

The format conversion will be done in software instead of hardware (FE).

I hope I was able to be more clear now. Thanks for taking time to 
understand this. I owe you a coffee. :)

Best regards,
Codrin

[1] 
https://elixir.bootlin.com/linux/v5.12-rc4/source/sound/soc/codecs/ad193x.c#L366
[2] 
https://elixir.bootlin.com/linux/v5.12-rc4/source/sound/core/pcm_lib.c#L1141
[3] 
https://elixir.bootlin.com/linux/v5.12-rc4/source/sound/core/pcm_native.c#L692



Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-03-24 Thread Pierre-Louis Bossart




I am using hw_params_fixup, but it's not enough. The first thing I do is
to not add the BE HW constraint rules in runtime->hw_constraints,
because this will potentially affect the FE HW params. If the FE does
sampling rate conversion, for example, applying the sampling rate
constrain rules from a BE codec on FE is useless. The second thing I do
is to apply these BE HW constraint rules to the BE HW params. It's true
that the BE HW params can be fine tuned via hw_params_fixup (topology,
device-tree or even static parameters) and set in such a way that avoid
the BE HW constraints, so we could ignore the constraint rules added by
their drivers. It's not every elegant and running the BE HW constraint
rules for the fixup BE HW params can be a sanity check. Also, I am
thinking that if the FE does no conversion (be_hw_params_fixup missing)
and more than one BEs are available, applying the HW constraint rules on
the same set of BE HW params could rule out the incompatible BE DAI
links and start the compatible ones only. I am not sure this is a real
usecase.


Even after a second cup of coffee I was not able to follow why the 
hw_params_fixup was not enough? That paragraph is rather dense.


And to be frank I don't fully understand the problem statement above: 
"separate the FE HW constraints from the BE HW constraints". We have 
existing solutions with a DSP-based SRC adjusting FE rates to what is 
required by the BE dailink.


Maybe it would help to show examples of what you can do today and what 
you cannot, so that we are on the same wavelength on what the 
limitations are and how to remove them?




Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-03-24 Thread Codrin.Ciubotariu
On 23.03.2021 21:25, Pierre-Louis Bossart wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> the content is safe
> 
> On 3/23/21 6:43 AM, Codrin Ciubotariu wrote:
>> HW constraints are needed to set limitations for HW parameters used to
>> configure the DAIs. All DAIs on the same link must agree upon the HW
>> parameters, so the parameters are affected by the DAIs' features and
>> their limitations. In case of DPCM, the FE DAIs can be used to perform
>> different kind of conversions, such as format or rate changing, bringing
>> the audio stream to a configuration supported by all the DAIs of the BE's
>> link. For this reason, the limitations of the BE DAIs are not always
>> important for the HW parameters between user-space and FE, only for the
>> paratemers between FE and BE DAI links. This brings us to this patch-set,
>> which aims to separate the FE HW constraints from the BE HW constraints.
>> This way, the FE can be used to perform more efficient HW conversions, on
>> the initial audio stream parameters, to parameters supported by the BE
>> DAIs.
>> To achieve this, the first thing needed is to detect whether a HW
>> constraint rule is enforced by a FE or a BE DAI. This means that
>> snd_pcm_hw_rule_add() needs to be able to differentiate between the two
>> type of DAIs. For this, the runtime pointer to struct snd_pcm_runtime is
>> replaced with a pointer to struct snd_pcm_substream, to be able to reach
>> substream->pcm->internal to differentiate between FE and BE DAIs.
>> This change affects many sound drivers (and one gpu drm driver).
>> All these changes are included in the first patch, to have a better
>> overview of the implications created by this change.
>> The second patch adds a new struct snd_pcm_hw_constraints under struct
>> snd_soc_dpcm_runtime, which is used to store the HW constraint rules
>> added by the BE DAIs. This structure is initialized with a subset of the
>> runtime constraint rules which does not include the rules that affect
>> the buffer or period size. snd_pcm_hw_rule_add() will add the BE rules
>> to the new struct snd_pcm_hw_constraints.
>> The third and last patch will apply the BE rule constraints, after the
>> fixup callback. If the fixup HW parameters do not respect the BE
>> constraint rules, the rules will exit with an error. The FE mask and
>> interval constraints are copied to the BE ones, to satisfy the
>> dai_link->dpcm_merged_* flags. The dai_link->dpcm_merged_* flags are
>> used to know if the FE does format or sampling rate conversion.
>>
>> I tested with ad1934 and wm8731 codecs as BEs, with a not-yet-mainlined
>> ASRC as FE, that can also do format conversion. I realize that the
>> change to snd_pcm_hw_rule_add() has a big impact, even though all the
>> drivers use snd_pcm_hw_rule_add() with substream->runtime, so passing
>> substream instead of runtime is not that risky.
> 
> can you use the BE hw_params_fixup instead?
> 
> That's what we use for SOF.
> 
> The FE hw_params are propagated to the BE, and then the BE can update
> the hw_params based on its own limitations and pass the result
> downstream, e.g. to a codec.
> 
> I'll copy below my understanding of the flow, which we discussed
> recently in the SOF team:
> 
> my understanding is that we start with the front-end hw_params as the
> basis for the back-end hw_params.
> 
> static int dpcm_fe_dai_hw_params(struct snd_pcm_substream *substream,
>   struct snd_pcm_hw_params *params)
> {
>      struct snd_soc_pcm_runtime *fe = asoc_substream_to_rtd(substream);
>      int ret, stream = substream->stream;
> 
>      mutex_lock_nested(>card->mutex, SND_SOC_CARD_CLASS_RUNTIME);
>      dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE);
> 
>      memcpy(>dpcm[stream].hw_params, params,
>      sizeof(struct snd_pcm_hw_params));
>      ret = dpcm_be_dai_hw_params(fe, stream);
> <<< the BE is handled first.
>      if (ret < 0) {
>      dev_err(fe->dev,"ASoC: hw_params BE failed %d\n", ret);
>      goto out;
>      }
> 
>      dev_dbg(fe->dev, "ASoC: hw_params FE %s rate %d chan %x fmt %d\n",
>      fe->dai_link->name, params_rate(params),
>      params_channels(params), params_format(params));
> 
>      /* call hw_params on the frontend */
>      ret = soc_pcm_hw_params(substream, params);
> 
> then each BE will be configured
> 
> int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream)
> {
>      struct snd_soc_dpcm *dpcm;
>      int ret;
> 
>      for_each_dpcm_be(fe, stream, dpcm) {
> 
>      struct snd_soc_pcm_runtime *be = dpcm->be;
>      struct snd_pcm_substream *be_substream =
>      snd_soc_dpcm_get_substream(be, stream);
> 
>      /* is this op for this BE ? */
>      if (!snd_soc_dpcm_be_can_update(fe, be, stream))
>      continue;
> 
>      /* copy params for each dpcm */
>      memcpy(>hw_params, >dpcm[stream].hw_params,
>      sizeof(struct 

Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-03-23 Thread Pierre-Louis Bossart




On 3/23/21 6:43 AM, Codrin Ciubotariu wrote:

HW constraints are needed to set limitations for HW parameters used to
configure the DAIs. All DAIs on the same link must agree upon the HW
parameters, so the parameters are affected by the DAIs' features and
their limitations. In case of DPCM, the FE DAIs can be used to perform
different kind of conversions, such as format or rate changing, bringing
the audio stream to a configuration supported by all the DAIs of the BE's
link. For this reason, the limitations of the BE DAIs are not always
important for the HW parameters between user-space and FE, only for the
paratemers between FE and BE DAI links. This brings us to this patch-set,
which aims to separate the FE HW constraints from the BE HW constraints.
This way, the FE can be used to perform more efficient HW conversions, on
the initial audio stream parameters, to parameters supported by the BE
DAIs.
To achieve this, the first thing needed is to detect whether a HW
constraint rule is enforced by a FE or a BE DAI. This means that
snd_pcm_hw_rule_add() needs to be able to differentiate between the two
type of DAIs. For this, the runtime pointer to struct snd_pcm_runtime is
replaced with a pointer to struct snd_pcm_substream, to be able to reach
substream->pcm->internal to differentiate between FE and BE DAIs.
This change affects many sound drivers (and one gpu drm driver).
All these changes are included in the first patch, to have a better
overview of the implications created by this change.
The second patch adds a new struct snd_pcm_hw_constraints under struct
snd_soc_dpcm_runtime, which is used to store the HW constraint rules
added by the BE DAIs. This structure is initialized with a subset of the
runtime constraint rules which does not include the rules that affect
the buffer or period size. snd_pcm_hw_rule_add() will add the BE rules
to the new struct snd_pcm_hw_constraints.
The third and last patch will apply the BE rule constraints, after the
fixup callback. If the fixup HW parameters do not respect the BE
constraint rules, the rules will exit with an error. The FE mask and
interval constraints are copied to the BE ones, to satisfy the
dai_link->dpcm_merged_* flags. The dai_link->dpcm_merged_* flags are
used to know if the FE does format or sampling rate conversion.

I tested with ad1934 and wm8731 codecs as BEs, with a not-yet-mainlined
ASRC as FE, that can also do format conversion. I realize that the
change to snd_pcm_hw_rule_add() has a big impact, even though all the
drivers use snd_pcm_hw_rule_add() with substream->runtime, so passing
substream instead of runtime is not that risky.


can you use the BE hw_params_fixup instead?

That's what we use for SOF.

The FE hw_params are propagated to the BE, and then the BE can update 
the hw_params based on its own limitations and pass the result 
downstream, e.g. to a codec.


I'll copy below my understanding of the flow, which we discussed 
recently in the SOF team:


my understanding is that we start with the front-end hw_params as the 
basis for the back-end hw_params.


static int dpcm_fe_dai_hw_params(struct snd_pcm_substream *substream,
 struct snd_pcm_hw_params *params)
{
struct snd_soc_pcm_runtime *fe = asoc_substream_to_rtd(substream);
int ret, stream = substream->stream;

mutex_lock_nested(>card->mutex, SND_SOC_CARD_CLASS_RUNTIME);
dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE);

memcpy(>dpcm[stream].hw_params, params,
sizeof(struct snd_pcm_hw_params));
ret = dpcm_be_dai_hw_params(fe, stream);
<<< the BE is handled first.
if (ret < 0) {
dev_err(fe->dev,"ASoC: hw_params BE failed %d\n", ret);
goto out;
}

dev_dbg(fe->dev, "ASoC: hw_params FE %s rate %d chan %x fmt %d\n",
fe->dai_link->name, params_rate(params),
params_channels(params), params_format(params));

/* call hw_params on the frontend */
ret = soc_pcm_hw_params(substream, params);

then each BE will be configured

int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream)
{
struct snd_soc_dpcm *dpcm;
int ret;

for_each_dpcm_be(fe, stream, dpcm) {

struct snd_soc_pcm_runtime *be = dpcm->be;
struct snd_pcm_substream *be_substream =
snd_soc_dpcm_get_substream(be, stream);

/* is this op for this BE ? */
if (!snd_soc_dpcm_be_can_update(fe, be, stream))
continue;

/* copy params for each dpcm */
memcpy(>hw_params, >dpcm[stream].hw_params,
sizeof(struct snd_pcm_hw_params));

/* perform any hw_params fixups */
ret = snd_soc_link_be_hw_params_fixup(be, >hw_params);

The fixup is the key, in SOF this is where we are going to look for 
information from the topology.


/* fixup the BE DAI link to match any values from topology */
int sof_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct 
snd_pcm_hw_params *params)

{
struct 

Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-03-23 Thread Codrin.Ciubotariu
On 23.03.2021 14:15, Jaroslav Kysela wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> Dne 23. 03. 21 v 12:43 Codrin Ciubotariu napsal(a):
> 
>> To achieve this, the first thing needed is to detect whether a HW
>> constraint rule is enforced by a FE or a BE DAI. This means that
>> snd_pcm_hw_rule_add() needs to be able to differentiate between the two
>> type of DAIs. For this, the runtime pointer to struct snd_pcm_runtime is
>> replaced with a pointer to struct snd_pcm_substream, to be able to reach
>> substream->pcm->internal to differentiate between FE and BE DAIs.
> 
> Think about other not-so-invasive solution. What about to use
> 'runtime->private_data' (struct snd_soc_pcm_runtime *) to determine FE / BE?
> 

I think struct snd_soc_pcm_runtime * is placed in 
substream->private_data, while runtime->private_data is used more by the 
platform drivers. runtime->trigger_master could be an idea, but it looks 
like it's initialized just before the trigger callback is called, way 
after the constraint rules are added and I am not sure it can be 
initialized earlier...

Best regards,
Codrin


Re: [RFC PATCH 0/3] Separate BE DAI HW constraints from FE ones

2021-03-23 Thread Jaroslav Kysela
Dne 23. 03. 21 v 12:43 Codrin Ciubotariu napsal(a):

> To achieve this, the first thing needed is to detect whether a HW
> constraint rule is enforced by a FE or a BE DAI. This means that
> snd_pcm_hw_rule_add() needs to be able to differentiate between the two
> type of DAIs. For this, the runtime pointer to struct snd_pcm_runtime is
> replaced with a pointer to struct snd_pcm_substream, to be able to reach
> substream->pcm->internal to differentiate between FE and BE DAIs.

Think about other not-so-invasive solution. What about to use
'runtime->private_data' (struct snd_soc_pcm_runtime *) to determine FE / BE?

Jaroslav
-- 
Jaroslav Kysela 
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.