Benoit Cousson | 11 Mar 12:17 2014

[RFC] ASoC: core: Add support for DAI multicodec

From: Misael Lopez Cruz <misael.lopez <at> ti.com>

DAI link assumes a one to one mapping between CPU DAI and CODEC. In
some cases, the same CPU DAI can be connected to several codecs.
This is the case for example, if you connect two mono codecs to the
same I2S link in order to have a stereo card.
The current ASoC implementation does not allow such setup.

Add support for DAI links composed of a single CPU DAI and multiple
CODECs. Sound cards have to pass the CODECs array in the corresponding
DAI link through a new 'snd_soc_dai_link_codec' struct. Each CODEC in
this array is described in the same manner single CODEC DAIs are
(either DT/OF node or codec_name).

CPU DAI in a multicodec DAI link can have more channels than what each
CODEC has. The number of channels each CODEC is responsible for is
machine specific, hence it's fixed up in machine drivers in a similar
way to what DPCM does.

Add few helpers to make the code more readable and fix a trailing
space in the header as well.

Signed-off-by: Misael Lopez Cruz <misael.lopez <at> ti.com>
[bcousson <at> baylibre.com: Adapt the original series to 3.14]
Signed-off-by: Benoit Cousson <bcousson <at> baylibre.com>
---

Hi Mark and Liam,

I'm not sure this is the only way to address that problem, but
(Continue reading)

Mark Brown | 12 Mar 01:28 2014

Re: [RFC] ASoC: core: Add support for DAI multicodec

On Tue, Mar 11, 2014 at 12:17:24PM +0100, Benoit Cousson wrote:
> From: Misael Lopez Cruz <misael.lopez <at> ti.com>
> 
> DAI link assumes a one to one mapping between CPU DAI and CODEC. In
> some cases, the same CPU DAI can be connected to several codecs.
> This is the case for example, if you connect two mono codecs to the
> same I2S link in order to have a stereo card.
> The current ASoC implementation does not allow such setup.

First up thanks for working on this, it's a feature which has been
requested for a long time but nobody stepped forward to do it before
now.

This is rather large so I've not had time to review it today, I'll try
to get at least a first pass at that done tomorrow.  I did notice that
in your comment about rebasing you mentioned a series - it'd be good if
we could see this as a series, splitting it up would make review easier.

> CPU DAI in a multicodec DAI link can have more channels than what each
> CODEC has. The number of channels each CODEC is responsible for is
> machine specific, hence it's fixed up in machine drivers in a similar
> way to what DPCM does.

This one is interesting.  It feels like most things will want a static
mapping because that's what the hardware does but there will doubtless
be things that could use flexibility.  Liam has looked at this in the
past (more for TDM IIRC, I thought about it for that as well and I seem
to recall Liam's ideas covering it).  It feels like we should start out
with static mappings and build up dynamic later on but equally well
getting something merged would mean we could improve on it.
(Continue reading)

Benoit Cousson | 13 Mar 11:42 2014

Re: [RFC] ASoC: core: Add support for DAI multicodec

Hi Mark,

On 12/03/2014 01:28, Mark Brown wrote:
> On Tue, Mar 11, 2014 at 12:17:24PM +0100, Benoit Cousson wrote:
>> From: Misael Lopez Cruz <misael.lopez <at> ti.com>
>>
>> DAI link assumes a one to one mapping between CPU DAI and CODEC. In
>> some cases, the same CPU DAI can be connected to several codecs.
>> This is the case for example, if you connect two mono codecs to the
>> same I2S link in order to have a stereo card.
>> The current ASoC implementation does not allow such setup.
>
> First up thanks for working on this, it's a feature which has been
> requested for a long time but nobody stepped forward to do it before
> now.
>
> This is rather large so I've not had time to review it today, I'll try
> to get at least a first pass at that done tomorrow.  I did notice that
> in your comment about rebasing you mentioned a series - it'd be good if
> we could see this as a series, splitting it up would make review easier.

Yeah, I know. The issue is that the original series was done on a 3.8 
Android branch with few refactor patches before that one to introduce 
some helpers. During the 3.8 -> 3.13 rebase then 3.13 -> 3.14 rebase, a 
lot of them did not survive due to internal asoc changes and cleanups 
that happened since 3.8.

At least, there are still few helpers that can be introduced sooner, but 
it will still make the main patch pretty huge. I don't think we can make 
the transition in smaller chunks.
(Continue reading)

Mark Brown | 12 Mar 23:51 2014

Re: [RFC] ASoC: core: Add support for DAI multicodec

On Tue, Mar 11, 2014 at 12:17:24PM +0100, Benoit Cousson wrote:

> +struct snd_soc_dai_link_codec {
> +	const char *codec_name;
> +	const struct device_node *codec_of_node;
> +	const char *codec_dai_name;
> +
> +	struct snd_soc_codec *codec;
> +	struct snd_soc_dai *codec_dai;
> +
> +	int (*hw_params_fixup)(struct snd_soc_pcm_runtime *rtd,
> +			       struct snd_pcm_hw_params *params);
> +	struct snd_pcm_hw_params hw_params;
> +};

The implementation looks basically fine, it's possible there's something
nasty in there but the patch is rather large and not quite repetitive
enough.  Most of the interface seems good - I'm not super thrilled with
having the separate CODECs list but equally well the idea of updating
all the machine drivers isn't super awesome either and should be punted
for a cleanup run later.

I would like to see something nicer for the fixup though - I think we
can avoid doing it if we use the TDM API to specify the slots that are
in use by a CODEC.  Xiubo has done some nice work there recently which
is handy.  Instead of having a fixup function if we specified a TDM
and channel map configuration then the core core could override the
params so that the channel count was clamped by how many channels are
actually being sent to the device - so if there's two TDM slots active
the device would be told to play stereo.  Would that work for your use
(Continue reading)

Benoit Cousson | 13 Mar 12:46 2014

Re: [RFC] ASoC: core: Add support for DAI multicodec

On 12/03/2014 23:51, Mark Brown wrote:
> On Tue, Mar 11, 2014 at 12:17:24PM +0100, Benoit Cousson wrote:
>
>> +struct snd_soc_dai_link_codec {
>> +	const char *codec_name;
>> +	const struct device_node *codec_of_node;
>> +	const char *codec_dai_name;
>> +
>> +	struct snd_soc_codec *codec;
>> +	struct snd_soc_dai *codec_dai;
>> +
>> +	int (*hw_params_fixup)(struct snd_soc_pcm_runtime *rtd,
>> +			       struct snd_pcm_hw_params *params);
>> +	struct snd_pcm_hw_params hw_params;
>> +};
>
> The implementation looks basically fine, it's possible there's something
> nasty in there but the patch is rather large and not quite repetitive
> enough.

I think some non-regression tests on that series will be good, because, 
so far it was used on Misa platform (dra7-evm AFAIR) and my own BBB + 
audio cape platform and BBB + 2 mono Codecs from NXP (to be upstreamed 
soon :-)).

We need to ensure that it will not break the thousand ASoC 
implementations we have in mainline today.

> Most of the interface seems good - I'm not super thrilled with
> having the separate CODECs list but equally well the idea of updating
(Continue reading)

Mark Brown | 13 Mar 20:22 2014

Re: [RFC] ASoC: core: Add support for DAI multicodec

On Thu, Mar 13, 2014 at 12:46:43PM +0100, Benoit Cousson wrote:
> On 12/03/2014 23:51, Mark Brown wrote:
> >On Tue, Mar 11, 2014 at 12:17:24PM +0100, Benoit Cousson wrote:

> >is handy.  Instead of having a fixup function if we specified a TDM
> >and channel map configuration then the core core could override the
> >params so that the channel count was clamped by how many channels are
> >actually being sent to the device - so if there's two TDM slots active
> >the device would be told to play stereo.  Would that work for your use
> >cases?

> Yes, I think so. My current setup is pretty basic: 2 mono Class D amplifiers
> connected to the same I2S link to build a stereo card.

> OK, so I'll rebase the patch to asoc/next. I'll try to split the huge patch
> at least into a series of 2 or 3 patches, and I'll remove the fixup part.

Sounds good.  We will need some configuration here so I think adding TDM
parameters to the various structs (I guess slot size and overall counts
should go in the link rather than per CODEC) but we could probably even
get away with doing this after the core patch.
On Thu, Mar 13, 2014 at 12:46:43PM +0100, Benoit Cousson wrote:
> On 12/03/2014 23:51, Mark Brown wrote:
> >On Tue, Mar 11, 2014 at 12:17:24PM +0100, Benoit Cousson wrote:

> >is handy.  Instead of having a fixup function if we specified a TDM
> >and channel map configuration then the core core could override the
> >params so that the channel count was clamped by how many channels are
(Continue reading)


Gmane