Hi Andrzej, On Wednesday 23 April 2014 11:02:21 Andrzej Hajda wrote: > On 04/21/2014 02:28 PM, YoungJun Cho wrote: > > This patch adds DT bindings for s6e3fa0 panel. > > The bindings describes panel resources, display timings and cpu timings. > > > > Changelog v2: > > - Adds unit address (commented by Sachin Kamat) > > Changelog v3: > > - Removes optional delay, size properties (commented by Laurent Pinchart) > > - Adds OLED detection, TE gpio properties > > Changelog v4: > > - Moves CPU timings relevant properties from FIMD DT > > > > (commeted by Laurent Pinchart, Andrzej Hajda) > > > > Signed-off-by: YoungJun Cho <yj44.cho@xxxxxxxxxxx> > > Acked-by: Inki Dae <inki.dae@xxxxxxxxxxx> > > Acked-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> > > --- > > > > .../devicetree/bindings/panel/samsung,s6e3fa0.txt | 63 +++++++++++++++ > > 1 file changed, 63 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt> > > diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > > b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt new file > > mode 100644 > > index 0000000..9eeb38b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > > @@ -0,0 +1,63 @@ > > +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel > > + > > +Required properties: > > + - compatible: "samsung,s6e3fa0" > > + - reg: the virtual channel number of a DSI peripheral > > + - vdd3-supply: core voltage supply > > + - vci-supply: voltage supply for analog circuits > > + - reset-gpio: a GPIO spec for the reset pin > > + - det-gpio: a GPIO spec for the OLED detection pin > > + - te-gpio: a GPIO spec for the TE pin > > + - display-timings: timings for the connected panel as described by [1] > > Which properties of display-timings are relevant for CPU mode? > I guess width and height. Anything more? > > > + - cpu-timings: CPU interface timings for the connected panel, and it > > contains > > + following optional properties. > > + - cs-setup: clock cycles for the active period of address > > signal > > + enable until chip select is enable in CPU interface > > + - wr-setup: clock cycles for the active period of CS signal > > enable > > + until write signal is enable in CPU interface > > + - wr-act: clock cycles for the active period of CS enable in > > CPU > > + interface What about calling this property wr-active ? I had called this wr-cycle in a previous implementation, that could be an option as well. > > + - wr-hold: clock cycles for the active period of CS disable > > until > > + write signal is disable in CPU interface > > cpu-timings name does not sound to be related to display. > I wonder if it would not be better to merge cpu-timings into > display-timings but this will require more discussion I guess. The panel has a memory-like interface with chip select, read/write and access strobe, address (1 bit) and data signals. The interface is defined in the MIPI DBI specification (a quick search turned up http://www.scribd.com/doc/206899854/MIPI-DPI-Specification-v2, there might be direct PDF downloads available), even if some panels implement a similar interface that predates MIPI DBI (with names such as i80 or SYS-80 probably due to the similarities with the 8051 external bus). The cpu-timings properties describe the read and write access timings for those signals, unrelated to the display video timings. I thus believe that they should be separate from the display timings. We will probably need to add properties for the read signal as well, but that can be done later. > If you want to stay with separate node please consider to make it > optional as whole node or make some properties required. Making node > required with all sub-properties optional is weird. > By the way I hope all timings properties are generic for CPU mode, > if not they should be prefixed by vendor or model. The timings description should be pretty generic I believe, especially given that the interface is standardized by the MIPI alliance. Could you have a quick look at the spec and share your opinion ? > > + > > +Optional properties: > > + > > +The device node can contain one 'port' child node with one child > > +'endpoint' node, according to the bindings defined in [2]. This > > +node should describe panel's video bus. > > + > > +[1]: Documentation/devicetree/bindings/video/display-timing.txt > > +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt > > + > > +Example: > > + > > + panel@0 { > > + compatible = "samsung,s6e3fa0"; > > + reg = <0>; > > + vdd3-supply = <&vcclcd_reg>; > > + vci-supply = <&vlcd_reg>; > > + reset-gpio = <&gpy7 4 0>; > > + det-gpio = <&gpg0 6 0>; > > + te-gpio = <&gpd1 7 0>; > > + > > + display-timings { > > + timing0: timing-0 { > > + clock-frequency = <0>; > > + hactive = <1080>; > > + vactive = <1920>; > > + hfront-porch = <2>; > > + hback-porch = <2>; > > + hsync-len = <1>; > > + vfront-porch = <1>; > > + vback-porch = <4>; > > + vsync-len = <1>; > > + }; > > + }; > > + > > + cpu-timings { > > + cs-setup = <0>; > > + wr-setup = <0>; > > + wr-act = <1>; > > + wr-hold = <0>; > > + }; > > + }; -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
