| Message ID | 20230103064615.5311-4-a-bhatia1@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: Introduce dual-link panels & panel-vendors | expand |
On 03/01/2023 07:46, Aradhya Bhatia wrote: > Dual-link LVDS interfaces have 2 links, with even pixels traveling on > one link, and odd pixels on the other. These panels are also generic in > nature, with no documented constraints, much like their single-link > counterparts, "panel-lvds". > > Add a new compatible, "panel-dual-lvds", and a dt-binding document for > these panels. > > Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> > --- > .../display/panel/panel-dual-lvds.yaml | 157 ++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 158 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml > new file mode 100644 > index 000000000000..88a7aa2410be > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml > @@ -0,0 +1,157 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/panel/panel-dual-lvds.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Generic Dual-Link LVDS Display Panel > + > +maintainers: > + - Aradhya Bhatia <a-bhatia1@ti.com> > + - Thierry Reding <thierry.reding@gmail.com> > + > +description: | > + A dual-LVDS interface is a dual-link connection with the even pixels > + traveling on one link, and the odd pixels traveling on the other. > + > +allOf: > + - $ref: panel-common.yaml# > + - $ref: /schemas/display/lvds.yaml/# Drop trailing / > + > +properties: > + compatible: > + oneOf: > + - items: > + - enum: > + - lincolntech,lcd185-101ct > + - microtips,13-101hieb0hf0-s > + - const: panel-dual-lvds > + - const: panel-dual-lvds You cannot have this compatible alone. > + > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/$defs/port-base > + unevaluatedProperties: false > + description: The sink for first set of LVDS pixels. > + > + properties: > + dual-lvds-odd-pixels: > + type: boolean > + > + dual-lvds-even-pixels: > + type: boolean > + > + oneOf: > + - required: [dual-lvds-odd-pixels] > + - required: [dual-lvds-even-pixels] > + > + port@1: > + $ref: /schemas/graph.yaml#/$defs/port-base > + unevaluatedProperties: false > + description: The sink for second set of LVDS pixels. > + > + properties: > + dual-lvds-even-pixels: > + type: boolean > + > + dual-lvds-odd-pixels: > + type: boolean > + > + oneOf: > + - required: [dual-lvds-even-pixels] > + - required: [dual-lvds-odd-pixels] > + > + allOf: > + - if: > + properties: > + port@0: > + properties: > + dual-lvds-odd-pixels: true That's not correct clause. It has no effect. > + required: > + - dual-lvds-odd-pixels > + then: > + properties: > + port@1: > + properties: > + dual-lvds-even-pixels: true > + dual-lvds-odd-pixels: false Why do you need this? Your oneOf before already solves it. > + > + - if: > + properties: > + port@0: > + properties: > + dual-lvds-even-pixels: true > + required: > + - dual-lvds-even-pixels > + then: > + properties: > + port@1: > + properties: > + dual-lvds-odd-pixels: true > + dual-lvds-even-pixels: false > + > + required: > + - port@0 > + - port@1 > + > + port: false > + > +unevaluatedProperties: false > + > +required: > + - compatible > + - width-mm > + - height-mm > + - data-mapping > + - panel-timing > + - ports > + > +examples: > + - |+ Drop + > + panel-dual-lvds { Just "panel". Node names should be generic. > + compatible = "microtips,13-101hieb0hf0-s", "panel-dual-lvds"; > + > + width-mm = <217>; > + height-mm = <136>; > + Best regards, Krzysztof
Hi Krzysztof, Thank you for reviewing the patches! On 03-Jan-23 14:02, Krzysztof Kozlowski wrote: > On 03/01/2023 07:46, Aradhya Bhatia wrote: >> Dual-link LVDS interfaces have 2 links, with even pixels traveling on >> one link, and odd pixels on the other. These panels are also generic in >> nature, with no documented constraints, much like their single-link >> counterparts, "panel-lvds". >> >> Add a new compatible, "panel-dual-lvds", and a dt-binding document for >> these panels. >> >> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >> --- >> .../display/panel/panel-dual-lvds.yaml | 157 ++++++++++++++++++ >> MAINTAINERS | 1 + >> 2 files changed, 158 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >> >> diff --git a/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >> new file mode 100644 >> index 000000000000..88a7aa2410be >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >> @@ -0,0 +1,157 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/display/panel/panel-dual-lvds.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Generic Dual-Link LVDS Display Panel >> + >> +maintainers: >> + - Aradhya Bhatia <a-bhatia1@ti.com> >> + - Thierry Reding <thierry.reding@gmail.com> >> + >> +description: | >> + A dual-LVDS interface is a dual-link connection with the even pixels >> + traveling on one link, and the odd pixels traveling on the other. >> + >> +allOf: >> + - $ref: panel-common.yaml# >> + - $ref: /schemas/display/lvds.yaml/# > > Drop trailing / Okay, will do! > >> + >> +properties: >> + compatible: >> + oneOf: >> + - items: >> + - enum: >> + - lincolntech,lcd185-101ct >> + - microtips,13-101hieb0hf0-s >> + - const: panel-dual-lvds >> + - const: panel-dual-lvds > > You cannot have this compatible alone Okay, will make the change! > >> + >> + ports: >> + $ref: /schemas/graph.yaml#/properties/ports >> + >> + properties: >> + port@0: >> + $ref: /schemas/graph.yaml#/$defs/port-base >> + unevaluatedProperties: false >> + description: The sink for first set of LVDS pixels. >> + >> + properties: >> + dual-lvds-odd-pixels: >> + type: boolean >> + >> + dual-lvds-even-pixels: >> + type: boolean >> + >> + oneOf: >> + - required: [dual-lvds-odd-pixels] >> + - required: [dual-lvds-even-pixels] >> + >> + port@1: >> + $ref: /schemas/graph.yaml#/$defs/port-base >> + unevaluatedProperties: false >> + description: The sink for second set of LVDS pixels. >> + >> + properties: >> + dual-lvds-even-pixels: >> + type: boolean >> + >> + dual-lvds-odd-pixels: >> + type: boolean >> + >> + oneOf: >> + - required: [dual-lvds-even-pixels] >> + - required: [dual-lvds-odd-pixels] >> + >> + allOf: >> + - if: >> + properties: >> + port@0: >> + properties: >> + dual-lvds-odd-pixels: true > > That's not correct clause. It has no effect. The idea behind this is to check the presence of the boolean property. if (dual-lvds-odd-pixels is present) then [..] I tried implementing this: [..] dual-lvds-odd-pixels: - const: true [..] But this is throwing an error. I am confused what else could be done. Can you please suggest what might be a more accurate check here? > >> + required: >> + - dual-lvds-odd-pixels >> + then: >> + properties: >> + port@1: >> + properties: >> + dual-lvds-even-pixels: true >> + dual-lvds-odd-pixels: false > > Why do you need this? Your oneOf before already solves it. I agree with your comment here. It makes sense to only have dual-lvds-even-pixels: true and have the oneOf condition take care of the other. But, I just tested this and it was unable to pick-up this intentionally-added error. I added 'dual-lvds-odd-pixels' property to both the nodes, and dt_binding_check passes successfully (which it should have not.) Instead, if I only keep this, dual-lvds-odd-pixels: false then the dt_binding_check detects the error as it should. Regardless, I am curious why the first method doesn't work. Will try to explore more on that. > >> + >> + - if: >> + properties: >> + port@0: >> + properties: >> + dual-lvds-even-pixels: true >> + required: >> + - dual-lvds-even-pixels >> + then: >> + properties: >> + port@1: >> + properties: >> + dual-lvds-odd-pixels: true >> + dual-lvds-even-pixels: false >> + >> + required: >> + - port@0 >> + - port@1 >> + >> + port: false >> + >> +unevaluatedProperties: false >> + >> +required: >> + - compatible >> + - width-mm >> + - height-mm >> + - data-mapping >> + - panel-timing >> + - ports >> + >> +examples: >> + - |+ > > Drop + Okay! > >> + panel-dual-lvds { > > Just "panel". Node names should be generic. Alright. Will make the change! > >> + compatible = "microtips,13-101hieb0hf0-s", "panel-dual-lvds"; >> + >> + width-mm = <217>; >> + height-mm = <136>; >> + > Regards Aradhya
On 03/01/2023 12:02, Aradhya Bhatia wrote: > But this is throwing an error. I am confused what else could be done. > Can you please suggest what might be a more accurate check here? > >> >>> + required: >>> + - dual-lvds-odd-pixels >>> + then: >>> + properties: >>> + port@1: >>> + properties: >>> + dual-lvds-even-pixels: true >>> + dual-lvds-odd-pixels: false >> >> Why do you need this? Your oneOf before already solves it. > > I agree with your comment here. It makes sense to only have > > dual-lvds-even-pixels: true > > and have the oneOf condition take care of the other. But, I just tested > this and it was unable to pick-up this intentionally-added error. > > I added 'dual-lvds-odd-pixels' property to both the nodes, and > dt_binding_check passes successfully (which it should have not.) > > Instead, if I only keep this, > > dual-lvds-odd-pixels: false > > then the dt_binding_check detects the error as it should. > > Regardless, I am curious why the first method doesn't work. Will try to > explore more on that. The check for presence of properties is only against required:, but you added there properties. Like this: https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/mfd/samsung,s5m8767.yaml#L155 Other way is to drop your both oneOf and entire allOf from ports and use: oneOf: - properties: ports: $ref: /schemas/graph.yaml#/properties/ports properties: port@0: required: - dual-lvds-odd-pixels port@1: required: - dual-lvds-even-pixels - properties: ports: $ref: /schemas/graph.yaml#/properties/ports properties: port@1: required: - dual-lvds-odd-pixels port@0: required: - dual-lvds-even-pixels Best regards, Krzysztof
Il 03/01/23 07:46, Aradhya Bhatia ha scritto: > Dual-link LVDS interfaces have 2 links, with even pixels traveling on > one link, and odd pixels on the other. These panels are also generic in > nature, with no documented constraints, much like their single-link > counterparts, "panel-lvds". > > Add a new compatible, "panel-dual-lvds", and a dt-binding document for > these panels. > > Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> > --- > .../display/panel/panel-dual-lvds.yaml | 157 ++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 158 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml > new file mode 100644 > index 000000000000..88a7aa2410be > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml > @@ -0,0 +1,157 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/panel/panel-dual-lvds.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Generic Dual-Link LVDS Display Panel > + > +maintainers: > + - Aradhya Bhatia <a-bhatia1@ti.com> > + - Thierry Reding <thierry.reding@gmail.com> > + > +description: | > + A dual-LVDS interface is a dual-link connection with the even pixels > + traveling on one link, and the odd pixels traveling on the other. > + > +allOf: > + - $ref: panel-common.yaml# > + - $ref: /schemas/display/lvds.yaml/# > + > +properties: > + compatible: > + oneOf: > + - items: > + - enum: > + - lincolntech,lcd185-101ct > + - microtips,13-101hieb0hf0-s > + - const: panel-dual-lvds > + - const: panel-dual-lvds > + > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/$defs/port-base > + unevaluatedProperties: false > + description: The sink for first set of LVDS pixels. > + > + properties: > + dual-lvds-odd-pixels: > + type: boolean > + > + dual-lvds-even-pixels: > + type: boolean > + > + oneOf: > + - required: [dual-lvds-odd-pixels] One question: why do we need a "panel-dual-lvds" compatible? A Dual-LVDS panel is a LVDS panel using two ports, hence still a panel-lvds. If you're doing this to clearly distinguish, for human readability purposes, single-link vs dual-link panels, I think that this would still be clear even if we use panel-lvds alone because dual-link panels, as you wrote in this binding, does *require* two ports, with "dual-lvds-{odd,even}-pixels" properties. So... the devicetree node would look like this: panel { compatible = "vendor,panel", "panel-lvds"; .... ports { port@0 { ..... -> dual-lvds-odd-pixels <- } port@1 { ..... -> dual-lvds-even-pixels <- }; }; }; > + - required: [dual-lvds-even-pixels] ...Though, if you expect dual-lvds panels to get other quirks in the future, that's a whole different story and you may actually need the panel-dual-lvds compatible. Regards, Angelo
Hi Aradhya, Thank you for the patch. On Tue, Jan 03, 2023 at 12:16:14PM +0530, Aradhya Bhatia wrote: > Dual-link LVDS interfaces have 2 links, with even pixels traveling on > one link, and odd pixels on the other. These panels are also generic in > nature, with no documented constraints, much like their single-link > counterparts, "panel-lvds". > > Add a new compatible, "panel-dual-lvds", and a dt-binding document for > these panels. > > Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> > --- > .../display/panel/panel-dual-lvds.yaml | 157 ++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 158 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml > new file mode 100644 > index 000000000000..88a7aa2410be > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml > @@ -0,0 +1,157 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/panel/panel-dual-lvds.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Generic Dual-Link LVDS Display Panel > + > +maintainers: > + - Aradhya Bhatia <a-bhatia1@ti.com> > + - Thierry Reding <thierry.reding@gmail.com> > + > +description: | > + A dual-LVDS interface is a dual-link connection with the even pixels > + traveling on one link, and the odd pixels traveling on the other. > + > +allOf: > + - $ref: panel-common.yaml# > + - $ref: /schemas/display/lvds.yaml/# > + > +properties: > + compatible: > + oneOf: > + - items: > + - enum: > + - lincolntech,lcd185-101ct > + - microtips,13-101hieb0hf0-s > + - const: panel-dual-lvds > + - const: panel-dual-lvds A device-specific compatible string should be required, "panel-dual-lvds" alone shouldn't be allowed. Otherwise it won't be possible to tell different models apart later should this be required. > + > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/$defs/port-base > + unevaluatedProperties: false > + description: The sink for first set of LVDS pixels. > + > + properties: > + dual-lvds-odd-pixels: > + type: boolean > + > + dual-lvds-even-pixels: > + type: boolean > + > + oneOf: > + - required: [dual-lvds-odd-pixels] > + - required: [dual-lvds-even-pixels] > + > + port@1: > + $ref: /schemas/graph.yaml#/$defs/port-base > + unevaluatedProperties: false > + description: The sink for second set of LVDS pixels. > + > + properties: > + dual-lvds-even-pixels: > + type: boolean > + > + dual-lvds-odd-pixels: > + type: boolean > + > + oneOf: > + - required: [dual-lvds-even-pixels] > + - required: [dual-lvds-odd-pixels] > + > + allOf: > + - if: > + properties: > + port@0: > + properties: > + dual-lvds-odd-pixels: true > + required: > + - dual-lvds-odd-pixels > + then: > + properties: > + port@1: > + properties: > + dual-lvds-even-pixels: true > + dual-lvds-odd-pixels: false > + > + - if: > + properties: > + port@0: > + properties: > + dual-lvds-even-pixels: true > + required: > + - dual-lvds-even-pixels > + then: > + properties: > + port@1: > + properties: > + dual-lvds-odd-pixels: true > + dual-lvds-even-pixels: false > + > + required: > + - port@0 > + - port@1 > + > + port: false > + > +unevaluatedProperties: false > + > +required: > + - compatible > + - width-mm > + - height-mm > + - data-mapping > + - panel-timing > + - ports > + > +examples: > + - |+ > + panel-dual-lvds { > + compatible = "microtips,13-101hieb0hf0-s", "panel-dual-lvds"; > + > + width-mm = <217>; > + height-mm = <136>; > + > + data-mapping = "vesa-24"; > + > + panel-timing { > + clock-frequency = <150275000>; > + hactive = <1920>; > + vactive = <1200>; > + hfront-porch = <32>; > + hsync-len = <52>; > + hback-porch = <24>; > + vfront-porch = <24>; > + vsync-len = <8>; > + vback-porch = <3>; > + de-active = <1>; > + }; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + dual-lvds-odd-pixels; > + lcd_in0: endpoint { > + remote-endpoint = <&oldi_out0>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + dual-lvds-even-pixels; > + lcd_in1: endpoint { > + remote-endpoint = <&oldi_out1>; > + }; > + }; > + }; > + }; > + > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index 7f86d02cb427..c13f24293ab1 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6595,6 +6595,7 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > S: Maintained > F: drivers/gpu/drm/panel/panel-lvds.c > F: Documentation/devicetree/bindings/display/lvds.yaml > +F: Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml > F: Documentation/devicetree/bindings/display/panel/panel-lvds.yaml > > DRM DRIVER FOR MANTIX MLAF057WE51 PANELS
Hi Krzysztof, On 03-Jan-23 16:58, Krzysztof Kozlowski wrote: > On 03/01/2023 12:02, Aradhya Bhatia wrote: >> But this is throwing an error. I am confused what else could be done. >> Can you please suggest what might be a more accurate check here? >> >>> >>>> + required: >>>> + - dual-lvds-odd-pixels >>>> + then: >>>> + properties: >>>> + port@1: >>>> + properties: >>>> + dual-lvds-even-pixels: true >>>> + dual-lvds-odd-pixels: false >>> >>> Why do you need this? Your oneOf before already solves it. >> >> I agree with your comment here. It makes sense to only have >> >> dual-lvds-even-pixels: true >> >> and have the oneOf condition take care of the other. But, I just tested >> this and it was unable to pick-up this intentionally-added error. >> >> I added 'dual-lvds-odd-pixels' property to both the nodes, and >> dt_binding_check passes successfully (which it should have not.) >> >> Instead, if I only keep this, >> >> dual-lvds-odd-pixels: false >> >> then the dt_binding_check detects the error as it should. >> >> Regardless, I am curious why the first method doesn't work. Will try to >> explore more on that. > > The check for presence of properties is only against required:, but you > added there properties. Like this: > > https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/mfd/samsung,s5m8767.yaml#L155 > > > Other way is to drop your both oneOf and entire allOf from ports and use: > > oneOf: > - properties: > ports: > $ref: /schemas/graph.yaml#/properties/ports > properties: > port@0: > required: > - dual-lvds-odd-pixels > port@1: > required: > - dual-lvds-even-pixels > - properties: > ports: > $ref: /schemas/graph.yaml#/properties/ports > properties: > port@1: > required: > - dual-lvds-odd-pixels > port@0: > required: > - dual-lvds-even-pixels > Thank you for the suggestions. I tested the both of them, and they seem to be working as expected. V2 will reflect all the required fixes. Regards Aradhya
Hi Angelo, Thanks for taking a look at the patches! On 03-Jan-23 17:21, AngeloGioacchino Del Regno wrote: > Il 03/01/23 07:46, Aradhya Bhatia ha scritto: >> Dual-link LVDS interfaces have 2 links, with even pixels traveling on >> one link, and odd pixels on the other. These panels are also generic in >> nature, with no documented constraints, much like their single-link >> counterparts, "panel-lvds". >> >> Add a new compatible, "panel-dual-lvds", and a dt-binding document for >> these panels. >> >> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >> --- >> .../display/panel/panel-dual-lvds.yaml | 157 ++++++++++++++++++ >> MAINTAINERS | 1 + >> 2 files changed, 158 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >> >> diff --git >> a/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >> b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >> new file mode 100644 >> index 000000000000..88a7aa2410be >> --- /dev/null >> +++ >> b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >> @@ -0,0 +1,157 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/display/panel/panel-dual-lvds.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Generic Dual-Link LVDS Display Panel >> + >> +maintainers: >> + - Aradhya Bhatia <a-bhatia1@ti.com> >> + - Thierry Reding <thierry.reding@gmail.com> >> + >> +description: | >> + A dual-LVDS interface is a dual-link connection with the even pixels >> + traveling on one link, and the odd pixels traveling on the other. >> + >> +allOf: >> + - $ref: panel-common.yaml# >> + - $ref: /schemas/display/lvds.yaml/# >> + >> +properties: >> + compatible: >> + oneOf: >> + - items: >> + - enum: >> + - lincolntech,lcd185-101ct >> + - microtips,13-101hieb0hf0-s >> + - const: panel-dual-lvds >> + - const: panel-dual-lvds >> + >> + ports: >> + $ref: /schemas/graph.yaml#/properties/ports >> + >> + properties: >> + port@0: >> + $ref: /schemas/graph.yaml#/$defs/port-base >> + unevaluatedProperties: false >> + description: The sink for first set of LVDS pixels. >> + >> + properties: >> + dual-lvds-odd-pixels: >> + type: boolean >> + >> + dual-lvds-even-pixels: >> + type: boolean >> + >> + oneOf: >> + - required: [dual-lvds-odd-pixels] > > One question: why do we need a "panel-dual-lvds" compatible? > A Dual-LVDS panel is a LVDS panel using two ports, hence still a panel-lvds. > > If you're doing this to clearly distinguish, for human readability purposes, > single-link vs dual-link panels, I think that this would still be clear even > if we use panel-lvds alone because dual-link panels, as you wrote in this > binding, does *require* two ports, with "dual-lvds-{odd,even}-pixels" properties. Yes, while they are both LVDS based panels the extra LVDS sink in these panels, and the capability to decode and display the 2 sets of signals are enough hardware differences that warrant for an addition of a new compatible. > > So... the devicetree node would look like this: > > panel { > compatible = "vendor,panel", "panel-lvds"; > .... > ports { > port@0 { > ..... > -> dual-lvds-odd-pixels <- > } > > port@1 { > ..... > -> dual-lvds-even-pixels <- > }; > }; > }; > >> + - required: [dual-lvds-even-pixels] > > ...Though, if you expect dual-lvds panels to get other quirks in the future, > that's a whole different story and you may actually need the panel-dual-lvds > compatible. Yes, exactly. Even while being non-smart, there are going to be more quirks in future. And it would be better if they have their own compatible/binding, and are not getting appended in an ever-growing if-else ladder. :) Regards Aradhya
Hi Laurent, Thank you for reviewing the patches! On 08-Jan-23 12:26, Laurent Pinchart wrote: > Hi Aradhya, > > Thank you for the patch. > > On Tue, Jan 03, 2023 at 12:16:14PM +0530, Aradhya Bhatia wrote: >> Dual-link LVDS interfaces have 2 links, with even pixels traveling on >> one link, and odd pixels on the other. These panels are also generic in >> nature, with no documented constraints, much like their single-link >> counterparts, "panel-lvds". >> >> Add a new compatible, "panel-dual-lvds", and a dt-binding document for >> these panels. >> >> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >> --- >> .../display/panel/panel-dual-lvds.yaml | 157 ++++++++++++++++++ >> MAINTAINERS | 1 + >> 2 files changed, 158 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >> >> diff --git a/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >> new file mode 100644 >> index 000000000000..88a7aa2410be >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >> @@ -0,0 +1,157 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/display/panel/panel-dual-lvds.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Generic Dual-Link LVDS Display Panel >> + >> +maintainers: >> + - Aradhya Bhatia <a-bhatia1@ti.com> >> + - Thierry Reding <thierry.reding@gmail.com> >> + >> +description: | >> + A dual-LVDS interface is a dual-link connection with the even pixels >> + traveling on one link, and the odd pixels traveling on the other. >> + >> +allOf: >> + - $ref: panel-common.yaml# >> + - $ref: /schemas/display/lvds.yaml/# >> + >> +properties: >> + compatible: >> + oneOf: >> + - items: >> + - enum: >> + - lincolntech,lcd185-101ct >> + - microtips,13-101hieb0hf0-s >> + - const: panel-dual-lvds >> + - const: panel-dual-lvds > > A device-specific compatible string should be required, > "panel-dual-lvds" alone shouldn't be allowed. Otherwise it won't be > possible to tell different models apart later should this be required. > Understood! Will make the fix in the next revision. >> + >> + ports: >> + $ref: /schemas/graph.yaml#/properties/ports >> + >> + properties: >> + port@0: >> + $ref: /schemas/graph.yaml#/$defs/port-base >> + unevaluatedProperties: false >> + description: The sink for first set of LVDS pixels. >> + >> + properties: >> + dual-lvds-odd-pixels: >> + type: boolean >> + >> + dual-lvds-even-pixels: >> + type: boolean >> + >> + oneOf: >> + - required: [dual-lvds-odd-pixels] >> + - required: [dual-lvds-even-pixels] >> + >> + port@1: >> + $ref: /schemas/graph.yaml#/$defs/port-base >> + unevaluatedProperties: false >> + description: The sink for second set of LVDS pixels. >> + >> + properties: >> + dual-lvds-even-pixels: >> + type: boolean >> + >> + dual-lvds-odd-pixels: >> + type: boolean >> + >> + oneOf: >> + - required: [dual-lvds-even-pixels] >> + - required: [dual-lvds-odd-pixels] >> + >> + allOf: >> + - if: >> + properties: >> + port@0: >> + properties: >> + dual-lvds-odd-pixels: true >> + required: >> + - dual-lvds-odd-pixels >> + then: >> + properties: >> + port@1: >> + properties: >> + dual-lvds-even-pixels: true >> + dual-lvds-odd-pixels: false >> + >> + - if: >> + properties: >> + port@0: >> + properties: >> + dual-lvds-even-pixels: true >> + required: >> + - dual-lvds-even-pixels >> + then: >> + properties: >> + port@1: >> + properties: >> + dual-lvds-odd-pixels: true >> + dual-lvds-even-pixels: false >> + >> + required: >> + - port@0 >> + - port@1 >> + >> + port: false >> + >> +unevaluatedProperties: false >> + >> +required: >> + - compatible >> + - width-mm >> + - height-mm >> + - data-mapping >> + - panel-timing >> + - ports >> + >> +examples: >> + - |+ >> + panel-dual-lvds { >> + compatible = "microtips,13-101hieb0hf0-s", "panel-dual-lvds"; >> + >> + width-mm = <217>; >> + height-mm = <136>; >> + >> + data-mapping = "vesa-24"; >> + >> + panel-timing { >> + clock-frequency = <150275000>; >> + hactive = <1920>; >> + vactive = <1200>; >> + hfront-porch = <32>; >> + hsync-len = <52>; >> + hback-porch = <24>; >> + vfront-porch = <24>; >> + vsync-len = <8>; >> + vback-porch = <3>; >> + de-active = <1>; >> + }; >> + >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + dual-lvds-odd-pixels; >> + lcd_in0: endpoint { >> + remote-endpoint = <&oldi_out0>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + dual-lvds-even-pixels; >> + lcd_in1: endpoint { >> + remote-endpoint = <&oldi_out1>; >> + }; >> + }; >> + }; >> + }; >> + >> +... >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 7f86d02cb427..c13f24293ab1 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -6595,6 +6595,7 @@ T: git git://anongit.freedesktop.org/drm/drm-misc >> S: Maintained >> F: drivers/gpu/drm/panel/panel-lvds.c >> F: Documentation/devicetree/bindings/display/lvds.yaml >> +F: Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >> F: Documentation/devicetree/bindings/display/panel/panel-lvds.yaml >> >> DRM DRIVER FOR MANTIX MLAF057WE51 PANELS > Regards Aradhya
On 09/01/2023 18:21, Aradhya Bhatia wrote: > Hi Angelo, > > Thanks for taking a look at the patches! > > On 03-Jan-23 17:21, AngeloGioacchino Del Regno wrote: >> Il 03/01/23 07:46, Aradhya Bhatia ha scritto: >>> Dual-link LVDS interfaces have 2 links, with even pixels traveling on >>> one link, and odd pixels on the other. These panels are also generic in >>> nature, with no documented constraints, much like their single-link >>> counterparts, "panel-lvds". >>> >>> Add a new compatible, "panel-dual-lvds", and a dt-binding document for >>> these panels. >>> >>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >>> --- >>> .../display/panel/panel-dual-lvds.yaml | 157 ++++++++++++++++++ >>> MAINTAINERS | 1 + >>> 2 files changed, 158 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >>> >>> diff --git >>> a/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >>> new file mode 100644 >>> index 000000000000..88a7aa2410be >>> --- /dev/null >>> +++ >>> b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >>> @@ -0,0 +1,157 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/display/panel/panel-dual-lvds.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Generic Dual-Link LVDS Display Panel >>> + >>> +maintainers: >>> + - Aradhya Bhatia <a-bhatia1@ti.com> >>> + - Thierry Reding <thierry.reding@gmail.com> >>> + >>> +description: | >>> + A dual-LVDS interface is a dual-link connection with the even pixels >>> + traveling on one link, and the odd pixels traveling on the other. >>> + >>> +allOf: >>> + - $ref: panel-common.yaml# >>> + - $ref: /schemas/display/lvds.yaml/# >>> + >>> +properties: >>> + compatible: >>> + oneOf: >>> + - items: >>> + - enum: >>> + - lincolntech,lcd185-101ct >>> + - microtips,13-101hieb0hf0-s >>> + - const: panel-dual-lvds >>> + - const: panel-dual-lvds >>> + >>> + ports: >>> + $ref: /schemas/graph.yaml#/properties/ports >>> + >>> + properties: >>> + port@0: >>> + $ref: /schemas/graph.yaml#/$defs/port-base >>> + unevaluatedProperties: false >>> + description: The sink for first set of LVDS pixels. >>> + >>> + properties: >>> + dual-lvds-odd-pixels: >>> + type: boolean >>> + >>> + dual-lvds-even-pixels: >>> + type: boolean >>> + >>> + oneOf: >>> + - required: [dual-lvds-odd-pixels] >> >> One question: why do we need a "panel-dual-lvds" compatible? >> A Dual-LVDS panel is a LVDS panel using two ports, hence still a >> panel-lvds. >> >> If you're doing this to clearly distinguish, for human readability >> purposes, >> single-link vs dual-link panels, I think that this would still be >> clear even >> if we use panel-lvds alone because dual-link panels, as you wrote in this >> binding, does *require* two ports, with "dual-lvds-{odd,even}-pixels" >> properties. > > Yes, while they are both LVDS based panels the extra LVDS sink in these > panels, and the capability to decode and display the 2 sets of signals > are enough hardware differences that warrant for an addition of a new > compatible. > >> >> So... the devicetree node would look like this: >> >> panel { >> compatible = "vendor,panel", "panel-lvds"; >> .... >> ports { >> port@0 { >> ..... >> -> dual-lvds-odd-pixels <- >> } >> >> port@1 { >> ..... >> -> dual-lvds-even-pixels <- >> }; >> }; >> }; >> >>> + - required: [dual-lvds-even-pixels] >> >> ...Though, if you expect dual-lvds panels to get other quirks in the >> future, >> that's a whole different story and you may actually need the >> panel-dual-lvds >> compatible. > > Yes, exactly. Even while being non-smart, there are going to be more > quirks in future. And it would be better if they have their own > compatible/binding, and are not getting appended in an ever-growing > if-else ladder. :) I can imagine a panel which you can use with a single LVDS link if the clock is high enough, or two LVDS links if the clock has to be lower. Is that a dual-lvds panel? =) But probably that situation is no different than a panel that can work with DSI or DPI input. Still, I'm agree with Angelo in that a new compatible string for dual link lvds feels a bit odd. That said, it's possible the panel-lvds bindings might get rather confusing. So I don't have a strong feeling here. Tomi
Hi Tomi, Thank you for taking a look at the patches! On 17-Jan-23 18:08, Tomi Valkeinen wrote: > On 09/01/2023 18:21, Aradhya Bhatia wrote: >> Hi Angelo, >> >> Thanks for taking a look at the patches! >> >> On 03-Jan-23 17:21, AngeloGioacchino Del Regno wrote: >>> Il 03/01/23 07:46, Aradhya Bhatia ha scritto: >>>> Dual-link LVDS interfaces have 2 links, with even pixels traveling on >>>> one link, and odd pixels on the other. These panels are also generic in >>>> nature, with no documented constraints, much like their single-link >>>> counterparts, "panel-lvds". >>>> >>>> Add a new compatible, "panel-dual-lvds", and a dt-binding document for >>>> these panels. >>>> >>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >>>> --- >>>> .../display/panel/panel-dual-lvds.yaml | 157 >>>> ++++++++++++++++++ >>>> MAINTAINERS | 1 + >>>> 2 files changed, 158 insertions(+) >>>> create mode 100644 >>>> Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >>>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >>>> new file mode 100644 >>>> index 000000000000..88a7aa2410be >>>> --- /dev/null >>>> +++ >>>> b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml >>>> @@ -0,0 +1,157 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/display/panel/panel-dual-lvds.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: Generic Dual-Link LVDS Display Panel >>>> + >>>> +maintainers: >>>> + - Aradhya Bhatia <a-bhatia1@ti.com> >>>> + - Thierry Reding <thierry.reding@gmail.com> >>>> + >>>> +description: | >>>> + A dual-LVDS interface is a dual-link connection with the even pixels >>>> + traveling on one link, and the odd pixels traveling on the other. >>>> + >>>> +allOf: >>>> + - $ref: panel-common.yaml# >>>> + - $ref: /schemas/display/lvds.yaml/# >>>> + >>>> +properties: >>>> + compatible: >>>> + oneOf: >>>> + - items: >>>> + - enum: >>>> + - lincolntech,lcd185-101ct >>>> + - microtips,13-101hieb0hf0-s >>>> + - const: panel-dual-lvds >>>> + - const: panel-dual-lvds >>>> + >>>> + ports: >>>> + $ref: /schemas/graph.yaml#/properties/ports >>>> + >>>> + properties: >>>> + port@0: >>>> + $ref: /schemas/graph.yaml#/$defs/port-base >>>> + unevaluatedProperties: false >>>> + description: The sink for first set of LVDS pixels. >>>> + >>>> + properties: >>>> + dual-lvds-odd-pixels: >>>> + type: boolean >>>> + >>>> + dual-lvds-even-pixels: >>>> + type: boolean >>>> + >>>> + oneOf: >>>> + - required: [dual-lvds-odd-pixels] >>> >>> One question: why do we need a "panel-dual-lvds" compatible? >>> A Dual-LVDS panel is a LVDS panel using two ports, hence still a >>> panel-lvds. >>> >>> If you're doing this to clearly distinguish, for human readability purposes, >>> single-link vs dual-link panels, I think that this would still be clear even >>> if we use panel-lvds alone because dual-link panels, as you wrote in this >>> binding, does *require* two ports, with "dual-lvds-{odd,even}-pixels" properties. >> >> Yes, while they are both LVDS based panels the extra LVDS sink in these >> panels, and the capability to decode and display the 2 sets of signals >> are enough hardware differences that warrant for an addition of a new >> compatible. >> >>> >>> So... the devicetree node would look like this: >>> >>> panel { >>> compatible = "vendor,panel", "panel-lvds"; >>> .... >>> ports { >>> port@0 { >>> ..... >>> -> dual-lvds-odd-pixels <- >>> } >>> >>> port@1 { >>> ..... >>> -> dual-lvds-even-pixels <- >>> }; >>> }; >>> }; >>> >>>> + - required: [dual-lvds-even-pixels] >>> >>> ...Though, if you expect dual-lvds panels to get other quirks in the future, >>> that's a whole different story and you may actually need the panel-dual-lvds >>> compatible. >> >> Yes, exactly. Even while being non-smart, there are going to be more >> quirks in future. And it would be better if they have their own >> compatible/binding, and are not getting appended in an ever-growing >> if-else ladder. :) > > I can imagine a panel which you can use with a single LVDS link if the > clock is high enough, or two LVDS links if the clock has to be lower. Is > that a dual-lvds panel? =) Hmm, I can see what you are saying here. If one wants to run a dual-link panel, with 1 link (given enough clock frequency), then the bindings here will *not* allow for a single port with the odd/even properties absent. In such a case, the compatible will indeed need to be changed to "panel-lvds". While it does feel a tad bit odd, I believe it is still worth maintaining the clarity and HW differences between the single and dual link panels. :) > > But probably that situation is no different than a panel that can work > with DSI or DPI input. > > Still, I'm agree with Angelo in that a new compatible string for dual > link lvds feels a bit odd. That said, it's possible the panel-lvds > bindings might get rather confusing. So I don't have a strong feeling here. Regards Aradhya
diff --git a/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml new file mode 100644 index 000000000000..88a7aa2410be --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml @@ -0,0 +1,157 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/panel-dual-lvds.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Generic Dual-Link LVDS Display Panel + +maintainers: + - Aradhya Bhatia <a-bhatia1@ti.com> + - Thierry Reding <thierry.reding@gmail.com> + +description: | + A dual-LVDS interface is a dual-link connection with the even pixels + traveling on one link, and the odd pixels traveling on the other. + +allOf: + - $ref: panel-common.yaml# + - $ref: /schemas/display/lvds.yaml/# + +properties: + compatible: + oneOf: + - items: + - enum: + - lincolntech,lcd185-101ct + - microtips,13-101hieb0hf0-s + - const: panel-dual-lvds + - const: panel-dual-lvds + + ports: + $ref: /schemas/graph.yaml#/properties/ports + + properties: + port@0: + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + description: The sink for first set of LVDS pixels. + + properties: + dual-lvds-odd-pixels: + type: boolean + + dual-lvds-even-pixels: + type: boolean + + oneOf: + - required: [dual-lvds-odd-pixels] + - required: [dual-lvds-even-pixels] + + port@1: + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + description: The sink for second set of LVDS pixels. + + properties: + dual-lvds-even-pixels: + type: boolean + + dual-lvds-odd-pixels: + type: boolean + + oneOf: + - required: [dual-lvds-even-pixels] + - required: [dual-lvds-odd-pixels] + + allOf: + - if: + properties: + port@0: + properties: + dual-lvds-odd-pixels: true + required: + - dual-lvds-odd-pixels + then: + properties: + port@1: + properties: + dual-lvds-even-pixels: true + dual-lvds-odd-pixels: false + + - if: + properties: + port@0: + properties: + dual-lvds-even-pixels: true + required: + - dual-lvds-even-pixels + then: + properties: + port@1: + properties: + dual-lvds-odd-pixels: true + dual-lvds-even-pixels: false + + required: + - port@0 + - port@1 + + port: false + +unevaluatedProperties: false + +required: + - compatible + - width-mm + - height-mm + - data-mapping + - panel-timing + - ports + +examples: + - |+ + panel-dual-lvds { + compatible = "microtips,13-101hieb0hf0-s", "panel-dual-lvds"; + + width-mm = <217>; + height-mm = <136>; + + data-mapping = "vesa-24"; + + panel-timing { + clock-frequency = <150275000>; + hactive = <1920>; + vactive = <1200>; + hfront-porch = <32>; + hsync-len = <52>; + hback-porch = <24>; + vfront-porch = <24>; + vsync-len = <8>; + vback-porch = <3>; + de-active = <1>; + }; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dual-lvds-odd-pixels; + lcd_in0: endpoint { + remote-endpoint = <&oldi_out0>; + }; + }; + + port@1 { + reg = <1>; + dual-lvds-even-pixels; + lcd_in1: endpoint { + remote-endpoint = <&oldi_out1>; + }; + }; + }; + }; + +... diff --git a/MAINTAINERS b/MAINTAINERS index 7f86d02cb427..c13f24293ab1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6595,6 +6595,7 @@ T: git git://anongit.freedesktop.org/drm/drm-misc S: Maintained F: drivers/gpu/drm/panel/panel-lvds.c F: Documentation/devicetree/bindings/display/lvds.yaml +F: Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml F: Documentation/devicetree/bindings/display/panel/panel-lvds.yaml DRM DRIVER FOR MANTIX MLAF057WE51 PANELS
Dual-link LVDS interfaces have 2 links, with even pixels traveling on one link, and odd pixels on the other. These panels are also generic in nature, with no documented constraints, much like their single-link counterparts, "panel-lvds". Add a new compatible, "panel-dual-lvds", and a dt-binding document for these panels. Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> --- .../display/panel/panel-dual-lvds.yaml | 157 ++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 158 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/panel-dual-lvds.yaml