On 04/23/2014 01:34 PM, Laurent Pinchart wrote: > 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. 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. Regards Andrzej >> 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>; >>> + }; >>> + }; -- 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
