[PATCH v2 00/13] R-Car DU: Add support for LVDS mode selection

[PATCH v2 00/13] 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.

Compared to v1, the LVDS panel DT bindings (02/13) have been reworked to
document common panel properties in a common file (01/13), with Mitsubishi
panels bindings now properly documented (03/13). Compared to the existing DPI
panel bindings that are currently abused by the R-Car DU driver for LVDS
panel, these new bindings specify the LVDS more explicitly.

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

Patches 04/13 to 06/13 add a new DRM panel driver for LVDS panels compatible
with the DT bindings. Patches 07/13 to 09/13 update the device tree of the
R-Car H3 Salvator-X board to add panel backlight support, and patch 10/13
switches two Mitsubishi panels to use the new bindings. This series doesn't
strictly depend on them so they can be merged separately at a later point if
needed.

Patches 11/13 to 13/13 finally move the R-Car DU driver to use the DRM panel
API, and add support for LVDS mode selection.

Laurent Pinchart (13):
  devicetree/bindings: display: Document common panel properties
  devicetree/bindings: display: Add bindings for LVDS panels
  devicetree/bindings: display: Add bindings for two Mitsubishi panels
  drm: Add data mirror bus flag
  drm: panels: Constify device node argument to of_drm_find_panel()
  drm: panels: Add LVDS panel driver
  arm64: dts: r8a7795: Add PWM support
  arm64: dts: r8a7795: salvator-x: Add DU LVDS output endpoint
  arm64: dts: r8a7795: salvator-x: Add panel backlight support
  ARM: shmobile: dts: Switch to panel-lvds bindings for Mitsubishi
    panels
  drm: rcar-du: Switch to encoder .atomic_mode_set() helper function
  drm: rcar-du: Use the DRM panel API
  drm: rcar-du: Add support for LVDS mode selection

 .../display/panel/mitsubishi,aa104xd12.txt         |  47 ++++
 .../display/panel/mitsubishi,aa121td01.txt         |  47 ++++
 .../bindings/display/panel/panel-common.txt        |  91 +++++++
 .../bindings/display/panel/panel-lvds.txt          | 120 +++++++++
 arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi     |   3 +-
 arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi     |   3 +-
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts |  26 ++
 arch/arm64/boot/dts/renesas/r8a7795.dtsi           |  63 +++++
 drivers/gpu/drm/drm_panel.c                        |   2 +-
 drivers/gpu/drm/panel/Kconfig                      |  10 +
 drivers/gpu/drm/panel/Makefile                     |   1 +
 drivers/gpu/drm/panel/panel-lvds.c                 | 284 +++++++++++++++++++++
 drivers/gpu/drm/rcar-du/Kconfig                    |   1 +
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c          |  50 +++-
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h          |   3 +
 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c          |   8 +-
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c          |  68 ++---
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c          |  11 +-
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h          |  13 +
 include/drm/drm_connector.h                        |   2 +
 include/drm/drm_panel.h                            |   4 +-
 21 files changed, 798 insertions(+), 59 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-common.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.txt
 create mode 100644 drivers/gpu/drm/panel/panel-lvds.c

-- 
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 v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Document properties common to several display panels in a central
location that can be referenced by the panel device tree bindings.

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

diff --git a/Documentation/devicetree/bindings/display/panel/panel-common.txt b/Documentation/devicetree/bindings/display/panel/panel-common.txt
new file mode 100644
index 000000000000..ec52c472c845
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt
@@ -0,0 +1,91 @@
+Common Properties for Display Panel
+===================================
+
+This document defines device tree properties common to several classes of
+display panels. It doesn't constitue a device tree binding specification by
+itself but is meant to be referenced by device tree bindings.
+
+When referenced from panel device tree bindings the properties defined in this
+document are defined as follows. The panel device tree bindings are
+responsible for defining whether each property is required or optional.
+
+
+Descriptive Properties
+----------------------
+
+- width-mm,
+- height-mm: The width-mm and height-mm specify the width and height of the
+  physical area where images are displayed. These properties are expressed in
+  millimeters and rounded to the closest unit.
+
+- label: The label property specifies a symbolic name for the panel as a
+  string suitable for use by humans. It typically contains a name inscribed on
+  the system (e.g. as an affixed label) or specified in the system's
+  documentation (e.g. in the user's manual).
+
+  If no such name exists, and unless the property is mandatory according to
+  device tree bindings, it shall rather be omitted than constructed of
+  non-descriptive information. For instance an LCD panel in a system that
+  contains a single panel shall not be labelled "LCD" if that name is not
+  inscribed on the system or used in a descriptive fashion in system
+  documentation.
+
+
+Display Timings
+---------------
+
+- panel-timing: Most display panels are restricted to a single resolution and
+  require specific display timings. The panel-timing subnode expresses those
+  timings as specified in the timing subnode section of the display timing
+  bindings defined in
+  Documentation/devicetree/bindings/display/display-timing.txt.
+
+
+Connectivity
+------------
+
+- ports: Panels receive video data through one or multiple connections. While
+  the nature of those connections is specific to the panel type, the
+  connectivity is expressed in a standard fashion using ports as specified in
+  the device graph bindings defined in
+  Documentation/devicetree/bindings/graph.txt.
+
+- ddc-i2c-bus: Some panels expose EDID information through an I2C-compatible
+  bus such as DDC2 or E-DDC. For such panels the ddc-i2c-bus contains a
+  phandle to the system I2C controller connected to that bus.
+
+
+Control I/Os
+------------
+
+Many display panels can be controlled through pins driven by GPIOs. The nature
+and timing of those control signals are device-specific and left for panel
+device tree bindings to specify. The following GPIO specifiers can however be
+used for panels that implement compatible control signals.
+
+- enable-gpios: Specifier for a GPIO connected to the panel enable control
+  signal. The enable signal is active high and enables operation of the panel.
+  This property can also be used for panels implementing an active low power
+  down signal, which is a negated version of the enable signal. Active low
+  enable signals (or active high power down signals) can be supported by
+  inverting the GPIO specifier polarity flag.
+
+  Note that the enable signal control panel operation only and must not be
+  confused with a backlight enable signal.
+
+- reset-gpios: Specifier for a GPIO coonnected to the panel reset control
+  signal. The reset signal is active low and resets the panel internal logic
+  while active. Active high reset signals can be supported by inverting the
+  GPIO specifier polarity flag.
+
+
+Backlight
+---------
+
+Most display panels include a backlight. Some of them also include a backlight
+controller exposed through a control bus such as I2C or DSI. Others expose
+backlight control through GPIO, PWM or other signals connected to an external
+backlight controller.
+
+- backlight: For panels whose backlight is controlled by an external backlight
+  controller, this property contains a phandle that references the controller.
-- 
Regards,

Laurent Pinchart

[PATCH v2 02/13] 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          | 120 +++++++++++++++++++++
 1 file changed, 120 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..b938269f841e
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
@@ -0,0 +1,120 @@
+LVDS Display 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" in addition to a mandatory
+  panel-specific compatible string defined in individual panel bindings. The
+  "panel-lvds" value shall never be used on its own.
+- width-mm: See panel-common.txt.
+- height-mm: See panel-common.txt.
+- data-mapping: The color signals mapping order, "jeida-18", "jeida-24"
+  or "vesa-24".
+
+Optional properties:
+
+- label: See panel-common.txt.
+- gpios: See panel-common.txt.
+- backlight: See panel-common.txt.
+- data-mirror: If set, reverse the bit order described in the data mappings
+  below on all data lanes, transmitting bits for slots 6 to 0 instead of
+  0 to 6.
+
+Required nodes:
+
+- panel-timing: See panel-common.txt.
+- ports: See panel-common.txt. These bindings require a single port subnode
+  corresponding to the panel LVDS input.
+
+
+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";
+
+	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_encoder>;
+		};
+	};
+};
-- 
Regards,

Laurent Pinchart

[PATCH v2 03/13] devicetree/bindings: display: Add bindings for two Mitsubishi panels

[Laurent Pinchart]

The AA104XD12 and AA121TD01 are LVDS display panels. Their bindings are
modelled on the the LVS panel bindings.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
---
 .../display/panel/mitsubishi,aa104xd12.txt         | 47 ++++++++++++++++++++++
 .../display/panel/mitsubishi,aa121td01.txt         | 47 ++++++++++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt

diff --git a/Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt b/Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt
new file mode 100644
index 000000000000..ced0121aed7d
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt
@@ -0,0 +1,47 @@
+Mitsubishi AA204XD12 LVDS Display Panel
+=======================================
+
+The AA104XD12 is a 10.4" XGA TFT-LCD display panel.
+
+These DT bindings follow the LVDS panel bindings defined in panel-lvds.txt
+with the following device-specific properties.
+
+
+Required properties:
+
+- compatible: Shall contain "mitsubishi,aa121td01" and "panel-lvds", in that
+  order.
+- vcc-supply: Reference to the regulator powering the panel VCC pins.
+
+
+Example
+-------
+
+panel {
+	compatible = "mitsubishi,aa104xd12", "panel-lvds";
+	vcc-supply = <&vcc_3v3>;
+
+	width-mm = <210>;
+	height-mm = <158>;
+
+	data-mapping = "jeida-24";
+
+	panel-timing {
+		/* 1024x768 @65Hz */
+		clock-frequency = <65000000>;
+		hactive = <1024>;
+		vactive = <768>;
+		hsync-len = <136>;
+		hfront-porch = <20>;
+		hback-porch = <160>;
+		vfront-porch = <3>;
+		vback-porch = <29>;
+		vsync-len = <6>;
+	};
+
+	port {
+		panel_in: endpoint {
+			remote-endpoint = <&lvds_encoder>;
+		};
+	};
+};
diff --git a/Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt b/Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt
new file mode 100644
index 000000000000..d6e1097504fe
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt
@@ -0,0 +1,47 @@
+Mitsubishi AA121TD01 LVDS Display Panel
+=======================================
+
+The AA121TD01 is a 12.1" WXGA TFT-LCD display panel.
+
+These DT bindings follow the LVDS panel bindings defined in panel-lvds.txt
+with the following device-specific properties.
+
+
+Required properties:
+
+- compatible: Shall contain "mitsubishi,aa121td01" and "panel-lvds", in that
+  order.
+- vcc-supply: Reference to the regulator powering the panel VCC pins.
+
+
+Example
+-------
+
+panel {
+	compatible = "mitsubishi,aa121td01", "panel-lvds";
+	vcc-supply = <&vcc_3v3>;
+
+	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_encoder>;
+		};
+	};
+};
-- 
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

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Rob Herring]

On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> Document properties common to several display panels in a central
> location that can be referenced by the panel device tree bindings.
> 

Looks good. Just one comment...

[...]

> +Connectivity
> +------------
> +
> +- ports: Panels receive video data through one or multiple connections. While
> +  the nature of those connections is specific to the panel type, the
> +  connectivity is expressed in a standard fashion using ports as specified in
> +  the device graph bindings defined in
> +  Documentation/devicetree/bindings/graph.txt.

We allow panels to either use graph binding or be a child of the display 
controller. Using the graph is preferred, but in the simple cases just a 
child node is sufficient. This should be described here or somewhere in 
this doc.

Rob

Re: [PATCH v2 02/13] devicetree/bindings: display: Add bindings for LVDS panels

[Rob Herring]

On Sat, Nov 19, 2016 at 05:28:02AM +0200, 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          | 120 +++++++++++++++++++++
>  1 file changed, 120 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.txt

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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 v2 03/13] devicetree/bindings: display: Add bindings for two Mitsubishi panels

[Rob Herring]

On Sat, Nov 19, 2016 at 05:28:03AM +0200, Laurent Pinchart wrote:
> The AA104XD12 and AA121TD01 are LVDS display panels. Their bindings are
> modelled on the the LVS panel bindings.

s/LVS/LVDS/

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../display/panel/mitsubishi,aa104xd12.txt         | 47 ++++++++++++++++++++++
>  .../display/panel/mitsubishi,aa121td01.txt         | 47 ++++++++++++++++++++++
>  2 files changed, 94 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt
>  create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt

With that,

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Rob,

On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> > Document properties common to several display panels in a central
> > location that can be referenced by the panel device tree bindings.
> 
> Looks good. Just one comment...
> 
> [...]
> 
> > +Connectivity
> > +------------
> > +
> > +- ports: Panels receive video data through one or multiple connections.
> > While
> > +  the nature of those connections is specific to the panel type, the
> > +  connectivity is expressed in a standard fashion using ports as
> > specified in
> > +  the device graph bindings defined in
> > +  Documentation/devicetree/bindings/graph.txt.
> 
> We allow panels to either use graph binding or be a child of the display
> controller.

I knew that some display controllers use a phandle to the panel (see the 
fsl,panel and nvidia,panel properties), but I didn't know we had panels as 
children of display controller nodes. I don't think we should allow that for 
anything but DSI panels, as the DT hierarchy is based on control buses. Are 
you sure we have other panels instantiated through that mechanism ?

> Using the graph is preferred, but in the simple cases just a child node is
> sufficient. This should be described here or somewhere in this doc.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v2 02/13] devicetree/bindings: display: Add bindings for LVDS panels

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3215 bytes --]

On Sat, Nov 19, 2016 at 05:28:02AM +0200, 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-ryLnwIuWjnhk3lzF8UVTdg@public.gmane.orgm>
> ---
>  .../bindings/display/panel/panel-lvds.txt          | 120 +++++++++++++++++++++
>  1 file changed, 120 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..b938269f841e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> @@ -0,0 +1,120 @@
> +LVDS Display 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" in addition to a mandatory
> +  panel-specific compatible string defined in individual panel bindings. The
> +  "panel-lvds" value shall never be used on its own.

What good is it if it shall never be used on its own? The above sounds
to me like the panel-specific compatible string implies the LVDS
binding, in a way that many compatible strings imply the simple binding.
Note that initially we did the very same thing with "panel-simple", only
to realize that it's completely redundant because it is never used.

> +- width-mm: See panel-common.txt.
> +- height-mm: See panel-common.txt.
> +- data-mapping: The color signals mapping order, "jeida-18", "jeida-24"
> +  or "vesa-24".
> +
> +Optional properties:
> +
> +- label: See panel-common.txt.
> +- gpios: See panel-common.txt.
> +- backlight: See panel-common.txt.
> +- data-mirror: If set, reverse the bit order described in the data mappings
> +  below on all data lanes, transmitting bits for slots 6 to 0 instead of
> +  0 to 6.
> +
> +Required nodes:
> +
> +- panel-timing: See panel-common.txt.
> +- ports: See panel-common.txt. These bindings require a single port subnode
> +  corresponding to the panel LVDS input.

Looks like I should go read the patch that introduces panel-common.txt
first...

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3023 bytes --]

On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> Document properties common to several display panels in a central
> location that can be referenced by the panel device tree bindings.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnhk3lzF8UVTdg@public.gmane.orgm>
> ---
>  .../bindings/display/panel/panel-common.txt        | 91 ++++++++++++++++++++++
>  1 file changed, 91 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-common.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-common.txt b/Documentation/devicetree/bindings/display/panel/panel-common.txt
> new file mode 100644
> index 000000000000..ec52c472c845
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt
> @@ -0,0 +1,91 @@
> +Common Properties for Display Panel
> +===================================
> +
> +This document defines device tree properties common to several classes of
> +display panels. It doesn't constitue a device tree binding specification by
> +itself but is meant to be referenced by device tree bindings.
> +
> +When referenced from panel device tree bindings the properties defined in this
> +document are defined as follows. The panel device tree bindings are
> +responsible for defining whether each property is required or optional.
> +
> +
> +Descriptive Properties
> +----------------------
> +
> +- width-mm,
> +- height-mm: The width-mm and height-mm specify the width and height of the
> +  physical area where images are displayed. These properties are expressed in
> +  millimeters and rounded to the closest unit.

Erm... this is already implied by the compatible string. Having this in
device tree is completely redundant.

> +- label: The label property specifies a symbolic name for the panel as a
> +  string suitable for use by humans. It typically contains a name inscribed on
> +  the system (e.g. as an affixed label) or specified in the system's
> +  documentation (e.g. in the user's manual).
> +
> +  If no such name exists, and unless the property is mandatory according to
> +  device tree bindings, it shall rather be omitted than constructed of
> +  non-descriptive information. For instance an LCD panel in a system that
> +  contains a single panel shall not be labelled "LCD" if that name is not
> +  inscribed on the system or used in a descriptive fashion in system
> +  documentation.
> +
> +
> +Display Timings
> +---------------
> +
> +- panel-timing: Most display panels are restricted to a single resolution and
> +  require specific display timings. The panel-timing subnode expresses those
> +  timings as specified in the timing subnode section of the display timing
> +  bindings defined in
> +  Documentation/devicetree/bindings/display/display-timing.txt.

Why? That's also implied by the compatible string. Honestly, I thought
by now we had been over this often enough...

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Thierry,

On Tuesday 22 Nov 2016 12:05:48 Thierry Reding wrote:
> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> > Document properties common to several display panels in a central
> > location that can be referenced by the panel device tree bindings.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  .../bindings/display/panel/panel-common.txt        | 91 +++++++++++++++++
> >  1 file changed, 91 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/display/panel/panel-common.txt> 
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/panel-common.txt
> > b/Documentation/devicetree/bindings/display/panel/panel-common.txt
> > new file mode 100644
> > index 000000000000..ec52c472c845
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt
> > @@ -0,0 +1,91 @@
> > +Common Properties for Display Panel
> > +===================================
> > +
> > +This document defines device tree properties common to several classes of
> > +display panels. It doesn't constitue a device tree binding specification
> > by
> > +itself but is meant to be referenced by device tree bindings.
> > +
> > +When referenced from panel device tree bindings the properties defined in
> > this
> > +document are defined as follows. The panel device tree bindings are
> > +responsible for defining whether each property is required or optional.
> > +
> > +
> > +Descriptive Properties
> > +----------------------
> > +
> > +- width-mm,
> > +- height-mm: The width-mm and height-mm specify the width and height of
> > the
> > +  physical area where images are displayed. These properties are
> > expressed in
> > +  millimeters and rounded to the closest unit.
> 
> Erm... this is already implied by the compatible string. Having this in
> device tree is completely redundant.

Nothing new under the sun here, we already have plenty of properties that 
could be implied by compatible strings. For instance for SoC IP cores many 
vendors use both an SoC-specific compatible string and a generic compatible 
string (e.g. "renesas,gpio-r8a7795" and "renesas,gpio-rcar"). The SoC-
compatible string implies register addresses, clocks and interrupts, but we 
still describe them in DT.

At the end of the day information about devices and their integration in the 
system needs to be available, either from DT or from C code. DT bindings 
should be designed to strike a good balance there, avoiding redundant 
information in DT (and thus keeping the bindings simple) while still providing 
enough information to allow for a reasonable level of genericity in OS 
implementations.

> > +- label: The label property specifies a symbolic name for the panel as a
> > +  string suitable for use by humans. It typically contains a name
> > inscribed on
> > +  the system (e.g. as an affixed label) or specified in the system's
> > +  documentation (e.g. in the user's manual).
> > +
> > +  If no such name exists, and unless the property is mandatory according
> > to
> > +  device tree bindings, it shall rather be omitted than constructed of
> > +  non-descriptive information. For instance an LCD panel in a system that
> > +  contains a single panel shall not be labelled "LCD" if that name is not
> > +  inscribed on the system or used in a descriptive fashion in system
> > +  documentation.
> > +
> > +
> > +Display Timings
> > +---------------
> > +
> > +- panel-timing: Most display panels are restricted to a single resolution
> > and
> > +  require specific display timings. The panel-timing subnode expresses
> > those
> > +  timings as specified in the timing subnode section of the display
> > timing
> > +  bindings defined in
> > +  Documentation/devicetree/bindings/display/display-timing.txt.
> 
> Why? That's also implied by the compatible string. Honestly, I thought
> by now we had been over this often enough...

Same argument as above. I won't try to change your mind and fix the simple 
panel driver, but I still stand firm on my belief that expressing the size and 
timings in DT is the right solution in a wide variety of cases (and yes I've 
read http://sietch-tagr.blogspot.fi/2016/04/display-panels-are-not-special.html, and while I agree with the title, I still believe size and 
timings in DT are not wrong).

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v2 02/13] devicetree/bindings: display: Add bindings for LVDS panels

[Laurent Pinchart]

Hi Thierry,

On Tuesday 22 Nov 2016 12:02:41 Thierry Reding wrote:
> On Sat, Nov 19, 2016 at 05:28:02AM +0200, 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          | 120 ++++++++++++++++
> >  1 file changed, 120 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..b938269f841e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> > @@ -0,0 +1,120 @@
> > +LVDS Display 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" in addition to a mandatory
> > +  panel-specific compatible string defined in individual panel bindings.
> > The
> > +  "panel-lvds" value shall never be used on its own.
> 
> What good is it if it shall never be used on its own? The above sounds
> to me like the panel-specific compatible string implies the LVDS
> binding, in a way that many compatible strings imply the simple binding.
> Note that initially we did the very same thing with "panel-simple", only
> to realize that it's completely redundant because it is never used.

DT allows specifying multiple compatible strings in decreasing order of 
genericity to make generic OS implementations possible while retaining the 
ability to later introduce device-specific code if/when the need arises 
(mostly because of information that were overlooked, misunderstood or just not 
available at implementation time - we unfortunately can't produce 100% perfect 
solutions all the time, I very much regret that). This is exactly what the 
LVDS panel bindings mandate.

> > +- width-mm: See panel-common.txt.
> > +- height-mm: See panel-common.txt.
> > +- data-mapping: The color signals mapping order, "jeida-18", "jeida-24"
> > +  or "vesa-24".
> > +
> > +Optional properties:
> > +
> > +- label: See panel-common.txt.
> > +- gpios: See panel-common.txt.
> > +- backlight: See panel-common.txt.
> > +- data-mirror: If set, reverse the bit order described in the data
> > mappings
> > +  below on all data lanes, transmitting bits for slots 6 to 0 instead of
> > +  0 to 6.
> > +
> > +Required nodes:
> > +
> > +- panel-timing: See panel-common.txt.
> > +- ports: See panel-common.txt. These bindings require a single port
> > subnode
> > +  corresponding to the panel LVDS input.
> 
> Looks like I should go read the patch that introduces panel-common.txt
> first...

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Rob Herring]

On Tue, Nov 22, 2016 at 5:05 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
>> Document properties common to several display panels in a central
>> location that can be referenced by the panel device tree bindings.
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> ---
>>  .../bindings/display/panel/panel-common.txt        | 91 ++++++++++++++++++++++
>>  1 file changed, 91 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-common.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/panel/panel-common.txt b/Documentation/devicetree/bindings/display/panel/panel-common.txt
>> new file mode 100644
>> index 000000000000..ec52c472c845
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt
>> @@ -0,0 +1,91 @@
>> +Common Properties for Display Panel
>> +===================================
>> +
>> +This document defines device tree properties common to several classes of
>> +display panels. It doesn't constitue a device tree binding specification by
>> +itself but is meant to be referenced by device tree bindings.
>> +
>> +When referenced from panel device tree bindings the properties defined in this
>> +document are defined as follows. The panel device tree bindings are
>> +responsible for defining whether each property is required or optional.
>> +
>> +
>> +Descriptive Properties
>> +----------------------
>> +
>> +- width-mm,
>> +- height-mm: The width-mm and height-mm specify the width and height of the
>> +  physical area where images are displayed. These properties are expressed in
>> +  millimeters and rounded to the closest unit.
>
> Erm... this is already implied by the compatible string. Having this in
> device tree is completely redundant.
>
>> +- label: The label property specifies a symbolic name for the panel as a
>> +  string suitable for use by humans. It typically contains a name inscribed on
>> +  the system (e.g. as an affixed label) or specified in the system's
>> +  documentation (e.g. in the user's manual).
>> +
>> +  If no such name exists, and unless the property is mandatory according to
>> +  device tree bindings, it shall rather be omitted than constructed of
>> +  non-descriptive information. For instance an LCD panel in a system that
>> +  contains a single panel shall not be labelled "LCD" if that name is not
>> +  inscribed on the system or used in a descriptive fashion in system
>> +  documentation.
>> +
>> +
>> +Display Timings
>> +---------------
>> +
>> +- panel-timing: Most display panels are restricted to a single resolution and
>> +  require specific display timings. The panel-timing subnode expresses those
>> +  timings as specified in the timing subnode section of the display timing
>> +  bindings defined in
>> +  Documentation/devicetree/bindings/display/display-timing.txt.
>
> Why? That's also implied by the compatible string. Honestly, I thought
> by now we had been over this often enough...

While I completely agree we don't want *only* generic compatibles nor
generic gpio and power control, I think timing values in DT are fine.
They are just data copied out of datasheets and aren't tweaked per
platform. If the same data would make sense to put into a display
EDID, I think it also makes sense to put that data in DT.

Rob

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Rob,

On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> > On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> >> Document properties common to several display panels in a central
> >> location that can be referenced by the panel device tree bindings.
> > 
> > Looks good. Just one comment...
> > 
> > [...]
> > 
> >> +Connectivity
> >> +------------
> >> +
> >> +- ports: Panels receive video data through one or multiple connections.
> >> While
> >> +  the nature of those connections is specific to the panel type, the
> >> +  connectivity is expressed in a standard fashion using ports as
> >> specified in
> >> +  the device graph bindings defined in
> >> +  Documentation/devicetree/bindings/graph.txt.
> > 
> > We allow panels to either use graph binding or be a child of the display
> > controller.
> 
> I knew that some display controllers use a phandle to the panel (see the
> fsl,panel and nvidia,panel properties), but I didn't know we had panels as
> children of display controller nodes. I don't think we should allow that for
> anything but DSI panels, as the DT hierarchy is based on control buses. Are
> you sure we have other panels instantiated through that mechanism ?

Ping ?

Please note that this file documents properties common to multiple panel DT 
bindings, but in no way makes it mandatory to use the OF graph bindings for 
panels. The decision is left to individual bindings.

> > Using the graph is preferred, but in the simple cases just a child node is
> > sufficient. This should be described here or somewhere in this doc.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Rob Herring]

On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
>> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
>> > On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
>> >> Document properties common to several display panels in a central
>> >> location that can be referenced by the panel device tree bindings.
>> >
>> > Looks good. Just one comment...
>> >
>> > [...]
>> >
>> >> +Connectivity
>> >> +------------
>> >> +
>> >> +- ports: Panels receive video data through one or multiple connections.
>> >> While
>> >> +  the nature of those connections is specific to the panel type, the
>> >> +  connectivity is expressed in a standard fashion using ports as
>> >> specified in
>> >> +  the device graph bindings defined in
>> >> +  Documentation/devicetree/bindings/graph.txt.
>> >
>> > We allow panels to either use graph binding or be a child of the display
>> > controller.
>>
>> I knew that some display controllers use a phandle to the panel (see the
>> fsl,panel and nvidia,panel properties), but I didn't know we had panels as
>> children of display controller nodes. I don't think we should allow that for
>> anything but DSI panels, as the DT hierarchy is based on control buses. Are
>> you sure we have other panels instantiated through that mechanism ?

Some panels have no control bus, so were do we place them? I would say
the hierarchy is based on buses with a preference for the control bus
when there are multiple buses. I'm not a fan of just sticking things
are the top level.

> Ping ?
>
> Please note that this file documents properties common to multiple panel DT
> bindings, but in no way makes it mandatory to use the OF graph bindings for
> panels. The decision is left to individual bindings.

It is mandatory in the sense that we don't want more cases of "fsl,panel".

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

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Rob,

On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
> On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
> > On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
> >> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> >>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> >>>> Document properties common to several display panels in a central
> >>>> location that can be referenced by the panel device tree bindings.
> >>> 
> >>> Looks good. Just one comment...
> >>> 
> >>> [...]
> >>> 
> >>>> +Connectivity
> >>>> +------------
> >>>> +
> >>>> +- ports: Panels receive video data through one or multiple
> >>>> connections. While
> >>>> +  the nature of those connections is specific to the panel type, the
> >>>> +  connectivity is expressed in a standard fashion using ports as
> >>>> specified in
> >>>> +  the device graph bindings defined in
> >>>> +  Documentation/devicetree/bindings/graph.txt.
> >>> 
> >>> We allow panels to either use graph binding or be a child of the
> >>> display controller.
> >> 
> >> I knew that some display controllers use a phandle to the panel (see the
> >> fsl,panel and nvidia,panel properties), but I didn't know we had panels
> >> as children of display controller nodes. I don't think we should allow
> >> that for anything but DSI panels, as the DT hierarchy is based on control
> >> buses. Are you sure we have other panels instantiated through that
> >> mechanism ?
>
> Some panels have no control bus, so were do we place them?

I'd say under the root node, like all similar control-less devices.

> I would say the hierarchy is based on buses with a preference for the
> control bus when there are multiple buses. I'm not a fan of just sticking
> things are the top level.

OK, so much for my comment a few lines up :-)

The problem with placing non-DSI panels as children of the display controller 
and not using OF graph is that the panel bindings become dependent of the 
display controller being used. A display controller using OF graph would 
require the panel to do the same, while a display controller expecting a panel 
child node (with a specific name) would require DT properties for the panel 
node.

I'm also not sure the complexity of OF graph is really that prohibitive if you 
compare it to panels as child nodes. To get the panel driver to bind to the 
panel DT node the display controller driver would need to create a platform 
device for the panel and register it. That's not very difficult, but parsing a 
single port and endpoint isn't either (and we could even provide a helper 
function for that, a version of of_drm_find_panel() that would take as an 
argument the display controller device node instead of the panel device node).

> > Ping ?
> > 
> > Please note that this file documents properties common to multiple panel
> > DT bindings, but in no way makes it mandatory to use the OF graph bindings
> > for panels. The decision is left to individual bindings.
> 
> It is mandatory in the sense that we don't want more cases of "fsl,panel".

That I agree with :-)

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Rob,

On Tuesday 29 Nov 2016 20:23:41 Laurent Pinchart wrote:
> On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
> > On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
> >> On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
> >>> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> >>>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> >>>>> Document properties common to several display panels in a central
> >>>>> location that can be referenced by the panel device tree bindings.
> >>>> 
> >>>> Looks good. Just one comment...
> >>>> 
> >>>> [...]
> >>>> 
> >>>>> +Connectivity
> >>>>> +------------
> >>>>> +
> >>>>> +- ports: Panels receive video data through one or multiple
> >>>>> connections. While
> >>>>> +  the nature of those connections is specific to the panel type, the
> >>>>> +  connectivity is expressed in a standard fashion using ports as
> >>>>> specified in
> >>>>> +  the device graph bindings defined in
> >>>>> +  Documentation/devicetree/bindings/graph.txt.
> >>>> 
> >>>> We allow panels to either use graph binding or be a child of the
> >>>> display controller.
> >>> 
> >>> I knew that some display controllers use a phandle to the panel (see
> >>> the fsl,panel and nvidia,panel properties), but I didn't know we had
> >>> panels as children of display controller nodes. I don't think we should
> >>> allow that for anything but DSI panels, as the DT hierarchy is based on
> >>> control buses. Are you sure we have other panels instantiated through
> >>> that mechanism ?
> > 
> > Some panels have no control bus, so were do we place them?
> 
> I'd say under the root node, like all similar control-less devices.
> 
> > I would say the hierarchy is based on buses with a preference for the
> > control bus when there are multiple buses. I'm not a fan of just sticking
> > things are the top level.
> 
> OK, so much for my comment a few lines up :-)
> 
> The problem with placing non-DSI panels as children of the display
> controller and not using OF graph is that the panel bindings become
> dependent of the display controller being used. A display controller using
> OF graph would require the panel to do the same, while a display controller
> expecting a panel child node (with a specific name) would require DT
> properties for the panel node.
> 
> I'm also not sure the complexity of OF graph is really that prohibitive if
> you compare it to panels as child nodes. To get the panel driver to bind to
> the panel DT node the display controller driver would need to create a
> platform device for the panel and register it. That's not very difficult,
> but parsing a single port and endpoint isn't either (and we could even
> provide a helper function for that, a version of of_drm_find_panel() that
> would take as an argument the display controller device node instead of the
> panel device node).

Ping ?

I'd like to standardize on one model for panel DT bindings, but I'm not sure 
that can be achieved given that we already have multiple competing models. In 
any case, is that blocking to merge this patch ? I only describe one 
connectivity model here as that's what my panel driver needs, but I have no 
issue adding more models later when needed. I believe this patch is a good 
step forward already.

> >> Ping ?
> >> 
> >> Please note that this file documents properties common to multiple panel
> >> DT bindings, but in no way makes it mandatory to use the OF graph
> >> bindings for panels. The decision is left to individual bindings.
> > 
> > It is mandatory in the sense that we don't want more cases of "fsl,panel".
> 
> That I agree with :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Rob Herring]

On Sun, Dec 18, 2016 at 2:54 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Tuesday 29 Nov 2016 20:23:41 Laurent Pinchart wrote:
>> On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
>> > On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
>> >> On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
>> >>> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
>> >>>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
>> >>>>> Document properties common to several display panels in a central
>> >>>>> location that can be referenced by the panel device tree bindings.
>> >>>>
>> >>>> Looks good. Just one comment...
>> >>>>
>> >>>> [...]
>> >>>>
>> >>>>> +Connectivity
>> >>>>> +------------
>> >>>>> +
>> >>>>> +- ports: Panels receive video data through one or multiple
>> >>>>> connections. While
>> >>>>> +  the nature of those connections is specific to the panel type, the
>> >>>>> +  connectivity is expressed in a standard fashion using ports as
>> >>>>> specified in
>> >>>>> +  the device graph bindings defined in
>> >>>>> +  Documentation/devicetree/bindings/graph.txt.
>> >>>>
>> >>>> We allow panels to either use graph binding or be a child of the
>> >>>> display controller.
>> >>>
>> >>> I knew that some display controllers use a phandle to the panel (see
>> >>> the fsl,panel and nvidia,panel properties), but I didn't know we had
>> >>> panels as children of display controller nodes. I don't think we should
>> >>> allow that for anything but DSI panels, as the DT hierarchy is based on
>> >>> control buses. Are you sure we have other panels instantiated through
>> >>> that mechanism ?
>> >
>> > Some panels have no control bus, so were do we place them?
>>
>> I'd say under the root node, like all similar control-less devices.
>>
>> > I would say the hierarchy is based on buses with a preference for the
>> > control bus when there are multiple buses. I'm not a fan of just sticking
>> > things are the top level.
>>
>> OK, so much for my comment a few lines up :-)
>>
>> The problem with placing non-DSI panels as children of the display
>> controller and not using OF graph is that the panel bindings become
>> dependent of the display controller being used. A display controller using
>> OF graph would require the panel to do the same, while a display controller
>> expecting a panel child node (with a specific name) would require DT
>> properties for the panel node.

Not sure I follow. They become dependent on the controller driver to
probe the panel, but the contents of the panel node would not be
controller dependent.

>> I'm also not sure the complexity of OF graph is really that prohibitive if
>> you compare it to panels as child nodes. To get the panel driver to bind to
>> the panel DT node the display controller driver would need to create a
>> platform device for the panel and register it. That's not very difficult,
>> but parsing a single port and endpoint isn't either (and we could even
>> provide a helper function for that, a version of of_drm_find_panel() that
>> would take as an argument the display controller device node instead of the
>> panel device node).
>
> Ping ?
>
> I'd like to standardize on one model for panel DT bindings, but I'm not sure
> that can be achieved given that we already have multiple competing models. In
> any case, is that blocking to merge this patch ? I only describe one
> connectivity model here as that's what my panel driver needs, but I have no
> issue adding more models later when needed. I believe this patch is a good
> step forward already.

It is an improvement which I appreciate, so yes I guess we can address
it later when needed.

Rob

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Rob,

On Monday 19 Dec 2016 09:38:49 Rob Herring wrote:
> On Sun, Dec 18, 2016 at 2:54 PM, Laurent Pinchart wrote:
> > On Tuesday 29 Nov 2016 20:23:41 Laurent Pinchart wrote:
> >> On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
> >>> On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
> >>>> On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
> >>>>> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> >>>>>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> >>>>>>> Document properties common to several display panels in a central
> >>>>>>> location that can be referenced by the panel device tree bindings.
> >>>>>> 
> >>>>>> Looks good. Just one comment...
> >>>>>> 
> >>>>>> [...]
> >>>>>> 
> >>>>>>> +Connectivity
> >>>>>>> +------------
> >>>>>>> +
> >>>>>>> +- ports: Panels receive video data through one or multiple
> >>>>>>> connections. While
> >>>>>>> +  the nature of those connections is specific to the panel type,
> >>>>>>> the
> >>>>>>> +  connectivity is expressed in a standard fashion using ports as
> >>>>>>> specified in
> >>>>>>> +  the device graph bindings defined in
> >>>>>>> +  Documentation/devicetree/bindings/graph.txt.
> >>>>>> 
> >>>>>> We allow panels to either use graph binding or be a child of the
> >>>>>> display controller.
> >>>>> 
> >>>>> I knew that some display controllers use a phandle to the panel (see
> >>>>> the fsl,panel and nvidia,panel properties), but I didn't know we had
> >>>>> panels as children of display controller nodes. I don't think we
> >>>>> should allow that for anything but DSI panels, as the DT hierarchy is
> >>>>> based on control buses. Are you sure we have other panels instantiated
> >>>>> through that mechanism ?
> >>> 
> >>> Some panels have no control bus, so were do we place them?
> >> 
> >> I'd say under the root node, like all similar control-less devices.
> >> 
> >>> I would say the hierarchy is based on buses with a preference for the
> >>> control bus when there are multiple buses. I'm not a fan of just
> >>> sticking things are the top level.
> >> 
> >> OK, so much for my comment a few lines up :-)
> >> 
> >> The problem with placing non-DSI panels as children of the display
> >> controller and not using OF graph is that the panel bindings become
> >> dependent of the display controller being used. A display controller
> >> using OF graph would require the panel to do the same, while a display
> >> controller expecting a panel child node (with a specific name) would
> >> require DT properties for the panel node.
> 
> Not sure I follow.

Sorry, I meant "would not requite DT properties".

> They become dependent on the controller driver to probe the panel, but the
> contents of the panel node would not be controller dependent.

If a display controller uses OF graph then the panel DT node has to declare 
ports. If the display controller doesn't use OF graph but instead expects the 
panel to be a direct subnode, or points to the panel using a property such as 
fsl,panel, then the panel DT node will not have ports.

> >> I'm also not sure the complexity of OF graph is really that prohibitive
> >> if you compare it to panels as child nodes. To get the panel driver to
> >> bind to the panel DT node the display controller driver would need to
> >> create a platform device for the panel and register it. That's not very
> >> difficult, but parsing a single port and endpoint isn't either (and we
> >> could even provide a helper function for that, a version of
> >> of_drm_find_panel() that would take as an argument the display controller
> >> device node instead of the panel device node).
> > 
> > Ping ?
> > 
> > I'd like to standardize on one model for panel DT bindings, but I'm not
> > sure that can be achieved given that we already have multiple competing
> > models. In any case, is that blocking to merge this patch ? I only
> > describe one connectivity model here as that's what my panel driver
> > needs, but I have no issue adding more models later when needed. I
> > believe this patch is a good step forward already.
> 
> It is an improvement which I appreciate, so yes I guess we can address
> it later when needed.

Thank you. Can I get your ack then ? :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Rob Herring]

On Mon, Dec 19, 2016 at 10:54 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Monday 19 Dec 2016 09:38:49 Rob Herring wrote:
>> On Sun, Dec 18, 2016 at 2:54 PM, Laurent Pinchart wrote:
>> > On Tuesday 29 Nov 2016 20:23:41 Laurent Pinchart wrote:
>> >> On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
>> >>> On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
>> >>>> On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
>> >>>>> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
>> >>>>>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
>> >>>>>>> Document properties common to several display panels in a central
>> >>>>>>> location that can be referenced by the panel device tree bindings.
>> >>>>>>
>> >>>>>> Looks good. Just one comment...
>> >>>>>>
>> >>>>>> [...]
>> >>>>>>
>> >>>>>>> +Connectivity
>> >>>>>>> +------------
>> >>>>>>> +
>> >>>>>>> +- ports: Panels receive video data through one or multiple
>> >>>>>>> connections. While
>> >>>>>>> +  the nature of those connections is specific to the panel type,
>> >>>>>>> the
>> >>>>>>> +  connectivity is expressed in a standard fashion using ports as
>> >>>>>>> specified in
>> >>>>>>> +  the device graph bindings defined in
>> >>>>>>> +  Documentation/devicetree/bindings/graph.txt.
>> >>>>>>
>> >>>>>> We allow panels to either use graph binding or be a child of the
>> >>>>>> display controller.
>> >>>>>
>> >>>>> I knew that some display controllers use a phandle to the panel (see
>> >>>>> the fsl,panel and nvidia,panel properties), but I didn't know we had
>> >>>>> panels as children of display controller nodes. I don't think we
>> >>>>> should allow that for anything but DSI panels, as the DT hierarchy is
>> >>>>> based on control buses. Are you sure we have other panels instantiated
>> >>>>> through that mechanism ?
>> >>>
>> >>> Some panels have no control bus, so were do we place them?
>> >>
>> >> I'd say under the root node, like all similar control-less devices.
>> >>
>> >>> I would say the hierarchy is based on buses with a preference for the
>> >>> control bus when there are multiple buses. I'm not a fan of just
>> >>> sticking things are the top level.
>> >>
>> >> OK, so much for my comment a few lines up :-)
>> >>
>> >> The problem with placing non-DSI panels as children of the display
>> >> controller and not using OF graph is that the panel bindings become
>> >> dependent of the display controller being used. A display controller
>> >> using OF graph would require the panel to do the same, while a display
>> >> controller expecting a panel child node (with a specific name) would
>> >> require DT properties for the panel node.
>>
>> Not sure I follow.
>
> Sorry, I meant "would not requite DT properties".
>
>> They become dependent on the controller driver to probe the panel, but the
>> contents of the panel node would not be controller dependent.
>
> If a display controller uses OF graph then the panel DT node has to declare
> ports. If the display controller doesn't use OF graph but instead expects the
> panel to be a direct subnode, or points to the panel using a property such as
> fsl,panel, then the panel DT node will not have ports.

The controller should just ask for the panel via a common function.
That function should then look for either a child node (called
'panel') or a graph port. Of course, for more complex cases, only OF
graph may work. It really just the simple case of a controller with a
single output and single panel that I'm talking about.


>> >> I'm also not sure the complexity of OF graph is really that prohibitive
>> >> if you compare it to panels as child nodes. To get the panel driver to
>> >> bind to the panel DT node the display controller driver would need to
>> >> create a platform device for the panel and register it. That's not very
>> >> difficult, but parsing a single port and endpoint isn't either (and we
>> >> could even provide a helper function for that, a version of
>> >> of_drm_find_panel() that would take as an argument the display controller
>> >> device node instead of the panel device node).
>> >
>> > Ping ?
>> >
>> > I'd like to standardize on one model for panel DT bindings, but I'm not
>> > sure that can be achieved given that we already have multiple competing
>> > models. In any case, is that blocking to merge this patch ? I only
>> > describe one connectivity model here as that's what my panel driver
>> > needs, but I have no issue adding more models later when needed. I
>> > believe this patch is a good step forward already.
>>
>> It is an improvement which I appreciate, so yes I guess we can address
>> it later when needed.
>
> Thank you. Can I get your ack then ? :-)

Yes.

Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Emil Velikov]

Hi Laurent,

Pardon for reviving this old thread. I've noticed a couple of things
which might want some attention.

On 19 November 2016 at 03:28, Laurent Pinchart
<laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:

> +
> +- panel-timing: Most display panels are restricted to a single resolution and
> +  require specific display timings. The panel-timing subnode expresses those
> +  timings as specified in the timing subnode section of the display timing
> +  bindings defined in
> +  Documentation/devicetree/bindings/display/display-timing.txt.
Cannot find such a file in linux-next. Perhaps you meant
Documentation/devicetree/bindings/display/panel/display-timing.txt?

Documentation/devicetree/bindings/display/panel/panel.txt includes a
"rotation" property, which we might want to fold here.

Regards,
Emil
--
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 v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Emil,

On Sunday 09 Apr 2017 12:47:01 Emil Velikov wrote:
> Hi Laurent,
> 
> Pardon for reviving this old thread. I've noticed a couple of things
> which might want some attention.
> 
> On 19 November 2016 at 03:28, Laurent Pinchart wrote:
> > +
> > +- panel-timing: Most display panels are restricted to a single resolution
> > and +  require specific display timings. The panel-timing subnode
> > expresses those +  timings as specified in the timing subnode section of
> > the display timing +  bindings defined in
> > +  Documentation/devicetree/bindings/display/display-timing.txt.
> 
> Cannot find such a file in linux-next. Perhaps you meant
> Documentation/devicetree/bindings/display/panel/display-timing.txt?

Oops. My bad, I'll fix that. Thank you for noticing it.

> Documentation/devicetree/bindings/display/panel/panel.txt includes a
> "rotation" property, which we might want to fold here.

I believe that panel.txt and panel-common.txt were added concurrently. We 
should indeed merge the two.

-- 
Regards,

Laurent Pinchart