diff mbox series

[v2,05/15] ASoC: soc-core.c: enable multi Component

Message ID 87fs5wo94v.wl-kuninori.morimoto.gx@renesas.com
State Accepted
Commit 45655ec69cb954d7fa594054bec33d6d5b99f8d5
Headers show
Series [v2,01/15] ASoC: soc-core: protect dlc->of_node under mutex | expand

Commit Message

Kuninori Morimoto July 10, 2023, 1:20 a.m. UTC
Current ASoC Card is using dlc (snd_soc_dai_link_component) to find
target DAI / Component to be used.
Current dlc has below 3 items to identify DAI / Component

	(a) name	for Component
	(b) of_node	for Component
	(c) dai_name	for DAI

(a) or (b) is used to identify target Component, and (c) is used
to identify DAI.

One of the biggest issue on it today is dlc needs "name matching"
for "dai_name" (c).

It was not a big deal when we were using platform_device, because we
could specify nessesary "dai_name" via its platform_data.

But we need to find DAI name pointer from whole registered datas and/or
each related driver somehow in case of DT, because we can't specify it.
Therefore, Card driver parses DT and assumes the DAI, and find its name
pointer. How to assume is based on each Component and/or Card.

Next biggest issue is Component node (a)/(b).

Basically, Component is registered when CPU/Codec driver was
probed() (X). Here, 1 Component is possible to have some DAIs.

	int xxx_probe(struct platform_device *pdev)
	{
		...
(X)		ret = devm_snd_soc_register_component(pdev->dev,
					&component_driver,
					&dai_driver, dai_driver_num);
		...
	}

The image of each data will be like below.
One note here is "driver" is included for later explanation.

	+-driver------+
	|+-component-+|
	||       dai0||
	||       dai1||
	||        ...||
	|+-----------+|
	+-------------+

The point here is 1 driver has 1 Component, because basically driver
calles snd_soc_register_component() (= X) once.

Here is the very basic CPU/Codec connection image.

	HW image			SW image
	+-- Board ------------+		+-card--------------------------+
	|+-----+      +------+|		|+-driver------+ +-driver------+|
	|| CPU | <--> |CodecA||		||+-component-+| |+-component-+||
	|+-----+      +------+|		|||        dai|<=>|dai        |||
	+---------------------+		||+-----------+| |+-----------+||
					|+-------------+ +-------------+|
					+-------------------------------+

It will be very complex if it has multi DAIs.
Here is intuitive easy to understandable HW / SW example.

	HW image			SW image
	+-- Board ---------------+	+-card--------------------------+
	|+--------+      +------+|	|+-driver------+ +-driver------+|
	|| CPU ch0| <--> |CodecA||	||+-component-+| |+-component-+||
	||        |      +------+|	|||    ch0 dai|<=>|dai        |||
	||        |      +------+|	|||           || |+-----------+||
	||     ch1| <--> |CodecB||	|||           || +-------------+|
	|+--------+      +------+|	|||           || +-driver------+|
	+------------------------+	|||           || |+-component-+||
					|||    ch1 dai|<=>|dai        |||
					||+-----------+| |+-----------+||
					|+-------------+ +-------------+|
					+-------------------------------+

It will be handled as multi interface as "one Card".

	card0,0: CPU-ch0 - CodecA
	card0,1: CPU-ch1 - CodecB
	    ^

But, here is the HW image example which will be more complex

	+-- Basic Board ---------+
	|+--------+      +------+|
	|| CPU ch0| <--> |CodecA||
	||     ch1| <-+  +------+|
	|+--------+   |          |
	+-------------|----------+
	+-- expansion board -----+
	|             |  +------+|
	|             +->|CodecB||
	|                +------+|
	+------------------------+

We intuitively think we want to handle these as "2 Sound Cards".

	card0,0: CPU-ch0 - CodecA
	card1,0: CPU-ch1 - CodecB
	    ^

But below image which we can register today doesn't allow it,
because the same Component will be connected to both Card0/1,
but it will be rejected by (Z).

	 +-driver------+
	 |+-component-+|
	+-card0-------------------------+
	|||           || +-driver------+|
	|||           || |+-component-+||
	|||    ch0 dai|<=>|dai        |||
	|||           || |+-----------+||
	|||           || +-------------+|
	+-------------------------------+
	 ||           ||
	+-card1-------------------------+
	|||           || +-driver------+|
	|||           || |+-component-+||
	|||    ch1 dai|<=>|dai        |||
	|||           || |+-----------+||
	|||           || +-------------+|
	+-------------------------------+
	 |+-----------+|
	 +-------------+

	static int soc_probe_component()
	{
		...
		if (component->card) {
(Z)			if (component->card != card) {
				dev_err(component->dev, ...);
				return -ENODEV;
			}
			return 0;
		}
		...
	}

So, how about to call snd_soc_register_component() (= X) multiple times
on probe() to avoid buplicated component->card limitation, to be like
below ?

	 +-driver------+
	+-card0-------------------------+
	||             | +-driver------+|
	||+-component-+| |+-component-+||
	|||    ch0 dai|<=>|dai        |||
	||+-----------+| |+-----------+||
	||             | +-------------+|
	+-------------------------------+
	 |             |
	+-card1-------------------------+
	||             | +-driver------+|
	||+-component-+| |+-component-+||
	|||    ch1 dai|<=>|dai        |||
	||+-----------+| |+-----------+||
	||             | +-------------+|
	+-------------------------------+
         +-------------+

Yes, looks good. But unfortunately it doesn't help us for now.
Let's see soc_component_to_node() and snd_soc_is_matching_component()

	static struct device_node
	*soc_component_to_node(struct snd_soc_component *component)
	{
		...
(A)		of_node = component->dev->of_node;
		...
	}

	static int snd_soc_is_matching_component(...)
	{
		...
(B)		if (dlc->of_node && component_of_node != dlc->of_node)
		...
	}

dlc checkes "of_node" to identify target component (B),
but this "of_node" came from component->dev (A) which is added
by snd_soc_register_component() (X) on probe().

This means we can have different "component->card", but have same
"component->dev" in this case.

Even though we calls snd_soc_register_component() (= X) multiple times,
all Components have same driver's dev, thus it is impossible to
identified the Component.
And if it was impossible to identify Component, it is impossible to
identify DAI on current implementation.

So, how to handle above complex HW image today is 2 patterns.
One is handles it as "1 big sound card".
The SW image is like below.

SW image
	+-card--------------------------+
	|+-driver------+ +-driver------+|
	||+-component-+| |+-component-+||
	|||    ch0 dai|<=>|dai        |||
	|||           || |+-----------+||
	|||           || +-------------+|
	|||           || +-driver------+|
	|||           || |+-component-+||
	|||    ch1 dai|<->|dai        |||
	||+-----------+| |+-----------+||
	|+-------------+ +-------------+|
	+-------------------------------+

But the problem is not intuitive.
We want to handle it as "2 Cards".

2nd pattern is like below.

SW image
	+-card0-------------------------+
	|+-driver------+ +-driver------+|
	||+-component-+| |+-component-+||
	|||    ch0 dai|<=>|dai        |||
	||+-----------+| |+-----------+||
	|+-------------+ +-------------+|
	+-------------------------------+

	+-card1-------------------------+
	|+-driver------+ +-driver------+|
	||+-component-+| |+-component-+||
	|||    ch1 dai|<=>|dai        |||
	||+-----------+| |+-----------+||
	|+-------------+ +-------------+|
	+-------------------------------+

It handles as "2 Cards", but CPU part needs to be probed as 2 drivers.
It is also not intuitive.

To solve this issue, we need to have multi Component support.

In current implementation, we need to identify Component first
to identify DAI, and it is using name matching to identify DAI.

But how about to be enable to directly identify DAI by unique way
instead of name matching ? In such case, we can directly identify DAI,
then it can identify Component from DAI.

For example Simple-Card / Audio-Graph-Card case, it is specifying DAI
via its node.

Simple-Card

	sound-dai = <&cpu-sound>;

Audio-Graph-Card

	dais = <&cpu-sound>;

If each CPU/Codec driver keeps this property when probing,
we can identify DAI directly from Card.
Being able to identify DAI directly means being able to identify its
Component as well even though Component has same dev (= B).

This patch adds new "dai_node" for it.

To keeping compatibility, it checks "dai_node" first if it has,
otherwise, use existing method (name matching).

Link: https://lore.kernel.org/r/87fskz5yrr.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc-dai.h |  1 +
 include/sound/soc.h     |  1 +
 sound/soc/soc-core.c    | 32 ++++++++++++++++++++++++++++++--
 3 files changed, 32 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index a4538040e88d..a33d803fe548 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -399,6 +399,7 @@  struct snd_soc_dai_driver {
 	unsigned int id;
 	unsigned int base;
 	struct snd_soc_dobj dobj;
+	struct of_phandle_args *dai_args;
 
 	/* DAI driver callbacks */
 	int (*probe)(struct snd_soc_dai *dai);
diff --git a/include/sound/soc.h b/include/sound/soc.h
index b27f84580c5b..dda731795bd4 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -651,6 +651,7 @@  struct snd_soc_dai_link_component {
 	const char *name;
 	struct device_node *of_node;
 	const char *dai_name;
+	struct of_phandle_args *dai_args;
 };
 
 struct snd_soc_dai_link_codec_ch_map {
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index ee309d3fe89c..8487a4c12753 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -238,9 +238,25 @@  static inline void snd_soc_debugfs_exit(void) { }
 
 #endif
 
+static int snd_soc_is_match_dai_args(struct of_phandle_args *args1,
+				     struct of_phandle_args *args2)
+{
+	if (!args1 || !args2)
+		return 0;
+
+	if (args1->np != args2->np)
+		return 0;
+
+	for (int i = 0; i < args1->args_count; i++)
+		if (args1->args[i] != args2->args[i])
+			return 0;
+
+	return 1;
+}
+
 static inline int snd_soc_dlc_component_is_empty(struct snd_soc_dai_link_component *dlc)
 {
-	return !(dlc->name || dlc->of_node);
+	return !(dlc->dai_args || dlc->name || dlc->of_node);
 }
 
 static inline int snd_soc_dlc_component_is_invalid(struct snd_soc_dai_link_component *dlc)
@@ -250,7 +266,7 @@  static inline int snd_soc_dlc_component_is_invalid(struct snd_soc_dai_link_compo
 
 static inline int snd_soc_dlc_dai_is_empty(struct snd_soc_dai_link_component *dlc)
 {
-	return !dlc->dai_name;
+	return !(dlc->dai_args || dlc->dai_name);
 }
 
 static int snd_soc_is_matching_dai(const struct snd_soc_dai_link_component *dlc,
@@ -259,6 +275,9 @@  static int snd_soc_is_matching_dai(const struct snd_soc_dai_link_component *dlc,
 	if (!dlc)
 		return 0;
 
+	if (dlc->dai_args)
+		return snd_soc_is_match_dai_args(dai->driver->dai_args, dlc->dai_args);
+
 	if (!dlc->dai_name)
 		return 1;
 
@@ -799,6 +818,15 @@  static int snd_soc_is_matching_component(
 	if (!dlc)
 		return 0;
 
+	if (dlc->dai_args) {
+		struct snd_soc_dai *dai;
+
+		for_each_component_dais(component, dai)
+			if (snd_soc_is_matching_dai(dlc, dai))
+				return 1;
+		return 0;
+	}
+
 	component_of_node = soc_component_to_node(component);
 
 	if (dlc->of_node && component_of_node != dlc->of_node)