Re: [RFC v2 PATCH v4 09/14] ARM: dts: s6e3fa0: add DT bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andrzej,

On 04/23/2014 10:33 PM, Andrzej Hajda wrote:
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.

Isn't it too generic? I prefer cpu-mode-timings.


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.

The current 'display-timings' is for video mode interface,
but cpu mode interface requires it.

If we use mipi-dbi-timings, we should change previous video mode
relevant ones at all.

Thank you.
Best regards YJ


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>;
+		};
+	};



_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux