RE: [PATCH v2 5/9] dt-bindings: display: bridge: Add ITE IT6263 LVDS to HDMI converter

From: Biju Das
Date: Mon Oct 14 2024 - 04:32:05 EST

Hi Liu,

> -----Original Message-----
> From: Liu Ying <victor.liu@xxxxxxx>
> Sent: Monday, October 14, 2024 6:34 AM
> Subject: Re: [PATCH v2 5/9] dt-bindings: display: bridge: Add ITE IT6263 LVDS to HDMI converter
>
> On 10/14/2024, Dmitry Baryshkov wrote:
> > On Sat, Oct 12, 2024 at 05:14:13PM +0800, Liu Ying wrote:
> >> On 10/12/2024, Dmitry Baryshkov wrote:
> >>> On Sat, Oct 12, 2024 at 03:35:39PM +0800, Liu Ying wrote:
> >>>> Document ITE IT6263 LVDS to HDMI converter.
> >>>>
> >>>> Product link:
> >>>> https://www.ite.com.tw/en/product/cate1/IT6263
> >>>>
> >>>> Signed-off-by: Liu Ying <victor.liu@xxxxxxx>
> >>>> ---
> >>>> v2:
> >>>> * Document number of LVDS link data lanes. (Biju)
> >>>> * Simplify ports property by dropping "oneOf". (Rob)
> >>>>
> >>>> .../bindings/display/bridge/ite,it6263.yaml | 276 ++++++++++++++++++
> >>>> 1 file changed, 276 insertions(+)
> >>>> create mode 100644
> >>>> Documentation/devicetree/bindings/display/bridge/ite,it6263.yaml
> >>>>
> >>>> diff --git
> >>>> a/Documentation/devicetree/bindings/display/bridge/ite,it6263.yaml
> >>>> b/Documentation/devicetree/bindings/display/bridge/ite,it6263.yaml
> >>>> new file mode 100644
> >>>> index 000000000000..bc2bbec07623
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/display/bridge/ite,it6263.y
> >>>> +++ aml
> >>>> @@ -0,0 +1,276 @@
> >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML
> >>>> +1.2
> >>>> +---
> >>>> +$id: http://devicetree.org/schemas/display/bridge/ite,it6263.yaml#
> >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>> +
> >>>> +title: ITE IT6263 LVDS to HDMI converter
> >>>> +
> >>>> +maintainers:
> >>>> + - Liu Ying <victor.liu@xxxxxxx>
> >>>> +
> >>>> +description: |
> >>>> + The IT6263 is a high-performance single-chip De-SSC(De-Spread
> >>>> +Spectrum) LVDS
> >>>> + to HDMI converter. Combined with LVDS receiver and HDMI 1.4a
> >>>> +transmitter,
> >>>> + the IT6263 supports LVDS input and HDMI 1.4 output by conversion function.
> >>>> + The built-in LVDS receiver can support single-link and dual-link
> >>>> +LVDS inputs,
> >>>> + and the built-in HDMI transmitter is fully compliant with HDMI
> >>>> +1.4a/3D, HDCP
> >>>> + 1.2 and backward compatible with DVI 1.0 specification.
> >>>> +
> >>>> + The IT6263 also encodes and transmits up to 8 channels of I2S
> >>>> + digital audio, with sampling rate up to 192KHz and sample size
> >>>> + up to 24 bits. In addition, an S/PDIF input port takes in compressed audio of up to 192KHz
> frame rate.
> >>>> +
> >>>> + The newly supported High-Bit Rate(HBR) audio by HDMI
> >>>> + specifications v1.3 is provided by the IT6263 in two interfaces:
> >>>> + the four I2S input ports or the S/PDIF input port. With both
> >>>> + interfaces the highest possible HBR frame rate is supported at up to 768KHz.
> >>>> +
> >>>> +properties:
> >>>
> >>> No LVDS data-mapping support?
> >>
> >> It is enough to document number of LVDS link data lanes because OS
> >> should be able to determine the data-mapping by looking at the number
> >> and the data-mapping capability of the other side of the LVDS link.
> >
> > From what I can see, data-mapping is specified on the consumer sink
> > side of the LVDS link. This means it should go to the bridge's device node.
>
> Then, I won't define data-lanes, because data-mapping implies it, e.g., jeida-24 implies data lanes
> 0/1/2/3, see lvds-data-mapping.yaml.
>
> Please let me know which one you prefer.

For consistency, maybe use data-mapping. See [1]

[1]
https://elixir.bootlin.com/linux/v6.0-rc4/source/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml#L26

Cheers,
Biju

>
> >
> >>
> >>>
> >>>> + compatible:
> >>>> + const: ite,it6263
> >>>> +
> >>>> + reg:
> >>>> + maxItems: 1
> >>>> +
> >>>> + clocks:
> >>>> + maxItems: 1
> >>>> + description: audio master clock
> >>>> +
> >>>> + clock-names:
> >>>> + const: mclk
> >>>> +
> >>>> + reset-gpios:
> >>>> + maxItems: 1
> >>>> +
> >>>> + ivdd-supply:
> >>>> + description: 1.8V digital logic power
> >>>> +
> >>>> + ovdd-supply:
> >>>> + description: 3.3V I/O pin power
> >>>> +
> >>>> + txavcc18-supply:
> >>>> + description: 1.8V HDMI analog frontend power
> >>>> +
> >>>> + txavcc33-supply:
> >>>> + description: 3.3V HDMI analog frontend power
> >>>> +
> >>>> + pvcc1-supply:
> >>>> + description: 1.8V HDMI frontend core PLL power
> >>>> +
> >>>> + pvcc2-supply:
> >>>> + description: 1.8V HDMI frontend filter PLL power
> >>>> +
> >>>> + avcc-supply:
> >>>> + description: 3.3V LVDS frontend power
> >>>> +
> >>>> + anvdd-supply:
> >>>> + description: 1.8V LVDS frontend analog power
> >>>> +
> >>>> + apvdd-supply:
> >>>> + description: 1.8V LVDS frontend PLL power
> >>>> +
> >>>> + "#sound-dai-cells":
> >>>> + const: 0
> >>>> +
> >>>> + ite,lvds-link-num-data-lanes:
> >>>> + $ref: /schemas/types.yaml#/definitions/uint8
> >>>> + enum: [3, 4, 5]
> >>>> + description: number of data lanes per LVDS link
> >>>
> >>> Please use data-lanes property inside the OF graph.
> >>
> >> In both port@0 and port@1?
> >
> > Yes
>
> I'm assuming if data-mapping is defined, then no need to define data-lanes.
>
> >
> >>
> >>>
> >>>> +
> >>>> + ite,i2s-audio-fifo-sources:
> >>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
> >>>> + minItems: 1
> >>>> + maxItems: 4
> >>>> + items:
> >>>> + enum: [0, 1, 2, 3]
> >>>> + description:
> >>>> + Each array element indicates the pin number of an I2S serial data input
> >>>> + line which is connected to an audio FIFO, from audio FIFO0 to FIFO3.
> >>>
> >>> What does that mean from the board point of view? Routed audio links
> >>> for the multichannel audio?
> >>
> >> Yes, also for single channel audio.
> >>
> >>>
> >>>> +
> >>>> + ite,rl-channel-swap-audio-sources:
> >>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
> >>>> + minItems: 1
> >>>> + maxItems: 4
> >>>> + uniqueItems: true
> >>>> + items:
> >>>> + enum: [0, 1, 2, 3]
> >>>> + description:
> >>>> + Each array element indicates an audio source whose right channel and left
> >>>> + channel are swapped by this converter. For I2S, the element is the pin
> >>>> + number of an I2S serial data input line. For S/PDIF, the element is always
> >>>> + 0.
> >>>
> >>> Why?
> >>
> >> Because this converter has the capability to swap right channel and
> >> left channel and S/PDIF always uses audio source0.
> >>
> >>>
> >>>> +
> >>>> + ports:
> >>>> + $ref: /schemas/graph.yaml#/properties/ports
> >>>> +
> >>>> + properties:
> >>>> + port@0:
> >>>> + $ref: /schemas/graph.yaml#/$defs/port-base
> >>>> + unevaluatedProperties: false
> >>>> + description:
> >>>> + The first LVDS input link.
> >>>> + In dual-link LVDS mode, this link works together with the second LVDS
> >>>> + input link, and one link receives odd pixels while the other receives
> >>>> + even pixels. Specify the pixels with one of the dual-lvds-odd-pixels
> >>>> + and dual-lvds-even-pixels properties when and only when dual-link LVDS
> >>>> + mode is used.
> >>>> +
> >>>> + properties:
> >>>> + dual-lvds-odd-pixels:
> >>>> + type: boolean
> >>>> + description: the first sink port for odd pixels
> >>>> +
> >>>> + dual-lvds-even-pixels:
> >>>> + type: boolean
> >>>> + description: the first sink port for even pixels
> >>>> +
> >>>> + port@1:
> >>>> + $ref: /schemas/graph.yaml#/$defs/port-base
> >>>> + unevaluatedProperties: false
> >>>> + description: the second LVDS input link
> >>>> +
> >>>> + properties:
> >>>> + dual-lvds-even-pixels:
> >>>> + type: boolean
> >>>> + description: the second sink port for even pixels
> >>>> +
> >>>> + dual-lvds-odd-pixels:
> >>>> + type: boolean
> >>>> + description: the second sink port for odd pixels
> >>>> +
> >>>> + oneOf:
> >>>> + - required: [dual-lvds-even-pixels]
> >>>> + - required: [dual-lvds-odd-pixels]
> >>>
> >>> This still allows one to specify that both ports provide odd pixels.
> >>> Is that expected? Also why do you need two properties which specify
> >>> the same option.
> >>
> >> No, that is not expected. Description for port@0 already mentions one
> >> link receives odd pixels while the other receives even pixels.
> >
> > That's not expected, but permitted by the bindings.
>
> It is not permitted by port@1. If "dual-lvds-odd-pixels;" is added to port@1 in the dual-link LVDS
> example, the below warning will be generated by dt_binding_check.
>
> Documentation/devicetree/bindings/display/bridge/ite,it6263.example.dtb: hdmi@4c: ports:port@1:
> {'reg': [[1]], 'dual-lvds-even-pixels': True, 'dual-lvds-odd-pixels': True, 'endpoint': {'remote-
> endpoint': [[4294967295]]}} is valid under each of {'required': ['dual-lvds-odd-pixels']},
> {'required': ['dual-lvds-even-pixels']}
> from schema $id: http://devicetree.org/schemas/display/bridge/ite,it6263.yaml#
>
> However, the binding for port@0 does allow DT writers to specify both even and odd pixels. I raised
> similar concerns in v1 discussion.
> According to Rob's comment in #devicetree IRC, the ports property is simplified to this form and more
> description for port@0 is added to tell when to specify the even/odd pixels. If you know any better
> way to indicate the constraints, please shout.
>
> >
> >>
> >> Two options are supported for dual-link LVDS, not the same option:
> >> 1) LVDS link1(port@0) gets odd pixels
> >> and
> >> LVDS link2(port@1) gets even pixels.
> >>
> >> 2) LVDS link1(port@0) gets even pixels
> >> and
> >> LVDS link2(port@1) gets odd pixels.
> >> That's the reason why each port needs two properties to define
> >> odd/even pixels.
> >>
> >>>
> >>> My suggestion would be to add a single root-level property which
> >>> specifies which port provides even pixel data.
> >>
> >> That won't work. The LVDS source side expects the ports of the sink
> >> side specify dual-lvds-{odd,even}-pixels properties.
> >
> > I didn't notice that these properties are already defined.
> >
> > As these properties are common between several schema files, please
> > extract them to a common schema file (like lvds.yaml).
>
> I'm not sure how to do that. Is it obvious?
> Please shed some light.
>
> Only two panel schema files are defining even/odd pixels now - advantech,idk-2121wr.yaml and panel-
> simple-lvds-dual-ports.yaml.
> Maybe, extract them later when more schema files(especially for
> bridges) try to define the same? I'd like to keep a low profile for now.
>
> >
> >>
> >>>
> >>>> +
> >>>> + port@2:
> >>>> + $ref: /schemas/graph.yaml#/properties/port
> >>>> + description: video port for the HDMI output
> >>>> +
> >>>> + port@3:
> >>>> + $ref: /schemas/graph.yaml#/properties/port
> >>>> + description: sound input port
> >>>> +
> >>>> + required:
> >>>> + - port@0
> >>>> + - port@2
> >>>> +
> >>>> +required:
> >>>> + - compatible
> >>>> + - reg
> >>>> + - ivdd-supply
> >>>> + - ovdd-supply
> >>>> + - txavcc18-supply
> >>>> + - txavcc33-supply
> >>>> + - pvcc1-supply
> >>>> + - pvcc2-supply
> >>>> + - avcc-supply
> >>>> + - anvdd-supply
> >>>> + - apvdd-supply
> >>>> + - ite,lvds-link-num-data-lanes
> >>>> + - ports
> >>>> +
> >>>> +additionalProperties: false
> >>>> +
> >>>> +examples:
> >>>> + - |
> >>>> + /* single-link LVDS input */
> >>>> + #include <dt-bindings/gpio/gpio.h>
> >>>> +
> >>>> + i2c {
> >>>> + #address-cells = <1>;
> >>>> + #size-cells = <0>;
> >>>> +
> >>>> + hdmi@4c {
> >>>> + compatible = "ite,it6263";
> >>>> + reg = <0x4c>;
> >>>> + reset-gpios = <&gpio1 10 GPIO_ACTIVE_LOW>;
> >>>> + ivdd-supply = <&reg_buck5>;
> >>>> + ovdd-supply = <&reg_vext_3v3>;
> >>>> + txavcc18-supply = <&reg_buck5>;
> >>>> + txavcc33-supply = <&reg_vext_3v3>;
> >>>> + pvcc1-supply = <&reg_buck5>;
> >>>> + pvcc2-supply = <&reg_buck5>;
> >>>> + avcc-supply = <&reg_vext_3v3>;
> >>>> + anvdd-supply = <&reg_buck5>;
> >>>> + apvdd-supply = <&reg_buck5>;
> >>>> + ite,lvds-link-num-data-lanes = /bits/ 8 <4>;
> >>>> +
> >>>> + ports {
> >>>> + #address-cells = <1>;
> >>>> + #size-cells = <0>;
> >>>> +
> >>>> + port@0 {
> >>>> + reg = <0>;
> >>>> +
> >>>> + it6263_lvds_link1: endpoint {
> >>>> + remote-endpoint = <&ldb_lvds_ch0>;
> >>>> + };
> >>>> + };
> >>>> +
> >>>> + port@2 {
> >>>> + reg = <2>;
> >>>> +
> >>>> + it6263_out: endpoint {
> >>>> + remote-endpoint = <&hdmi_in>;
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> +
> >>>> + - |
> >>>> + /* dual-link LVDS input */
> >>>> + #include <dt-bindings/gpio/gpio.h>
> >>>> +
> >>>> + i2c {
> >>>> + #address-cells = <1>;
> >>>> + #size-cells = <0>;
> >>>> +
> >>>> + hdmi@4c {
> >>>> + compatible = "ite,it6263";
> >>>> + reg = <0x4c>;
> >>>> + reset-gpios = <&gpio1 10 GPIO_ACTIVE_LOW>;
> >>>> + ivdd-supply = <&reg_buck5>;
> >>>> + ovdd-supply = <&reg_vext_3v3>;
> >>>> + txavcc18-supply = <&reg_buck5>;
> >>>> + txavcc33-supply = <&reg_vext_3v3>;
> >>>> + pvcc1-supply = <&reg_buck5>;
> >>>> + pvcc2-supply = <&reg_buck5>;
> >>>> + avcc-supply = <&reg_vext_3v3>;
> >>>> + anvdd-supply = <&reg_buck5>;
> >>>> + apvdd-supply = <&reg_buck5>;
> >>>> + ite,lvds-link-num-data-lanes = /bits/ 8 <4>;
> >>>> +
> >>>> + ports {
> >>>> + #address-cells = <1>;
> >>>> + #size-cells = <0>;
> >>>> +
> >>>> + port@0 {
> >>>> + reg = <0>;
> >>>> + dual-lvds-odd-pixels;
> >>>> +
> >>>> + it6263_lvds_link1_dual: endpoint {
> >>>> + remote-endpoint = <&ldb_lvds_ch0>;
> >>>> + };
> >>>> + };
> >>>> +
> >>>> + port@1 {
> >>>> + reg = <1>;
> >>>> + dual-lvds-even-pixels;
> >>>> +
> >>>> + it6263_lvds_link2_dual: endpoint {
> >>>> + remote-endpoint = <&ldb_lvds_ch1>;
> >>>> + };
> >>>> + };
> >>>> +
> >>>> + port@2 {
> >>>> + reg = <2>;
> >>>> +
> >>>> + it6263_out_dual: endpoint {
> >>>> + remote-endpoint = <&hdmi_in>;
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> --
> >>>> 2.34.1
> >>>>
> >>>
> >>
> >> --
> >> Regards,
> >> Liu Ying
> >>
> >
>
> --
> Regards,
> Liu Ying