[0/9] ASoC: meson: gx: add audio output support
mbox series

Message ID 20200213155159.3235792-1-jbrunet@baylibre.com
Headers show
Series
  • ASoC: meson: gx: add audio output support
Related show

Message

Jerome Brunet Feb. 13, 2020, 3:51 p.m. UTC
This patchset adds support for the i2s and spdif audio outputs of the
amlogic GX SoC family, such the S905, S905X/D, S912 and S805X. These SoCs
are used by a fair amount of boards actively maintained upstream.

This was tested on:
 * amlogic s912 q200
 * libretech s805x-ac (frite)
 * libretech s905x-cc (potato)
 * libretech s905d-pc (tartiflette)

This could also possibly support meson8 32bits SoCs but I have not tested
it myself and it could require some further tweaks.

The audio subsystem found on these SoCs has now been dropped in the newer
designs. All recent SoCs families (like g12a and sm1) derive from the AXG
audio architecture.

Jerome Brunet (9):
  ASoC: core: allow a dt node to provide several components
  ASoC: meson: g12a: extract codec-to-codec utils
  ASoC: meson: aiu: add audio output dt-bindings
  ASoC: meson: aiu: add i2s and spdif support
  ASoC: meson: aiu: add hdmi codec control support
  ASoC: meson: aiu: add internal dac codec control support
  ASoC: meson: axg: extract sound card utils
  ASoC: meson: gx: add sound card dt-binding documentation
  ASoC: meson: gx: add sound card support

 .../bindings/sound/amlogic,aiu.yaml           | 111 +++++
 .../bindings/sound/amlogic,gx-sound-card.yaml | 113 +++++
 include/dt-bindings/sound/meson-aiu.h         |  18 +
 sound/soc/meson/Kconfig                       |  24 ++
 sound/soc/meson/Makefile                      |  15 +
 sound/soc/meson/aiu-acodec-ctrl.c             | 205 +++++++++
 sound/soc/meson/aiu-codec-ctrl.c              | 152 +++++++
 sound/soc/meson/aiu-encoder-i2s.c             | 324 ++++++++++++++
 sound/soc/meson/aiu-encoder-spdif.c           | 209 +++++++++
 sound/soc/meson/aiu-fifo-i2s.c                | 153 +++++++
 sound/soc/meson/aiu-fifo-spdif.c              | 186 ++++++++
 sound/soc/meson/aiu-fifo.c                    | 223 ++++++++++
 sound/soc/meson/aiu-fifo.h                    |  50 +++
 sound/soc/meson/aiu.c                         | 390 +++++++++++++++++
 sound/soc/meson/aiu.h                         |  91 ++++
 sound/soc/meson/axg-card.c                    | 403 ++----------------
 sound/soc/meson/g12a-tohdmitx.c               | 219 ++--------
 sound/soc/meson/gx-card.c                     | 141 ++++++
 sound/soc/meson/meson-card-utils.c            | 385 +++++++++++++++++
 sound/soc/meson/meson-card.h                  |  55 +++
 sound/soc/meson/meson-codec-glue.c            | 149 +++++++
 sound/soc/meson/meson-codec-glue.h            |  32 ++
 sound/soc/soc-core.c                          |   8 +
 23 files changed, 3104 insertions(+), 552 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/amlogic,aiu.yaml
 create mode 100644 Documentation/devicetree/bindings/sound/amlogic,gx-sound-card.yaml
 create mode 100644 include/dt-bindings/sound/meson-aiu.h
 create mode 100644 sound/soc/meson/aiu-acodec-ctrl.c
 create mode 100644 sound/soc/meson/aiu-codec-ctrl.c
 create mode 100644 sound/soc/meson/aiu-encoder-i2s.c
 create mode 100644 sound/soc/meson/aiu-encoder-spdif.c
 create mode 100644 sound/soc/meson/aiu-fifo-i2s.c
 create mode 100644 sound/soc/meson/aiu-fifo-spdif.c
 create mode 100644 sound/soc/meson/aiu-fifo.c
 create mode 100644 sound/soc/meson/aiu-fifo.h
 create mode 100644 sound/soc/meson/aiu.c
 create mode 100644 sound/soc/meson/aiu.h
 create mode 100644 sound/soc/meson/gx-card.c
 create mode 100644 sound/soc/meson/meson-card-utils.c
 create mode 100644 sound/soc/meson/meson-card.h
 create mode 100644 sound/soc/meson/meson-codec-glue.c
 create mode 100644 sound/soc/meson/meson-codec-glue.h

Comments

Mark Brown Feb. 13, 2020, 5:18 p.m. UTC | #1
On Thu, Feb 13, 2020 at 04:51:51PM +0100, Jerome Brunet wrote:

> At the moment, querying the dai_name will stop of the first component
> matching the dt node. This does not allow a device (single dt node) to
> provide several ASoC components which could then be used through DT.

> This change let the search go on if the xlate function of the component
> returns an error, giving the possibility to another component to match
> and return the dai_name.

My first question here would be why you'd want to do that rather than
combine everything into a single component since the hardware seems to
be doing that anyway.  Hopefully the rest of the series will answer this
but it'd be good in the changelog here.
Jerome Brunet Feb. 13, 2020, 5:37 p.m. UTC | #2
On Thu 13 Feb 2020 at 18:18, Mark Brown <broonie@kernel.org> wrote:

> On Thu, Feb 13, 2020 at 04:51:51PM +0100, Jerome Brunet wrote:
>
>> At the moment, querying the dai_name will stop of the first component
>> matching the dt node. This does not allow a device (single dt node) to
>> provide several ASoC components which could then be used through DT.
>
>> This change let the search go on if the xlate function of the component
>> returns an error, giving the possibility to another component to match
>> and return the dai_name.
>
> My first question here would be why you'd want to do that rather than
> combine everything into a single component since the hardware seems to
> be doing that anyway.  Hopefully the rest of the series will answer this
> but it'd be good in the changelog here.

Hi Mark,

Sorry if I was not clear enough.

This HW is messy. It is indeed one monolithic device which
provides several functions/sub-devices/components

I tried several approaches:

* Just 1 component: This was ugly because the part that is present only on 1
SoC variant, I needed to reconstruct the dai, widget, route and control
table which involved a fair amount of useless copies.

* A lot of devices (and components) with syscon: This ended up being even
  uglier, difficult to work with since it did not really reflected the
  actual HW.

The solution proposed here is just one device with 3 possible
components (groups):
* The CPU producers a associated path
* The HDMI control
* The Internal DAC control

The impact on ASoC is rather small, the driver reflect quite well what
the HW is and, with a sound-dai-cell=2, it fairly simple in DT as well.

Do you think there is something wrong with a linux device providing
several ASoC components ?
Mark Brown Feb. 13, 2020, 5:40 p.m. UTC | #3
On Thu, Feb 13, 2020 at 06:37:41PM +0100, Jerome Brunet wrote:

> > My first question here would be why you'd want to do that rather than
> > combine everything into a single component since the hardware seems to
> > be doing that anyway.  Hopefully the rest of the series will answer this
> > but it'd be good in the changelog here.

> Do you think there is something wrong with a linux device providing
> several ASoC components ?

I don't know that it's actively wrong, it's more a comment about the
changelog only describing the what of the change and not the why - the
original idea for a component was that there should be a 1:1 mapping
between components and devices but as you say it's not actually a big
change to let things get split up more.
Mark Brown Feb. 13, 2020, 6:21 p.m. UTC | #4
On Thu, Feb 13, 2020 at 04:51:55PM +0100, Jerome Brunet wrote:

> +int aiu_add_component(struct device *dev,
> +		      const struct snd_soc_component_driver *component_driver,
> +		      struct snd_soc_dai_driver *dai_drv,
> +		      int num_dai,
> +		      const char *debugfs_prefix)
> +{
> +	struct snd_soc_component *component;
> +
> +	component = devm_kzalloc(dev, sizeof(*component), GFP_KERNEL);
> +	if (!component)
> +		return -ENOMEM;
> +
> +#ifdef CONFIG_DEBUG_FS
> +	component->debugfs_prefix = debugfs_prefix;
> +#endif

You really shouldn't be doing this as it could conflict with something
the machine driver wants to do however it's probably not going to be an
issue in practice as it's not like there's going to be multiple SoCs in
the card at once and if there were there'd doubltess be other issues.
Jerome Brunet Feb. 14, 2020, 1:16 p.m. UTC | #5
On Thu 13 Feb 2020 at 19:21, Mark Brown <broonie@kernel.org> wrote:

> On Thu, Feb 13, 2020 at 04:51:55PM +0100, Jerome Brunet wrote:
>
>> +int aiu_add_component(struct device *dev,
>> +		      const struct snd_soc_component_driver *component_driver,
>> +		      struct snd_soc_dai_driver *dai_drv,
>> +		      int num_dai,
>> +		      const char *debugfs_prefix)
>> +{
>> +	struct snd_soc_component *component;
>> +
>> +	component = devm_kzalloc(dev, sizeof(*component), GFP_KERNEL);
>> +	if (!component)
>> +		return -ENOMEM;
>> +
>> +#ifdef CONFIG_DEBUG_FS
>> +	component->debugfs_prefix = debugfs_prefix;
>> +#endif
>
> You really shouldn't be doing this as it could conflict with something
> the machine driver wants to do however it's probably not going to be an
> issue in practice as it's not like there's going to be multiple SoCs in
> the card at once and if there were there'd doubltess be other issues.

I'm not sure I understand (and I'd prefer to :) )

As you said before, initially the there was supposed to be a 1:1 mapping
between device and component. The component name is directly derived
from the device name, and the debugfs directory is created from component name.

I would have preferred to use snd_soc_register_component() directly, but
with multiple components from the same device I got:

debugfs: Directory 'c1105400.audio-controller' with parent 'AWESOME-CARD' already present!
debugfs: Directory 'c1105400.audio-controller' with parent 'AWESOME-CARD' already present!

I copied the code above from other direct users of
snd_soc_add_component() (soc-generic-dmaengine-pcm.c and
stm32_adfsdm.c). I suppose they had the same name collision issue.

Instead of addressing the debugfs side effect, maybe  we could just make
sure that each component name is unique within ASoC ? I'd be happy submit
something if you think this can helpful.
Mark Brown Feb. 14, 2020, 1:22 p.m. UTC | #6
On Fri, Feb 14, 2020 at 02:16:10PM +0100, Jerome Brunet wrote:
> On Thu 13 Feb 2020 at 19:21, Mark Brown <broonie@kernel.org> wrote:

> >> +#ifdef CONFIG_DEBUG_FS
> >> +	component->debugfs_prefix = debugfs_prefix;
> >> +#endif

> > You really shouldn't be doing this as it could conflict with something
> > the machine driver wants to do however it's probably not going to be an
> > issue in practice as it's not like there's going to be multiple SoCs in
> > the card at once and if there were there'd doubltess be other issues.

> I'm not sure I understand (and I'd prefer to :) )

> As you said before, initially the there was supposed to be a 1:1 mapping
> between device and component. The component name is directly derived
> from the device name, and the debugfs directory is created from component name.

I understand why you're doing it but that feature is intended for the
use of cards when they're integrating components, not for devices
trying to register multiple components on the same device.  This means
that a card that tries to use the feature will conflict with what the
driver is doing, but like I say there's no obvious use case for a card
doing that.

> Instead of addressing the debugfs side effect, maybe  we could just make
> sure that each component name is unique within ASoC ? I'd be happy submit
> something if you think this can helpful.

That'd be better.