| Message ID | 1533264572-6364-2-git-send-email-abhinavk@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5,1/2] drm/panel: Add support for Truly NT35597 panel | expand |
On Fri, Aug 3, 2018 at 4:49 AM Abhinav Kumar <abhinavk@codeaurora.org> wrote: > From: "abhinavk@codeaurora.org" <abhinavk@codeaurora.org> > > Add the device tree bindings for Truly NT35597 panel. > This panel supports both single DSI and dual DSI. I do not think this is a panel but a panel driver that can be used with several physical panels. Can you confirm? I suspect this since the command sequence in the driver seems to contain a command for setting up the actual resolution and a bunch of clocking properties for the physical panel. > +Required properties: > +- compatible: should be "truly,nt35597" As with eg ili9322 I think this should have dual compatible strings identifying the system it is used with since the resolution, clocking etc is apparently actually configurable. compatible: "truly,nt35597", "qcom,reference-design1-name-display"; "truly,nt35597", "qcom,reference-design2-name-display"; Then you send the command setting up resolution etc only for that one system. > +- vdda-supply: phandle of the regulator that provides the supply voltage > + Power IC supply > +- vdispp-supply: phandle of the regulator that provides the supply voltage > + for positive LCD bias > +- vdispn-supply: phandle of the regulator that provides the supply voltage > + for negative LCD bias > +- reset-gpios: phandle of gpio for reset line > + This should be 8mA, gpio can be configured using mux, pinctrl, pinctrl-names > +- mode-gpios: phandle of the gpio for choosing the mode of the display > + for single DSI or Dual DSI > + This should be low for dual DSI and high for single DSI mode > +- display-timings: Node for the Panel timings I don't think this belongs in the device tree at all. Provide the timings from the driver based on the compatible string instead, as you actually send commands to set up a certain timing in the display controller. (See ili9322 driver for an example of how I do this.) > +- ports: This device has two video ports driven by two DSIs. Their connections > + are modelled using the OF graph bindings specified in > + Documentation/devicetree/bindings/graph.txt. > + - port@0: DSI input port driven by master DSI > + - port@1: DSI input port driven by secondary DSI The rest seems fine. Yours, Linus Walleij
Hi Linus Thanks for your valuable comments. Yes, we agree with your comments here and will address them. Some details below on how we will take care of it. Thanks Abhinav On 2018-08-03 04:20, Linus Walleij wrote: > On Fri, Aug 3, 2018 at 4:49 AM Abhinav Kumar <abhinavk@codeaurora.org> > wrote: > >> From: "abhinavk@codeaurora.org" <abhinavk@codeaurora.org> >> >> Add the device tree bindings for Truly NT35597 panel. >> This panel supports both single DSI and dual DSI. > > I do not think this is a panel but a panel driver that can be used > with several physical panels. Can you confirm? Yes, from the documentation I have I can see that this is a panel driver and can support other panels/resolutions. > > I suspect this since the command sequence in the driver seems > to contain a command for setting up the actual resolution and > a bunch of clocking properties for the physical panel. > >> +Required properties: >> +- compatible: should be "truly,nt35597" > > As with eg ili9322 I think this should have dual compatible strings > identifying the system it is used with since the resolution, clocking > etc is apparently > actually configurable. > > compatible: > "truly,nt35597", "qcom,reference-design1-name-display"; > "truly,nt35597", "qcom,reference-design2-name-display"; > > Then you send the command setting up resolution etc only for that > one system. > Yes, I agree we will can have dual compatible strings and we will pick resolution/modes based on the compatible string similar to this: https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/panel/panel-ilitek-ili9322.c#L659 >> +- vdda-supply: phandle of the regulator that provides the supply >> voltage >> + Power IC supply >> +- vdispp-supply: phandle of the regulator that provides the supply >> voltage >> + for positive LCD bias >> +- vdispn-supply: phandle of the regulator that provides the supply >> voltage >> + for negative LCD bias >> +- reset-gpios: phandle of gpio for reset line >> + This should be 8mA, gpio can be configured using mux, pinctrl, >> pinctrl-names >> +- mode-gpios: phandle of the gpio for choosing the mode of the >> display >> + for single DSI or Dual DSI >> + This should be low for dual DSI and high for single DSI mode >> +- display-timings: Node for the Panel timings > > I don't think this belongs in the device tree at all. > > Provide the timings from the driver based on the compatible string > instead, as you actually send commands to set up a certain timing in > the display controller. > > (See ili9322 driver for an example of how I do this.) Yes, will follow the example of ili9322 and do the same. > >> +- ports: This device has two video ports driven by two DSIs. Their >> connections >> + are modelled using the OF graph bindings specified in >> + Documentation/devicetree/bindings/graph.txt. >> + - port@0: DSI input port driven by master DSI >> + - port@1: DSI input port driven by secondary DSI > > The rest seems fine. > > Yours, > Linus Walleij
diff --git a/Documentation/devicetree/bindings/display/truly,nt35597.txt b/Documentation/devicetree/bindings/display/truly,nt35597.txt new file mode 100644 index 0000000..5d297b8 --- /dev/null +++ b/Documentation/devicetree/bindings/display/truly,nt35597.txt @@ -0,0 +1,69 @@ +Truly model NT35597 1440x2560 DSI Panel + +Required properties: +- compatible: should be "truly,nt35597" +- vdda-supply: phandle of the regulator that provides the supply voltage + Power IC supply +- vdispp-supply: phandle of the regulator that provides the supply voltage + for positive LCD bias +- vdispn-supply: phandle of the regulator that provides the supply voltage + for negative LCD bias +- reset-gpios: phandle of gpio for reset line + This should be 8mA, gpio can be configured using mux, pinctrl, pinctrl-names +- mode-gpios: phandle of the gpio for choosing the mode of the display + for single DSI or Dual DSI + This should be low for dual DSI and high for single DSI mode +- display-timings: Node for the Panel timings +- ports: This device has two video ports driven by two DSIs. Their connections + are modelled using the OF graph bindings specified in + Documentation/devicetree/bindings/graph.txt. + - port@0: DSI input port driven by master DSI + - port@1: DSI input port driven by secondary DSI + +Example: + + dsi@ae94000 { + panel@0 { + compatible = "truly,nt35597"; + reg = <0>; + vdda-supply = <&pm8998_l14>; + vdispp-supply = <&lab_regulator>; + vdispn-supply = <&ibb_regulator>; + pinctrl-names = "default", "suspend"; + pinctrl-0 = <&dpu_dsi_active>; + pinctrl-1 = <&dpu_dsi_suspend>; + + reset-gpios = <&tlmm 6 0>; + mode-gpios = <&tlmm 52 0>; + display-timings { + timing0: timing-0 { + clock-frequency = <268316138>; + hactive = <1440>; + vactive = <2560>; + hfront-porch = <200>; + hback-porch = <64>; + hsync-len = <32>; + vfront-porch = <8>; + vback-porch = <7>; + vsync-len = <1>; + }; + }; + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + reg = <0>; + panel0_in: endpoint { + remote-endpoint = <&dsi0_out>; + }; + }; + + port@1 { + reg = <1>; + panel1_in: endpoint { + remote-endpoint = <&dsi1_out>; + }; + }; + }; + }; + };