[PATCH 0/2] R-Car DU: Add support for LVDS mode selection

[PATCH 0/2] R-Car DU: Add support for LVDS mode selection

[Laurent Pinchart]

Hello,

This patch series adds support for LVDS mode selection in the R-Car DU driver.

Patch 1/2 defines a new DT binding for LVDS panel. Compared to the existing
DPI panel bindings that are currently abused by the driver for LVDS panel,
this new binding specifies the LVDS more explicitly. Patch 2/2 then adds
support for the new bindings to the driver, retrieving the mode from DT and
configuring the hardware accordingly.

The LVDS panel DT bindings are based on the relevant standards I have been
able to find, as well as on existing LVDS-related code and DT bindings
available in the mainline kernel. Those include

- the LVDS formats MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
  MEDIA_BUS_FMT_RGB888_1X7X4_SPWG and MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA
  (Documentation/media/uapi/v4l/subdev-formats.rst)
- iMX display DT bindings available in
  (Documentation/devicetree/bindings/display/imx/ldb.txt)
- the drivers/gpu/drm/imx/ driver

In addition to the three modes specified in the LVDS panel DT bindings, the
Renesas R-Car DU also supports the following two modes.

Slot       0       1       2       3       4       5       6
       ________________                         _________________
Clock                  \_______________________/
         ______  ______  ______  ______  ______  ______  ______
DATA0  ><_CTL0_><__R7__><__R6__><__R5__><__R4__><__R3__><__R2__><
DATA1  ><_CTL1_><__G7__><__G6__><__G5__><__G4__><__G3__><__G2__><
DATA2  ><_CTL2_><__B7__><__B6__><__B5__><__B4__><__B3__><__B2__><
DATA3  ><_CTL3_><__B1__><__B0__><__G1__><__G0__><__R1__><__R0__><
         ______  ______  ______  ______  ______  ______  ______
DATA0  ><_CTL0_><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
DATA1  ><_CTL1_><__G5__><__G4__><__G3__><__G2__><__G1__><__G0__><
DATA2  ><_CTL2_><__B5__><__B4__><__B3__><__B2__><__B1__><__B0__><
DATA3  ><_CTL3_><__B7__><__B6__><__G7__><__G6__><__R7__><__R6__><

and their mirrored version.

I haven't been able to find any standard defining those data mappings, nor any
panel using them. The control signals positions correspond to DC-balanced LVDS
(see figure 18 on page 19 of http://www.ti.com/lit/ds/symlink/ds90cf388.pdf),
but the R-Car DU doesn't support DC-balanced LVDS as far as I can tell, so
it's not a match. If anyone knows of other devices supporting these data
mappings or of standards defining them I would appreciate the information and
will update the bindings accordingly.


Laurent Pinchart (2):
  devicetree/bindings: display: Add bindings for LVDS panels
  drm: rcar-du: Add support for LVDS mode selection

 .../bindings/display/panel/panel-lvds.txt          | 119 +++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c          |  29 +++++
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c          |  11 +-
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h          |  13 +++
 4 files changed, 170 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.txt

-- 
Regards,

Laurent Pinchart

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

[PATCH 0/2] R-Car DU: Add support for LVDS mode selection

[Laurent Pinchart]

Hello,

This patch series adds support for LVDS mode selection in the R-Car DU driver.

Patch 1/2 defines a new DT binding for LVDS panel. Compared to the existing
DPI panel bindings that are currently abused by the driver for LVDS panel,
this new binding specifies the LVDS more explicitly. Patch 2/2 then adds
support for the new bindings to the driver, retrieving the mode from DT and
configuring the hardware accordingly.

The LVDS panel DT bindings are based on the relevant standards I have been
able to find, as well as on existing LVDS-related code and DT bindings
available in the mainline kernel. Those include

- the LVDS formats MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
  MEDIA_BUS_FMT_RGB888_1X7X4_SPWG and MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA
  (Documentation/media/uapi/v4l/subdev-formats.rst)
- iMX display DT bindings available in
  (Documentation/devicetree/bindings/display/imx/ldb.txt)
- the drivers/gpu/drm/imx/ driver

In addition to the three modes specified in the LVDS panel DT bindings, the
Renesas R-Car DU also supports the following two modes.

Slot       0       1       2       3       4       5       6
       ________________                         _________________
Clock                  \_______________________/
         ______  ______  ______  ______  ______  ______  ______
DATA0  ><_CTL0_><__R7__><__R6__><__R5__><__R4__><__R3__><__R2__><
DATA1  ><_CTL1_><__G7__><__G6__><__G5__><__G4__><__G3__><__G2__><
DATA2  ><_CTL2_><__B7__><__B6__><__B5__><__B4__><__B3__><__B2__><
DATA3  ><_CTL3_><__B1__><__B0__><__G1__><__G0__><__R1__><__R0__><
         ______  ______  ______  ______  ______  ______  ______
DATA0  ><_CTL0_><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
DATA1  ><_CTL1_><__G5__><__G4__><__G3__><__G2__><__G1__><__G0__><
DATA2  ><_CTL2_><__B5__><__B4__><__B3__><__B2__><__B1__><__B0__><
DATA3  ><_CTL3_><__B7__><__B6__><__G7__><__G6__><__R7__><__R6__><

and their mirrored version.

I haven't been able to find any standard defining those data mappings, nor any
panel using them. The control signals positions correspond to DC-balanced LVDS
(see figure 18 on page 19 of http://www.ti.com/lit/ds/symlink/ds90cf388.pdf),
but the R-Car DU doesn't support DC-balanced LVDS as far as I can tell, so
it's not a match. If anyone knows of other devices supporting these data
mappings or of standards defining them I would appreciate the information and
will update the bindings accordingly.


Laurent Pinchart (2):
  devicetree/bindings: display: Add bindings for LVDS panels
  drm: rcar-du: Add support for LVDS mode selection

 .../bindings/display/panel/panel-lvds.txt          | 119 +++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c          |  29 +++++
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c          |  11 +-
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h          |  13 +++
 4 files changed, 170 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.txt

-- 
Regards,

Laurent Pinchart

[PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Laurent Pinchart]

LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
Multiple incompatible data link layers have been used over time to
transmit image data to LVDS panels. This binding supports display panels
compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
specifications.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../bindings/display/panel/panel-lvds.txt          | 119 +++++++++++++++++++++
 1 file changed, 119 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.txt

diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
new file mode 100644
index 000000000000..250861f2673e
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
@@ -0,0 +1,119 @@
+Generic LVDS Panel
+==================
+
+LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. Multiple
+incompatible data link layers have been used over time to transmit image data
+to LVDS panels. This bindings supports display panels compatible with the
+following specifications.
+
+[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, February
+1999 (Version 1.0), Japan Electronic Industry Development Association (JEIDA)
+[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
+Semiconductor
+[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
+Electronics Standards Association (VESA)
+
+Device compatible with those specifications have been marketed under the
+FPD-Link and FlatLink brands.
+
+
+Required properties:
+- compatible: shall contain "panel-lvds"
+- width-mm: panel display width in millimeters
+- height-mm: panel display height in millimeters
+- data-mapping: the color signals mapping order, "jeida-18", "jeida-24"
+  or "vesa-24"
+
+Optional properties:
+- label: a symbolic name for the panel
+- avdd-supply: reference to the regulator that powers the panel analog supply
+- dvdd-supply: reference to the regulator that powers the panel digital supply
+- data-mirror: if set, reverse the bit order on all data lanes (6 to 0 instead
+  of 0 to 6)
+
+Required nodes:
+- One "panel-timing" node containing video timings, in accordance with the
+  display timing bindings defined in
+  Documentation/devicetree/bindings/display/display-timing.txt.
+- One "port" child node for the LVDS input port, in accordance with the
+  video interface bindings defined in
+  Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+
+LVDS data mappings are defined as follows.
+
+- "jeida-18" - 18-bit data mapping compatible with the [JEIDA], [LDI] and
+  [VESA] specifications. Data are transferred as follows on 3 LVDS lanes.
+
+Slot	    0       1       2       3       4       5       6
+	________________                         _________________
+Clock	                \_______________________/
+	  ______  ______  ______  ______  ______  ______  ______
+DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
+DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
+DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
+
+- "jeida-24" - 24-bit data mapping compatible with the [DSIM] and [LDI]
+  specifications. Data are transferred as follows on 4 LVDS lanes.
+
+Slot	    0       1       2       3       4       5       6
+	________________                         _________________
+Clock	                \_______________________/
+	  ______  ______  ______  ______  ______  ______  ______
+DATA0	><__G2__><__R7__><__R6__><__R5__><__R4__><__R3__><__R2__><
+DATA1	><__B3__><__B2__><__G7__><__G6__><__G5__><__G4__><__G3__><
+DATA2	><_CTL2_><_CTL1_><_CTL0_><__B7__><__B6__><__B5__><__B4__><
+DATA3	><_CTL3_><__B1__><__B0__><__G1__><__G0__><__R1__><__R0__><
+
+- "vesa-24" - 24-bit data mapping compatible with the [VESA] specification.
+  Data are transferred as follows on 4 LVDS lanes.
+
+Slot	    0       1       2       3       4       5       6
+	________________                         _________________
+Clock	                \_______________________/
+	  ______  ______  ______  ______  ______  ______  ______
+DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
+DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
+DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
+DATA3	><_CTL3_><__B7__><__B6__><__G7__><__G6__><__R7__><__R6__><
+
+
+Control signals are mapped as follows.
+
+CTL0: HSync
+CTL1: VSync
+CTL2: Data Enable
+CTL3: 0
+
+
+Example
+-------
+
+panel {
+	compatible = "mitsubishi,aa121td01", "panel-lvds";
+	label = "lcd";
+
+	width-mm = <261>;
+	height-mm = <163>;
+
+	data-mapping = "jeida-24";
+
+	panel-timing {
+		/* 1280x800 @60Hz */
+		clock-frequency = <71000000>;
+		hactive = <1280>;
+		vactive = <800>;
+		hsync-len = <70>;
+		hfront-porch = <20>;
+		hback-porch = <70>;
+		vsync-len = <5>;
+		vfront-porch = <3>;
+		vback-porch = <15>;
+	};
+
+	port {
+		panel_in: endpoint {
+			remote-endpoint = <&lvds_connector>;
+		};
+	};
+};
-- 
Regards,

Laurent Pinchart

[PATCH 2/2] drm: rcar-du: Add support for LVDS mode selection

[Laurent Pinchart]

Retrieve the LVDS mode from the panel node and configure the LVDS
encoder accordingly. LVDS mode selection is static as LVDS panels can't
be hot-plugged on any of the device supported by the driver. Support for
dynamic mode selection can be implemented in the future if needed
without any impact on the DT bindings.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
---
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 29 +++++++++++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 11 +++++++++--
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h | 13 +++++++++++++
 3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
index 64e9f0b86e58..71a70b06f16b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
@@ -24,6 +24,7 @@
 #include "rcar_du_encoder.h"
 #include "rcar_du_kms.h"
 #include "rcar_du_lvdscon.h"
+#include "rcar_du_lvdsenc.h"
 
 struct rcar_du_lvds_connector {
 	struct rcar_du_connector connector;
@@ -33,6 +34,8 @@ struct rcar_du_lvds_connector {
 		unsigned int height_mm;		/* Panel height in mm */
 		struct videomode mode;
 	} panel;
+
+	unsigned int mode;
 };
 
 #define to_rcar_lvds_connector(c) \
@@ -100,6 +103,32 @@ int rcar_du_lvds_connector_init(struct rcar_du_device *rcdu,
 	of_property_read_u32(np, "width-mm", &lvdscon->panel.width_mm);
 	of_property_read_u32(np, "height-mm", &lvdscon->panel.height_mm);
 
+	if (of_device_is_compatible(np, "panel-lvds")) {
+		enum rcar_lvds_mode mode = 0;
+		const char *mapping = NULL;
+
+		of_property_read_string(np, "data-mapping", &mapping);
+		if (!mapping) {
+			dev_dbg(rcdu->dev,
+				"required property %s not found in %s node\n",
+				"data-mapping", np->full_name);
+			return -EINVAL;
+		}
+
+		if (!strcmp(mapping, "jeida-18") ||
+		    !strcmp(mapping, "jeida-24"))
+			mode = RCAR_LVDS_MODE_JEIDA;
+		else if (!strcmp(mapping, "vesa-24"))
+			mode = RCAR_LVDS_MODE_VESA;
+		else
+			return -EINVAL;
+
+		if (of_find_property(np, "data-mirror", NULL))
+			mode |= RCAR_LVDS_MODE_MIRROR;
+
+		rcar_du_lvdsenc_set_mode(renc->lvds, mode);
+	}
+
 	connector = &lvdscon->connector.connector;
 	connector->display_info.width_mm = lvdscon->panel.width_mm;
 	connector->display_info.height_mm = lvdscon->panel.height_mm;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
index b74105a80a6e..c8c22850947a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
@@ -31,6 +31,7 @@ struct rcar_du_lvdsenc {
 	bool enabled;
 
 	enum rcar_lvds_input input;
+	enum rcar_lvds_mode mode;
 };
 
 static void rcar_lvds_write(struct rcar_du_lvdsenc *lvds, u32 reg, u32 data)
@@ -61,7 +62,7 @@ static void rcar_du_lvdsenc_start_gen2(struct rcar_du_lvdsenc *lvds,
 	/* Select the input, hardcode mode 0, enable LVDS operation and turn
 	 * bias circuitry on.
 	 */
-	lvdcr0 = LVDCR0_BEN | LVDCR0_LVEN;
+	lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_BEN | LVDCR0_LVEN;
 	if (rcrtc->index == 2)
 		lvdcr0 |= LVDCR0_DUSEL;
 	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
@@ -107,7 +108,7 @@ static void rcar_du_lvdsenc_start_gen3(struct rcar_du_lvdsenc *lvds,
 	/* Turn the PLL on, set it to LVDS normal mode, wait for the startup
 	 * delay and turn the output on.
 	 */
-	lvdcr0 = LVDCR0_PLLON;
+	lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_PLLON;
 	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
 
 	lvdcr0 |= LVDCR0_PWD;
@@ -210,6 +211,12 @@ void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds,
 		mode->clock = clamp(mode->clock, 25175, 148500);
 }
 
+void rcar_du_lvdsenc_set_mode(struct rcar_du_lvdsenc *lvds,
+			      enum rcar_lvds_mode mode)
+{
+	lvds->mode = mode;
+}
+
 static int rcar_du_lvdsenc_get_resources(struct rcar_du_lvdsenc *lvds,
 					 struct platform_device *pdev)
 {
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
index dfdba746edf4..7218ac89333e 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
@@ -26,8 +26,17 @@ enum rcar_lvds_input {
 	RCAR_LVDS_INPUT_DU2,
 };
 
+/* Keep in sync with the LVDCR0.LVMD hardware register values. */
+enum rcar_lvds_mode {
+	RCAR_LVDS_MODE_JEIDA = 0,
+	RCAR_LVDS_MODE_MIRROR = 1,
+	RCAR_LVDS_MODE_VESA = 4,
+};
+
 #if IS_ENABLED(CONFIG_DRM_RCAR_LVDS)
 int rcar_du_lvdsenc_init(struct rcar_du_device *rcdu);
+void rcar_du_lvdsenc_set_mode(struct rcar_du_lvdsenc *lvds,
+			      enum rcar_lvds_mode mode);
 int rcar_du_lvdsenc_enable(struct rcar_du_lvdsenc *lvds,
 			   struct drm_crtc *crtc, bool enable);
 void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds,
@@ -37,6 +46,10 @@ static inline int rcar_du_lvdsenc_init(struct rcar_du_device *rcdu)
 {
 	return 0;
 }
+static inline void rcar_du_lvdsenc_set_mode(struct rcar_du_lvdsenc *lvds,
+					    enum rcar_lvds_mode mode)
+{
+}
 static inline int rcar_du_lvdsenc_enable(struct rcar_du_lvdsenc *lvds,
 					 struct drm_crtc *crtc, bool enable)
 {
-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] drm: rcar-du: Add support for LVDS mode selection

[Laurent Pinchart]

Retrieve the LVDS mode from the panel node and configure the LVDS
encoder accordingly. LVDS mode selection is static as LVDS panels can't
be hot-plugged on any of the device supported by the driver. Support for
dynamic mode selection can be implemented in the future if needed
without any impact on the DT bindings.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 29 +++++++++++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 11 +++++++++--
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h | 13 +++++++++++++
 3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
index 64e9f0b86e58..71a70b06f16b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
@@ -24,6 +24,7 @@
 #include "rcar_du_encoder.h"
 #include "rcar_du_kms.h"
 #include "rcar_du_lvdscon.h"
+#include "rcar_du_lvdsenc.h"
 
 struct rcar_du_lvds_connector {
 	struct rcar_du_connector connector;
@@ -33,6 +34,8 @@ struct rcar_du_lvds_connector {
 		unsigned int height_mm;		/* Panel height in mm */
 		struct videomode mode;
 	} panel;
+
+	unsigned int mode;
 };
 
 #define to_rcar_lvds_connector(c) \
@@ -100,6 +103,32 @@ int rcar_du_lvds_connector_init(struct rcar_du_device *rcdu,
 	of_property_read_u32(np, "width-mm", &lvdscon->panel.width_mm);
 	of_property_read_u32(np, "height-mm", &lvdscon->panel.height_mm);
 
+	if (of_device_is_compatible(np, "panel-lvds")) {
+		enum rcar_lvds_mode mode = 0;
+		const char *mapping = NULL;
+
+		of_property_read_string(np, "data-mapping", &mapping);
+		if (!mapping) {
+			dev_dbg(rcdu->dev,
+				"required property %s not found in %s node\n",
+				"data-mapping", np->full_name);
+			return -EINVAL;
+		}
+
+		if (!strcmp(mapping, "jeida-18") ||
+		    !strcmp(mapping, "jeida-24"))
+			mode = RCAR_LVDS_MODE_JEIDA;
+		else if (!strcmp(mapping, "vesa-24"))
+			mode = RCAR_LVDS_MODE_VESA;
+		else
+			return -EINVAL;
+
+		if (of_find_property(np, "data-mirror", NULL))
+			mode |= RCAR_LVDS_MODE_MIRROR;
+
+		rcar_du_lvdsenc_set_mode(renc->lvds, mode);
+	}
+
 	connector = &lvdscon->connector.connector;
 	connector->display_info.width_mm = lvdscon->panel.width_mm;
 	connector->display_info.height_mm = lvdscon->panel.height_mm;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
index b74105a80a6e..c8c22850947a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
@@ -31,6 +31,7 @@ struct rcar_du_lvdsenc {
 	bool enabled;
 
 	enum rcar_lvds_input input;
+	enum rcar_lvds_mode mode;
 };
 
 static void rcar_lvds_write(struct rcar_du_lvdsenc *lvds, u32 reg, u32 data)
@@ -61,7 +62,7 @@ static void rcar_du_lvdsenc_start_gen2(struct rcar_du_lvdsenc *lvds,
 	/* Select the input, hardcode mode 0, enable LVDS operation and turn
 	 * bias circuitry on.
 	 */
-	lvdcr0 = LVDCR0_BEN | LVDCR0_LVEN;
+	lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_BEN | LVDCR0_LVEN;
 	if (rcrtc->index == 2)
 		lvdcr0 |= LVDCR0_DUSEL;
 	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
@@ -107,7 +108,7 @@ static void rcar_du_lvdsenc_start_gen3(struct rcar_du_lvdsenc *lvds,
 	/* Turn the PLL on, set it to LVDS normal mode, wait for the startup
 	 * delay and turn the output on.
 	 */
-	lvdcr0 = LVDCR0_PLLON;
+	lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_PLLON;
 	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
 
 	lvdcr0 |= LVDCR0_PWD;
@@ -210,6 +211,12 @@ void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds,
 		mode->clock = clamp(mode->clock, 25175, 148500);
 }
 
+void rcar_du_lvdsenc_set_mode(struct rcar_du_lvdsenc *lvds,
+			      enum rcar_lvds_mode mode)
+{
+	lvds->mode = mode;
+}
+
 static int rcar_du_lvdsenc_get_resources(struct rcar_du_lvdsenc *lvds,
 					 struct platform_device *pdev)
 {
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
index dfdba746edf4..7218ac89333e 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
@@ -26,8 +26,17 @@ enum rcar_lvds_input {
 	RCAR_LVDS_INPUT_DU2,
 };
 
+/* Keep in sync with the LVDCR0.LVMD hardware register values. */
+enum rcar_lvds_mode {
+	RCAR_LVDS_MODE_JEIDA = 0,
+	RCAR_LVDS_MODE_MIRROR = 1,
+	RCAR_LVDS_MODE_VESA = 4,
+};
+
 #if IS_ENABLED(CONFIG_DRM_RCAR_LVDS)
 int rcar_du_lvdsenc_init(struct rcar_du_device *rcdu);
+void rcar_du_lvdsenc_set_mode(struct rcar_du_lvdsenc *lvds,
+			      enum rcar_lvds_mode mode);
 int rcar_du_lvdsenc_enable(struct rcar_du_lvdsenc *lvds,
 			   struct drm_crtc *crtc, bool enable);
 void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds,
@@ -37,6 +46,10 @@ static inline int rcar_du_lvdsenc_init(struct rcar_du_device *rcdu)
 {
 	return 0;
 }
+static inline void rcar_du_lvdsenc_set_mode(struct rcar_du_lvdsenc *lvds,
+					    enum rcar_lvds_mode mode)
+{
+}
 static inline int rcar_du_lvdsenc_enable(struct rcar_du_lvdsenc *lvds,
 					 struct drm_crtc *crtc, bool enable)
 {
-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Rob Herring]

On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> Multiple incompatible data link layers have been used over time to
> transmit image data to LVDS panels. This binding supports display panels
> compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> specifications.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../bindings/display/panel/panel-lvds.txt          | 119 +++++++++++++++++++++
>  1 file changed, 119 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> new file mode 100644
> index 000000000000..250861f2673e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> @@ -0,0 +1,119 @@
> +Generic LVDS Panel
> +==================
> +
> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. Multiple
> +incompatible data link layers have been used over time to transmit image data
> +to LVDS panels. This bindings supports display panels compatible with the
> +following specifications.
> +
> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, February
> +1999 (Version 1.0), Japan Electronic Industry Development Association (JEIDA)
> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
> +Semiconductor
> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
> +Electronics Standards Association (VESA)
> +
> +Device compatible with those specifications have been marketed under the
> +FPD-Link and FlatLink brands.
> +
> +
> +Required properties:
> +- compatible: shall contain "panel-lvds"

Maybe as a fallback, but on its own, no way.

> +- width-mm: panel display width in millimeters
> +- height-mm: panel display height in millimeters

This is already documented for all panels IIRC.

> +- data-mapping: the color signals mapping order, "jeida-18", "jeida-24"
> +  or "vesa-24"

Maybe this should be part of the compatible.

> +
> +Optional properties:
> +- label: a symbolic name for the panel

Could be for any panel or display connector.

> +- avdd-supply: reference to the regulator that powers the panel 
analog supply
> +- dvdd-supply: reference to the regulator that powers the panel digital supply

Which one has to be powered on first, what voltage, and with what time 
in between? This is why "generic" or "simple" bindings don't work.

> +- data-mirror: if set, reverse the bit order on all data lanes (6 to 0 instead
> +  of 0 to 6)
> +
> +Required nodes:
> +- One "panel-timing" node containing video timings, in accordance with the
> +  display timing bindings defined in
> +  Documentation/devicetree/bindings/display/display-timing.txt.

I'll let Thierry comment on this one.

> +- One "port" child node for the LVDS input port, in accordance with the
> +  video interface bindings defined in
> +  Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +
> +LVDS data mappings are defined as follows.
> +
> +- "jeida-18" - 18-bit data mapping compatible with the [JEIDA], [LDI] and
> +  [VESA] specifications. Data are transferred as follows on 3 LVDS lanes.
> +
> +Slot	    0       1       2       3       4       5       6
> +	________________                         _________________
> +Clock	                \_______________________/
> +	  ______  ______  ______  ______  ______  ______  ______
> +DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
> +DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
> +DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
> +
> +- "jeida-24" - 24-bit data mapping compatible with the [DSIM] and [LDI]
> +  specifications. Data are transferred as follows on 4 LVDS lanes.
> +
> +Slot	    0       1       2       3       4       5       6
> +	________________                         _________________
> +Clock	                \_______________________/
> +	  ______  ______  ______  ______  ______  ______  ______
> +DATA0	><__G2__><__R7__><__R6__><__R5__><__R4__><__R3__><__R2__><
> +DATA1	><__B3__><__B2__><__G7__><__G6__><__G5__><__G4__><__G3__><
> +DATA2	><_CTL2_><_CTL1_><_CTL0_><__B7__><__B6__><__B5__><__B4__><
> +DATA3	><_CTL3_><__B1__><__B0__><__G1__><__G0__><__R1__><__R0__><
> +
> +- "vesa-24" - 24-bit data mapping compatible with the [VESA] specification.
> +  Data are transferred as follows on 4 LVDS lanes.
> +
> +Slot	    0       1       2       3       4       5       6
> +	________________                         _________________
> +Clock	                \_______________________/
> +	  ______  ______  ______  ______  ______  ______  ______
> +DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
> +DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
> +DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
> +DATA3	><_CTL3_><__B7__><__B6__><__G7__><__G6__><__R7__><__R6__><
> +
> +
> +Control signals are mapped as follows.
> +
> +CTL0: HSync
> +CTL1: VSync
> +CTL2: Data Enable
> +CTL3: 0
> +
> +
> +Example
> +-------
> +
> +panel {
> +	compatible = "mitsubishi,aa121td01", "panel-lvds";
> +	label = "lcd";

Kind of useless in your example.

> +
> +	width-mm = <261>;
> +	height-mm = <163>;
> +
> +	data-mapping = "jeida-24";
> +
> +	panel-timing {
> +		/* 1280x800 @60Hz */
> +		clock-frequency = <71000000>;
> +		hactive = <1280>;
> +		vactive = <800>;
> +		hsync-len = <70>;
> +		hfront-porch = <20>;
> +		hback-porch = <70>;
> +		vsync-len = <5>;
> +		vfront-porch = <3>;
> +		vback-porch = <15>;
> +	};
> +
> +	port {
> +		panel_in: endpoint {
> +			remote-endpoint = <&lvds_connector>;
> +		};
> +	};
> +};
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Rob Herring]

On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> Multiple incompatible data link layers have been used over time to
> transmit image data to LVDS panels. This binding supports display panels
> compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> specifications.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../bindings/display/panel/panel-lvds.txt          | 119 +++++++++++++++++++++
>  1 file changed, 119 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> new file mode 100644
> index 000000000000..250861f2673e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> @@ -0,0 +1,119 @@
> +Generic LVDS Panel
> +==================
> +
> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. Multiple
> +incompatible data link layers have been used over time to transmit image data
> +to LVDS panels. This bindings supports display panels compatible with the
> +following specifications.
> +
> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, February
> +1999 (Version 1.0), Japan Electronic Industry Development Association (JEIDA)
> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
> +Semiconductor
> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
> +Electronics Standards Association (VESA)
> +
> +Device compatible with those specifications have been marketed under the
> +FPD-Link and FlatLink brands.
> +
> +
> +Required properties:
> +- compatible: shall contain "panel-lvds"

Maybe as a fallback, but on its own, no way.

> +- width-mm: panel display width in millimeters
> +- height-mm: panel display height in millimeters

This is already documented for all panels IIRC.

> +- data-mapping: the color signals mapping order, "jeida-18", "jeida-24"
> +  or "vesa-24"

Maybe this should be part of the compatible.

> +
> +Optional properties:
> +- label: a symbolic name for the panel

Could be for any panel or display connector.

> +- avdd-supply: reference to the regulator that powers the panel 
analog supply
> +- dvdd-supply: reference to the regulator that powers the panel digital supply

Which one has to be powered on first, what voltage, and with what time 
in between? This is why "generic" or "simple" bindings don't work.

> +- data-mirror: if set, reverse the bit order on all data lanes (6 to 0 instead
> +  of 0 to 6)
> +
> +Required nodes:
> +- One "panel-timing" node containing video timings, in accordance with the
> +  display timing bindings defined in
> +  Documentation/devicetree/bindings/display/display-timing.txt.

I'll let Thierry comment on this one.

> +- One "port" child node for the LVDS input port, in accordance with the
> +  video interface bindings defined in
> +  Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +
> +LVDS data mappings are defined as follows.
> +
> +- "jeida-18" - 18-bit data mapping compatible with the [JEIDA], [LDI] and
> +  [VESA] specifications. Data are transferred as follows on 3 LVDS lanes.
> +
> +Slot	    0       1       2       3       4       5       6
> +	________________                         _________________
> +Clock	                \_______________________/
> +	  ______  ______  ______  ______  ______  ______  ______
> +DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
> +DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
> +DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
> +
> +- "jeida-24" - 24-bit data mapping compatible with the [DSIM] and [LDI]
> +  specifications. Data are transferred as follows on 4 LVDS lanes.
> +
> +Slot	    0       1       2       3       4       5       6
> +	________________                         _________________
> +Clock	                \_______________________/
> +	  ______  ______  ______  ______  ______  ______  ______
> +DATA0	><__G2__><__R7__><__R6__><__R5__><__R4__><__R3__><__R2__><
> +DATA1	><__B3__><__B2__><__G7__><__G6__><__G5__><__G4__><__G3__><
> +DATA2	><_CTL2_><_CTL1_><_CTL0_><__B7__><__B6__><__B5__><__B4__><
> +DATA3	><_CTL3_><__B1__><__B0__><__G1__><__G0__><__R1__><__R0__><
> +
> +- "vesa-24" - 24-bit data mapping compatible with the [VESA] specification.
> +  Data are transferred as follows on 4 LVDS lanes.
> +
> +Slot	    0       1       2       3       4       5       6
> +	________________                         _________________
> +Clock	                \_______________________/
> +	  ______  ______  ______  ______  ______  ______  ______
> +DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
> +DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
> +DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
> +DATA3	><_CTL3_><__B7__><__B6__><__G7__><__G6__><__R7__><__R6__><
> +
> +
> +Control signals are mapped as follows.
> +
> +CTL0: HSync
> +CTL1: VSync
> +CTL2: Data Enable
> +CTL3: 0
> +
> +
> +Example
> +-------
> +
> +panel {
> +	compatible = "mitsubishi,aa121td01", "panel-lvds";
> +	label = "lcd";

Kind of useless in your example.

> +
> +	width-mm = <261>;
> +	height-mm = <163>;
> +
> +	data-mapping = "jeida-24";
> +
> +	panel-timing {
> +		/* 1280x800 @60Hz */
> +		clock-frequency = <71000000>;
> +		hactive = <1280>;
> +		vactive = <800>;
> +		hsync-len = <70>;
> +		hfront-porch = <20>;
> +		hback-porch = <70>;
> +		vsync-len = <5>;
> +		vfront-porch = <3>;
> +		vback-porch = <15>;
> +	};
> +
> +	port {
> +		panel_in: endpoint {
> +			remote-endpoint = <&lvds_connector>;
> +		};
> +	};
> +};
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Laurent Pinchart]

Hi Rob,

On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> > LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> > Multiple incompatible data link layers have been used over time to
> > transmit image data to LVDS panels. This binding supports display panels
> > compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> > specifications.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++++++
> >  1 file changed, 119 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/display/panel/panel-lvds.txt> 
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> > b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt new file
> > mode 100644
> > index 000000000000..250861f2673e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> > @@ -0,0 +1,119 @@
> > +Generic LVDS Panel
> > +==================
> > +
> > +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> > Multiple
> > +incompatible data link layers have been used over time to transmit image
> > data
> > +to LVDS panels. This bindings supports display panels compatible with the
> > +following specifications.
> > +
> > +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> > February
> > +1999 (Version 1.0), Japan Electronic Industry Development Association
> > (JEIDA)
> > +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
> > +Semiconductor
> > +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
> > +Electronics Standards Association (VESA)
> > +
> > +Device compatible with those specifications have been marketed under the
> > +FPD-Link and FlatLink brands.
> > +
> > +
> > +Required properties:
> > +- compatible: shall contain "panel-lvds"
> 
> Maybe as a fallback, but on its own, no way.

Which brings an interesting question: when designing generic DT bindings, 
what's the rule regarding 

> > +- width-mm: panel display width in millimeters
> > +- height-mm: panel display height in millimeters
> 
> This is already documented for all panels IIRC.

Note that this DT binding has nothing to do with the simple-panel binding. It 
is instead similar to the panel-dpi and panel-dsi-cm bindings (which currently 
don't but should specify the panel size in DT). The LVDS panel driver will 
*not* include any panel-specific information such as size or timings, these 
are specified in DT.

> > +- data-mapping: the color signals mapping order, "jeida-18", "jeida-24"
> > +  or "vesa-24"
> 
> Maybe this should be part of the compatible.

I've thought about it, but given that some panels support selecting between 
multiple modes (through a mode pin that is usually hardwired), I believe a 
separate DT property makes sense.

Furthermore, LVDS data organization is controlled by the combination of both 
data-mapping and data-mirror. It makes little sense from my point of view to 
handle one as part of the compatible string and the other one as a separate 
property.

> > +Optional properties:
> > +- label: a symbolic name for the panel
> 
> Could be for any panel or display connector.

Yes, but I'm not sure to understand how that's relevant :-)

> > +- avdd-supply: reference to the regulator that powers the panel
> analog supply
> > +- dvdd-supply: reference to the regulator that powers the panel digital
> > supply
>
> Which one has to be powered on first, what voltage, and with what time
> in between? This is why "generic" or "simple" bindings don't work.

The above-mentioned specifications also define connectors, pinouts and power 
supplies, but many LVDS panels compatible with the LVDS physical and data 
layers use a different connector with small differences in power supplies.

I believe the voltage is irrelevant here, it doesn't need to be controlled by 
the operating system. Power supplies order and timing is relevant, I'll 
investigate the level of differences between panels. I'm also fine with 
dropping those properties for now.

> > +- data-mirror: if set, reverse the bit order on all data lanes (6 to 0
> > instead
> > +  of 0 to 6)
> > +
> > +Required nodes:
> > +- One "panel-timing" node containing video timings, in accordance with
> > the
> > +  display timing bindings defined in
> > +  Documentation/devicetree/bindings/display/display-timing.txt.
> 
> I'll let Thierry comment on this one.
> 
> > +- One "port" child node for the LVDS input port, in accordance with the
> > +  video interface bindings defined in
> > +  Documentation/devicetree/bindings/media/video-interfaces.txt.
> > +
> > +
> > +LVDS data mappings are defined as follows.
> > +
> > +- "jeida-18" - 18-bit data mapping compatible with the [JEIDA], [LDI] and
> > +  [VESA] specifications. Data are transferred as follows on 3 LVDS lanes.
> > +
> > +Slot	    0       1       2       3       4       5       6
> > +	________________                         _________________
> > +Clock	                \_______________________/
> > +	  ______  ______  ______  ______  ______  ______  ______
> > +DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
> > +DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
> > +DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
> > +
> > +- "jeida-24" - 24-bit data mapping compatible with the [DSIM] and [LDI]
> > +  specifications. Data are transferred as follows on 4 LVDS lanes.
> > +
> > +Slot	    0       1       2       3       4       5       6
> > +	________________                         _________________
> > +Clock	                \_______________________/
> > +	  ______  ______  ______  ______  ______  ______  ______
> > +DATA0	><__G2__><__R7__><__R6__><__R5__><__R4__><__R3__><__R2__><
> > +DATA1	><__B3__><__B2__><__G7__><__G6__><__G5__><__G4__><__G3__><
> > +DATA2	><_CTL2_><_CTL1_><_CTL0_><__B7__><__B6__><__B5__><__B4__><
> > +DATA3	><_CTL3_><__B1__><__B0__><__G1__><__G0__><__R1__><__R0__><
> > +
> > +- "vesa-24" - 24-bit data mapping compatible with the [VESA]
> > specification. +  Data are transferred as follows on 4 LVDS lanes.
> > +
> > +Slot	    0       1       2       3       4       5       6
> > +	________________                         _________________
> > +Clock	                \_______________________/
> > +	  ______  ______  ______  ______  ______  ______  ______
> > +DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
> > +DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
> > +DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
> > +DATA3	><_CTL3_><__B7__><__B6__><__G7__><__G6__><__R7__><__R6__><
> > +
> > +
> > +Control signals are mapped as follows.
> > +
> > +CTL0: HSync
> > +CTL1: VSync
> > +CTL2: Data Enable
> > +CTL3: 0
> > +
> > +
> > +Example
> > +-------
> > +
> > +panel {
> > +	compatible = "mitsubishi,aa121td01", "panel-lvds";
> > +	label = "lcd";
> 
> Kind of useless in your example.

I can rename that to "LCD0" for instance.

> > +
> > +	width-mm = <261>;
> > +	height-mm = <163>;
> > +
> > +	data-mapping = "jeida-24";
> > +
> > +	panel-timing {
> > +		/* 1280x800 @60Hz */
> > +		clock-frequency = <71000000>;
> > +		hactive = <1280>;
> > +		vactive = <800>;
> > +		hsync-len = <70>;
> > +		hfront-porch = <20>;
> > +		hback-porch = <70>;
> > +		vsync-len = <5>;
> > +		vfront-porch = <3>;
> > +		vback-porch = <15>;
> > +	};
> > +
> > +	port {
> > +		panel_in: endpoint {
> > +			remote-endpoint = <&lvds_connector>;
> > +		};
> > +	};
> > +};

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Laurent Pinchart]

Hi Rob,

On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> > LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> > Multiple incompatible data link layers have been used over time to
> > transmit image data to LVDS panels. This binding supports display panels
> > compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> > specifications.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++++++
> >  1 file changed, 119 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/display/panel/panel-lvds.txt> 
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> > b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt new file
> > mode 100644
> > index 000000000000..250861f2673e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> > @@ -0,0 +1,119 @@
> > +Generic LVDS Panel
> > +==================
> > +
> > +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> > Multiple
> > +incompatible data link layers have been used over time to transmit image
> > data
> > +to LVDS panels. This bindings supports display panels compatible with the
> > +following specifications.
> > +
> > +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> > February
> > +1999 (Version 1.0), Japan Electronic Industry Development Association
> > (JEIDA)
> > +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
> > +Semiconductor
> > +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
> > +Electronics Standards Association (VESA)
> > +
> > +Device compatible with those specifications have been marketed under the
> > +FPD-Link and FlatLink brands.
> > +
> > +
> > +Required properties:
> > +- compatible: shall contain "panel-lvds"
> 
> Maybe as a fallback, but on its own, no way.

Which brings an interesting question: when designing generic DT bindings, 
what's the rule regarding 

> > +- width-mm: panel display width in millimeters
> > +- height-mm: panel display height in millimeters
> 
> This is already documented for all panels IIRC.

Note that this DT binding has nothing to do with the simple-panel binding. It 
is instead similar to the panel-dpi and panel-dsi-cm bindings (which currently 
don't but should specify the panel size in DT). The LVDS panel driver will 
*not* include any panel-specific information such as size or timings, these 
are specified in DT.

> > +- data-mapping: the color signals mapping order, "jeida-18", "jeida-24"
> > +  or "vesa-24"
> 
> Maybe this should be part of the compatible.

I've thought about it, but given that some panels support selecting between 
multiple modes (through a mode pin that is usually hardwired), I believe a 
separate DT property makes sense.

Furthermore, LVDS data organization is controlled by the combination of both 
data-mapping and data-mirror. It makes little sense from my point of view to 
handle one as part of the compatible string and the other one as a separate 
property.

> > +Optional properties:
> > +- label: a symbolic name for the panel
> 
> Could be for any panel or display connector.

Yes, but I'm not sure to understand how that's relevant :-)

> > +- avdd-supply: reference to the regulator that powers the panel
> analog supply
> > +- dvdd-supply: reference to the regulator that powers the panel digital
> > supply
>
> Which one has to be powered on first, what voltage, and with what time
> in between? This is why "generic" or "simple" bindings don't work.

The above-mentioned specifications also define connectors, pinouts and power 
supplies, but many LVDS panels compatible with the LVDS physical and data 
layers use a different connector with small differences in power supplies.

I believe the voltage is irrelevant here, it doesn't need to be controlled by 
the operating system. Power supplies order and timing is relevant, I'll 
investigate the level of differences between panels. I'm also fine with 
dropping those properties for now.

> > +- data-mirror: if set, reverse the bit order on all data lanes (6 to 0
> > instead
> > +  of 0 to 6)
> > +
> > +Required nodes:
> > +- One "panel-timing" node containing video timings, in accordance with
> > the
> > +  display timing bindings defined in
> > +  Documentation/devicetree/bindings/display/display-timing.txt.
> 
> I'll let Thierry comment on this one.
> 
> > +- One "port" child node for the LVDS input port, in accordance with the
> > +  video interface bindings defined in
> > +  Documentation/devicetree/bindings/media/video-interfaces.txt.
> > +
> > +
> > +LVDS data mappings are defined as follows.
> > +
> > +- "jeida-18" - 18-bit data mapping compatible with the [JEIDA], [LDI] and
> > +  [VESA] specifications. Data are transferred as follows on 3 LVDS lanes.
> > +
> > +Slot	    0       1       2       3       4       5       6
> > +	________________                         _________________
> > +Clock	                \_______________________/
> > +	  ______  ______  ______  ______  ______  ______  ______
> > +DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
> > +DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
> > +DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
> > +
> > +- "jeida-24" - 24-bit data mapping compatible with the [DSIM] and [LDI]
> > +  specifications. Data are transferred as follows on 4 LVDS lanes.
> > +
> > +Slot	    0       1       2       3       4       5       6
> > +	________________                         _________________
> > +Clock	                \_______________________/
> > +	  ______  ______  ______  ______  ______  ______  ______
> > +DATA0	><__G2__><__R7__><__R6__><__R5__><__R4__><__R3__><__R2__><
> > +DATA1	><__B3__><__B2__><__G7__><__G6__><__G5__><__G4__><__G3__><
> > +DATA2	><_CTL2_><_CTL1_><_CTL0_><__B7__><__B6__><__B5__><__B4__><
> > +DATA3	><_CTL3_><__B1__><__B0__><__G1__><__G0__><__R1__><__R0__><
> > +
> > +- "vesa-24" - 24-bit data mapping compatible with the [VESA]
> > specification. +  Data are transferred as follows on 4 LVDS lanes.
> > +
> > +Slot	    0       1       2       3       4       5       6
> > +	________________                         _________________
> > +Clock	                \_______________________/
> > +	  ______  ______  ______  ______  ______  ______  ______
> > +DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
> > +DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
> > +DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
> > +DATA3	><_CTL3_><__B7__><__B6__><__G7__><__G6__><__R7__><__R6__><
> > +
> > +
> > +Control signals are mapped as follows.
> > +
> > +CTL0: HSync
> > +CTL1: VSync
> > +CTL2: Data Enable
> > +CTL3: 0
> > +
> > +
> > +Example
> > +-------
> > +
> > +panel {
> > +	compatible = "mitsubishi,aa121td01", "panel-lvds";
> > +	label = "lcd";
> 
> Kind of useless in your example.

I can rename that to "LCD0" for instance.

> > +
> > +	width-mm = <261>;
> > +	height-mm = <163>;
> > +
> > +	data-mapping = "jeida-24";
> > +
> > +	panel-timing {
> > +		/* 1280x800 @60Hz */
> > +		clock-frequency = <71000000>;
> > +		hactive = <1280>;
> > +		vactive = <800>;
> > +		hsync-len = <70>;
> > +		hfront-porch = <20>;
> > +		hback-porch = <70>;
> > +		vsync-len = <5>;
> > +		vfront-porch = <3>;
> > +		vback-porch = <15>;
> > +	};
> > +
> > +	port {
> > +		panel_in: endpoint {
> > +			remote-endpoint = <&lvds_connector>;
> > +		};
> > +	};
> > +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Rob Herring]

On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
>> > LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> > Multiple incompatible data link layers have been used over time to
>> > transmit image data to LVDS panels. This binding supports display panels
>> > compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
>> > specifications.
>> >
>> > Signed-off-by: Laurent Pinchart
>> > <laurent.pinchart+renesas@ideasonboard.com>
>> > ---
>> >
>> >  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++++++
>> >  1 file changed, 119 insertions(+)
>> >  create mode 100644
>> >  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
>> > diff --git
>> > a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> > b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt new file
>> > mode 100644
>> > index 000000000000..250861f2673e
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> > @@ -0,0 +1,119 @@
>> > +Generic LVDS Panel
>> > +==================
>> > +
>> > +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> > Multiple
>> > +incompatible data link layers have been used over time to transmit image
>> > data
>> > +to LVDS panels. This bindings supports display panels compatible with the
>> > +following specifications.
>> > +
>> > +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
>> > February
>> > +1999 (Version 1.0), Japan Electronic Industry Development Association
>> > (JEIDA)
>> > +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
>> > +Semiconductor
>> > +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
>> > +Electronics Standards Association (VESA)
>> > +
>> > +Device compatible with those specifications have been marketed under the
>> > +FPD-Link and FlatLink brands.
>> > +
>> > +
>> > +Required properties:
>> > +- compatible: shall contain "panel-lvds"
>>
>> Maybe as a fallback, but on its own, no way.
>
> Which brings an interesting question: when designing generic DT bindings,
> what's the rule regarding

Call it "simple" so I can easily NAK it. :)

Define a generic structure, not a single binding trying to serve all.

>
>> > +- width-mm: panel display width in millimeters
>> > +- height-mm: panel display height in millimeters
>>
>> This is already documented for all panels IIRC.
>
> Note that this DT binding has nothing to do with the simple-panel binding. It
> is instead similar to the panel-dpi and panel-dsi-cm bindings (which currently
> don't but should specify the panel size in DT). The LVDS panel driver will
> *not* include any panel-specific information such as size or timings, these
> are specified in DT.

The panel bindings aren't really different. The biggest difference was
location in the tree, but we now generally allow panels to be either a
child of the LCD controller or connected with OF graph. We probably
need to work on restructuring the panel bindings a bit. We should have
an inheritance with a base panel binding of things like size, label,
graph, backlight, etc, then perhaps an interface specific bindings for
LVDS, DSI, and parallel, then a panel specific binding. With this the
panel specific binding is typically just a compatible string and which
inherited properties apply to it.


>> > +- data-mapping: the color signals mapping order, "jeida-18", "jeida-24"
>> > +  or "vesa-24"
>>
>> Maybe this should be part of the compatible.
>
> I've thought about it, but given that some panels support selecting between
> multiple modes (through a mode pin that is usually hardwired), I believe a
> separate DT property makes sense.

Okay.

> Furthermore, LVDS data organization is controlled by the combination of both
> data-mapping and data-mirror. It makes little sense from my point of view to
> handle one as part of the compatible string and the other one as a separate
> property.
>
>> > +Optional properties:
>> > +- label: a symbolic name for the panel
>>
>> Could be for any panel or display connector.
>
> Yes, but I'm not sure to understand how that's relevant :-)

Meaning it should be a common property.

>> > +- avdd-supply: reference to the regulator that powers the panel
>> analog supply
>> > +- dvdd-supply: reference to the regulator that powers the panel digital
>> > supply
>>
>> Which one has to be powered on first, what voltage, and with what time
>> in between? This is why "generic" or "simple" bindings don't work.
>
> The above-mentioned specifications also define connectors, pinouts and power
> supplies, but many LVDS panels compatible with the LVDS physical and data
> layers use a different connector with small differences in power supplies.
>
> I believe the voltage is irrelevant here, it doesn't need to be controlled by
> the operating system. Power supplies order and timing is relevant, I'll
> investigate the level of differences between panels. I'm also fine with
> dropping those properties for now.

Whether you have control of the supplies is dependent on the board.
Dropping them is just puts us in the simple binding trap. The simple
bindings start out that way and then people keep adding to them.

>
>> > +- data-mirror: if set, reverse the bit order on all data lanes (6 to 0
>> > instead
>> > +  of 0 to 6)

On this one, make the name describe the order. "mirror" requires that
I know what is normal ordering. Perhaps "data-msb-first".

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Rob Herring]

On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
>> > LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> > Multiple incompatible data link layers have been used over time to
>> > transmit image data to LVDS panels. This binding supports display panels
>> > compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
>> > specifications.
>> >
>> > Signed-off-by: Laurent Pinchart
>> > <laurent.pinchart+renesas@ideasonboard.com>
>> > ---
>> >
>> >  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++++++
>> >  1 file changed, 119 insertions(+)
>> >  create mode 100644
>> >  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
>> > diff --git
>> > a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> > b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt new file
>> > mode 100644
>> > index 000000000000..250861f2673e
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> > @@ -0,0 +1,119 @@
>> > +Generic LVDS Panel
>> > +==================
>> > +
>> > +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> > Multiple
>> > +incompatible data link layers have been used over time to transmit image
>> > data
>> > +to LVDS panels. This bindings supports display panels compatible with the
>> > +following specifications.
>> > +
>> > +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
>> > February
>> > +1999 (Version 1.0), Japan Electronic Industry Development Association
>> > (JEIDA)
>> > +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
>> > +Semiconductor
>> > +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
>> > +Electronics Standards Association (VESA)
>> > +
>> > +Device compatible with those specifications have been marketed under the
>> > +FPD-Link and FlatLink brands.
>> > +
>> > +
>> > +Required properties:
>> > +- compatible: shall contain "panel-lvds"
>>
>> Maybe as a fallback, but on its own, no way.
>
> Which brings an interesting question: when designing generic DT bindings,
> what's the rule regarding

Call it "simple" so I can easily NAK it. :)

Define a generic structure, not a single binding trying to serve all.

>
>> > +- width-mm: panel display width in millimeters
>> > +- height-mm: panel display height in millimeters
>>
>> This is already documented for all panels IIRC.
>
> Note that this DT binding has nothing to do with the simple-panel binding. It
> is instead similar to the panel-dpi and panel-dsi-cm bindings (which currently
> don't but should specify the panel size in DT). The LVDS panel driver will
> *not* include any panel-specific information such as size or timings, these
> are specified in DT.

The panel bindings aren't really different. The biggest difference was
location in the tree, but we now generally allow panels to be either a
child of the LCD controller or connected with OF graph. We probably
need to work on restructuring the panel bindings a bit. We should have
an inheritance with a base panel binding of things like size, label,
graph, backlight, etc, then perhaps an interface specific bindings for
LVDS, DSI, and parallel, then a panel specific binding. With this the
panel specific binding is typically just a compatible string and which
inherited properties apply to it.


>> > +- data-mapping: the color signals mapping order, "jeida-18", "jeida-24"
>> > +  or "vesa-24"
>>
>> Maybe this should be part of the compatible.
>
> I've thought about it, but given that some panels support selecting between
> multiple modes (through a mode pin that is usually hardwired), I believe a
> separate DT property makes sense.

Okay.

> Furthermore, LVDS data organization is controlled by the combination of both
> data-mapping and data-mirror. It makes little sense from my point of view to
> handle one as part of the compatible string and the other one as a separate
> property.
>
>> > +Optional properties:
>> > +- label: a symbolic name for the panel
>>
>> Could be for any panel or display connector.
>
> Yes, but I'm not sure to understand how that's relevant :-)

Meaning it should be a common property.

>> > +- avdd-supply: reference to the regulator that powers the panel
>> analog supply
>> > +- dvdd-supply: reference to the regulator that powers the panel digital
>> > supply
>>
>> Which one has to be powered on first, what voltage, and with what time
>> in between? This is why "generic" or "simple" bindings don't work.
>
> The above-mentioned specifications also define connectors, pinouts and power
> supplies, but many LVDS panels compatible with the LVDS physical and data
> layers use a different connector with small differences in power supplies.
>
> I believe the voltage is irrelevant here, it doesn't need to be controlled by
> the operating system. Power supplies order and timing is relevant, I'll
> investigate the level of differences between panels. I'm also fine with
> dropping those properties for now.

Whether you have control of the supplies is dependent on the board.
Dropping them is just puts us in the simple binding trap. The simple
bindings start out that way and then people keep adding to them.

>
>> > +- data-mirror: if set, reverse the bit order on all data lanes (6 to 0
>> > instead
>> > +  of 0 to 6)

On this one, make the name describe the order. "mirror" requires that
I know what is normal ordering. Perhaps "data-msb-first".

Rob

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Laurent Pinchart]

Hi Rob,

On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
> > On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> >> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> >>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>> Multiple incompatible data link layers have been used over time to
> >>> transmit image data to LVDS panels. This binding supports display
> >>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> >>> specifications.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> <laurent.pinchart+renesas@ideasonboard.com>
> >>> ---
> >>> 
> >>>  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++++
> >>>  1 file changed, 119 insertions(+)
> >>>  create mode 100644
> >>>  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
> >>> 
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>> new file mode 100644
> >>> index 000000000000..250861f2673e
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>> @@ -0,0 +1,119 @@
> >>> +Generic LVDS Panel
> >>> +==================
> >>> +
> >>> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>> Multiple
> >>> +incompatible data link layers have been used over time to transmit
> >>> image data
> >>> +to LVDS panels. This bindings supports display panels compatible with
> >>> the
> >>> +following specifications.
> >>> +
> >>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> >>> February
> >>> +1999 (Version 1.0), Japan Electronic Industry Development Association
> >>> (JEIDA)
> >>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
> >>> +Semiconductor
> >>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
> >>> Video
> >>> +Electronics Standards Association (VESA)
> >>> +
> >>> +Device compatible with those specifications have been marketed under
> >>> the
> >>> +FPD-Link and FlatLink brands.
> >>> +
> >>> +
> >>> +Required properties:
> >>> +- compatible: shall contain "panel-lvds"
> >> 
> >> Maybe as a fallback, but on its own, no way.
> > 
> > Which brings an interesting question: when designing generic DT bindings,
> > what's the rule regarding

Looks like I forgot part of the question. I meant to ask what is the rule 
regarding usage of more precise compatible strings ?

For instance (but perhaps not the best example), the input/rotary-encoder.txt 
bindings define a "rotary-encoder" compatible string, with no other bindings 
defining more precise compatible strings for the exact rotary encoder model. 
When it comes to panels I believe it makes sense to define model-specific 
compatible strings even if they're unused by drivers. I'm however wondering 
what the rule is there, and where those device-specific compatible strings 
should be defined. I'd like to avoid using one file per panel model as done 
today for the simple-panel bindings.

> Call it "simple" so I can easily NAK it. :)
> 
> Define a generic structure, not a single binding trying to serve all.
> 
> >> > +- width-mm: panel display width in millimeters
> >> > +- height-mm: panel display height in millimeters
> >> 
> >> This is already documented for all panels IIRC.
> > 
> > Note that this DT binding has nothing to do with the simple-panel binding.
> > It is instead similar to the panel-dpi and panel-dsi-cm bindings (which
> > currently don't but should specify the panel size in DT). The LVDS panel
> > driver will *not* include any panel-specific information such as size or
> > timings, these are specified in DT.
> 
> The panel bindings aren't really different. The biggest difference was
> location in the tree, but we now generally allow panels to be either a
> child of the LCD controller or connected with OF graph. We probably
> need to work on restructuring the panel bindings a bit. We should have
> an inheritance with a base panel binding of things like size, label,
> graph, backlight, etc, then perhaps an interface specific bindings for
> LVDS, DSI, and parallel, then a panel specific binding. With this the
> panel specific binding is typically just a compatible string and which
> inherited properties apply to it.

That sounds good to me, but we have multiple models for panel bindings.

As you mentioned panels can be referenced through an LCD controller node 
property containing a phandle to the panel node, or through OF graph. That's a 
situation we have today, and we need to keep supporting both (at least for 
existing panels, perhaps not for the new ones).

Another difference is how to express panel data such as size and timings. The 
simple-panel DT bindings don't contain such data and expects the drivers to 
contain a table of panel data for all models supported, while the DPI, DSI and 
now the proposed LVDS panel bindings contain properties for panel data.

How would you like to reconcile all that ?

> >>> +- data-mapping: the color signals mapping order, "jeida-18",
> >>> "jeida-24"
> >>> +  or "vesa-24"
> >> 
> >> Maybe this should be part of the compatible.
> > 
> > I've thought about it, but given that some panels support selecting
> > between multiple modes (through a mode pin that is usually hardwired), I
> > believe a separate DT property makes sense.
> 
> Okay.
> 
> > Furthermore, LVDS data organization is controlled by the combination of
> > both data-mapping and data-mirror. It makes little sense from my point of
> > view to handle one as part of the compatible string and the other one as
> > a separate property.
> > 
> >> > +Optional properties:
> >> > +- label: a symbolic name for the panel
> >> 
> >> Could be for any panel or display connector.
> > 
> > Yes, but I'm not sure to understand how that's relevant :-)
> 
> Meaning it should be a common property.

Sure. So you expect me to reorganize all the panels and connectors DT bindings 
in order to get this one merged ? :-)

> >>> +- avdd-supply: reference to the regulator that powers the panel
> >>> analog supply
> >>> +- dvdd-supply: reference to the regulator that powers the panel
> >>> digital supply
> >> 
> >> Which one has to be powered on first, what voltage, and with what time
> >> in between? This is why "generic" or "simple" bindings don't work.
> > 
> > The above-mentioned specifications also define connectors, pinouts and
> > power supplies, but many LVDS panels compatible with the LVDS physical
> > and data layers use a different connector with small differences in power
> > supplies.
> > 
> > I believe the voltage is irrelevant here, it doesn't need to be controlled
> > by the operating system. Power supplies order and timing is relevant,
> > I'll investigate the level of differences between panels. I'm also fine
> > with dropping those properties for now.
> 
> Whether you have control of the supplies is dependent on the board.
> Dropping them is just puts us in the simple binding trap. The simple
> bindings start out that way and then people keep adding to them.

Damn, you can't be fooled easily ;-)

On a more serious note, I'd like to design the bindings in a way that wouldn't 
require adding device-specific code in the driver for each panel model, given 
that in most cases power supply handling will be generic. What's your opinion 
about a generic power supply model that would be used in the default case, 
with the option to override it with device-specific code when needed ?

> >>> +- data-mirror: if set, reverse the bit order on all data lanes (6 to 0
> >>> instead
> >>> +  of 0 to 6)
> 
> On this one, make the name describe the order. "mirror" requires that
> I know what is normal ordering. Perhaps "data-msb-first".

Sounds good to me, I'll use that.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Laurent Pinchart]

Hi Rob,

On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
> > On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> >> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> >>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>> Multiple incompatible data link layers have been used over time to
> >>> transmit image data to LVDS panels. This binding supports display
> >>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> >>> specifications.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> <laurent.pinchart+renesas@ideasonboard.com>
> >>> ---
> >>> 
> >>>  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++++
> >>>  1 file changed, 119 insertions(+)
> >>>  create mode 100644
> >>>  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
> >>> 
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>> new file mode 100644
> >>> index 000000000000..250861f2673e
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>> @@ -0,0 +1,119 @@
> >>> +Generic LVDS Panel
> >>> +==================
> >>> +
> >>> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>> Multiple
> >>> +incompatible data link layers have been used over time to transmit
> >>> image data
> >>> +to LVDS panels. This bindings supports display panels compatible with
> >>> the
> >>> +following specifications.
> >>> +
> >>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> >>> February
> >>> +1999 (Version 1.0), Japan Electronic Industry Development Association
> >>> (JEIDA)
> >>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
> >>> +Semiconductor
> >>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
> >>> Video
> >>> +Electronics Standards Association (VESA)
> >>> +
> >>> +Device compatible with those specifications have been marketed under
> >>> the
> >>> +FPD-Link and FlatLink brands.
> >>> +
> >>> +
> >>> +Required properties:
> >>> +- compatible: shall contain "panel-lvds"
> >> 
> >> Maybe as a fallback, but on its own, no way.
> > 
> > Which brings an interesting question: when designing generic DT bindings,
> > what's the rule regarding

Looks like I forgot part of the question. I meant to ask what is the rule 
regarding usage of more precise compatible strings ?

For instance (but perhaps not the best example), the input/rotary-encoder.txt 
bindings define a "rotary-encoder" compatible string, with no other bindings 
defining more precise compatible strings for the exact rotary encoder model. 
When it comes to panels I believe it makes sense to define model-specific 
compatible strings even if they're unused by drivers. I'm however wondering 
what the rule is there, and where those device-specific compatible strings 
should be defined. I'd like to avoid using one file per panel model as done 
today for the simple-panel bindings.

> Call it "simple" so I can easily NAK it. :)
> 
> Define a generic structure, not a single binding trying to serve all.
> 
> >> > +- width-mm: panel display width in millimeters
> >> > +- height-mm: panel display height in millimeters
> >> 
> >> This is already documented for all panels IIRC.
> > 
> > Note that this DT binding has nothing to do with the simple-panel binding.
> > It is instead similar to the panel-dpi and panel-dsi-cm bindings (which
> > currently don't but should specify the panel size in DT). The LVDS panel
> > driver will *not* include any panel-specific information such as size or
> > timings, these are specified in DT.
> 
> The panel bindings aren't really different. The biggest difference was
> location in the tree, but we now generally allow panels to be either a
> child of the LCD controller or connected with OF graph. We probably
> need to work on restructuring the panel bindings a bit. We should have
> an inheritance with a base panel binding of things like size, label,
> graph, backlight, etc, then perhaps an interface specific bindings for
> LVDS, DSI, and parallel, then a panel specific binding. With this the
> panel specific binding is typically just a compatible string and which
> inherited properties apply to it.

That sounds good to me, but we have multiple models for panel bindings.

As you mentioned panels can be referenced through an LCD controller node 
property containing a phandle to the panel node, or through OF graph. That's a 
situation we have today, and we need to keep supporting both (at least for 
existing panels, perhaps not for the new ones).

Another difference is how to express panel data such as size and timings. The 
simple-panel DT bindings don't contain such data and expects the drivers to 
contain a table of panel data for all models supported, while the DPI, DSI and 
now the proposed LVDS panel bindings contain properties for panel data.

How would you like to reconcile all that ?

> >>> +- data-mapping: the color signals mapping order, "jeida-18",
> >>> "jeida-24"
> >>> +  or "vesa-24"
> >> 
> >> Maybe this should be part of the compatible.
> > 
> > I've thought about it, but given that some panels support selecting
> > between multiple modes (through a mode pin that is usually hardwired), I
> > believe a separate DT property makes sense.
> 
> Okay.
> 
> > Furthermore, LVDS data organization is controlled by the combination of
> > both data-mapping and data-mirror. It makes little sense from my point of
> > view to handle one as part of the compatible string and the other one as
> > a separate property.
> > 
> >> > +Optional properties:
> >> > +- label: a symbolic name for the panel
> >> 
> >> Could be for any panel or display connector.
> > 
> > Yes, but I'm not sure to understand how that's relevant :-)
> 
> Meaning it should be a common property.

Sure. So you expect me to reorganize all the panels and connectors DT bindings 
in order to get this one merged ? :-)

> >>> +- avdd-supply: reference to the regulator that powers the panel
> >>> analog supply
> >>> +- dvdd-supply: reference to the regulator that powers the panel
> >>> digital supply
> >> 
> >> Which one has to be powered on first, what voltage, and with what time
> >> in between? This is why "generic" or "simple" bindings don't work.
> > 
> > The above-mentioned specifications also define connectors, pinouts and
> > power supplies, but many LVDS panels compatible with the LVDS physical
> > and data layers use a different connector with small differences in power
> > supplies.
> > 
> > I believe the voltage is irrelevant here, it doesn't need to be controlled
> > by the operating system. Power supplies order and timing is relevant,
> > I'll investigate the level of differences between panels. I'm also fine
> > with dropping those properties for now.
> 
> Whether you have control of the supplies is dependent on the board.
> Dropping them is just puts us in the simple binding trap. The simple
> bindings start out that way and then people keep adding to them.

Damn, you can't be fooled easily ;-)

On a more serious note, I'd like to design the bindings in a way that wouldn't 
require adding device-specific code in the driver for each panel model, given 
that in most cases power supply handling will be generic. What's your opinion 
about a generic power supply model that would be used in the default case, 
with the option to override it with device-specific code when needed ?

> >>> +- data-mirror: if set, reverse the bit order on all data lanes (6 to 0
> >>> instead
> >>> +  of 0 to 6)
> 
> On this one, make the name describe the order. "mirror" requires that
> I know what is normal ordering. Perhaps "data-msb-first".

Sounds good to me, I'll use that.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Laurent Pinchart]

Hi Rob,

Ping ?

On Monday 17 Oct 2016 15:42:56 Laurent Pinchart wrote:
> On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
> > On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
> >> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> >>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> >>>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>> Multiple incompatible data link layers have been used over time to
> >>>> transmit image data to LVDS panels. This binding supports display
> >>>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> >>>> specifications.
> >>>> 
> >>>> Signed-off-by: Laurent Pinchart
> >>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>> ---
> >>>> 
> >>>>  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++
> >>>>  1 file changed, 119 insertions(+)
> >>>>  create mode 100644
> >>>>  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
> >>>> 
> >>>> diff --git
> >>>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> new file mode 100644
> >>>> index 000000000000..250861f2673e
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> @@ -0,0 +1,119 @@
> >>>> +Generic LVDS Panel
> >>>> +==================
> >>>> +
> >>>> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>> Multiple
> >>>> +incompatible data link layers have been used over time to transmit
> >>>> image data
> >>>> +to LVDS panels. This bindings supports display panels compatible with
> >>>> the
> >>>> +following specifications.
> >>>> +
> >>>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> >>>> February
> >>>> +1999 (Version 1.0), Japan Electronic Industry Development Association
> >>>> (JEIDA)
> >>>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95),
> >>>> National
> >>>> +Semiconductor
> >>>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
> >>>> Video
> >>>> +Electronics Standards Association (VESA)
> >>>> +
> >>>> +Device compatible with those specifications have been marketed under
> >>>> the
> >>>> +FPD-Link and FlatLink brands.
> >>>> +
> >>>> +
> >>>> +Required properties:
> >>>> +- compatible: shall contain "panel-lvds"
> >>> 
> >>> Maybe as a fallback, but on its own, no way.
> >> 
> >> Which brings an interesting question: when designing generic DT
> >> bindings, what's the rule regarding
> 
> Looks like I forgot part of the question. I meant to ask what is the rule
> regarding usage of more precise compatible strings ?
> 
> For instance (but perhaps not the best example), the
> input/rotary-encoder.txt bindings define a "rotary-encoder" compatible
> string, with no other bindings defining more precise compatible strings for
> the exact rotary encoder model. When it comes to panels I believe it makes
> sense to define model-specific compatible strings even if they're unused by
> drivers. I'm however wondering what the rule is there, and where those
> device-specific compatible strings should be defined. I'd like to avoid
> using one file per panel model as done today for the simple-panel bindings.
> 
> > Call it "simple" so I can easily NAK it. :)
> > 
> > Define a generic structure, not a single binding trying to serve all.
> > 
> >>> > +- width-mm: panel display width in millimeters
> >>> > +- height-mm: panel display height in millimeters
> >>> 
> >>> This is already documented for all panels IIRC.
> >> 
> >> Note that this DT binding has nothing to do with the simple-panel
> >> binding. It is instead similar to the panel-dpi and panel-dsi-cm bindings
> >> (which currently don't but should specify the panel size in DT). The LVDS
> >> panel driver will *not* include any panel-specific information such as
> >> size or timings, these are specified in DT.
> > 
> > The panel bindings aren't really different. The biggest difference was
> > location in the tree, but we now generally allow panels to be either a
> > child of the LCD controller or connected with OF graph. We probably
> > need to work on restructuring the panel bindings a bit. We should have
> > an inheritance with a base panel binding of things like size, label,
> > graph, backlight, etc, then perhaps an interface specific bindings for
> > LVDS, DSI, and parallel, then a panel specific binding. With this the
> > panel specific binding is typically just a compatible string and which
> > inherited properties apply to it.
> 
> That sounds good to me, but we have multiple models for panel bindings.
> 
> As you mentioned panels can be referenced through an LCD controller node
> property containing a phandle to the panel node, or through OF graph. That's
> a situation we have today, and we need to keep supporting both (at least
> for existing panels, perhaps not for the new ones).
> 
> Another difference is how to express panel data such as size and timings.
> The simple-panel DT bindings don't contain such data and expects the
> drivers to contain a table of panel data for all models supported, while
> the DPI, DSI and now the proposed LVDS panel bindings contain properties
> for panel data.
> 
> How would you like to reconcile all that ?
> 
> >>>> +- data-mapping: the color signals mapping order, "jeida-18",
> >>>> "jeida-24"
> >>>> +  or "vesa-24"
> >>> 
> >>> Maybe this should be part of the compatible.
> >> 
> >> I've thought about it, but given that some panels support selecting
> >> between multiple modes (through a mode pin that is usually hardwired), I
> >> believe a separate DT property makes sense.
> > 
> > Okay.
> > 
> >> Furthermore, LVDS data organization is controlled by the combination of
> >> both data-mapping and data-mirror. It makes little sense from my point
> >> of view to handle one as part of the compatible string and the other one
> >> as a separate property.
> >> 
> >>> > +Optional properties:
> >>> > +- label: a symbolic name for the panel
> >>> 
> >>> Could be for any panel or display connector.
> >> 
> >> Yes, but I'm not sure to understand how that's relevant :-)
> > 
> > Meaning it should be a common property.
> 
> Sure. So you expect me to reorganize all the panels and connectors DT
> bindings in order to get this one merged ? :-)
> 
> >>>> +- avdd-supply: reference to the regulator that powers the panel
> >>>> analog supply
> >>>> +- dvdd-supply: reference to the regulator that powers the panel
> >>>> digital supply
> >>> 
> >>> Which one has to be powered on first, what voltage, and with what time
> >>> in between? This is why "generic" or "simple" bindings don't work.
> >> 
> >> The above-mentioned specifications also define connectors, pinouts and
> >> power supplies, but many LVDS panels compatible with the LVDS physical
> >> and data layers use a different connector with small differences in
> >> power
> >> supplies.
> >> 
> >> I believe the voltage is irrelevant here, it doesn't need to be
> >> controlled by the operating system. Power supplies order and timing is
> >> relevant, I'll investigate the level of differences between panels. I'm
> >> also fine with dropping those properties for now.
> > 
> > Whether you have control of the supplies is dependent on the board.
> > Dropping them is just puts us in the simple binding trap. The simple
> > bindings start out that way and then people keep adding to them.
> 
> Damn, you can't be fooled easily ;-)
> 
> On a more serious note, I'd like to design the bindings in a way that
> wouldn't require adding device-specific code in the driver for each panel
> model, given that in most cases power supply handling will be generic.
> What's your opinion about a generic power supply model that would be used
> in the default case, with the option to override it with device-specific
> code when needed ?
>
> >>>> +- data-mirror: if set, reverse the bit order on all data lanes (6 to
> >>>> 0 instead
> >>>> +  of 0 to 6)
> > 
> > On this one, make the name describe the order. "mirror" requires that
> > I know what is normal ordering. Perhaps "data-msb-first".
> 
> Sounds good to me, I'll use that.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Laurent Pinchart]

Hi Rob,

Ping ?

On Monday 17 Oct 2016 15:42:56 Laurent Pinchart wrote:
> On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
> > On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
> >> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> >>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> >>>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>> Multiple incompatible data link layers have been used over time to
> >>>> transmit image data to LVDS panels. This binding supports display
> >>>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> >>>> specifications.
> >>>> 
> >>>> Signed-off-by: Laurent Pinchart
> >>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>> ---
> >>>> 
> >>>>  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++
> >>>>  1 file changed, 119 insertions(+)
> >>>>  create mode 100644
> >>>>  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
> >>>> 
> >>>> diff --git
> >>>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> new file mode 100644
> >>>> index 000000000000..250861f2673e
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> @@ -0,0 +1,119 @@
> >>>> +Generic LVDS Panel
> >>>> +==================
> >>>> +
> >>>> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>> Multiple
> >>>> +incompatible data link layers have been used over time to transmit
> >>>> image data
> >>>> +to LVDS panels. This bindings supports display panels compatible with
> >>>> the
> >>>> +following specifications.
> >>>> +
> >>>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> >>>> February
> >>>> +1999 (Version 1.0), Japan Electronic Industry Development Association
> >>>> (JEIDA)
> >>>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95),
> >>>> National
> >>>> +Semiconductor
> >>>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
> >>>> Video
> >>>> +Electronics Standards Association (VESA)
> >>>> +
> >>>> +Device compatible with those specifications have been marketed under
> >>>> the
> >>>> +FPD-Link and FlatLink brands.
> >>>> +
> >>>> +
> >>>> +Required properties:
> >>>> +- compatible: shall contain "panel-lvds"
> >>> 
> >>> Maybe as a fallback, but on its own, no way.
> >> 
> >> Which brings an interesting question: when designing generic DT
> >> bindings, what's the rule regarding
> 
> Looks like I forgot part of the question. I meant to ask what is the rule
> regarding usage of more precise compatible strings ?
> 
> For instance (but perhaps not the best example), the
> input/rotary-encoder.txt bindings define a "rotary-encoder" compatible
> string, with no other bindings defining more precise compatible strings for
> the exact rotary encoder model. When it comes to panels I believe it makes
> sense to define model-specific compatible strings even if they're unused by
> drivers. I'm however wondering what the rule is there, and where those
> device-specific compatible strings should be defined. I'd like to avoid
> using one file per panel model as done today for the simple-panel bindings.
> 
> > Call it "simple" so I can easily NAK it. :)
> > 
> > Define a generic structure, not a single binding trying to serve all.
> > 
> >>> > +- width-mm: panel display width in millimeters
> >>> > +- height-mm: panel display height in millimeters
> >>> 
> >>> This is already documented for all panels IIRC.
> >> 
> >> Note that this DT binding has nothing to do with the simple-panel
> >> binding. It is instead similar to the panel-dpi and panel-dsi-cm bindings
> >> (which currently don't but should specify the panel size in DT). The LVDS
> >> panel driver will *not* include any panel-specific information such as
> >> size or timings, these are specified in DT.
> > 
> > The panel bindings aren't really different. The biggest difference was
> > location in the tree, but we now generally allow panels to be either a
> > child of the LCD controller or connected with OF graph. We probably
> > need to work on restructuring the panel bindings a bit. We should have
> > an inheritance with a base panel binding of things like size, label,
> > graph, backlight, etc, then perhaps an interface specific bindings for
> > LVDS, DSI, and parallel, then a panel specific binding. With this the
> > panel specific binding is typically just a compatible string and which
> > inherited properties apply to it.
> 
> That sounds good to me, but we have multiple models for panel bindings.
> 
> As you mentioned panels can be referenced through an LCD controller node
> property containing a phandle to the panel node, or through OF graph. That's
> a situation we have today, and we need to keep supporting both (at least
> for existing panels, perhaps not for the new ones).
> 
> Another difference is how to express panel data such as size and timings.
> The simple-panel DT bindings don't contain such data and expects the
> drivers to contain a table of panel data for all models supported, while
> the DPI, DSI and now the proposed LVDS panel bindings contain properties
> for panel data.
> 
> How would you like to reconcile all that ?
> 
> >>>> +- data-mapping: the color signals mapping order, "jeida-18",
> >>>> "jeida-24"
> >>>> +  or "vesa-24"
> >>> 
> >>> Maybe this should be part of the compatible.
> >> 
> >> I've thought about it, but given that some panels support selecting
> >> between multiple modes (through a mode pin that is usually hardwired), I
> >> believe a separate DT property makes sense.
> > 
> > Okay.
> > 
> >> Furthermore, LVDS data organization is controlled by the combination of
> >> both data-mapping and data-mirror. It makes little sense from my point
> >> of view to handle one as part of the compatible string and the other one
> >> as a separate property.
> >> 
> >>> > +Optional properties:
> >>> > +- label: a symbolic name for the panel
> >>> 
> >>> Could be for any panel or display connector.
> >> 
> >> Yes, but I'm not sure to understand how that's relevant :-)
> > 
> > Meaning it should be a common property.
> 
> Sure. So you expect me to reorganize all the panels and connectors DT
> bindings in order to get this one merged ? :-)
> 
> >>>> +- avdd-supply: reference to the regulator that powers the panel
> >>>> analog supply
> >>>> +- dvdd-supply: reference to the regulator that powers the panel
> >>>> digital supply
> >>> 
> >>> Which one has to be powered on first, what voltage, and with what time
> >>> in between? This is why "generic" or "simple" bindings don't work.
> >> 
> >> The above-mentioned specifications also define connectors, pinouts and
> >> power supplies, but many LVDS panels compatible with the LVDS physical
> >> and data layers use a different connector with small differences in
> >> power
> >> supplies.
> >> 
> >> I believe the voltage is irrelevant here, it doesn't need to be
> >> controlled by the operating system. Power supplies order and timing is
> >> relevant, I'll investigate the level of differences between panels. I'm
> >> also fine with dropping those properties for now.
> > 
> > Whether you have control of the supplies is dependent on the board.
> > Dropping them is just puts us in the simple binding trap. The simple
> > bindings start out that way and then people keep adding to them.
> 
> Damn, you can't be fooled easily ;-)
> 
> On a more serious note, I'd like to design the bindings in a way that
> wouldn't require adding device-specific code in the driver for each panel
> model, given that in most cases power supply handling will be generic.
> What's your opinion about a generic power supply model that would be used
> in the default case, with the option to override it with device-specific
> code when needed ?
>
> >>>> +- data-mirror: if set, reverse the bit order on all data lanes (6 to
> >>>> 0 instead
> >>>> +  of 0 to 6)
> > 
> > On this one, make the name describe the order. "mirror" requires that
> > I know what is normal ordering. Perhaps "data-msb-first".
> 
> Sounds good to me, I'll use that.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Rob Herring]

On Mon, Oct 17, 2016 at 7:42 AM, Laurent Pinchart
<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> Hi Rob,
>
> On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
>> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
>> > On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
>> >> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
>> >>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> >>> Multiple incompatible data link layers have been used over time to
>> >>> transmit image data to LVDS panels. This binding supports display
>> >>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
>> >>> specifications.
>> >>>
>> >>> Signed-off-by: Laurent Pinchart
>> >>> <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>> >>> ---
>> >>>
>> >>>  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++++
>> >>>  1 file changed, 119 insertions(+)
>> >>>  create mode 100644
>> >>>  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
>> >>>
>> >>> diff --git
>> >>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> >>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> >>> new file mode 100644
>> >>> index 000000000000..250861f2673e
>> >>> --- /dev/null
>> >>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> >>> @@ -0,0 +1,119 @@
>> >>> +Generic LVDS Panel
>> >>> +==================
>> >>> +
>> >>> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> >>> Multiple
>> >>> +incompatible data link layers have been used over time to transmit
>> >>> image data
>> >>> +to LVDS panels. This bindings supports display panels compatible with
>> >>> the
>> >>> +following specifications.
>> >>> +
>> >>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
>> >>> February
>> >>> +1999 (Version 1.0), Japan Electronic Industry Development Association
>> >>> (JEIDA)
>> >>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
>> >>> +Semiconductor
>> >>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
>> >>> Video
>> >>> +Electronics Standards Association (VESA)
>> >>> +
>> >>> +Device compatible with those specifications have been marketed under
>> >>> the
>> >>> +FPD-Link and FlatLink brands.
>> >>> +
>> >>> +
>> >>> +Required properties:
>> >>> +- compatible: shall contain "panel-lvds"
>> >>
>> >> Maybe as a fallback, but on its own, no way.
>> >
>> > Which brings an interesting question: when designing generic DT bindings,
>> > what's the rule regarding
>
> Looks like I forgot part of the question. I meant to ask what is the rule
> regarding usage of more precise compatible strings ?

When in doubt, always have one. If there's any chance at all that s/w
will need to know or care, then we should have one.

> For instance (but perhaps not the best example), the input/rotary-encoder.txt
> bindings define a "rotary-encoder" compatible string, with no other bindings
> defining more precise compatible strings for the exact rotary encoder model.
> When it comes to panels I believe it makes sense to define model-specific
> compatible strings even if they're unused by drivers. I'm however wondering
> what the rule is there, and where those device-specific compatible strings
> should be defined. I'd like to avoid using one file per panel model as done
> today for the simple-panel bindings.

There's a few exceptions like this where there is not any sort of
model to base a compatible on. For example, a GPIO connected LED is
truly generic. The only way to have a more specific compatible would
be something with the board name in it.

Your case here is in the middle. It seems like it's generic and
passive, but perhaps power control is not. Rather than trying to
decide, we can just cover our ass and put both a generic and specific
compatible in.

>> Call it "simple" so I can easily NAK it. :)
>>
>> Define a generic structure, not a single binding trying to serve all.
>>
>> >> > +- width-mm: panel display width in millimeters
>> >> > +- height-mm: panel display height in millimeters
>> >>
>> >> This is already documented for all panels IIRC.
>> >
>> > Note that this DT binding has nothing to do with the simple-panel binding.
>> > It is instead similar to the panel-dpi and panel-dsi-cm bindings (which
>> > currently don't but should specify the panel size in DT). The LVDS panel
>> > driver will *not* include any panel-specific information such as size or
>> > timings, these are specified in DT.
>>
>> The panel bindings aren't really different. The biggest difference was
>> location in the tree, but we now generally allow panels to be either a
>> child of the LCD controller or connected with OF graph. We probably
>> need to work on restructuring the panel bindings a bit. We should have
>> an inheritance with a base panel binding of things like size, label,
>> graph, backlight, etc, then perhaps an interface specific bindings for
>> LVDS, DSI, and parallel, then a panel specific binding. With this the
>> panel specific binding is typically just a compatible string and which
>> inherited properties apply to it.
>
> That sounds good to me, but we have multiple models for panel bindings.
>
> As you mentioned panels can be referenced through an LCD controller node
> property containing a phandle to the panel node, or through OF graph. That's a
> situation we have today, and we need to keep supporting both (at least for
> existing panels, perhaps not for the new ones).
>
> Another difference is how to express panel data such as size and timings. The
> simple-panel DT bindings don't contain such data and expects the drivers to
> contain a table of panel data for all models supported, while the DPI, DSI and
> now the proposed LVDS panel bindings contain properties for panel data.
>
> How would you like to reconcile all that ?

Thierry has outlined the position[1] that simple-panel follows many
times and I generally agree. I could be convinced that perhaps panel
timings could go into DT. However, you can't really describe
*everything*, so we're not going to get away from panel specific
compatible strings and panel specifics in the kernel.

>> >>> +- data-mapping: the color signals mapping order, "jeida-18",
>> >>> "jeida-24"
>> >>> +  or "vesa-24"
>> >>
>> >> Maybe this should be part of the compatible.
>> >
>> > I've thought about it, but given that some panels support selecting
>> > between multiple modes (through a mode pin that is usually hardwired), I
>> > believe a separate DT property makes sense.
>>
>> Okay.
>>
>> > Furthermore, LVDS data organization is controlled by the combination of
>> > both data-mapping and data-mirror. It makes little sense from my point of
>> > view to handle one as part of the compatible string and the other one as
>> > a separate property.
>> >
>> >> > +Optional properties:
>> >> > +- label: a symbolic name for the panel
>> >>
>> >> Could be for any panel or display connector.
>> >
>> > Yes, but I'm not sure to understand how that's relevant :-)
>>
>> Meaning it should be a common property.
>
> Sure. So you expect me to reorganize all the panels and connectors DT bindings
> in order to get this one merged ? :-)

No, because I don't think label is widely defined. Just put it in a
common place and reference it. Any other panels can be fixed later.
Really, the "simple panel" binding should probably morph into the
common binding.

>> >>> +- avdd-supply: reference to the regulator that powers the panel
>> >>> analog supply
>> >>> +- dvdd-supply: reference to the regulator that powers the panel
>> >>> digital supply
>> >>
>> >> Which one has to be powered on first, what voltage, and with what time
>> >> in between? This is why "generic" or "simple" bindings don't work.
>> >
>> > The above-mentioned specifications also define connectors, pinouts and
>> > power supplies, but many LVDS panels compatible with the LVDS physical
>> > and data layers use a different connector with small differences in power
>> > supplies.
>> >
>> > I believe the voltage is irrelevant here, it doesn't need to be controlled
>> > by the operating system. Power supplies order and timing is relevant,
>> > I'll investigate the level of differences between panels. I'm also fine
>> > with dropping those properties for now.
>>
>> Whether you have control of the supplies is dependent on the board.
>> Dropping them is just puts us in the simple binding trap. The simple
>> bindings start out that way and then people keep adding to them.
>
> Damn, you can't be fooled easily ;-)

I guess you can count all the simple bindings to see how many times I
can be fooled. :)

> On a more serious note, I'd like to design the bindings in a way that wouldn't
> require adding device-specific code in the driver for each panel model, given
> that in most cases power supply handling will be generic. What's your opinion
> about a generic power supply model that would be used in the default case,
> with the option to override it with device-specific code when needed ?

I don't agree. Read Thierry's post on the subject[1].

Rob

[1] http://sietch-tagr.blogspot.com/2016/04/display-panels-are-not-special.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Rob Herring]

On Mon, Oct 17, 2016 at 7:42 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
>> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
>> > On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
>> >> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
>> >>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> >>> Multiple incompatible data link layers have been used over time to
>> >>> transmit image data to LVDS panels. This binding supports display
>> >>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
>> >>> specifications.
>> >>>
>> >>> Signed-off-by: Laurent Pinchart
>> >>> <laurent.pinchart+renesas@ideasonboard.com>
>> >>> ---
>> >>>
>> >>>  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++++
>> >>>  1 file changed, 119 insertions(+)
>> >>>  create mode 100644
>> >>>  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
>> >>>
>> >>> diff --git
>> >>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> >>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> >>> new file mode 100644
>> >>> index 000000000000..250861f2673e
>> >>> --- /dev/null
>> >>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> >>> @@ -0,0 +1,119 @@
>> >>> +Generic LVDS Panel
>> >>> +==================
>> >>> +
>> >>> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> >>> Multiple
>> >>> +incompatible data link layers have been used over time to transmit
>> >>> image data
>> >>> +to LVDS panels. This bindings supports display panels compatible with
>> >>> the
>> >>> +following specifications.
>> >>> +
>> >>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
>> >>> February
>> >>> +1999 (Version 1.0), Japan Electronic Industry Development Association
>> >>> (JEIDA)
>> >>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
>> >>> +Semiconductor
>> >>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
>> >>> Video
>> >>> +Electronics Standards Association (VESA)
>> >>> +
>> >>> +Device compatible with those specifications have been marketed under
>> >>> the
>> >>> +FPD-Link and FlatLink brands.
>> >>> +
>> >>> +
>> >>> +Required properties:
>> >>> +- compatible: shall contain "panel-lvds"
>> >>
>> >> Maybe as a fallback, but on its own, no way.
>> >
>> > Which brings an interesting question: when designing generic DT bindings,
>> > what's the rule regarding
>
> Looks like I forgot part of the question. I meant to ask what is the rule
> regarding usage of more precise compatible strings ?

When in doubt, always have one. If there's any chance at all that s/w
will need to know or care, then we should have one.

> For instance (but perhaps not the best example), the input/rotary-encoder.txt
> bindings define a "rotary-encoder" compatible string, with no other bindings
> defining more precise compatible strings for the exact rotary encoder model.
> When it comes to panels I believe it makes sense to define model-specific
> compatible strings even if they're unused by drivers. I'm however wondering
> what the rule is there, and where those device-specific compatible strings
> should be defined. I'd like to avoid using one file per panel model as done
> today for the simple-panel bindings.

There's a few exceptions like this where there is not any sort of
model to base a compatible on. For example, a GPIO connected LED is
truly generic. The only way to have a more specific compatible would
be something with the board name in it.

Your case here is in the middle. It seems like it's generic and
passive, but perhaps power control is not. Rather than trying to
decide, we can just cover our ass and put both a generic and specific
compatible in.

>> Call it "simple" so I can easily NAK it. :)
>>
>> Define a generic structure, not a single binding trying to serve all.
>>
>> >> > +- width-mm: panel display width in millimeters
>> >> > +- height-mm: panel display height in millimeters
>> >>
>> >> This is already documented for all panels IIRC.
>> >
>> > Note that this DT binding has nothing to do with the simple-panel binding.
>> > It is instead similar to the panel-dpi and panel-dsi-cm bindings (which
>> > currently don't but should specify the panel size in DT). The LVDS panel
>> > driver will *not* include any panel-specific information such as size or
>> > timings, these are specified in DT.
>>
>> The panel bindings aren't really different. The biggest difference was
>> location in the tree, but we now generally allow panels to be either a
>> child of the LCD controller or connected with OF graph. We probably
>> need to work on restructuring the panel bindings a bit. We should have
>> an inheritance with a base panel binding of things like size, label,
>> graph, backlight, etc, then perhaps an interface specific bindings for
>> LVDS, DSI, and parallel, then a panel specific binding. With this the
>> panel specific binding is typically just a compatible string and which
>> inherited properties apply to it.
>
> That sounds good to me, but we have multiple models for panel bindings.
>
> As you mentioned panels can be referenced through an LCD controller node
> property containing a phandle to the panel node, or through OF graph. That's a
> situation we have today, and we need to keep supporting both (at least for
> existing panels, perhaps not for the new ones).
>
> Another difference is how to express panel data such as size and timings. The
> simple-panel DT bindings don't contain such data and expects the drivers to
> contain a table of panel data for all models supported, while the DPI, DSI and
> now the proposed LVDS panel bindings contain properties for panel data.
>
> How would you like to reconcile all that ?

Thierry has outlined the position[1] that simple-panel follows many
times and I generally agree. I could be convinced that perhaps panel
timings could go into DT. However, you can't really describe
*everything*, so we're not going to get away from panel specific
compatible strings and panel specifics in the kernel.

>> >>> +- data-mapping: the color signals mapping order, "jeida-18",
>> >>> "jeida-24"
>> >>> +  or "vesa-24"
>> >>
>> >> Maybe this should be part of the compatible.
>> >
>> > I've thought about it, but given that some panels support selecting
>> > between multiple modes (through a mode pin that is usually hardwired), I
>> > believe a separate DT property makes sense.
>>
>> Okay.
>>
>> > Furthermore, LVDS data organization is controlled by the combination of
>> > both data-mapping and data-mirror. It makes little sense from my point of
>> > view to handle one as part of the compatible string and the other one as
>> > a separate property.
>> >
>> >> > +Optional properties:
>> >> > +- label: a symbolic name for the panel
>> >>
>> >> Could be for any panel or display connector.
>> >
>> > Yes, but I'm not sure to understand how that's relevant :-)
>>
>> Meaning it should be a common property.
>
> Sure. So you expect me to reorganize all the panels and connectors DT bindings
> in order to get this one merged ? :-)

No, because I don't think label is widely defined. Just put it in a
common place and reference it. Any other panels can be fixed later.
Really, the "simple panel" binding should probably morph into the
common binding.

>> >>> +- avdd-supply: reference to the regulator that powers the panel
>> >>> analog supply
>> >>> +- dvdd-supply: reference to the regulator that powers the panel
>> >>> digital supply
>> >>
>> >> Which one has to be powered on first, what voltage, and with what time
>> >> in between? This is why "generic" or "simple" bindings don't work.
>> >
>> > The above-mentioned specifications also define connectors, pinouts and
>> > power supplies, but many LVDS panels compatible with the LVDS physical
>> > and data layers use a different connector with small differences in power
>> > supplies.
>> >
>> > I believe the voltage is irrelevant here, it doesn't need to be controlled
>> > by the operating system. Power supplies order and timing is relevant,
>> > I'll investigate the level of differences between panels. I'm also fine
>> > with dropping those properties for now.
>>
>> Whether you have control of the supplies is dependent on the board.
>> Dropping them is just puts us in the simple binding trap. The simple
>> bindings start out that way and then people keep adding to them.
>
> Damn, you can't be fooled easily ;-)

I guess you can count all the simple bindings to see how many times I
can be fooled. :)

> On a more serious note, I'd like to design the bindings in a way that wouldn't
> require adding device-specific code in the driver for each panel model, given
> that in most cases power supply handling will be generic. What's your opinion
> about a generic power supply model that would be used in the default case,
> with the option to override it with device-specific code when needed ?

I don't agree. Read Thierry's post on the subject[1].

Rob

[1] http://sietch-tagr.blogspot.com/2016/04/display-panels-are-not-special.html

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Laurent Pinchart]

Hi Rob,

On Monday 14 Nov 2016 19:40:26 Rob Herring wrote:
> On Mon, Oct 17, 2016 at 7:42 AM, Laurent Pinchart wrote:
> > On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
> >> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
> >>> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> >>>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> >>>>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>>> Multiple incompatible data link layers have been used over time to
> >>>>> transmit image data to LVDS panels. This binding supports display
> >>>>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> >>>>> specifications.
> >>>>> 
> >>>>> Signed-off-by: Laurent Pinchart
> >>>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>>> ---
> >>>>> 
> >>>>>  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++
> >>>>>  1 file changed, 119 insertions(+)
> >>>>>  create mode 100644
> >>>>>  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
> >>>>> 
> >>>>> diff --git
> >>>>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>>> new file mode 100644
> >>>>> index 000000000000..250861f2673e
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>>> @@ -0,0 +1,119 @@
> >>>>> +Generic LVDS Panel
> >>>>> +==================
> >>>>> +
> >>>>> +LVDS is a physical layer specification defined in
> >>>>> ANSI/TIA/EIA-644-A. Multiple
> >>>>> +incompatible data link layers have been used over time to transmit
> >>>>> image data
> >>>>> +to LVDS panels. This bindings supports display panels compatible
> >>>>> with the
> >>>>> +following specifications.
> >>>>> +
> >>>>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> >>>>> February
> >>>>> +1999 (Version 1.0), Japan Electronic Industry Development
> >>>>> Association (JEIDA)
> >>>>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95),
> >>>>> National
> >>>>> +Semiconductor
> >>>>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
> >>>>> Video
> >>>>> +Electronics Standards Association (VESA)
> >>>>> +
> >>>>> +Device compatible with those specifications have been marketed under
> >>>>> the
> >>>>> +FPD-Link and FlatLink brands.
> >>>>> +
> >>>>> +
> >>>>> +Required properties:
> >>>>> +- compatible: shall contain "panel-lvds"
> >>>> 
> >>>> Maybe as a fallback, but on its own, no way.
> >>> 
> >>> Which brings an interesting question: when designing generic DT
> >>> bindings, what's the rule regarding
> > 
> > Looks like I forgot part of the question. I meant to ask what is the rule
> > regarding usage of more precise compatible strings ?
> 
> When in doubt, always have one. If there's any chance at all that s/w
> will need to know or care, then we should have one.
> 
> > For instance (but perhaps not the best example), the
> > input/rotary-encoder.txt bindings define a "rotary-encoder" compatible
> > string, with no other bindings defining more precise compatible strings
> > for the exact rotary encoder model. When it comes to panels I believe it
> > makes sense to define model-specific compatible strings even if they're
> > unused by drivers. I'm however wondering what the rule is there, and
> > where those device-specific compatible strings should be defined. I'd
> > like to avoid using one file per panel model as done today for the
> > simple-panel bindings.
> 
> There's a few exceptions like this where there is not any sort of
> model to base a compatible on. For example, a GPIO connected LED is
> truly generic. The only way to have a more specific compatible would
> be something with the board name in it.
> 
> Your case here is in the middle. It seems like it's generic and
> passive, but perhaps power control is not. Rather than trying to
> decide, we can just cover our ass and put both a generic and specific
> compatible in.

That sounds good to me. I'll mention in the document that a more precise 
compatible is required.

> >> Call it "simple" so I can easily NAK it. :)
> >> 
> >> Define a generic structure, not a single binding trying to serve all.
> >> 
> >>>>> +- width-mm: panel display width in millimeters
> >>>>> +- height-mm: panel display height in millimeters
> >>>> 
> >>>> This is already documented for all panels IIRC.
> >>> 
> >>> Note that this DT binding has nothing to do with the simple-panel
> >>> binding. It is instead similar to the panel-dpi and panel-dsi-cm
> >>> bindings (which currently don't but should specify the panel size in
> >>> DT). The LVDS panel driver will *not* include any panel-specific
> >>> information such as size or timings, these are specified in DT.
> >> 
> >> The panel bindings aren't really different. The biggest difference was
> >> location in the tree, but we now generally allow panels to be either a
> >> child of the LCD controller or connected with OF graph. We probably
> >> need to work on restructuring the panel bindings a bit. We should have
> >> an inheritance with a base panel binding of things like size, label,
> >> graph, backlight, etc, then perhaps an interface specific bindings for
> >> LVDS, DSI, and parallel, then a panel specific binding. With this the
> >> panel specific binding is typically just a compatible string and which
> >> inherited properties apply to it.
> > 
> > That sounds good to me, but we have multiple models for panel bindings.
> > 
> > As you mentioned panels can be referenced through an LCD controller node
> > property containing a phandle to the panel node, or through OF graph.
> > That's a situation we have today, and we need to keep supporting both (at
> > least for existing panels, perhaps not for the new ones).
> > 
> > Another difference is how to express panel data such as size and timings.
> > The simple-panel DT bindings don't contain such data and expects the
> > drivers to contain a table of panel data for all models supported, while
> > the DPI, DSI and now the proposed LVDS panel bindings contain properties
> > for panel data.
> > 
> > How would you like to reconcile all that ?
> 
> Thierry has outlined the position[1] that simple-panel follows many
> times and I generally agree. I could be convinced that perhaps panel
> timings could go into DT. However, you can't really describe
> *everything*, so we're not going to get away from panel specific
> compatible strings and panel specifics in the kernel.

I'm fine with that middle ground, and will include panel timings in DT but not 
an information related to power sequencing or other panel-specific properties 
(beside the standardized properties defined in this binding of course).

> >>>>> +- data-mapping: the color signals mapping order, "jeida-18",
> >>>>> "jeida-24"
> >>>>> +  or "vesa-24"
> >>>> 
> >>>> Maybe this should be part of the compatible.
> >>> 
> >>> I've thought about it, but given that some panels support selecting
> >>> between multiple modes (through a mode pin that is usually hardwired),
> >>> I believe a separate DT property makes sense.
> >> 
> >> Okay.
> >> 
> >>> Furthermore, LVDS data organization is controlled by the combination of
> >>> both data-mapping and data-mirror. It makes little sense from my point
> >>> of view to handle one as part of the compatible string and the other one
> >>> as a separate property.
> >>> 
> >>>> > +Optional properties:
> >>>> > +- label: a symbolic name for the panel
> >>>> 
> >>>> Could be for any panel or display connector.
> >>> 
> >>> Yes, but I'm not sure to understand how that's relevant :-)
> >> 
> >> Meaning it should be a common property.
> > 
> > Sure. So you expect me to reorganize all the panels and connectors DT
> > bindings in order to get this one merged ? :-)
> 
> No, because I don't think label is widely defined. Just put it in a
> common place and reference it. Any other panels can be fixed later.
> Really, the "simple panel" binding should probably morph into the
> common binding.

The "label" property is actually defined in the "Devicetree Specification, 
Release 0.1" as recently published on devicetree.org. Where would you like me 
to define it in the bindings ?

> >>>>> +- avdd-supply: reference to the regulator that powers the panel
> >>>>> analog supply
> >>>>> +- dvdd-supply: reference to the regulator that powers the panel
> >>>>> digital supply
> >>>> 
> >>>> Which one has to be powered on first, what voltage, and with what time
> >>>> in between? This is why "generic" or "simple" bindings don't work.
> >>> 
> >>> The above-mentioned specifications also define connectors, pinouts and
> >>> power supplies, but many LVDS panels compatible with the LVDS physical
> >>> and data layers use a different connector with small differences in
> >>> power supplies.
> >>> 
> >>> I believe the voltage is irrelevant here, it doesn't need to be
> >>> controlled by the operating system. Power supplies order and timing is
> >>> relevant, I'll investigate the level of differences between panels. I'm
> >>> also fine with dropping those properties for now.
> >> 
> >> Whether you have control of the supplies is dependent on the board.
> >> Dropping them is just puts us in the simple binding trap. The simple
> >> bindings start out that way and then people keep adding to them.
> > 
> > Damn, you can't be fooled easily ;-)
> 
> I guess you can count all the simple bindings to see how many times I
> can be fooled. :)
> 
> > On a more serious note, I'd like to design the bindings in a way that
> > wouldn't require adding device-specific code in the driver for each panel
> > model, given that in most cases power supply handling will be generic.
> > What's your opinion about a generic power supply model that would be used
> > in the default case, with the option to override it with device-specific
> > code when needed ?
>
> I don't agree. Read Thierry's post on the subject[1].

I'm not sure you understood me correctly (and writing a clarification at 
4:00am might not help :-)). My point here is that a fair number of panels 
don't care about power sequencing and have no control GPIOs (that's certainly 
the case of the two Mitsubishi panels I use here that have a single power 
supply - good luck trying to convince me that this needs to be sequenced :-) - 
and no enable or reset control pin). This kind of panel should obviously be 
modelled in DT with both a specific and a generic ("panel-lvds") compatible 
string, to ensure that we'll have enough information available on the 
operating system side to control the panel properly. The power supply(ies) 
should be documented in relation to the specific compatible string and not 
mentioned by the generic bindings (whether that should go in one or multiple 
text files is bikeshedding).

This being said, on the driver side, I don't see a reason to list the specific 
compatible strings explicitly for those panels when a generic implementation 
matching the generic compatible string can handle them fine. This has no 
impact on the bindings and is an Linux implementation decision.

> [1]
> http://sietch-tagr.blogspot.com/2016/04/display-panels-are-not-special.html

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Laurent Pinchart]

Hi Rob,

On Monday 14 Nov 2016 19:40:26 Rob Herring wrote:
> On Mon, Oct 17, 2016 at 7:42 AM, Laurent Pinchart wrote:
> > On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
> >> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
> >>> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> >>>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> >>>>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>>> Multiple incompatible data link layers have been used over time to
> >>>>> transmit image data to LVDS panels. This binding supports display
> >>>>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> >>>>> specifications.
> >>>>> 
> >>>>> Signed-off-by: Laurent Pinchart
> >>>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>>> ---
> >>>>> 
> >>>>>  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++
> >>>>>  1 file changed, 119 insertions(+)
> >>>>>  create mode 100644
> >>>>>  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
> >>>>> 
> >>>>> diff --git
> >>>>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>>> new file mode 100644
> >>>>> index 000000000000..250861f2673e
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>>> @@ -0,0 +1,119 @@
> >>>>> +Generic LVDS Panel
> >>>>> +==================
> >>>>> +
> >>>>> +LVDS is a physical layer specification defined in
> >>>>> ANSI/TIA/EIA-644-A. Multiple
> >>>>> +incompatible data link layers have been used over time to transmit
> >>>>> image data
> >>>>> +to LVDS panels. This bindings supports display panels compatible
> >>>>> with the
> >>>>> +following specifications.
> >>>>> +
> >>>>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> >>>>> February
> >>>>> +1999 (Version 1.0), Japan Electronic Industry Development
> >>>>> Association (JEIDA)
> >>>>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95),
> >>>>> National
> >>>>> +Semiconductor
> >>>>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
> >>>>> Video
> >>>>> +Electronics Standards Association (VESA)
> >>>>> +
> >>>>> +Device compatible with those specifications have been marketed under
> >>>>> the
> >>>>> +FPD-Link and FlatLink brands.
> >>>>> +
> >>>>> +
> >>>>> +Required properties:
> >>>>> +- compatible: shall contain "panel-lvds"
> >>>> 
> >>>> Maybe as a fallback, but on its own, no way.
> >>> 
> >>> Which brings an interesting question: when designing generic DT
> >>> bindings, what's the rule regarding
> > 
> > Looks like I forgot part of the question. I meant to ask what is the rule
> > regarding usage of more precise compatible strings ?
> 
> When in doubt, always have one. If there's any chance at all that s/w
> will need to know or care, then we should have one.
> 
> > For instance (but perhaps not the best example), the
> > input/rotary-encoder.txt bindings define a "rotary-encoder" compatible
> > string, with no other bindings defining more precise compatible strings
> > for the exact rotary encoder model. When it comes to panels I believe it
> > makes sense to define model-specific compatible strings even if they're
> > unused by drivers. I'm however wondering what the rule is there, and
> > where those device-specific compatible strings should be defined. I'd
> > like to avoid using one file per panel model as done today for the
> > simple-panel bindings.
> 
> There's a few exceptions like this where there is not any sort of
> model to base a compatible on. For example, a GPIO connected LED is
> truly generic. The only way to have a more specific compatible would
> be something with the board name in it.
> 
> Your case here is in the middle. It seems like it's generic and
> passive, but perhaps power control is not. Rather than trying to
> decide, we can just cover our ass and put both a generic and specific
> compatible in.

That sounds good to me. I'll mention in the document that a more precise 
compatible is required.

> >> Call it "simple" so I can easily NAK it. :)
> >> 
> >> Define a generic structure, not a single binding trying to serve all.
> >> 
> >>>>> +- width-mm: panel display width in millimeters
> >>>>> +- height-mm: panel display height in millimeters
> >>>> 
> >>>> This is already documented for all panels IIRC.
> >>> 
> >>> Note that this DT binding has nothing to do with the simple-panel
> >>> binding. It is instead similar to the panel-dpi and panel-dsi-cm
> >>> bindings (which currently don't but should specify the panel size in
> >>> DT). The LVDS panel driver will *not* include any panel-specific
> >>> information such as size or timings, these are specified in DT.
> >> 
> >> The panel bindings aren't really different. The biggest difference was
> >> location in the tree, but we now generally allow panels to be either a
> >> child of the LCD controller or connected with OF graph. We probably
> >> need to work on restructuring the panel bindings a bit. We should have
> >> an inheritance with a base panel binding of things like size, label,
> >> graph, backlight, etc, then perhaps an interface specific bindings for
> >> LVDS, DSI, and parallel, then a panel specific binding. With this the
> >> panel specific binding is typically just a compatible string and which
> >> inherited properties apply to it.
> > 
> > That sounds good to me, but we have multiple models for panel bindings.
> > 
> > As you mentioned panels can be referenced through an LCD controller node
> > property containing a phandle to the panel node, or through OF graph.
> > That's a situation we have today, and we need to keep supporting both (at
> > least for existing panels, perhaps not for the new ones).
> > 
> > Another difference is how to express panel data such as size and timings.
> > The simple-panel DT bindings don't contain such data and expects the
> > drivers to contain a table of panel data for all models supported, while
> > the DPI, DSI and now the proposed LVDS panel bindings contain properties
> > for panel data.
> > 
> > How would you like to reconcile all that ?
> 
> Thierry has outlined the position[1] that simple-panel follows many
> times and I generally agree. I could be convinced that perhaps panel
> timings could go into DT. However, you can't really describe
> *everything*, so we're not going to get away from panel specific
> compatible strings and panel specifics in the kernel.

I'm fine with that middle ground, and will include panel timings in DT but not 
an information related to power sequencing or other panel-specific properties 
(beside the standardized properties defined in this binding of course).

> >>>>> +- data-mapping: the color signals mapping order, "jeida-18",
> >>>>> "jeida-24"
> >>>>> +  or "vesa-24"
> >>>> 
> >>>> Maybe this should be part of the compatible.
> >>> 
> >>> I've thought about it, but given that some panels support selecting
> >>> between multiple modes (through a mode pin that is usually hardwired),
> >>> I believe a separate DT property makes sense.
> >> 
> >> Okay.
> >> 
> >>> Furthermore, LVDS data organization is controlled by the combination of
> >>> both data-mapping and data-mirror. It makes little sense from my point
> >>> of view to handle one as part of the compatible string and the other one
> >>> as a separate property.
> >>> 
> >>>> > +Optional properties:
> >>>> > +- label: a symbolic name for the panel
> >>>> 
> >>>> Could be for any panel or display connector.
> >>> 
> >>> Yes, but I'm not sure to understand how that's relevant :-)
> >> 
> >> Meaning it should be a common property.
> > 
> > Sure. So you expect me to reorganize all the panels and connectors DT
> > bindings in order to get this one merged ? :-)
> 
> No, because I don't think label is widely defined. Just put it in a
> common place and reference it. Any other panels can be fixed later.
> Really, the "simple panel" binding should probably morph into the
> common binding.

The "label" property is actually defined in the "Devicetree Specification, 
Release 0.1" as recently published on devicetree.org. Where would you like me 
to define it in the bindings ?

> >>>>> +- avdd-supply: reference to the regulator that powers the panel
> >>>>> analog supply
> >>>>> +- dvdd-supply: reference to the regulator that powers the panel
> >>>>> digital supply
> >>>> 
> >>>> Which one has to be powered on first, what voltage, and with what time
> >>>> in between? This is why "generic" or "simple" bindings don't work.
> >>> 
> >>> The above-mentioned specifications also define connectors, pinouts and
> >>> power supplies, but many LVDS panels compatible with the LVDS physical
> >>> and data layers use a different connector with small differences in
> >>> power supplies.
> >>> 
> >>> I believe the voltage is irrelevant here, it doesn't need to be
> >>> controlled by the operating system. Power supplies order and timing is
> >>> relevant, I'll investigate the level of differences between panels. I'm
> >>> also fine with dropping those properties for now.
> >> 
> >> Whether you have control of the supplies is dependent on the board.
> >> Dropping them is just puts us in the simple binding trap. The simple
> >> bindings start out that way and then people keep adding to them.
> > 
> > Damn, you can't be fooled easily ;-)
> 
> I guess you can count all the simple bindings to see how many times I
> can be fooled. :)
> 
> > On a more serious note, I'd like to design the bindings in a way that
> > wouldn't require adding device-specific code in the driver for each panel
> > model, given that in most cases power supply handling will be generic.
> > What's your opinion about a generic power supply model that would be used
> > in the default case, with the option to override it with device-specific
> > code when needed ?
>
> I don't agree. Read Thierry's post on the subject[1].

I'm not sure you understood me correctly (and writing a clarification at 
4:00am might not help :-)). My point here is that a fair number of panels 
don't care about power sequencing and have no control GPIOs (that's certainly 
the case of the two Mitsubishi panels I use here that have a single power 
supply - good luck trying to convince me that this needs to be sequenced :-) - 
and no enable or reset control pin). This kind of panel should obviously be 
modelled in DT with both a specific and a generic ("panel-lvds") compatible 
string, to ensure that we'll have enough information available on the 
operating system side to control the panel properly. The power supply(ies) 
should be documented in relation to the specific compatible string and not 
mentioned by the generic bindings (whether that should go in one or multiple 
text files is bikeshedding).

This being said, on the driver side, I don't see a reason to list the specific 
compatible strings explicitly for those panels when a generic implementation 
matching the generic compatible string can handle them fine. This has no 
impact on the bindings and is an Linux implementation decision.

> [1]
> http://sietch-tagr.blogspot.com/2016/04/display-panels-are-not-special.html

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Rob Herring]

On Mon, Nov 14, 2016 at 8:11 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Monday 14 Nov 2016 19:40:26 Rob Herring wrote:
>> On Mon, Oct 17, 2016 at 7:42 AM, Laurent Pinchart wrote:
>> > On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
>> >> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
>> >>> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
>> >>>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
>> >>>>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> >>>>> Multiple incompatible data link layers have been used over time to
>> >>>>> transmit image data to LVDS panels. This binding supports display
>> >>>>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
>> >>>>> specifications.

[...]

>> >>> Furthermore, LVDS data organization is controlled by the combination of
>> >>> both data-mapping and data-mirror. It makes little sense from my point
>> >>> of view to handle one as part of the compatible string and the other one
>> >>> as a separate property.
>> >>>
>> >>>> > +Optional properties:
>> >>>> > +- label: a symbolic name for the panel
>> >>>>
>> >>>> Could be for any panel or display connector.
>> >>>
>> >>> Yes, but I'm not sure to understand how that's relevant :-)
>> >>
>> >> Meaning it should be a common property.
>> >
>> > Sure. So you expect me to reorganize all the panels and connectors DT
>> > bindings in order to get this one merged ? :-)
>>
>> No, because I don't think label is widely defined. Just put it in a
>> common place and reference it. Any other panels can be fixed later.
>> Really, the "simple panel" binding should probably morph into the
>> common binding.
>
> The "label" property is actually defined in the "Devicetree Specification,

Yes, so is reg, interrupts, etc. but we still list when those are used.

> Release 0.1" as recently published on devicetree.org. Where would you like me
> to define it in the bindings ?

Just split things into 2 files. Move everything that's used in other
panel bindings (label, width-mm, ports, etc.) to a panel/common.txt.
Then in lvds-panel.txt, it just refers to common.txt and is just the
LVDS specific things. Bonus points if simple-panel (should just go
away), panel-dpi, panel-dsi-cm are converted.

>> >>>>> +- avdd-supply: reference to the regulator that powers the panel
>> >>>>> analog supply
>> >>>>> +- dvdd-supply: reference to the regulator that powers the panel
>> >>>>> digital supply

I would not be against these being common either. It's somewhat better
than simple-panel's "power-supply" property.

>> >>>>
>> >>>> Which one has to be powered on first, what voltage, and with what time
>> >>>> in between? This is why "generic" or "simple" bindings don't work.
>> >>>
>> >>> The above-mentioned specifications also define connectors, pinouts and
>> >>> power supplies, but many LVDS panels compatible with the LVDS physical
>> >>> and data layers use a different connector with small differences in
>> >>> power supplies.
>> >>>
>> >>> I believe the voltage is irrelevant here, it doesn't need to be
>> >>> controlled by the operating system. Power supplies order and timing is
>> >>> relevant, I'll investigate the level of differences between panels. I'm
>> >>> also fine with dropping those properties for now.
>> >>
>> >> Whether you have control of the supplies is dependent on the board.
>> >> Dropping them is just puts us in the simple binding trap. The simple
>> >> bindings start out that way and then people keep adding to them.
>> >
>> > Damn, you can't be fooled easily ;-)
>>
>> I guess you can count all the simple bindings to see how many times I
>> can be fooled. :)
>>
>> > On a more serious note, I'd like to design the bindings in a way that
>> > wouldn't require adding device-specific code in the driver for each panel
>> > model, given that in most cases power supply handling will be generic.
>> > What's your opinion about a generic power supply model that would be used
>> > in the default case, with the option to override it with device-specific
>> > code when needed ?
>>
>> I don't agree. Read Thierry's post on the subject[1].
>
> I'm not sure you understood me correctly (and writing a clarification at
> 4:00am might not help :-)). My point here is that a fair number of panels
> don't care about power sequencing and have no control GPIOs (that's certainly
> the case of the two Mitsubishi panels I use here that have a single power
> supply - good luck trying to convince me that this needs to be sequenced :-) -
> and no enable or reset control pin). This kind of panel should obviously be
> modelled in DT with both a specific and a generic ("panel-lvds") compatible
> string, to ensure that we'll have enough information available on the
> operating system side to control the panel properly. The power supply(ies)
> should be documented in relation to the specific compatible string and not
> mentioned by the generic bindings (whether that should go in one or multiple
> text files is bikeshedding).

Let me rephrase, I don't agree with generic bindings, but I fully
support having a generic driver. So you can have a generic driver
function that can handle your case. Even slightly more complicated can
be handled: turn on all the supplies listed, de-assert any reset
gpios, assert any enable gpios (this is why I push for common naming
of "enable-gpios"). If this is enough for a panel, then the generic
compatible will work. If not, sorry, provide your own power sequencing
code and match on the more specific compatible.

> This being said, on the driver side, I don't see a reason to list the specific
> compatible strings explicitly for those panels when a generic implementation
> matching the generic compatible string can handle them fine. This has no
> impact on the bindings and is an Linux implementation decision.

I think we're in agreement.

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Rob Herring]

On Mon, Nov 14, 2016 at 8:11 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Monday 14 Nov 2016 19:40:26 Rob Herring wrote:
>> On Mon, Oct 17, 2016 at 7:42 AM, Laurent Pinchart wrote:
>> > On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
>> >> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
>> >>> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
>> >>>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
>> >>>>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> >>>>> Multiple incompatible data link layers have been used over time to
>> >>>>> transmit image data to LVDS panels. This binding supports display
>> >>>>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
>> >>>>> specifications.

[...]

>> >>> Furthermore, LVDS data organization is controlled by the combination of
>> >>> both data-mapping and data-mirror. It makes little sense from my point
>> >>> of view to handle one as part of the compatible string and the other one
>> >>> as a separate property.
>> >>>
>> >>>> > +Optional properties:
>> >>>> > +- label: a symbolic name for the panel
>> >>>>
>> >>>> Could be for any panel or display connector.
>> >>>
>> >>> Yes, but I'm not sure to understand how that's relevant :-)
>> >>
>> >> Meaning it should be a common property.
>> >
>> > Sure. So you expect me to reorganize all the panels and connectors DT
>> > bindings in order to get this one merged ? :-)
>>
>> No, because I don't think label is widely defined. Just put it in a
>> common place and reference it. Any other panels can be fixed later.
>> Really, the "simple panel" binding should probably morph into the
>> common binding.
>
> The "label" property is actually defined in the "Devicetree Specification,

Yes, so is reg, interrupts, etc. but we still list when those are used.

> Release 0.1" as recently published on devicetree.org. Where would you like me
> to define it in the bindings ?

Just split things into 2 files. Move everything that's used in other
panel bindings (label, width-mm, ports, etc.) to a panel/common.txt.
Then in lvds-panel.txt, it just refers to common.txt and is just the
LVDS specific things. Bonus points if simple-panel (should just go
away), panel-dpi, panel-dsi-cm are converted.

>> >>>>> +- avdd-supply: reference to the regulator that powers the panel
>> >>>>> analog supply
>> >>>>> +- dvdd-supply: reference to the regulator that powers the panel
>> >>>>> digital supply

I would not be against these being common either. It's somewhat better
than simple-panel's "power-supply" property.

>> >>>>
>> >>>> Which one has to be powered on first, what voltage, and with what time
>> >>>> in between? This is why "generic" or "simple" bindings don't work.
>> >>>
>> >>> The above-mentioned specifications also define connectors, pinouts and
>> >>> power supplies, but many LVDS panels compatible with the LVDS physical
>> >>> and data layers use a different connector with small differences in
>> >>> power supplies.
>> >>>
>> >>> I believe the voltage is irrelevant here, it doesn't need to be
>> >>> controlled by the operating system. Power supplies order and timing is
>> >>> relevant, I'll investigate the level of differences between panels. I'm
>> >>> also fine with dropping those properties for now.
>> >>
>> >> Whether you have control of the supplies is dependent on the board.
>> >> Dropping them is just puts us in the simple binding trap. The simple
>> >> bindings start out that way and then people keep adding to them.
>> >
>> > Damn, you can't be fooled easily ;-)
>>
>> I guess you can count all the simple bindings to see how many times I
>> can be fooled. :)
>>
>> > On a more serious note, I'd like to design the bindings in a way that
>> > wouldn't require adding device-specific code in the driver for each panel
>> > model, given that in most cases power supply handling will be generic.
>> > What's your opinion about a generic power supply model that would be used
>> > in the default case, with the option to override it with device-specific
>> > code when needed ?
>>
>> I don't agree. Read Thierry's post on the subject[1].
>
> I'm not sure you understood me correctly (and writing a clarification at
> 4:00am might not help :-)). My point here is that a fair number of panels
> don't care about power sequencing and have no control GPIOs (that's certainly
> the case of the two Mitsubishi panels I use here that have a single power
> supply - good luck trying to convince me that this needs to be sequenced :-) -
> and no enable or reset control pin). This kind of panel should obviously be
> modelled in DT with both a specific and a generic ("panel-lvds") compatible
> string, to ensure that we'll have enough information available on the
> operating system side to control the panel properly. The power supply(ies)
> should be documented in relation to the specific compatible string and not
> mentioned by the generic bindings (whether that should go in one or multiple
> text files is bikeshedding).

Let me rephrase, I don't agree with generic bindings, but I fully
support having a generic driver. So you can have a generic driver
function that can handle your case. Even slightly more complicated can
be handled: turn on all the supplies listed, de-assert any reset
gpios, assert any enable gpios (this is why I push for common naming
of "enable-gpios"). If this is enough for a panel, then the generic
compatible will work. If not, sorry, provide your own power sequencing
code and match on the more specific compatible.

> This being said, on the driver side, I don't see a reason to list the specific
> compatible strings explicitly for those panels when a generic implementation
> matching the generic compatible string can handle them fine. This has no
> impact on the bindings and is an Linux implementation decision.

I think we're in agreement.

Rob

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Laurent Pinchart]

Hi Rob,

On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchartwrote:
> > On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> >> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> >>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>> Multiple incompatible data link layers have been used over time to
> >>> transmit image data to LVDS panels. This binding supports display
> >>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> >>> specifications.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> <laurent.pinchart+renesas@ideasonboard.com>
> >>> ---
> >>> 
> >>>  .../bindings/display/panel/panel-lvds.txt          | 119 +++++++++++++
> >>>  1 file changed, 119 insertions(+)
> >>>  create mode 100644
> >>>  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
> >>> 
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt new
> >>> file
> >>> mode 100644
> >>> index 000000000000..250861f2673e
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>> @@ -0,0 +1,119 @@
> >>> +Generic LVDS Panel
> >>> +==================
> >>> +
> >>> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>> Multiple
> >>> +incompatible data link layers have been used over time to transmit
> >>> image data
> >>> +to LVDS panels. This bindings supports display panels compatible with
> >>> the
> >>> +following specifications.
> >>> +
> >>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> >>> February
> >>> +1999 (Version 1.0), Japan Electronic Industry Development Association
> >>> (JEIDA)
> >>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
> >>> +Semiconductor
> >>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
> >>> Video
> >>> +Electronics Standards Association (VESA)
> >>> +
> >>> +Device compatible with those specifications have been marketed under
> >>> the
> >>> +FPD-Link and FlatLink brands.
> >>> +
> >>> +
> >>> +Required properties:
> >>> +- compatible: shall contain "panel-lvds"
> >> 
> >> Maybe as a fallback, but on its own, no way.
> > 
> > Which brings an interesting question: when designing generic DT bindings,
> > what's the rule regarding
> 
> Call it "simple" so I can easily NAK it. :)
> 
> Define a generic structure, not a single binding trying to serve all.
> 
> >>> +- width-mm: panel display width in millimeters
> >>> +- height-mm: panel display height in millimeters
> >> 
> >> This is already documented for all panels IIRC.
> > 
> > Note that this DT binding has nothing to do with the simple-panel binding.
> > It is instead similar to the panel-dpi and panel-dsi-cm bindings (which
> > currently don't but should specify the panel size in DT). The LVDS panel
> > driver will *not* include any panel-specific information such as size or
> > timings, these are specified in DT.
> 
> The panel bindings aren't really different. The biggest difference was
> location in the tree, but we now generally allow panels to be either a
> child of the LCD controller or connected with OF graph. We probably
> need to work on restructuring the panel bindings a bit. We should have
> an inheritance with a base panel binding of things like size, label,
> graph, backlight, etc, then perhaps an interface specific bindings for
> LVDS, DSI, and parallel, then a panel specific binding. With this the
> panel specific binding is typically just a compatible string and which
> inherited properties apply to it.
> 
> >>> +- data-mapping: the color signals mapping order, "jeida-18",
> >>> "jeida-24"
> >>> +  or "vesa-24"
> >> 
> >> Maybe this should be part of the compatible.
> > 
> > I've thought about it, but given that some panels support selecting
> > between multiple modes (through a mode pin that is usually hardwired), I
> > believe a separate DT property makes sense.
> 
> Okay.
> 
> > Furthermore, LVDS data organization is controlled by the combination of
> > both data-mapping and data-mirror. It makes little sense from my point of
> > view to handle one as part of the compatible string and the other one as
> > a separate property.
> > 
> >>> +Optional properties:
> >>> +- label: a symbolic name for the panel
> >> 
> >> Could be for any panel or display connector.
> > 
> > Yes, but I'm not sure to understand how that's relevant :-)
> 
> Meaning it should be a common property.
> 
> >>> +- avdd-supply: reference to the regulator that powers the panel
> >>> analog supply
> >>> +- dvdd-supply: reference to the regulator that powers the panel
> >>> digital supply
> >> 
> >> Which one has to be powered on first, what voltage, and with what time
> >> in between? This is why "generic" or "simple" bindings don't work.
> > 
> > The above-mentioned specifications also define connectors, pinouts and
> > power supplies, but many LVDS panels compatible with the LVDS physical
> > and data layers use a different connector with small differences in power
> > supplies.
> > 
> > I believe the voltage is irrelevant here, it doesn't need to be controlled
> > by the operating system. Power supplies order and timing is relevant,
> > I'll investigate the level of differences between panels. I'm also fine
> > with dropping those properties for now.
> 
> Whether you have control of the supplies is dependent on the board.
> Dropping them is just puts us in the simple binding trap. The simple
> bindings start out that way and then people keep adding to them.
> 
> >>> +- data-mirror: if set, reverse the bit order on all data lanes (6 to 0
> >>> instead
> >>> +  of 0 to 6)
> 
> On this one, make the name describe the order. "mirror" requires that
> I know what is normal ordering. Perhaps "data-msb-first".

The normal order is defined just below in the same document, and actually 
transmits bits with MSB first (roughly). The bit ordering in LVDS is a bit 
messed up, I've tried to follow the specs as much as possible for these 
bindings. Data mirroring isn't defined in any spec I've found but is 
implemented by some devices that got the spec wrong :-)

-- 
Regards,

Laurent Pinchart