Hi Andrzej, On Wednesday 23 April 2014 14:48:31 Andrzej Hajda wrote: > On 04/23/2014 01:34 PM, Laurent Pinchart wrote: > > 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. > > cpu-timings suggests these timings are for CPU, but they are for display > panel in CPU mode. > I though rather about something like display-cpu-timings or i80-timings, > just to avoid confusion. mipi-dbi-timings maybe ? > >> 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
