[PATCH v3 5/6] dt-bindings: display: sii902x: Add HDMI audio bindings
Olivier MOYSAN
olivier.moysan at st.com
Thu Mar 14 10:11:50 UTC 2019
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 at 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 at 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 */
>>
>
>
More information about the dri-devel
mailing list