diff mbox series

[v2,09/14] ASoC: audio-graph-card2: add Yaml Document

Message ID 87wnplvk2a.wl-kuninori.morimoto.gx@renesas.com (mailing list archive)
State New, archived
Headers show
Series ASoC: add Audio Graph Card2 driver | expand

Commit Message

Kuninori Morimoto July 20, 2021, 1:41 a.m. UTC
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

This patch adds Audio Graph Card2 Yaml bindings.
It is similar to Audio Graph Card, but different.

	- audio-graph-card  used "dais"  to indicate DAI-links,
	  audio-graph-card2 uses "links" to it.

	- audio-graph-card  used "phandle" to indicate bitclock/frame-master,
	  audio-graph-card2 uses flag to it.

	- audio-graph-card  used "format" to indicate DAI format,
	  audio-graph-card2 assumes CPU/Codec drivers have .get_fmt support.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../sound/audio-graph-card2-items.yaml        | 80 +++++++++++++++++++
 .../bindings/sound/audio-graph-card2.yaml     | 51 ++++++++++++
 2 files changed, 131 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/audio-graph-card2-items.yaml
 create mode 100644 Documentation/devicetree/bindings/sound/audio-graph-card2.yaml

Comments

Rob Herring (Arm) July 20, 2021, 1:11 p.m. UTC | #1
On Tue, 20 Jul 2021 10:41:01 +0900, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> This patch adds Audio Graph Card2 Yaml bindings.
> It is similar to Audio Graph Card, but different.
> 
> 	- audio-graph-card  used "dais"  to indicate DAI-links,
> 	  audio-graph-card2 uses "links" to it.
> 
> 	- audio-graph-card  used "phandle" to indicate bitclock/frame-master,
> 	  audio-graph-card2 uses flag to it.
> 
> 	- audio-graph-card  used "format" to indicate DAI format,
> 	  audio-graph-card2 assumes CPU/Codec drivers have .get_fmt support.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  .../sound/audio-graph-card2-items.yaml        | 80 +++++++++++++++++++
>  .../bindings/sound/audio-graph-card2.yaml     | 51 ++++++++++++
>  2 files changed, 131 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/audio-graph-card2-items.yaml
>  create mode 100644 Documentation/devicetree/bindings/sound/audio-graph-card2.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/audio-graph-card2-items.yaml: patternProperties:^ports(@[0-1])?$:properties: 'port(@[0-9a-f]+)?' does not match '^[#$a-zA-Z][a-zA-Z0-9,+\\-._@]{0,63}$'
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/audio-graph-card2-items.yaml: ignoring, error in schema: patternProperties: ^ports(@[0-1])?$: properties
warning: no schema found in file: ./Documentation/devicetree/bindings/sound/audio-graph-card2-items.yaml
Documentation/devicetree/bindings/sound/audio-graph-card2.example.dt.yaml:0:0: /example-0/cpu: failed to match any schema with compatible: ['cpu-driver']
Documentation/devicetree/bindings/sound/audio-graph-card2.example.dt.yaml:0:0: /example-0/codec: failed to match any schema with compatible: ['codec-driver']
Documentation/devicetree/bindings/sound/audio-graph-card2-items.example.dt.yaml:0:0: /example-0/mix: failed to match any schema with compatible: ['audio-graph-card2-dsp']
Documentation/devicetree/bindings/sound/audio-graph-card2-items.example.dt.yaml:0:0: /example-0/multi: failed to match any schema with compatible: ['audio-graph-card2-multi']
Documentation/devicetree/bindings/sound/audio-graph-card2-items.example.dt.yaml:0:0: /example-0/codec2codec: failed to match any schema with compatible: ['audio-graph-card2-codec2codec']
\ndoc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1507357

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Rob Herring (Arm) July 20, 2021, 3:12 p.m. UTC | #2
On Mon, Jul 19, 2021 at 7:48 PM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
>
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> This patch adds Audio Graph Card2 Yaml bindings.
> It is similar to Audio Graph Card, but different.
>
>         - audio-graph-card  used "dais"  to indicate DAI-links,
>           audio-graph-card2 uses "links" to it.
>
>         - audio-graph-card  used "phandle" to indicate bitclock/frame-master,
>           audio-graph-card2 uses flag to it.
>
>         - audio-graph-card  used "format" to indicate DAI format,
>           audio-graph-card2 assumes CPU/Codec drivers have .get_fmt support.

Why do we need these changes? I'm not wild about a new generic binding
replacing an existing one which only has 2 or 3 users IIRC. Plus
there's already the Renesas variant. (On the flip side, only a few
users, easier to deprecate the old binding.)

I also would like to see the graph card replace the simple card
binding. Surely it can handle the 'simple' case too.

> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  .../sound/audio-graph-card2-items.yaml        | 80 +++++++++++++++++++
>  .../bindings/sound/audio-graph-card2.yaml     | 51 ++++++++++++
>  2 files changed, 131 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/audio-graph-card2-items.yaml
>  create mode 100644 Documentation/devicetree/bindings/sound/audio-graph-card2.yaml
>
> diff --git a/Documentation/devicetree/bindings/sound/audio-graph-card2-items.yaml b/Documentation/devicetree/bindings/sound/audio-graph-card2-items.yaml
> new file mode 100644
> index 000000000000..ec94cad6b939
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/audio-graph-card2-items.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/audio-graph-card2-items.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Audio Graph Card2 Items Bindings
> +
> +maintainers:
> +  - Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - audio-graph-card2-dsp
> +      - audio-graph-card2-multi
> +      - audio-graph-card2-codec2codec

This appears to be a significant change. Why do we need to encode this
info into the compatible? Can't walking the graph tell us this info?

> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +patternProperties:
> +  "^ports(@[0-1])?$":
> +    $ref: /schemas/graph.yaml#/properties/ports
> +    properties:
> +      port(@[0-9a-f]+)?:
> +        $ref: audio-graph-port.yaml#
> +        unevaluatedProperties: false
> +    additionalProperties: true
> +
> +required:
> +  - compatible
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    mix {
> +        compatible = "audio-graph-card2-dsp";
> +
> +        /* sample ports
> +        ports@0 {
> +            port@0 { mix_fe0_ep: endpoint { remote-endpoint = <&cpu0_ep>; }; };
> +            port@1 { mix_fe1_ep: endpoint { remote-endpoint = <&cpu1_ep>; }; };
> +        };
> +        ports@1 {
> +            port@0 { mix_be0_ep: endpoint { remote-endpoint = <&codec0_ep>; }; };
> +        };
> +        */
> +    };
> +
> +    multi {
> +        compatible = "audio-graph-card2-multi";
> +
> +        /* sample ports
> +        ports@0 {
> +            port@0 { multi_00_ep: endpoint { remote-endpoint = <&cpu2_ep>; }; };
> +            port@1 { multi_01_ep: endpoint { remote-endpoint = <&cpu3_ep>; }; };
> +        };
> +        ports@1 {
> +            port@0 { multi_10_ep: endpoint { remote-endpoint = <&codec1_ep>; }; };
> +            port@1 { multi_11_ep: endpoint { remote-endpoint = <&codec2_ep>; }; };
> +        };
> +        */
> +    };
> +
> +    codec2codec {
> +        compatible = "audio-graph-card2-codec2codec";
> +
> +        /* sample ports
> +        rate = <48000>;
> +        ports {
> +            port@0 { c2c_0_ep: endpoint { remote-endpoint = <&codec3_ep>; }; };
> +            port@1 { c2c_1_ep: endpoint { remote-endpoint = <&codec4_ep>; }; };
> +        };
> +        */
> +    };
> diff --git a/Documentation/devicetree/bindings/sound/audio-graph-card2.yaml b/Documentation/devicetree/bindings/sound/audio-graph-card2.yaml
> new file mode 100644
> index 000000000000..4975f88de025
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/audio-graph-card2.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/audio-graph-card2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Audio Graph Card2 Device Tree Bindings
> +
> +maintainers:
> +  - Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - audio-graph-card2
> +  links:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +  label:
> +    maxItems: 1
> +  routing:
> +    description: |
> +      A list of the connections between audio components.
> +      Each entry is a pair of strings, the first being the
> +      connection's sink, the second being the connection's source.
> +    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> +
> +required:
> +  - compatible
> +  - links
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    sound {
> +        compatible = "audio-graph-card2";
> +
> +        links = <&cpu_port>;
> +    };
> +
> +    cpu {
> +        compatible = "cpu-driver";
> +
> +        cpu_port: port { cpu_ep: endpoint { remote-endpoint = <&codec_ep>; }; };
> +    };
> +
> +    codec {
> +        compatible = "codec-driver";
> +
> +        port { codec_ep: endpoint { remote-endpoint = <&cpu_ep>; }; };
> +    };
> --
> 2.25.1
>
Kuninori Morimoto July 20, 2021, 11:32 p.m. UTC | #3
Hi Rob

> Why do we need these changes? I'm not wild about a new generic binding
> replacing an existing one which only has 2 or 3 users IIRC. Plus
> there's already the Renesas variant. (On the flip side, only a few
> users, easier to deprecate the old binding.)

Sorry I don't understand
	- Who is "2 or 3 users" ?
	- What is "Renesas variant" ?

audio-graph-card2 is based on audio-graph-card,
but different driver not minor variant.
Becase these are different, it can't keep compatibility.
This is the reason why we need audio-graph-card2 instead of expanding
audio-graph-card.

> I also would like to see the graph card replace the simple card
> binding. Surely it can handle the 'simple' case too.

Do you mean you want to merge audio-graph-card and simple-card DT binding ??
audio-graph-card and simple-card are different drivers.

Thank you for your help !!

Best regards
---
Kuninori Morimoto
Mark Brown July 21, 2021, 11:54 a.m. UTC | #4
On Wed, Jul 21, 2021 at 08:32:07AM +0900, Kuninori Morimoto wrote:

> > Why do we need these changes? I'm not wild about a new generic binding
> > replacing an existing one which only has 2 or 3 users IIRC. Plus
> > there's already the Renesas variant. (On the flip side, only a few
> > users, easier to deprecate the old binding.)

> Sorry I don't understand
> 	- Who is "2 or 3 users" ?

Just that there's not that many users of the existing audio-graph-card
(though it's a bit more than 2 or 3 and it's newer stuff rather than
old).

> 	- What is "Renesas variant" ?

I think that's the rsrc-card though that got removed.  There's also the
Tegra audio graph card though.

> audio-graph-card2 is based on audio-graph-card,
> but different driver not minor variant.
> Becase these are different, it can't keep compatibility.
> This is the reason why we need audio-graph-card2 instead of expanding
> audio-graph-card.

I think what Rob is looking for here is a more detailed description of
what the problems are with the existing binding that require a new
binding - what's driving these big changes?  TBH this is part of why
I've been holding off on review, I need to get my head round why we
can't fix the existing binding in place.

> > I also would like to see the graph card replace the simple card
> > binding. Surely it can handle the 'simple' case too.

> Do you mean you want to merge audio-graph-card and simple-card DT binding ??
> audio-graph-card and simple-card are different drivers.

It's more about making sure that new users that currently use
simple-card are using audio-graph-card instead - we need to keep
simple-card around for existing users (or at least the binding but
probably it's more effort than it's worth to merge the binding parsing
code elsewhere) but we should be avoiding adding new users of it.  I've
been pushing people to use audio-graph-card for a while, TBH we should
probably just go ahead and flag simple-card as deprecated in the binding
now since I don't think there's any reason anyone is forced to use it at
this point.
Kuninori Morimoto July 26, 2021, 2:19 a.m. UTC | #5
Hi Mark

Thank you for clearing the topics.
I think I could understand Rob and your expectation.

> It's more about making sure that new users that currently use
> simple-card are using audio-graph-card instead - we need to keep
> simple-card around for existing users (or at least the binding but
> probably it's more effort than it's worth to merge the binding parsing
> code elsewhere) but we should be avoiding adding new users of it.  I've
> been pushing people to use audio-graph-card for a while, TBH we should
> probably just go ahead and flag simple-card as deprecated in the binding
> now since I don't think there's any reason anyone is forced to use it at
> this point.
(snip)
> > > Why do we need these changes? I'm not wild about a new generic binding
> > > replacing an existing one which only has 2 or 3 users IIRC. Plus
> > > there's already the Renesas variant. (On the flip side, only a few
> > > users, easier to deprecate the old binding.)
> 
> > Sorry I don't understand
> > 	- Who is "2 or 3 users" ?
> 
> Just that there's not that many users of the existing audio-graph-card
> (though it's a bit more than 2 or 3 and it's newer stuff rather than
> old).
(snip)
> I think what Rob is looking for here is a more detailed description of
> what the problems are with the existing binding that require a new
> binding - what's driving these big changes?  TBH this is part of why
> I've been holding off on review, I need to get my head round why we
> can't fix the existing binding in place.

OK, let's cleanup the problem.

O : supported
- : not supported
x : Annotated

	simple-card
	 O: Normal connection
	 -: DPCM
	 -: Multi CPU/Codec
	 -: Codec2Codec

	audio-graph-card
(A)	 O: Normal connection
(B)	 x: DPCM
	 -: Multi CPU/Codec
	 -: Codec2Codec

	x: Tegra uses is as customize audio-graph-card

	audio-graph-card2
	 O: Normal connection
	 O: DPCM
	 O: Multi CPU/Codec
	 O: Codec2Codec

We need to keep simple-card, I think there is no discussion is needed here.

About audio-graph-card vs audio-graph-card2,
I think keeping (A) only on audio-graph-card2 is not super difficult
(But some message will be indicated. see below).
Supporting (B) on audio-graph-card2 is difficult.

I'm not sure detail, but we can do like this ?

	step 1)
	- add audio-graph-card2 which have (A) compatibility.
	- indicate "audio-graph-card will be deprecated" on audio-graph-card

	step 2)
	- Tegra switch to use audio-graph-card2
	- confirm that all existing audio-graph-card user have no problem on
	  audio-graph-card2 too.

	step 3)
	- remove audio-graph-card

My concerns are...

	- I'm not sure how DT is strict.
	  If we removed audio-graph-card, but user uses old Tegra DT on it...
	  We can't remove audio-graph-card forever if DT was super strict (?).

	- The naming of audio-graph-card vs audio-graph-card2 driver file.
	  because audio-graph-card will be removed later.

	- audio-graph-card2 can keep (A) compatible, but some features
	  are not recommended. Existing user will get such message.
	  And because of this compatibility, audio-graph-card2 can't remove
	  this "un-recommended" feature.

Thank you for your help !!

Best regards
---
Kuninori Morimoto
Mark Brown Aug. 3, 2021, 4:53 p.m. UTC | #6
On Mon, Jul 26, 2021 at 11:19:20AM +0900, Kuninori Morimoto wrote:

> 	audio-graph-card
> (A)	 O: Normal connection
> (B)	 x: DPCM
> 	 -: Multi CPU/Codec
> 	 -: Codec2Codec
> 
> 	x: Tegra uses is as customize audio-graph-card

TBH I'm not sure this is a bad solution for DPCM - there's the whole
thing with representing DPCM in device tree being fun going on :/

> 	audio-graph-card2
> 	 O: Normal connection
> 	 O: DPCM
> 	 O: Multi CPU/Codec
> 	 O: Codec2Codec

OK, so if there's issues with multi CPU/CODEC due to the representation
of inter-device links not being good enough we definitely need to fix
that and I can see that being a binding change.  For the CODEC<->CODEC
stuff I'd have thought we'd be able to get things working but if we're
changing things anyway perhaps it's not worth it.  It'd probably be
helpful to spell out the specific issues with the multi-device links.

> We need to keep simple-card, I think there is no discussion is needed here.

Yes.

> 
> About audio-graph-card vs audio-graph-card2,
> I think keeping (A) only on audio-graph-card2 is not super difficult
> (But some message will be indicated. see below).
> Supporting (B) on audio-graph-card2 is difficult.

> I'm not sure detail, but we can do like this ?

> 	step 1)
> 	- add audio-graph-card2 which have (A) compatibility.
> 	- indicate "audio-graph-card will be deprecated" on audio-graph-card

> 	step 2)
> 	- Tegra switch to use audio-graph-card2
> 	- confirm that all existing audio-graph-card user have no problem on
> 	  audio-graph-card2 too.

> 	step 3)
> 	- remove audio-graph-card

I guess one other option is to just keep the two audio graph bindings in
parallel, having it as something like a simple links and rich links
variant?  We're going to have to maintain compatibility I think and it'd
make it clearer what's going on, it wouldn't just be a version number
for the binding that's changed but rather something more descriptive.

> My concerns are...

> 	- I'm not sure how DT is strict.
> 	  If we removed audio-graph-card, but user uses old Tegra DT on it...
> 	  We can't remove audio-graph-card forever if DT was super strict (?).

> 	- The naming of audio-graph-card vs audio-graph-card2 driver file.
> 	  because audio-graph-card will be removed later.

Perhaps the approach above with a descriptive name for the new binding
and just keeping both around in parallel makes that all clearer/easier?

> 	- audio-graph-card2 can keep (A) compatible, but some features
> 	  are not recommended. Existing user will get such message.
> 	  And because of this compatibility, audio-graph-card2 can't remove
> 	  this "un-recommended" feature.

Right, some of this depends on how actively bad those features are - if
they're more just not recommended than actively bad then perhaps it's
not worth bothering to deprecate them.
Kuninori Morimoto Aug. 4, 2021, 12:49 a.m. UTC | #7
Hi Mark

Thank you for your review

> > 	audio-graph-card2
> > 	 O: Normal connection
> > 	 O: DPCM
> > 	 O: Multi CPU/Codec
> > 	 O: Codec2Codec
> 
> OK, so if there's issues with multi CPU/CODEC due to the representation
> of inter-device links not being good enough we definitely need to fix
> that and I can see that being a binding change.  For the CODEC<->CODEC
> stuff I'd have thought we'd be able to get things working but if we're
> changing things anyway perhaps it's not worth it.  It'd probably be
> helpful to spell out the specific issues with the multi-device links.

Maybe I'm misunderstanding you, but the reason why we can't 100% merge
audio-graph-card and audio-graph-card2 is that existing audio-graph-card
was focusing only for "Normal" connection, and didn't mind expansion for
advanced connections.

DPCM connection was added for my local use case (= for both simple-card/audio-graph-card),
but it was forcibly expansion, has limitation, no flexibility, etc, etc...
I'm happy that someone is using it, but...
Adding more connection variation (which needs flexibility) (with keeping compatibility)
to existing audio-graph-card is impossible I thought.

The issue is audio-graph-card's flexibility/compatibility, not ALSA SoC.

> > 	step 1)
> > 	- add audio-graph-card2 which have (A) compatibility.
> > 	- indicate "audio-graph-card will be deprecated" on audio-graph-card
> 
> > 	step 2)
> > 	- Tegra switch to use audio-graph-card2
> > 	- confirm that all existing audio-graph-card user have no problem on
> > 	  audio-graph-card2 too.
> 
> > 	step 3)
> > 	- remove audio-graph-card
> 
> I guess one other option is to just keep the two audio graph bindings in
> parallel, having it as something like a simple links and rich links
> variant?  We're going to have to maintain compatibility I think and it'd
> make it clearer what's going on, it wouldn't just be a version number
> for the binding that's changed but rather something more descriptive.

OK, it is nice idea for me, "descriptive" is difficult,
but for example...

	- audio-link-card
	- multi-graph-card
	- link-graph-card
	- audio-mf-graph-card (mf = multi functional)
	...

> Perhaps the approach above with a descriptive name for the new binding
> and just keeping both around in parallel makes that all clearer/easier?

Yes

> > 	- audio-graph-card2 can keep (A) compatible, but some features
> > 	  are not recommended. Existing user will get such message.
> > 	  And because of this compatibility, audio-graph-card2 can't remove
> > 	  this "un-recommended" feature.
> 
> Right, some of this depends on how actively bad those features are - if
> they're more just not recommended than actively bad then perhaps it's
> not worth bothering to deprecate them.

In my quick check (not deep),
for keeping (A) (= Normal) compatibility on new card point of view,
one of not recommended I indicated is property naming (= "dai" vs "link").
But, I noticed that it is not a *super* big deal.

Other one is that new card is assuming that using auto format
(= using .get_fmt on each driver), but we can use "format" property for it
and possible to overwrite.
So, I noticed that keeping Normal connection compatibility on new card
is not super difficult, and "un-recommended" is very small (In my quick check).

Ahh, new card is not supporting "platform" so far (it is supported on audio-graph-card),
and maybe other options/property which I'm not using too.
But it is not a big problem I think, we can add these later.

I want to tell here is that, we can add new card (by new name), and
I think we can keep audio-graph-card's *normal* compatibility on it, (not DPCM).
Of cource we can keep existing audio-graph-card, but easy to switch to new card (?).

I'm not sure it is OK for DT maintainer.

Thank you for your help !!

Best regards
---
Kuninori Morimoto
Mark Brown Aug. 4, 2021, 5:17 p.m. UTC | #8
On Wed, Aug 04, 2021 at 09:49:39AM +0900, Kuninori Morimoto wrote:

> OK, it is nice idea for me, "descriptive" is difficult,
> but for example...

> 	- audio-link-card
> 	- multi-graph-card
> 	- link-graph-card
> 	- audio-mf-graph-card (mf = multi functional)

The -mf- there reads unfortunately differently in English so we
definitely don't want to go with that one I think.  I do agree that it's
hard to come up with a name, possibly rich-link-graph-card or something?
Actually, looking at the bindings documents I'm not 100% clear what the
differences in the binding (as opposed to the code that parses it) are -
this may just be the examples being too cut down to show them.  I'm not
100% clear why we have the three different compatibles in there, that
feels like something that should just be in the graph description,
especially codec2codec since we might have for example both a DSP and a
codec2codec link in the same card.

> Other one is that new card is assuming that using auto format
> (= using .get_fmt on each driver), but we can use "format" property for it
> and possible to overwrite.
> So, I noticed that keeping Normal connection compatibility on new card
> is not super difficult, and "un-recommended" is very small (In my quick check).

> Ahh, new card is not supporting "platform" so far (it is supported on audio-graph-card),
> and maybe other options/property which I'm not using too.
> But it is not a big problem I think, we can add these later.

Yes, these both feel like things we can do on both cards.

> I want to tell here is that, we can add new card (by new name), and
> I think we can keep audio-graph-card's *normal* compatibility on it, (not DPCM).
> Of cource we can keep existing audio-graph-card, but easy to switch to new card (?).

> I'm not sure it is OK for DT maintainer.

Well, I think the big issue from a DT point of view is needing to add a
new generic card at all - there's much less problem with keeping the old
ones around than there is with keeping on adding new generic cards.
Kuninori Morimoto Aug. 4, 2021, 11:47 p.m. UTC | #9
Hi Mark

Thank you for your feedback

> The -mf- there reads unfortunately differently in English so we
> definitely don't want to go with that one I think.  I do agree that it's
> hard to come up with a name, possibly rich-link-graph-card or something?

Thanks. It is a little bit long name, so,
rich-graph-card, or rich-link-card is nice for me.

> Well, I think the big issue from a DT point of view is needing to add a
> new generic card at all - there's much less problem with keeping the old
> ones around than there is with keeping on adding new generic cards.

I guess/hope the DT issue will be disappear if new card can keep
existing binding...

> Actually, looking at the bindings documents I'm not 100% clear what the
> differences in the binding (as opposed to the code that parses it) are -
> this may just be the examples being too cut down to show them.  I'm not
> 100% clear why we have the three different compatibles in there, that
> feels like something that should just be in the graph description,

Ohhhh, yes, indeed. I didn't notice about that !
If my understanding was correct, it can be something like ...

	card {
		compatible = "rich-graph-card";
		...
		links = ...
		
		mix {
			...
		}
		multi {
			...
		}
		codec2codec {
			...
		}
	}

Hmm, nice idea.

> especially codec2codec since we might have for example both a DSP and a
> codec2codec link in the same card.

It is possible in my understanding, but am I misunderstanding ?

... is it naming issue ?
In my understanding, both "DSP" and "MIXer" are using "DPCM" connection,
but driver/sample is calling it as "DSP".
I think "MIXer" and "Codec2Codec" in same card is possible.
I'm not sure about "DSP" case...

Thank you for your help !!

Best regards
---
Kuninori Morimoto
Kuninori Morimoto Aug. 4, 2021, 11:51 p.m. UTC | #10
Hi Mark again

I will take Summer Vacation from tomorrow in 1 week.
I'm sorry for my long term no respoce then.

> Thank you for your feedback
> 
> > The -mf- there reads unfortunately differently in English so we
> > definitely don't want to go with that one I think.  I do agree that it's
> > hard to come up with a name, possibly rich-link-graph-card or something?
> 
> Thanks. It is a little bit long name, so,
> rich-graph-card, or rich-link-card is nice for me.
> 
> > Well, I think the big issue from a DT point of view is needing to add a
> > new generic card at all - there's much less problem with keeping the old
> > ones around than there is with keeping on adding new generic cards.
> 
> I guess/hope the DT issue will be disappear if new card can keep
> existing binding...
> 
> > Actually, looking at the bindings documents I'm not 100% clear what the
> > differences in the binding (as opposed to the code that parses it) are -
> > this may just be the examples being too cut down to show them.  I'm not
> > 100% clear why we have the three different compatibles in there, that
> > feels like something that should just be in the graph description,
> 
> Ohhhh, yes, indeed. I didn't notice about that !
> If my understanding was correct, it can be something like ...
> 
> 	card {
> 		compatible = "rich-graph-card";
> 		...
> 		links = ...
> 		
> 		mix {
> 			...
> 		}
> 		multi {
> 			...
> 		}
> 		codec2codec {
> 			...
> 		}
> 	}
> 
> Hmm, nice idea.
> 
> > especially codec2codec since we might have for example both a DSP and a
> > codec2codec link in the same card.
> 
> It is possible in my understanding, but am I misunderstanding ?
> 
> ... is it naming issue ?
> In my understanding, both "DSP" and "MIXer" are using "DPCM" connection,
> but driver/sample is calling it as "DSP".
> I think "MIXer" and "Codec2Codec" in same card is possible.
> I'm not sure about "DSP" case...
> 
> Thank you for your help !!
> 
> Best regards
> ---
> Kuninori Morimoto
Mark Brown Aug. 5, 2021, 12:52 p.m. UTC | #11
On Thu, Aug 05, 2021 at 08:51:50AM +0900, Kuninori Morimoto wrote:

> I will take Summer Vacation from tomorrow in 1 week.
> I'm sorry for my long term no respoce then.

No worries, thanks for all your hard work on this so far and please
enjoy your vacation!
Mark Brown Aug. 13, 2021, 7:43 p.m. UTC | #12
On Thu, Aug 05, 2021 at 08:47:46AM +0900, Kuninori Morimoto wrote:

> > The -mf- there reads unfortunately differently in English so we
> > definitely don't want to go with that one I think.  I do agree that it's
> > hard to come up with a name, possibly rich-link-graph-card or something?

> Thanks. It is a little bit long name, so,
> rich-graph-card, or rich-link-card is nice for me.

Yeah, let's go with that for now.

> 
> > Actually, looking at the bindings documents I'm not 100% clear what the
> > differences in the binding (as opposed to the code that parses it) are -
> > this may just be the examples being too cut down to show them.  I'm not
> > 100% clear why we have the three different compatibles in there, that
> > feels like something that should just be in the graph description,

> Ohhhh, yes, indeed. I didn't notice about that !
> If my understanding was correct, it can be something like ...

> 	card {
> 		compatible = "rich-graph-card";
> 		...
> 		links = ...
> 		
> 		mix {
> 			...
> 		}
> 		multi {
> 			...
> 		}
> 		codec2codec {
> 			...
> 		}
> 	}
> 
> Hmm, nice idea.

Can we merge some of these types - for example what happens if we get a
CODEC to CODEC link with TDM (eg, a DSP with a link to two mono speakers).
I think we should at least be able to merge TDM with anything else, I
guess we could have all three if we had a DPCM SoC with two CODECs on a
single link though that feels a bit pathological.

> > especially codec2codec since we might have for example both a DSP and a
> > codec2codec link in the same card.

> It is possible in my understanding, but am I misunderstanding ?

> ... is it naming issue ?
> In my understanding, both "DSP" and "MIXer" are using "DPCM" connection,
> but driver/sample is calling it as "DSP".
> I think "MIXer" and "Codec2Codec" in same card is possible.
> I'm not sure about "DSP" case...

I think you're understanding it right - I'm using DSP to mean a SoC
needing DPCM because of the DSP here, sorry that wasn't the clearest way
to describe things.
Kuninori Morimoto Aug. 16, 2021, 4:33 a.m. UTC | #13
Hi Mark

Thank you for your feedback.

> > rich-graph-card, or rich-link-card is nice for me.
> Yeah, let's go with that for now.

OK, thanks.

> Can we merge some of these types - for example what happens if we get a
> CODEC to CODEC link with TDM (eg, a DSP with a link to two mono speakers).
> I think we should at least be able to merge TDM with anything else, I
> guess we could have all three if we had a DPCM SoC with two CODECs on a
> single link though that feels a bit pathological.

Hmm... good question.
I need to double-check it before posting v3.

For this kind of "complex connection", "DT sample" which can be
easily use/test is very helpful for user I hope.

> I think you're understanding it right - I'm using DSP to mean a SoC
> needing DPCM because of the DSP here, sorry that wasn't the clearest way
> to describe things.

OK, complex enough :)

Thank you for your help !!

Best regards
---
Kuninori Morimoto
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/audio-graph-card2-items.yaml b/Documentation/devicetree/bindings/sound/audio-graph-card2-items.yaml
new file mode 100644
index 000000000000..ec94cad6b939
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/audio-graph-card2-items.yaml
@@ -0,0 +1,80 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/audio-graph-card2-items.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Audio Graph Card2 Items Bindings
+
+maintainers:
+  - Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
+
+properties:
+  compatible:
+    enum:
+      - audio-graph-card2-dsp
+      - audio-graph-card2-multi
+      - audio-graph-card2-codec2codec
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^ports(@[0-1])?$":
+    $ref: /schemas/graph.yaml#/properties/ports
+    properties:
+      port(@[0-9a-f]+)?:
+        $ref: audio-graph-port.yaml#
+        unevaluatedProperties: false
+    additionalProperties: true
+
+required:
+  - compatible
+
+additionalProperties: true
+
+examples:
+  - |
+    mix {
+        compatible = "audio-graph-card2-dsp";
+
+        /* sample ports
+        ports@0 {
+            port@0 { mix_fe0_ep: endpoint { remote-endpoint = <&cpu0_ep>; }; };
+            port@1 { mix_fe1_ep: endpoint { remote-endpoint = <&cpu1_ep>; }; };
+        };
+        ports@1 {
+            port@0 { mix_be0_ep: endpoint { remote-endpoint = <&codec0_ep>; }; };
+        };
+        */
+    };
+
+    multi {
+        compatible = "audio-graph-card2-multi";
+
+        /* sample ports
+        ports@0 {
+            port@0 { multi_00_ep: endpoint { remote-endpoint = <&cpu2_ep>; }; };
+            port@1 { multi_01_ep: endpoint { remote-endpoint = <&cpu3_ep>; }; };
+        };
+        ports@1 {
+            port@0 { multi_10_ep: endpoint { remote-endpoint = <&codec1_ep>; }; };
+            port@1 { multi_11_ep: endpoint { remote-endpoint = <&codec2_ep>; }; };
+        };
+        */
+    };
+
+    codec2codec {
+        compatible = "audio-graph-card2-codec2codec";
+
+        /* sample ports
+        rate = <48000>;
+        ports {
+            port@0 { c2c_0_ep: endpoint { remote-endpoint = <&codec3_ep>; }; };
+            port@1 { c2c_1_ep: endpoint { remote-endpoint = <&codec4_ep>; }; };
+        };
+        */
+    };
diff --git a/Documentation/devicetree/bindings/sound/audio-graph-card2.yaml b/Documentation/devicetree/bindings/sound/audio-graph-card2.yaml
new file mode 100644
index 000000000000..4975f88de025
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/audio-graph-card2.yaml
@@ -0,0 +1,51 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/audio-graph-card2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Audio Graph Card2 Device Tree Bindings
+
+maintainers:
+  - Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
+
+properties:
+  compatible:
+    enum:
+      - audio-graph-card2
+  links:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+  label:
+    maxItems: 1
+  routing:
+    description: |
+      A list of the connections between audio components.
+      Each entry is a pair of strings, the first being the
+      connection's sink, the second being the connection's source.
+    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+
+required:
+  - compatible
+  - links
+
+additionalProperties: false
+
+examples:
+  - |
+    sound {
+        compatible = "audio-graph-card2";
+
+        links = <&cpu_port>;
+    };
+
+    cpu {
+        compatible = "cpu-driver";
+
+        cpu_port: port { cpu_ep: endpoint { remote-endpoint = <&codec_ep>; }; };
+    };
+
+    codec {
+        compatible = "codec-driver";
+
+        port { codec_ep: endpoint { remote-endpoint = <&cpu_ep>; }; };
+    };