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 Laurent,

Thank you for comments.

On 04/23/2014 08: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.

Okay, I'll use wr-active. It's better.


+          - 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.

Yes, as I wrote another thread before, this cpu interface timings is
additional one. The video timings is required also.

Thank you.
Best regards YJ


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