| Message ID | e730f059db53a60b8992c8abfc133c00c49de5d1.1552492550.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: sii902x: HDMI-audio support and some fixes | expand |
Hello Jyri, Thank you for the patch. 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 ? > + 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>; > 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 */
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 */
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 */ >
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 */
On 13/03/2019 20:12, Laurent Pinchart wrote: > 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. > Usually (or always AFAIK) there is only one bit clock and one frame clock lane, and 1 - n data lanes. But still being more explicit does not hurt, let's make it i2s-data-lanes. Thanks for the prompt review. I'll try to make the changes for tomorrow. Best regards, Jyri
Hello Jyri, On 3/13/19 6:52 PM, Jyri Sarha wrote: > 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 > > My understanding is that of graph is now the preferred way to describe audio topology. So a think that it needs to be more detailed here. Moreover, the support of audio-graph-card requires a driver update. audio graph card parses endpoints to retrieve codec DAI id. For hdmi codec, get_dai_id() callback has to be implemented, as stated in snd_soc_get_dai_id(), to retrieve the right DAI id: * For example HDMI case, HDMI has video/sound port, * but ALSA SoC needs sound port number only. * Thus counting HDMI DT port/endpoint doesn't work. * Then, it should have .of_xlate_dai_id Regards Olivier >>>> 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 */ >> > >
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 */
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