diff mbox series

[v3,5/6] dt-bindings: display: sii902x: Add HDMI audio bindings

Message ID e730f059db53a60b8992c8abfc133c00c49de5d1.1552492550.git.jsarha@ti.com
State New
Headers show
Series None | expand

Commit Message

Jyri Sarha March 13, 2019, 4:01 p.m. UTC 
The sii902x chip family supports also HDMI audio. Add binding for
describing the necessary i2s and mclk wiring for it.

Signed-off-by: Jyri Sarha <jsarha@ti.com>

---
 .../bindings/display/bridge/sii902x.txt       | 34 +++++++++++++++++++
 include/dt-bindings/sound/sii902x-audio.h     | 17 ++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 include/dt-bindings/sound/sii902x-audio.h

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Comments

Laurent Pinchart March 13, 2019, 4:47 p.m. UTC | #1 
Hello again,

On Wed, Mar 13, 2019 at 06:29:19PM +0200, Laurent Pinchart wrote:
> On Wed, Mar 13, 2019 at 06:01:07PM +0200, Jyri Sarha wrote:

> > The sii902x chip family supports also HDMI audio. Add binding for

> > describing the necessary i2s and mclk wiring for it.

> > 

> > Signed-off-by: Jyri Sarha <jsarha@ti.com>

> > ---

> >  .../bindings/display/bridge/sii902x.txt       | 34 +++++++++++++++++++

> >  include/dt-bindings/sound/sii902x-audio.h     | 17 ++++++++++

> >  2 files changed, 51 insertions(+)

> >  create mode 100644 include/dt-bindings/sound/sii902x-audio.h

> > 

> > diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt

> > index c4c1855ca654..977756841193 100644

> > --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt

> > +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt

> > @@ -8,6 +8,29 @@ Optional properties:

> >  	- interrupts: describe the interrupt line used to inform the host

> >  	  about hotplug events.

> >  	- reset-gpios: OF device-tree gpio specification for RST_N pin.

> > +	- sil,i2s-fifo-routing: Array of exactly 4 integers indicating i2s

> > +	  pins for audio fifo routing. First integer defines routing to

> > +	  fifo 0 and second to fifo 1, etc. Integers can be filled with

> > +	  definitions from: include/dt-bindings/sound/sii902x-audio.h

> > +	  The available definitions are:

> > +	  - ENABLE_BIT:	enable this audio fifo

> > +	  - CONNECT_SD#: route audio input from SD0, SD1, SD2, or SD3 i2s

> > +			 data input pin

> > +	  - LEFT_RIGHT_SWAP_BIT: swap i2s input channels for this fifo

> 

> Are all combinations valid ? For instance, could we have D1 routed to

> the third FIFO, and all other FIFOs disabled ?


I found the answer to this question in the datasheet:

"Note that no gaps are allowed when mapping channels to FIFOs – SD pins
must be mapped to FIFO#0 and FIFO#1 before mapping a channel to FIFO#2,
and so on."

I think we can thus simplify the bindings, and use an approach similar
to the one taken by the data-lanes property for CSI-2. Furthermore, I
think this should be standardized, not left device-specific.

How about an sd-lanes property (better names are welcome) that would
store an array of N integers, where each sd-lanes[i] tells which SD pin
the i-th FIFO is connected to ?

> > +	  I2S HDMI audio is configured only if this property is found.

> > +	- clocks: phandle mclk

> 

> Maybe "clocks: phandle and clock specifier for each clock listed in the clock-names property" ?

> 

> > +	- clock-names: "mclk"

> > +	    Describes SII902x MCLK input. MCLK is used to produce

> > +	    HDMI audio CTS values. This property is required if

> > +	    "i2s-fifo-routing"-property is present. This property follows

> 

> The property is named sil,i2s-fifo-routing.

> 

> > +	    Documentation/devicetree/bindings/clock/clock-bindings.txt

> > +	    consumer binding.

> > +	- #sound-dai-cells = <0>: ASoC codec dai available for simple-card

> > +	    If audio properties are present sii902x provides an ASoC

> > +	    codec component driver that can be used by other ASoC

> > +	    components like simple-card. See binding document for

> > +	    details:

> > +	    Documentation/devicetree/bindings/sound/simple-card.txt

> >  

> >  Optional subnodes:

> >  	- video input: this subnode can contain a video input port node

> > @@ -21,6 +44,17 @@ Example:

> >  		compatible = "sil,sii9022";

> >  		reg = <0x39>;

> >  		reset-gpios = <&pioA 1 0>;

> > +

> > +		#sound-dai-cells = <0>;

> > +		sil,i2s-fifo-routing = <

> > +			(ENABLE_BIT|CONNECT_SD0)

> > +			0

> > +			0

> > +			0

> > +		>;

> > +		clocks = <&mclk>;

> > +		clock-names = "mclk";

> > +

> >  		ports {

> >  			#address-cells = <1>;

> >  			#size-cells = <0>;


I also forgot to mention that the audio connection should be modeled
using OF graph too.

> > diff --git a/include/dt-bindings/sound/sii902x-audio.h b/include/dt-bindings/sound/sii902x-audio.h

> > new file mode 100644

> > index 000000000000..0a849904754b

> > --- /dev/null

> > +++ b/include/dt-bindings/sound/sii902x-audio.h

> > @@ -0,0 +1,17 @@

> > +// SPDX-License-Identifier: GPL-2.0

> > +/*

> > + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/

> > + * Author: Jyri Sarha <jsarha@ti.com>

> > + */

> > +

> > +#ifndef __DT_SII9022_AUDIO_H

> > +#define __DT_SII9022_AUDIO_H

> > +

> > +#define ENABLE_BIT			0x80

> > +#define CONNECT_SD0			0x00

> > +#define CONNECT_SD1			0x10

> > +#define CONNECT_SD2			0x20

> > +#define CONNECT_SD3			0x30

> > +#define LEFT_RIGHT_SWAP_BIT		0x04

> 

> This is fairly generic, should you prefix the macros with SII9022 ?

> 

> > +

> > +#endif /* __DT_SII9022_AUDIO_H */


-- 
Regards,

Laurent Pinchart
Jyri Sarha March 13, 2019, 5:52 p.m. UTC | #2 
On 13/03/2019 18:47, Laurent Pinchart wrote:
> Hello again,

> 

> On Wed, Mar 13, 2019 at 06:29:19PM +0200, Laurent Pinchart wrote:

>> On Wed, Mar 13, 2019 at 06:01:07PM +0200, Jyri Sarha wrote:

>>> The sii902x chip family supports also HDMI audio. Add binding for

>>> describing the necessary i2s and mclk wiring for it.

>>>

>>> Signed-off-by: Jyri Sarha <jsarha@ti.com>

>>> ---

>>>  .../bindings/display/bridge/sii902x.txt       | 34 +++++++++++++++++++

>>>  include/dt-bindings/sound/sii902x-audio.h     | 17 ++++++++++

>>>  2 files changed, 51 insertions(+)

>>>  create mode 100644 include/dt-bindings/sound/sii902x-audio.h

>>>

>>> diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt

>>> index c4c1855ca654..977756841193 100644

>>> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt

>>> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt

>>> @@ -8,6 +8,29 @@ Optional properties:

>>>  	- interrupts: describe the interrupt line used to inform the host

>>>  	  about hotplug events.

>>>  	- reset-gpios: OF device-tree gpio specification for RST_N pin.

>>> +	- sil,i2s-fifo-routing: Array of exactly 4 integers indicating i2s

>>> +	  pins for audio fifo routing. First integer defines routing to

>>> +	  fifo 0 and second to fifo 1, etc. Integers can be filled with

>>> +	  definitions from: include/dt-bindings/sound/sii902x-audio.h

>>> +	  The available definitions are:

>>> +	  - ENABLE_BIT:	enable this audio fifo

>>> +	  - CONNECT_SD#: route audio input from SD0, SD1, SD2, or SD3 i2s

>>> +			 data input pin

>>> +	  - LEFT_RIGHT_SWAP_BIT: swap i2s input channels for this fifo

>>

>> Are all combinations valid ? For instance, could we have D1 routed to

>> the third FIFO, and all other FIFOs disabled ?

> 

> I found the answer to this question in the datasheet:

> 

> "Note that no gaps are allowed when mapping channels to FIFOs – SD pins

> must be mapped to FIFO#0 and FIFO#1 before mapping a channel to FIFO#2,

> and so on."

> 

> I think we can thus simplify the bindings, and use an approach similar

> to the one taken by the data-lanes property for CSI-2. Furthermore, I

> think this should be standardized, not left device-specific.

> 

> How about an sd-lanes property (better names are welcome) that would

> store an array of N integers, where each sd-lanes[i] tells which SD pin

> the i-th FIFO is connected to ?


I agree otherwise, but I would still rather use i2s than sd, because it
is more explicit. SD overlaps with so many other acronyms. So how would
i2s-lanes sound?

> 

>>> +	  I2S HDMI audio is configured only if this property is found.

>>> +	- clocks: phandle mclk

>>

>> Maybe "clocks: phandle and clock specifier for each clock listed in the clock-names property" ?

>>


Ok

>>> +	- clock-names: "mclk"

>>> +	    Describes SII902x MCLK input. MCLK is used to produce

>>> +	    HDMI audio CTS values. This property is required if

>>> +	    "i2s-fifo-routing"-property is present. This property follows

>>

>> The property is named sil,i2s-fifo-routing.

>>


Ok

>>> +	    Documentation/devicetree/bindings/clock/clock-bindings.txt

>>> +	    consumer binding.

>>> +	- #sound-dai-cells = <0>: ASoC codec dai available for simple-card

>>> +	    If audio properties are present sii902x provides an ASoC

>>> +	    codec component driver that can be used by other ASoC

>>> +	    components like simple-card. See binding document for

>>> +	    details:

>>> +	    Documentation/devicetree/bindings/sound/simple-card.txt

>>>  

>>>  Optional subnodes:

>>>  	- video input: this subnode can contain a video input port node

>>> @@ -21,6 +44,17 @@ Example:

>>>  		compatible = "sil,sii9022";

>>>  		reg = <0x39>;

>>>  		reset-gpios = <&pioA 1 0>;

>>> +

>>> +		#sound-dai-cells = <0>;

>>> +		sil,i2s-fifo-routing = <

>>> +			(ENABLE_BIT|CONNECT_SD0)

>>> +			0

>>> +			0

>>> +			0

>>> +		>;

>>> +		clocks = <&mclk>;

>>> +		clock-names = "mclk";

>>> +

>>>  		ports {

>>>  			#address-cells = <1>;

>>>  			#size-cells = <0>;

> 

> I also forgot to mention that the audio connection should be modeled

> using OF graph too.

> 


With the audio configuration sii902x becomes an ASoC codec component.
There are current more than one way to describe the connections between
ASoC components (see [1] and [2] for details). The individual ASoC DAI
drivers (digital audio interface) do not need to know about the audio
system topology, that is taken care of by the machine driver (or card
driver if you will), for instance simple-card or audio-graph-card.

However, since sii902x device is agnostic about the way the connections
are described, I don't think I should go into too many details about how
the possible bindings. However, I should add a reference at least to
audio-graph-card too.

[1] Documentation/devicetree/bindings/sound/simple-card.txt
[2] Documentation/devicetree/bindings/sound/audio-graph-card.txt


>>> diff --git a/include/dt-bindings/sound/sii902x-audio.h b/include/dt-bindings/sound/sii902x-audio.h

>>> new file mode 100644

>>> index 000000000000..0a849904754b

>>> --- /dev/null

>>> +++ b/include/dt-bindings/sound/sii902x-audio.h

>>> @@ -0,0 +1,17 @@

>>> +// SPDX-License-Identifier: GPL-2.0

>>> +/*

>>> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/

>>> + * Author: Jyri Sarha <jsarha@ti.com>

>>> + */

>>> +

>>> +#ifndef __DT_SII9022_AUDIO_H

>>> +#define __DT_SII9022_AUDIO_H

>>> +

>>> +#define ENABLE_BIT			0x80

>>> +#define CONNECT_SD0			0x00

>>> +#define CONNECT_SD1			0x10

>>> +#define CONNECT_SD2			0x20

>>> +#define CONNECT_SD3			0x30

>>> +#define LEFT_RIGHT_SWAP_BIT		0x04

>>

>> This is fairly generic, should you prefix the macros with SII9022 ?

>>

>>> +

>>> +#endif /* __DT_SII9022_AUDIO_H */

> 



-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Laurent Pinchart March 13, 2019, 6:12 p.m. UTC | #3 
Hi Jyri,

On Wed, Mar 13, 2019 at 07:52:08PM +0200, Jyri Sarha wrote:
> On 13/03/2019 18:47, Laurent Pinchart wrote:

> > On Wed, Mar 13, 2019 at 06:29:19PM +0200, Laurent Pinchart wrote:

> >> On Wed, Mar 13, 2019 at 06:01:07PM +0200, Jyri Sarha wrote:

> >>> The sii902x chip family supports also HDMI audio. Add binding for

> >>> describing the necessary i2s and mclk wiring for it.

> >>>

> >>> Signed-off-by: Jyri Sarha <jsarha@ti.com>

> >>> ---

> >>>  .../bindings/display/bridge/sii902x.txt       | 34 +++++++++++++++++++

> >>>  include/dt-bindings/sound/sii902x-audio.h     | 17 ++++++++++

> >>>  2 files changed, 51 insertions(+)

> >>>  create mode 100644 include/dt-bindings/sound/sii902x-audio.h

> >>>

> >>> diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt

> >>> index c4c1855ca654..977756841193 100644

> >>> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt

> >>> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt

> >>> @@ -8,6 +8,29 @@ Optional properties:

> >>>  	- interrupts: describe the interrupt line used to inform the host

> >>>  	  about hotplug events.

> >>>  	- reset-gpios: OF device-tree gpio specification for RST_N pin.

> >>> +	- sil,i2s-fifo-routing: Array of exactly 4 integers indicating i2s

> >>> +	  pins for audio fifo routing. First integer defines routing to

> >>> +	  fifo 0 and second to fifo 1, etc. Integers can be filled with

> >>> +	  definitions from: include/dt-bindings/sound/sii902x-audio.h

> >>> +	  The available definitions are:

> >>> +	  - ENABLE_BIT:	enable this audio fifo

> >>> +	  - CONNECT_SD#: route audio input from SD0, SD1, SD2, or SD3 i2s

> >>> +			 data input pin

> >>> +	  - LEFT_RIGHT_SWAP_BIT: swap i2s input channels for this fifo

> >>

> >> Are all combinations valid ? For instance, could we have D1 routed to

> >> the third FIFO, and all other FIFOs disabled ?

> > 

> > I found the answer to this question in the datasheet:

> > 

> > "Note that no gaps are allowed when mapping channels to FIFOs – SD pins

> > must be mapped to FIFO#0 and FIFO#1 before mapping a channel to FIFO#2,

> > and so on."

> > 

> > I think we can thus simplify the bindings, and use an approach similar

> > to the one taken by the data-lanes property for CSI-2. Furthermore, I

> > think this should be standardized, not left device-specific.

> > 

> > How about an sd-lanes property (better names are welcome) that would

> > store an array of N integers, where each sd-lanes[i] tells which SD pin

> > the i-th FIFO is connected to ?

> 

> I agree otherwise, but I would still rather use i2s than sd, because it

> is more explicit. SD overlaps with so many other acronyms. So how would

> i2s-lanes sound?


Sounds good to me. It's a better name, so it's welcome :-) I don't know
what terminology is usually used in the audio world for this, so I was
pretty sure my initial name proposal was bad.

Is there a risk of needing to describe the clock lane separately in the
future (for this or another I2S-related chip) ? If so, maybe
i2s-data-lanes, or just data-lanes, would be a better pick.

> >>> +	  I2S HDMI audio is configured only if this property is found.

> >>> +	- clocks: phandle mclk

> >>

> >> Maybe "clocks: phandle and clock specifier for each clock listed in

> >> the clock-names property" ?

> 

> Ok

> 

> >>> +	- clock-names: "mclk"

> >>> +	    Describes SII902x MCLK input. MCLK is used to produce

> >>> +	    HDMI audio CTS values. This property is required if

> >>> +	    "i2s-fifo-routing"-property is present. This property follows

> >>

> >> The property is named sil,i2s-fifo-routing.

> 

> Ok

> 

> >>> +	    Documentation/devicetree/bindings/clock/clock-bindings.txt

> >>> +	    consumer binding.

> >>> +	- #sound-dai-cells = <0>: ASoC codec dai available for simple-card

> >>> +	    If audio properties are present sii902x provides an ASoC

> >>> +	    codec component driver that can be used by other ASoC

> >>> +	    components like simple-card. See binding document for

> >>> +	    details:

> >>> +	    Documentation/devicetree/bindings/sound/simple-card.txt

> >>>  

> >>>  Optional subnodes:

> >>>  	- video input: this subnode can contain a video input port node

> >>> @@ -21,6 +44,17 @@ Example:

> >>>  		compatible = "sil,sii9022";

> >>>  		reg = <0x39>;

> >>>  		reset-gpios = <&pioA 1 0>;

> >>> +

> >>> +		#sound-dai-cells = <0>;

> >>> +		sil,i2s-fifo-routing = <

> >>> +			(ENABLE_BIT|CONNECT_SD0)

> >>> +			0

> >>> +			0

> >>> +			0

> >>> +		>;

> >>> +		clocks = <&mclk>;

> >>> +		clock-names = "mclk";

> >>> +

> >>>  		ports {

> >>>  			#address-cells = <1>;

> >>>  			#size-cells = <0>;

> > 

> > I also forgot to mention that the audio connection should be modeled

> > using OF graph too.

> 

> With the audio configuration sii902x becomes an ASoC codec component.

> There are current more than one way to describe the connections between

> ASoC components (see [1] and [2] for details). The individual ASoC DAI

> drivers (digital audio interface) do not need to know about the audio

> system topology, that is taken care of by the machine driver (or card

> driver if you will), for instance simple-card or audio-graph-card.

> 

> However, since sii902x device is agnostic about the way the connections

> are described, I don't think I should go into too many details about how

> the possible bindings. However, I should add a reference at least to

> audio-graph-card too.

> 

> [1] Documentation/devicetree/bindings/sound/simple-card.txt

> [2] Documentation/devicetree/bindings/sound/audio-graph-card.txt


References to those bindings would be helpful, yes.

> >>> diff --git a/include/dt-bindings/sound/sii902x-audio.h b/include/dt-bindings/sound/sii902x-audio.h

> >>> new file mode 100644

> >>> index 000000000000..0a849904754b

> >>> --- /dev/null

> >>> +++ b/include/dt-bindings/sound/sii902x-audio.h

> >>> @@ -0,0 +1,17 @@

> >>> +// SPDX-License-Identifier: GPL-2.0

> >>> +/*

> >>> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/

> >>> + * Author: Jyri Sarha <jsarha@ti.com>

> >>> + */

> >>> +

> >>> +#ifndef __DT_SII9022_AUDIO_H

> >>> +#define __DT_SII9022_AUDIO_H

> >>> +

> >>> +#define ENABLE_BIT			0x80

> >>> +#define CONNECT_SD0			0x00

> >>> +#define CONNECT_SD1			0x10

> >>> +#define CONNECT_SD2			0x20

> >>> +#define CONNECT_SD3			0x30

> >>> +#define LEFT_RIGHT_SWAP_BIT		0x04

> >>

> >> This is fairly generic, should you prefix the macros with SII9022 ?

> >>

> >>> +

> >>> +#endif /* __DT_SII9022_AUDIO_H */


-- 
Regards,

Laurent Pinchart
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
index c4c1855ca654..977756841193 100644
--- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
+++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
@@ -8,6 +8,29 @@  Optional properties:
 	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- sil,i2s-fifo-routing: Array of exactly 4 integers indicating i2s
+	  pins for audio fifo routing. First integer defines routing to
+	  fifo 0 and second to fifo 1, etc. Integers can be filled with
+	  definitions from: include/dt-bindings/sound/sii902x-audio.h
+	  The available definitions are:
+	  - ENABLE_BIT:	enable this audio fifo
+	  - CONNECT_SD#: route audio input from SD0, SD1, SD2, or SD3 i2s
+			 data input pin
+	  - LEFT_RIGHT_SWAP_BIT: swap i2s input channels for this fifo
+	  I2S HDMI audio is configured only if this property is found.
+	- clocks: phandle mclk
+	- clock-names: "mclk"
+	    Describes SII902x MCLK input. MCLK is used to produce
+	    HDMI audio CTS values. This property is required if
+	    "i2s-fifo-routing"-property is present. This property follows
+	    Documentation/devicetree/bindings/clock/clock-bindings.txt
+	    consumer binding.
+	- #sound-dai-cells = <0>: ASoC codec dai available for simple-card
+	    If audio properties are present sii902x provides an ASoC
+	    codec component driver that can be used by other ASoC
+	    components like simple-card. See binding document for
+	    details:
+	    Documentation/devicetree/bindings/sound/simple-card.txt
 
 Optional subnodes:
 	- video input: this subnode can contain a video input port node
@@ -21,6 +44,17 @@  Example:
 		compatible = "sil,sii9022";
 		reg = <0x39>;
 		reset-gpios = <&pioA 1 0>;
+
+		#sound-dai-cells = <0>;
+		sil,i2s-fifo-routing = <
+			(ENABLE_BIT|CONNECT_SD0)
+			0
+			0
+			0
+		>;
+		clocks = <&mclk>;
+		clock-names = "mclk";
+
 		ports {
 			#address-cells = <1>;
 			#size-cells = <0>;
diff --git a/include/dt-bindings/sound/sii902x-audio.h b/include/dt-bindings/sound/sii902x-audio.h
new file mode 100644
index 000000000000..0a849904754b
--- /dev/null
+++ b/include/dt-bindings/sound/sii902x-audio.h
@@ -0,0 +1,17 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Author: Jyri Sarha <jsarha@ti.com>
+ */
+
+#ifndef __DT_SII9022_AUDIO_H
+#define __DT_SII9022_AUDIO_H
+
+#define ENABLE_BIT			0x80
+#define CONNECT_SD0			0x00
+#define CONNECT_SD1			0x10
+#define CONNECT_SD2			0x20
+#define CONNECT_SD3			0x30
+#define LEFT_RIGHT_SWAP_BIT		0x04
+
+#endif /* __DT_SII9022_AUDIO_H */