On 04/23/2014 02:55 PM, Laurent Pinchart wrote: > 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 ? It looks OK. I guess only hactive, and vactive props of display-timings are used, maybe some flags? I wonder if it would not be better to drop display-timings node and place all props into mipi-dbi-timings or mipi-dbi-settings node. Or rather all timings props should be put into port/endpoint node with or without any container node. 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
