[PATCH v2 0/4] drm: Add support for the Amlogic Video Processing Unit

[PATCH v2 0/4] drm: Add support for the Amlogic Video Processing Unit

[Neil Armstrong]

This a repost of the previous version at [2] with fixes, the following patches will
be sent via a PULL Request once the DT maintainers acks the DT bindings.
The Amlogic maintainer will take the arm64 DT patches to avoid merges conflicts.

The Amlogic Meson SoCs embeds a Video Processing Unit able to output at least
a Composite/CVBS Video with embedded VDAC and an HDMI Link with Embedded HDMI
Transceiver.

Thus, the current driver does not support HDMI yet.

The Video Processig Unit is composed of multiple modules like the Video
Input Unit and the Video Post Processing that can be associated to a
CRTC with Planes management.
The last Unit is the Venc that embeds at least 3 Encoders, ENCI for Interlace
Video used by CVBS or HDMI, ENCP for Progressive Video used by the HDMI
Transceiver and ENCL for LCD Display.

The LCD Display is not planned to be supported on the Meson GX Family.

This driver is a DRM/KMS driver using the following DRM components :
 - GEM-CMA
 - PRIME-CMA
 - Atomic Modesetting
 - FBDev-CMA

For the following SoCs :
 - GXBB Family (S905)
 - GXL Family (S905X, S905D)
 - GXM Family (S912)

The current driver only supports the CVBS PAL/NTSC output modes, but the
CRTC/Planes management should support bigger modes.
But Advanced Colorspace Conversion, Scaling and HDMI Modes will be added in
a second time.

The Device Tree bindings makes use of the endpoints video interface definitions
to connect to the optional CVBS and in the future the HDMI Connector nodes.

The driver has been tested with Xorg modesetting driver and Weston DRM backend.

Changes since v1 at [2] :
 - Simplify bindings to have a "composite-video-connector" to represent the on-board composite connector
 - Remove the component_match binding since it's unused for now
 - Moved all DRM connector code back in the venc_cvbs file
 - Check for port endpoints validity to detech connector existence
 - Added Daniel Vetter's ack on non-dt patches commit messages

Changes since RFC at [1] :
 - Add maintainers entry
 - Move all Plane and CRTC code from backend to corresponding DRM code
 - Keep only init and common code in backend source files
 - Move the CVBS encoder out of the CVBS DT node, only keep the connector
 - Various cleanups using DRM helpers
 - Cleanup of copyright headers
 - Fixup of bindings documentation

[1] http://lkml.kernel.org/r/1480089791-12517-1-git-send-email-narmstrong at baylibre.com
[2] http://lkml.kernel.org/r/1480416469-9655-1-git-send-email-narmstrong at baylibre.com

Neil Armstrong (4):
  drm: Add support for Amlogic Meson Graphic Controller
  ARM64: dts: meson-gx: Add Graphic Controller nodes
  dt-bindings: display: add Amlogic Meson DRM Bindings
  MAINTAINERS: add entry for Amlogic DRM drivers

 .../bindings/display/meson/meson-drm.txt           |  101 ++
 MAINTAINERS                                        |    9 +
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi          |   19 +
 .../boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts    |   15 +
 arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi   |   15 +
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        |    4 +
 .../boot/dts/amlogic/meson-gxl-nexbox-a95x.dts     |   15 +
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi         |    4 +
 .../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts |   15 +
 arch/arm64/boot/dts/amlogic/meson-gxm.dtsi         |    4 +
 drivers/gpu/drm/Kconfig                            |    2 +
 drivers/gpu/drm/Makefile                           |    1 +
 drivers/gpu/drm/meson/Kconfig                      |    9 +
 drivers/gpu/drm/meson/Makefile                     |    4 +
 drivers/gpu/drm/meson/meson_canvas.c               |   68 +
 drivers/gpu/drm/meson/meson_canvas.h               |   42 +
 drivers/gpu/drm/meson/meson_crtc.c                 |  208 +++
 drivers/gpu/drm/meson/meson_crtc.h                 |   32 +
 drivers/gpu/drm/meson/meson_drv.c                  |  343 +++++
 drivers/gpu/drm/meson/meson_drv.h                  |   60 +
 drivers/gpu/drm/meson/meson_plane.c                |  230 ++++
 drivers/gpu/drm/meson/meson_plane.h                |   30 +
 drivers/gpu/drm/meson/meson_registers.h            | 1395 ++++++++++++++++++++
 drivers/gpu/drm/meson/meson_vclk.c                 |  167 +++
 drivers/gpu/drm/meson/meson_vclk.h                 |   34 +
 drivers/gpu/drm/meson/meson_venc.c                 |  254 ++++
 drivers/gpu/drm/meson/meson_venc.h                 |   72 +
 drivers/gpu/drm/meson/meson_venc_cvbs.c            |  293 ++++
 drivers/gpu/drm/meson/meson_venc_cvbs.h            |   41 +
 drivers/gpu/drm/meson/meson_viu.c                  |  331 +++++
 drivers/gpu/drm/meson/meson_viu.h                  |   64 +
 drivers/gpu/drm/meson/meson_vpp.c                  |  162 +++
 drivers/gpu/drm/meson/meson_vpp.h                  |   35 +
 33 files changed, 4078 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/meson/meson-drm.txt
 create mode 100644 drivers/gpu/drm/meson/Kconfig
 create mode 100644 drivers/gpu/drm/meson/Makefile
 create mode 100644 drivers/gpu/drm/meson/meson_canvas.c
 create mode 100644 drivers/gpu/drm/meson/meson_canvas.h
 create mode 100644 drivers/gpu/drm/meson/meson_crtc.c
 create mode 100644 drivers/gpu/drm/meson/meson_crtc.h
 create mode 100644 drivers/gpu/drm/meson/meson_drv.c
 create mode 100644 drivers/gpu/drm/meson/meson_drv.h
 create mode 100644 drivers/gpu/drm/meson/meson_plane.c
 create mode 100644 drivers/gpu/drm/meson/meson_plane.h
 create mode 100644 drivers/gpu/drm/meson/meson_registers.h
 create mode 100644 drivers/gpu/drm/meson/meson_vclk.c
 create mode 100644 drivers/gpu/drm/meson/meson_vclk.h
 create mode 100644 drivers/gpu/drm/meson/meson_venc.c
 create mode 100644 drivers/gpu/drm/meson/meson_venc.h
 create mode 100644 drivers/gpu/drm/meson/meson_venc_cvbs.c
 create mode 100644 drivers/gpu/drm/meson/meson_venc_cvbs.h
 create mode 100644 drivers/gpu/drm/meson/meson_viu.c
 create mode 100644 drivers/gpu/drm/meson/meson_viu.h
 create mode 100644 drivers/gpu/drm/meson/meson_vpp.c
 create mode 100644 drivers/gpu/drm/meson/meson_vpp.h

-- 
1.9.1

[PATCH v2 2/4] ARM64: dts: meson-gx: Add Graphic Controller nodes

[Neil Armstrong]

Add Video Processing Unit and CVBS Output nodes, and enable CVBS on selected
boards.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi             | 19 +++++++++++++++++++
 .../arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts | 15 +++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi      | 15 +++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi           |  4 ++++
 arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts | 15 +++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi            |  4 ++++
 arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts   | 15 +++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-gxm.dtsi            |  4 ++++
 8 files changed, 91 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index fc033c0..a27f881 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -356,5 +356,24 @@
 				status = "disabled";
 			};
 		};
+
+		vpu: vpu at d0100000 {
+			compatible = "amlogic,meson-gx-vpu";
+			reg = <0x0 0xd0100000 0x0 0x100000>,
+			      <0x0 0xc883c000 0x0 0x1000>,
+			      <0x0 0xc8838000 0x0 0x1000>;
+			reg-names = "vpu", "hhi", "dmc";
+			interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			/* CVBS VDAC output port */
+			port at 0 {
+				reg = <0>;
+
+				cvbs_vdac_out: endpoint {
+				};
+			};
+		};
 	};
 };
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
index 9696820..390f7db 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
@@ -142,6 +142,17 @@
 		clocks = <&wifi32k>;
 		clock-names = "ext_clock";
 	};
+
+	cvbs-connector {
+		compatible = "composite-video-connector";
+		label = "cvbs";
+
+		port {
+			cvbs_connector_in: endpoint {
+				remote-endpoint = <&cvbs_vdac_out>;
+			};
+		};
+	};
 };
 
 &uart_AO {
@@ -229,3 +240,7 @@
 	clocks = <&clkc CLKID_FCLK_DIV4>;
 	clock-names = "clkin0";
 };
+
+&cvbs_vdac_out {
+	remote-endpoint = <&cvbs_connector_in>;
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index 203be28..44bdebf 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -125,6 +125,17 @@
 		clocks = <&wifi32k>;
 		clock-names = "ext_clock";
 	};
+
+	cvbs-connector {
+		compatible = "composite-video-connector";
+		label = "cvbs";
+
+		port {
+			cvbs_connector_in: endpoint {
+				remote-endpoint = <&cvbs_vdac_out>;
+			};
+		};
+	};
 };
 
 /* This UART is brought out to the DB9 connector */
@@ -234,3 +245,7 @@
 	clocks = <&clkc CLKID_FCLK_DIV4>;
 	clock-names = "clkin0";
 };
+
+&cvbs_vdac_out {
+	remote-endpoint = <&cvbs_connector_in>;
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index ac5ad3b..5353a20 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -506,3 +506,7 @@
 		 <&clkc CLKID_FCLK_DIV2>;
 	clock-names = "core", "clkin0", "clkin1";
 };
+
+&vpu {
+	compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
index e99101a..7bd0538 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
@@ -117,6 +117,17 @@
 		clocks = <&wifi32k>;
 		clock-names = "ext_clock";
 	};
+
+	cvbs-connector {
+		compatible = "composite-video-connector";
+		label = "cvbs";
+
+		port {
+			cvbs_connector_in: endpoint {
+				remote-endpoint = <&cvbs_vdac_out>;
+			};
+		};
+	};
 };
 
 &uart_AO {
@@ -203,3 +214,7 @@
 	clocks = <&clkc CLKID_FCLK_DIV4>;
 	clock-names = "clkin0";
 };
+
+&cvbs_vdac_out {
+	remote-endpoint = <&cvbs_connector_in>;
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index 9f89b99..5c7a8fa 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -299,3 +299,7 @@
 		 <&clkc CLKID_FCLK_DIV2>;
 	clock-names = "core", "clkin0", "clkin1";
 };
+
+&vpu {
+	compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
index d320727..5b99749 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
@@ -90,6 +90,17 @@
 		compatible = "mmc-pwrseq-emmc";
 		reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>;
 	};
+
+	cvbs-connector {
+		compatible = "composite-video-connector";
+		label = "cvbs";
+
+		port {
+			cvbs_connector_in: endpoint {
+				remote-endpoint = <&cvbs_vdac_out>;
+			};
+		};
+	};
 };
 
 /* This UART is brought out to the DB9 connector */
@@ -167,3 +178,7 @@
 		max-speed = <1000>;
 	};
 };
+
+&cvbs_vdac_out {
+	remote-endpoint = <&cvbs_connector_in>;
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
index c1974bb..eb2f0c3 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
@@ -112,3 +112,7 @@
 		};
 	};
 };
+
+&vpu {
+	compatible = "amlogic,meson-gxm-vpu", "amlogic,meson-gx-vpu";
+};
-- 
1.9.1

[PATCH v2 3/4] dt-bindings: display: add Amlogic Meson DRM Bindings

[Neil Armstrong]

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../bindings/display/meson/meson-drm.txt           | 101 +++++++++++++++++++++
 1 file changed, 101 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/meson/meson-drm.txt

diff --git a/Documentation/devicetree/bindings/display/meson/meson-drm.txt b/Documentation/devicetree/bindings/display/meson/meson-drm.txt
new file mode 100644
index 0000000..e52869a
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/meson/meson-drm.txt
@@ -0,0 +1,101 @@
+Amlogic Meson Display Controller
+================================
+
+The Amlogic Meson Display controller is composed of several components
+that are going to be documented below:
+
+DMC|---------------VPU (Video Processing Unit)----------------|------HHI------|
+   | vd1   _______     _____________    _________________     |               |
+D  |-------|      |----|            |   |                |    |   HDMI PLL    |
+D  | vd2   | VIU  |    | Video Post |   | Video Encoders |<---|-----VCLK      |
+R  |-------|      |----| Processing |   |                |    |               |
+   | osd2  |      |    |            |---| Enci ----------|----|-----VDAC------|
+R  |-------| CSC  |----| Scalers    |   | Encp ----------|----|----HDMI-TX----|
+A  | osd1  |      |    | Blenders   |   | Encl ----------|----|---------------|
+M  |-------|______|----|____________|   |________________|    |               |
+___|__________________________________________________________|_______________|
+
+
+VIU: Video Input Unit
+---------------------
+
+The Video Input Unit is in charge of the pixel scanout from the DDR memory.
+It fetches the frames addresses, stride and parameters from the "Canvas" memory.
+This part is also in charge of the CSC (Colorspace Conversion).
+It can handle 2 OSD Planes and 2 Video Planes.
+
+VPP: Video Post Processing
+--------------------------
+
+The Video Post Processing is in charge of the scaling and blending of the
+various planes into a single pixel stream.
+There is a special "pre-blending" used by the video planes with a dedicated
+scaler and a "post-blending" to merge with the OSD Planes.
+The OSD planes also have a dedicated scaler for one of the OSD.
+
+VENC: Video Encoders
+--------------------
+
+The VENC is composed of the multiple pixel encoders :
+ - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
+ - ENCP : Progressive Video Encoder for HDMI
+ - ENCL : LCD LVDS Encoder
+The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and clock
+tree and provides the scanout clock to the VPP and VIU.
+The ENCI is connected to a single VDAC for Composite Output.
+The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
+
+Device Tree Bindings:
+---------------------
+
+VPU: Video Processing Unit
+--------------------------
+
+Required properties:
+- compatible: value should be different for each SoC family as :
+	- GXBB (S905) : "amlogic,meson-gxbb-vpu"
+	- GXL (S905X, S905D) : "amlogic,meson-gxl-vpu"
+	- GXM (S912) : "amlogic,meson-gxm-vpu"
+	followed by the common "amlogic,meson-gx-vpu"
+- reg: base address and size of he following memory-mapped regions :
+	- vpu
+	- hhi
+	- dmc
+- reg-names: should contain the names of the previous memory regions
+- interrupts: should contain the VENC Vsync interrupt number
+
+- ports: A ports node with endpoint definitions as defined in
+  Documentation/devicetree/bindings/media/video-interfaces.txt.
+  The first port should be connected to a CVBS connector endpoint if available.
+
+Example:
+
+tv: connector {
+	compatible = "composite-video-connector";
+	label = "cvbs";
+
+	port {
+		tv_connector_in: endpoint {
+			remote-endpoint = <&cvbs_vdac_out>;
+		};
+	};
+};
+
+vpu: vpu at d0100000 {
+	compatible = "amlogic,meson-gxbb-vpu";
+	reg = <0x0 0xd0100000 0x0 0x100000>,
+	      <0x0 0xc883c000 0x0 0x1000>,
+	      <0x0 0xc8838000 0x0 0x1000>;
+	reg-names = "vpu", "hhi", "dmc";
+	interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	/* CVBS VDAC output port */
+	port at 0 {
+		cvbs_vdac_out: endpoint {
+			reg = <0>;
+			remote-endpoint = <&tv_connector_in>;
+		};
+	};
+};
-- 
1.9.1

[PATCH v2 4/4] MAINTAINERS: add entry for Amlogic DRM drivers

[Neil Armstrong]

Add myself as maintainer for Amlogic DRM drivers.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1cd38a7..b2486fb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4078,6 +4078,15 @@ S:	Supported
 F:	drivers/gpu/drm/sun4i/
 F:	Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
 
+DRM DRIVERS FOR AMLOGIC SOCS
+M:	Neil Armstrong <narmstrong@baylibre.com>
+L:	dri-devel at lists.freedesktop.org
+L:	linux-amlogic at lists.infradead.org
+W:	http://linux-meson.com/
+S:	Supported
+F:	drivers/gpu/drm/meson/
+F:	Documentation/devicetree/bindings/display/meson/meson-drm.txt
+
 DRM DRIVERS FOR EXYNOS
 M:	Inki Dae <inki.dae@samsung.com>
 M:	Joonyoung Shim <jy0922.shim@samsung.com>
-- 
1.9.1

[PATCH v2 3/4] dt-bindings: display: add Amlogic Meson DRM Bindings

[Laurent Pinchart]

Hi Neil,

Thank you for the patch.

On Wednesday 30 Nov 2016 16:43:44 Neil Armstrong wrote:
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> .../bindings/display/meson/meson-drm.txt           | 101 ++++++++++++++++++
> 1 file changed, 101 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/meson/meson-drm.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/meson/meson-drm.txt
> b/Documentation/devicetree/bindings/display/meson/meson-drm.txt new file
> mode 100644
> index 0000000..e52869a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/meson/meson-drm.txt
> @@ -0,0 +1,101 @@
> +Amlogic Meson Display Controller
> +================================
> +
> +The Amlogic Meson Display controller is composed of several components
> +that are going to be documented below:
> +
> +DMC|---------------VPU (Video Processing Unit)------------|------HHI------|
> +   | vd1   _______     _____________    _____________     |               |
> +D  |-------|      |----|            |   |            |    |   HDMI PLL    |
> +D  | vd2   | VIU  |    | Video Post |   | Video Encs |<---|-----VCLK      |
> +R  |-------|      |----| Processing |   |            |    |               |
> +   | osd2  |      |    |            |---| Enci ------|----|-----VDAC------|
> +R  |-------| CSC  |----| Scalers    |   | Encp ------|----|----HDMI-TX----|
> +A  | osd1  |      |    | Blenders   |   | Encl-------|----|---------------|
> +M  |-------|______|----|____________|   |____________|    |               |
> +___|______________________________________________________|_______________|
> +
> +
> +VIU: Video Input Unit
> +---------------------
> +
> +The Video Input Unit is in charge of the pixel scanout from the DDR memory.
> +It fetches the frames addresses, stride and parameters from the "Canvas"
> memory.
> +This part is also in charge of the CSC (Colorspace Conversion).
> +It can handle 2 OSD Planes and 2 Video Planes.
> +
> +VPP: Video Post Processing
> +--------------------------
> +
> +The Video Post Processing is in charge of the scaling and blending of the
> +various planes into a single pixel stream.
> +There is a special "pre-blending" used by the video planes with a dedicated
> +scaler and a "post-blending" to merge with the OSD Planes.
> +The OSD planes also have a dedicated scaler for one of the OSD.
> +
> +VENC: Video Encoders
> +--------------------
> +
> +The VENC is composed of the multiple pixel encoders :
> + - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
> + - ENCP : Progressive Video Encoder for HDMI
> + - ENCL : LCD LVDS Encoder
> +The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and
> clock
> +tree and provides the scanout clock to the VPP and VIU.
> +The ENCI is connected to a single VDAC for Composite Output.
> +The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
> +
> +Device Tree Bindings:
> +---------------------
> +
> +VPU: Video Processing Unit
> +--------------------------
> +
> +Required properties:
> +- compatible: value should be different for each SoC family as :
> +	- GXBB (S905) : "amlogic,meson-gxbb-vpu"
> +	- GXL (S905X, S905D) : "amlogic,meson-gxl-vpu"
> +	- GXM (S912) : "amlogic,meson-gxm-vpu"
> +	followed by the common "amlogic,meson-gx-vpu"
> +- reg: base address and size of he following memory-mapped regions :
> +	- vpu
> +	- hhi
> +	- dmc
> +- reg-names: should contain the names of the previous memory regions
> +- interrupts: should contain the VENC Vsync interrupt number
> +
> +- ports: A ports node with endpoint definitions as defined in
> +  Documentation/devicetree/bindings/media/video-interfaces.txt.
> +  The first port should be connected to a CVBS connector endpoint if
> available.

This is a bit vague, I propose clarifying it with a description similar to the 
one in the renesas,du.txt bindings.

Required nodes:

The connections to the VPU output video ports are modeled using the OF graph
bindings specified in Documentation/devicetree/bindings/graph.txt.

The following table lists for each supported model the port number
corresponding to each DU output.

                Port 0          Port1           Port2           Port3
-----------------------------------------------------------------------------
 R8A7779 (H1)   DPAD 0          DPAD 1          -               -
 R8A7790 (H2)   DPAD            LVDS 0          LVDS 1          -
 R8A7791 (M2-W) DPAD            LVDS 0          -               -
 R8A7792 (V2H)  DPAD 0          DPAD 1          -               -
 R8A7793 (M2-N) DPAD            LVDS 0          -               -
 R8A7794 (E2)   DPAD 0          DPAD 1          -               -
 R8A7795 (H3)   DPAD            HDMI 0          HDMI 1          LVDS
 R8A7796 (M3-W) DPAD            HDMI            LVDS            -

(You should obviously replace the table with Amlogic data)

It doesn't matter if the current driver implementation only supports CVBS, the 
DT bindings can already document the other ports.

> +
> +Example:
> +
> +tv: connector {
> +	compatible = "composite-video-connector";
> +	label = "cvbs";

I'd remove the label here, as it doesn't bring any additional information. 
Unless the board you're using has a label for the connector, in case that 
label should be used.

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	port {
> +		tv_connector_in: endpoint {
> +			remote-endpoint = <&cvbs_vdac_out>;
> +		};
> +	};
> +};
> +
> +vpu: vpu at d0100000 {
> +	compatible = "amlogic,meson-gxbb-vpu";
> +	reg = <0x0 0xd0100000 0x0 0x100000>,
> +	      <0x0 0xc883c000 0x0 0x1000>,
> +	      <0x0 0xc8838000 0x0 0x1000>;
> +	reg-names = "vpu", "hhi", "dmc";
> +	interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	/* CVBS VDAC output port */
> +	port at 0 {
> +		cvbs_vdac_out: endpoint {
> +			reg = <0>;
> +			remote-endpoint = <&tv_connector_in>;
> +		};
> +	};
> +};

-- 
Regards,

Laurent Pinchart

[PATCH v2 3/4] dt-bindings: display: add Amlogic Meson DRM Bindings

[Laurent Pinchart]

Hi Neil,

On Wednesday 30 Nov 2016 16:43:44 Neil Armstrong wrote:
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  .../bindings/display/meson/meson-drm.txt           | 101 ++++++++++++++++++

I forgot to mention that the file should not be named meson-drm.txt as DRM is 
a Linux-specific concept. You can name it meson.txt, but a better option would 
be amlogic,meson.txt. By the way does it really need a subdirectory ?

>  1 file changed, 101 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/meson/meson-drm.txt

-- 
Regards,

Laurent Pinchart

[PATCH v2 4/4] MAINTAINERS: add entry for Amlogic DRM drivers

[Laurent Pinchart]

Hi Neil,

Thank you for the patch.

On Wednesday 30 Nov 2016 16:43:45 Neil Armstrong wrote:
> Add myself as maintainer for Amlogic DRM drivers.
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

After updating this patch due to the rename I proposed in patch 3/4,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  MAINTAINERS | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1cd38a7..b2486fb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4078,6 +4078,15 @@ S:	Supported
>  F:	drivers/gpu/drm/sun4i/
>  F:	Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> 
> +DRM DRIVERS FOR AMLOGIC SOCS
> +M:	Neil Armstrong <narmstrong@baylibre.com>
> +L:	dri-devel at lists.freedesktop.org
> +L:	linux-amlogic at lists.infradead.org
> +W:	http://linux-meson.com/
> +S:	Supported
> +F:	drivers/gpu/drm/meson/
> +F:	Documentation/devicetree/bindings/display/meson/meson-drm.txt
> +
>  DRM DRIVERS FOR EXYNOS
>  M:	Inki Dae <inki.dae@samsung.com>
>  M:	Joonyoung Shim <jy0922.shim@samsung.com>

-- 
Regards,

Laurent Pinchart

[PATCH v2 3/4] dt-bindings: display: add Amlogic Meson DRM Bindings

[Neil Armstrong]

Hi Laurent,
On 11/30/2016 04:58 PM, Laurent Pinchart wrote:
> Hi Neil,
> 
> On Wednesday 30 Nov 2016 16:43:44 Neil Armstrong wrote:
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  .../bindings/display/meson/meson-drm.txt           | 101 ++++++++++++++++++
> 
> I forgot to mention that the file should not be named meson-drm.txt as DRM is 
> a Linux-specific concept. You can name it meson.txt, but a better option would 
> be amlogic,meson.txt. By the way does it really need a subdirectory ?

I took example of the sun4i layout the naming, and no it does not need a subdirector..

I will move it to amlogic,meson.txt, seems far better.

Thanks,
Neil

>>  1 file changed, 101 insertions(+)
>>  create mode 100644
>> Documentation/devicetree/bindings/display/meson/meson-drm.txt
> 

[PATCH v2 2/4] ARM64: dts: meson-gx: Add Graphic Controller nodes

[Laurent Pinchart]

Hi Neil,

Thank you for the patch.

On Wednesday 30 Nov 2016 16:43:43 Neil Armstrong wrote:
> Add Video Processing Unit and CVBS Output nodes, and enable CVBS on selected
> boards.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gx.dtsi             | 19 ++++++++++++++++
>  .../arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts | 15 +++++++++++++++
>  arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi      | 15 +++++++++++++++
>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi           |  4 ++++
>  arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts | 15 +++++++++++++++
>  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi            |  4 ++++
>  arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts   | 15 +++++++++++++++
>  arch/arm64/boot/dts/amlogic/meson-gxm.dtsi            |  4 ++++
>  8 files changed, 91 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi index fc033c0..a27f881 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> @@ -356,5 +356,24 @@
>  				status = "disabled";
>  			};
>  		};
> +
> +		vpu: vpu at d0100000 {
> +			compatible = "amlogic,meson-gx-vpu";
> +			reg = <0x0 0xd0100000 0x0 0x100000>,
> +			      <0x0 0xc883c000 0x0 0x1000>,
> +			      <0x0 0xc8838000 0x0 0x1000>;
> +			reg-names = "vpu", "hhi", "dmc";
> +			interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			/* CVBS VDAC output port */
> +			port at 0 {
> +				reg = <0>;
> +
> +				cvbs_vdac_out: endpoint {
> +				};

Endpoints require a remote-endpoint property. You should move the endpoint to 
board DT files.

> +			};
> +		};
>  	};
>  };
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts index
> 9696820..390f7db 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> @@ -142,6 +142,17 @@
>  		clocks = <&wifi32k>;
>  		clock-names = "ext_clock";
>  	};
> +
> +	cvbs-connector {
> +		compatible = "composite-video-connector";
> +		label = "cvbs";

Unless the board has a label for the connector (either on the board, on the 
casing or in the user manual) I'd leave this out. Same comment for the other 
boards.

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +		port {
> +			cvbs_connector_in: endpoint {
> +				remote-endpoint = <&cvbs_vdac_out>;
> +			};
> +		};
> +	};
>  };
> 
>  &uart_AO {
> @@ -229,3 +240,7 @@
>  	clocks = <&clkc CLKID_FCLK_DIV4>;
>  	clock-names = "clkin0";
>  };
> +
> +&cvbs_vdac_out {
> +	remote-endpoint = <&cvbs_connector_in>;
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi index 203be28..44bdebf
> 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> @@ -125,6 +125,17 @@
>  		clocks = <&wifi32k>;
>  		clock-names = "ext_clock";
>  	};
> +
> +	cvbs-connector {
> +		compatible = "composite-video-connector";
> +		label = "cvbs";
> +
> +		port {
> +			cvbs_connector_in: endpoint {
> +				remote-endpoint = <&cvbs_vdac_out>;
> +			};
> +		};
> +	};
>  };
> 
>  /* This UART is brought out to the DB9 connector */
> @@ -234,3 +245,7 @@
>  	clocks = <&clkc CLKID_FCLK_DIV4>;
>  	clock-names = "clkin0";
>  };
> +
> +&cvbs_vdac_out {
> +	remote-endpoint = <&cvbs_connector_in>;
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi index ac5ad3b..5353a20 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> @@ -506,3 +506,7 @@
>  		 <&clkc CLKID_FCLK_DIV2>;
>  	clock-names = "core", "clkin0", "clkin1";
>  };
> +
> +&vpu {
> +	compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
> b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts index
> e99101a..7bd0538 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
> @@ -117,6 +117,17 @@
>  		clocks = <&wifi32k>;
>  		clock-names = "ext_clock";
>  	};
> +
> +	cvbs-connector {
> +		compatible = "composite-video-connector";
> +		label = "cvbs";
> +
> +		port {
> +			cvbs_connector_in: endpoint {
> +				remote-endpoint = <&cvbs_vdac_out>;
> +			};
> +		};
> +	};
>  };
> 
>  &uart_AO {
> @@ -203,3 +214,7 @@
>  	clocks = <&clkc CLKID_FCLK_DIV4>;
>  	clock-names = "clkin0";
>  };
> +
> +&cvbs_vdac_out {
> +	remote-endpoint = <&cvbs_connector_in>;
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi index 9f89b99..5c7a8fa 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> @@ -299,3 +299,7 @@
>  		 <&clkc CLKID_FCLK_DIV2>;
>  	clock-names = "core", "clkin0", "clkin1";
>  };
> +
> +&vpu {
> +	compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts index
> d320727..5b99749 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> @@ -90,6 +90,17 @@
>  		compatible = "mmc-pwrseq-emmc";
>  		reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>;
>  	};
> +
> +	cvbs-connector {
> +		compatible = "composite-video-connector";
> +		label = "cvbs";
> +
> +		port {
> +			cvbs_connector_in: endpoint {
> +				remote-endpoint = <&cvbs_vdac_out>;
> +			};
> +		};
> +	};
>  };
> 
>  /* This UART is brought out to the DB9 connector */
> @@ -167,3 +178,7 @@
>  		max-speed = <1000>;
>  	};
>  };
> +
> +&cvbs_vdac_out {
> +	remote-endpoint = <&cvbs_connector_in>;
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi index c1974bb..eb2f0c3 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
> @@ -112,3 +112,7 @@
>  		};
>  	};
>  };
> +
> +&vpu {
> +	compatible = "amlogic,meson-gxm-vpu", "amlogic,meson-gx-vpu";
> +};

-- 
Regards,

Laurent Pinchart

[PATCH v2 1/4] drm: Add support for Amlogic Meson Graphic Controller

[Laurent Pinchart]

Hi Neil,

Thank you for the patch.

I'm afraid I don't have time for a complete review, but could you please get 
rid of the of_machine_is_compatible() calls and match on the VPU compatible 
string instead ?

On Wednesday 30 Nov 2016 16:43:42 Neil Armstrong wrote:
> The Amlogic Meson Display controller is composed of several components :
> 
> DMC|---------------VPU (Video Processing
> Unit)----------------|------HHI------| |
> vd1   _______     _____________    _________________     |               |
> D  |-------|      |----|            |   |                |    |   HDMI
> PLL    | D  | vd2   | VIU  |    | Video Post |   | Video Encoders
> |<---|-----VCLK      | R  |-------|      |----| Processing
> |   |                |    |               | |
> osd2  |      |    |            |---| Enci ----------|----|-----VDAC------|
> R  |-------| CSC  |----| Scalers    |   | Encp
> ----------|----|----HDMI-TX----| A  | osd1  |      |    | Blenders   |   |
> Encl ----------|----|---------------|
> M  |-------|______|----|____________|   |________________|    |            
>    |
> ___|__________________________________________________________|____________
> ___|
> 
> VIU: Video Input Unit
> ---------------------
> 
> The Video Input Unit is in charge of the pixel scanout from the DDR memory.
> It fetches the frames addresses, stride and parameters from the "Canvas"
> memory. This part is also in charge of the CSC (Colorspace Conversion).
> It can handle 2 OSD Planes and 2 Video Planes.
> 
> VPP: Video Post Processing
> --------------------------
> 
> The Video Post Processing is in charge of the scaling and blending of the
> various planes into a single pixel stream.
> There is a special "pre-blending" used by the video planes with a dedicated
> scaler and a "post-blending" to merge with the OSD Planes.
> The OSD planes also have a dedicated scaler for one of the OSD.
> 
> VENC: Video Encoders
> --------------------
> 
> The VENC is composed of the multiple pixel encoders :
>  - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
>  - ENCP : Progressive Video Encoder for HDMI
>  - ENCL : LCD LVDS Encoder
> The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and clock
> tree and provides the scanout clock to the VPP and VIU.
> The ENCI is connected to a single VDAC for Composite Output.
> The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
> 
> This driver is a DRM/KMS driver using the following DRM components :
>  - GEM-CMA
>  - PRIME-CMA
>  - Atomic Modesetting
>  - FBDev-CMA
> 
> For the following SoCs :
>  - GXBB Family (S905)
>  - GXL Family (S905X, S905D)
>  - GXM Family (S912)
> 
> The current driver only supports the CVBS PAL/NTSC output modes, but the
> CRTC/Planes management should support bigger modes.
> But Advanced Colorspace Conversion, Scaling and HDMI Modes will be added in
> a second time.
> 
> The Device Tree bindings makes use of the endpoints video interface
> definitions to connect to the optional CVBS and in the future the HDMI
> Connector nodes.
> 
> HDMI Support is planned for a next release.
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/Kconfig                 |    2 +
>  drivers/gpu/drm/Makefile                |    1 +
>  drivers/gpu/drm/meson/Kconfig           |    9 +
>  drivers/gpu/drm/meson/Makefile          |    4 +
>  drivers/gpu/drm/meson/meson_canvas.c    |   68 ++
>  drivers/gpu/drm/meson/meson_canvas.h    |   42 +
>  drivers/gpu/drm/meson/meson_crtc.c      |  208 +++++
>  drivers/gpu/drm/meson/meson_crtc.h      |   32 +
>  drivers/gpu/drm/meson/meson_drv.c       |  343 ++++++++
>  drivers/gpu/drm/meson/meson_drv.h       |   60 ++
>  drivers/gpu/drm/meson/meson_plane.c     |  230 +++++
>  drivers/gpu/drm/meson/meson_plane.h     |   30 +
>  drivers/gpu/drm/meson/meson_registers.h | 1395 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/meson/meson_vclk.c      |  167 ++++
>  drivers/gpu/drm/meson/meson_vclk.h      |   34 +
>  drivers/gpu/drm/meson/meson_venc.c      |  254 ++++++
>  drivers/gpu/drm/meson/meson_venc.h      |   72 ++
>  drivers/gpu/drm/meson/meson_venc_cvbs.c |  293 +++++++
>  drivers/gpu/drm/meson/meson_venc_cvbs.h |   41 +
>  drivers/gpu/drm/meson/meson_viu.c       |  331 ++++++++
>  drivers/gpu/drm/meson/meson_viu.h       |   64 ++
>  drivers/gpu/drm/meson/meson_vpp.c       |  162 ++++
>  drivers/gpu/drm/meson/meson_vpp.h       |   35 +
>  23 files changed, 3877 insertions(+)
>  create mode 100644 drivers/gpu/drm/meson/Kconfig
>  create mode 100644 drivers/gpu/drm/meson/Makefile
>  create mode 100644 drivers/gpu/drm/meson/meson_canvas.c
>  create mode 100644 drivers/gpu/drm/meson/meson_canvas.h
>  create mode 100644 drivers/gpu/drm/meson/meson_crtc.c
>  create mode 100644 drivers/gpu/drm/meson/meson_crtc.h
>  create mode 100644 drivers/gpu/drm/meson/meson_drv.c
>  create mode 100644 drivers/gpu/drm/meson/meson_drv.h
>  create mode 100644 drivers/gpu/drm/meson/meson_plane.c
>  create mode 100644 drivers/gpu/drm/meson/meson_plane.h
>  create mode 100644 drivers/gpu/drm/meson/meson_registers.h
>  create mode 100644 drivers/gpu/drm/meson/meson_vclk.c
>  create mode 100644 drivers/gpu/drm/meson/meson_vclk.h
>  create mode 100644 drivers/gpu/drm/meson/meson_venc.c
>  create mode 100644 drivers/gpu/drm/meson/meson_venc.h
>  create mode 100644 drivers/gpu/drm/meson/meson_venc_cvbs.c
>  create mode 100644 drivers/gpu/drm/meson/meson_venc_cvbs.h
>  create mode 100644 drivers/gpu/drm/meson/meson_viu.c
>  create mode 100644 drivers/gpu/drm/meson/meson_viu.h
>  create mode 100644 drivers/gpu/drm/meson/meson_vpp.c
>  create mode 100644 drivers/gpu/drm/meson/meson_vpp.h

-- 
Regards,

Laurent Pinchart

[PATCH v2 3/4] dt-bindings: display: add Amlogic Meson DRM Bindings

[Neil Armstrong]

Hi Laurent,

On 11/30/2016 04:56 PM, Laurent Pinchart wrote:
> Hi Neil,
> 
> Thank you for the patch.
> 
> On Wednesday 30 Nov 2016 16:43:44 Neil Armstrong wrote:
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> .../bindings/display/meson/meson-drm.txt           | 101 ++++++++++++++++++
>> 1 file changed, 101 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/display/meson/meson-drm.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/meson/meson-drm.txt
>> b/Documentation/devicetree/bindings/display/meson/meson-drm.txt new file
>> mode 100644
>> index 0000000..e52869a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/meson/meson-drm.txt
>> @@ -0,0 +1,101 @@

[...]

>> +
>> +Device Tree Bindings:
>> +---------------------
>> +
>> +VPU: Video Processing Unit
>> +--------------------------
>> +
>> +Required properties:
>> +- compatible: value should be different for each SoC family as :
>> +	- GXBB (S905) : "amlogic,meson-gxbb-vpu"
>> +	- GXL (S905X, S905D) : "amlogic,meson-gxl-vpu"
>> +	- GXM (S912) : "amlogic,meson-gxm-vpu"
>> +	followed by the common "amlogic,meson-gx-vpu"
>> +- reg: base address and size of he following memory-mapped regions :
>> +	- vpu
>> +	- hhi
>> +	- dmc
>> +- reg-names: should contain the names of the previous memory regions
>> +- interrupts: should contain the VENC Vsync interrupt number
>> +
>> +- ports: A ports node with endpoint definitions as defined in
>> +  Documentation/devicetree/bindings/media/video-interfaces.txt.
>> +  The first port should be connected to a CVBS connector endpoint if
>> available.
> 
> This is a bit vague, I propose clarifying it with a description similar to the 
> one in the renesas,du.txt bindings.
> 
> Required nodes:
> 
> The connections to the VPU output video ports are modeled using the OF graph
> bindings specified in Documentation/devicetree/bindings/graph.txt.
> 
> The following table lists for each supported model the port number
> corresponding to each DU output.
> 
>                 Port 0          Port1           Port2           Port3
> -----------------------------------------------------------------------------
>  R8A7779 (H1)   DPAD 0          DPAD 1          -               -
>  R8A7790 (H2)   DPAD            LVDS 0          LVDS 1          -
>  R8A7791 (M2-W) DPAD            LVDS 0          -               -
>  R8A7792 (V2H)  DPAD 0          DPAD 1          -               -
>  R8A7793 (M2-N) DPAD            LVDS 0          -               -
>  R8A7794 (E2)   DPAD 0          DPAD 1          -               -
>  R8A7795 (H3)   DPAD            HDMI 0          HDMI 1          LVDS
>  R8A7796 (M3-W) DPAD            HDMI            LVDS            -
> 
> (You should obviously replace the table with Amlogic data)
> 
> It doesn't matter if the current driver implementation only supports CVBS, the 
> DT bindings can already document the other ports.
> 

Ok, it's a pretty table ! Will integrate this.

>> +
>> +Example:
>> +
>> +tv: connector {
>> +	compatible = "composite-video-connector";
>> +	label = "cvbs";
> 
> I'd remove the label here, as it doesn't bring any additional information. 
> Unless the board you're using has a label for the connector, in case that 
> label should be used.

Indeed, I already removed it in the dts.

> 
> Apart from that,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 

[...]

Thanks for the review,
Neil

[PATCH v2 1/4] drm: Add support for Amlogic Meson Graphic Controller

[Neil Armstrong]

On 11/30/2016 05:03 PM, Laurent Pinchart wrote:
> Hi Neil,
> 
> Thank you for the patch.
> 
> I'm afraid I don't have time for a complete review, but could you please get 
> rid of the of_machine_is_compatible() calls and match on the VPU compatible 
> string instead ?

Oops, I knew I forgot to change this one...

I will get rid of these for the v3.

Thanks,
Neil

[PATCH v2 2/4] ARM64: dts: meson-gx: Add Graphic Controller nodes

[Neil Armstrong]

On 11/30/2016 05:02 PM, Laurent Pinchart wrote:
> Hi Neil,
> 
> Thank you for the patch.
> 
> On Wednesday 30 Nov 2016 16:43:43 Neil Armstrong wrote:
>> Add Video Processing Unit and CVBS Output nodes, and enable CVBS on selected
>> boards.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  arch/arm64/boot/dts/amlogic/meson-gx.dtsi             | 19 ++++++++++++++++
>>  .../arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts | 15 +++++++++++++++
>>  arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi      | 15 +++++++++++++++
>>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi           |  4 ++++
>>  arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts | 15 +++++++++++++++
>>  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi            |  4 ++++
>>  arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts   | 15 +++++++++++++++
>>  arch/arm64/boot/dts/amlogic/meson-gxm.dtsi            |  4 ++++
>>  8 files changed, 91 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
>> b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi index fc033c0..a27f881 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
>> @@ -356,5 +356,24 @@
>>  				status = "disabled";
>>  			};
>>  		};
>> +
>> +		vpu: vpu at d0100000 {
>> +			compatible = "amlogic,meson-gx-vpu";
>> +			reg = <0x0 0xd0100000 0x0 0x100000>,
>> +			      <0x0 0xc883c000 0x0 0x1000>,
>> +			      <0x0 0xc8838000 0x0 0x1000>;
>> +			reg-names = "vpu", "hhi", "dmc";
>> +			interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			/* CVBS VDAC output port */
>> +			port at 0 {
>> +				reg = <0>;
>> +
>> +				cvbs_vdac_out: endpoint {
>> +				};
> 
> Endpoints require a remote-endpoint property. You should move the endpoint to 
> board DT files.

OK, I was wondering, it looked dirty to me.

> 
>> +			};
>> +		};
>>  	};
>>  };
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
>> b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts index
>> 9696820..390f7db 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
>> @@ -142,6 +142,17 @@
>>  		clocks = <&wifi32k>;
>>  		clock-names = "ext_clock";
>>  	};
>> +
>> +	cvbs-connector {
>> +		compatible = "composite-video-connector";
>> +		label = "cvbs";
> 
> Unless the board has a label for the connector (either on the board, on the 
> casing or in the user manual) I'd leave this out. Same comment for the other 
> boards.

OK,

Thanks,
Neil

> 
> Apart from that,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
>> +
>> +		port {
>> +			cvbs_connector_in: endpoint {
>> +				remote-endpoint = <&cvbs_vdac_out>;
>> +			};
>> +		};
>> +	};
>>  };
>>
>>  &uart_AO {
>> @@ -229,3 +240,7 @@
>>  	clocks = <&clkc CLKID_FCLK_DIV4>;
>>  	clock-names = "clkin0";
>>  };
>> +
>> +&cvbs_vdac_out {
>> +	remote-endpoint = <&cvbs_connector_in>;
>> +};
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
>> b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi index 203be28..44bdebf
>> 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
>> @@ -125,6 +125,17 @@
>>  		clocks = <&wifi32k>;
>>  		clock-names = "ext_clock";
>>  	};
>> +
>> +	cvbs-connector {
>> +		compatible = "composite-video-connector";
>> +		label = "cvbs";
>> +
>> +		port {
>> +			cvbs_connector_in: endpoint {
>> +				remote-endpoint = <&cvbs_vdac_out>;
>> +			};
>> +		};
>> +	};
>>  };
>>
>>  /* This UART is brought out to the DB9 connector */
>> @@ -234,3 +245,7 @@
>>  	clocks = <&clkc CLKID_FCLK_DIV4>;
>>  	clock-names = "clkin0";
>>  };
>> +
>> +&cvbs_vdac_out {
>> +	remote-endpoint = <&cvbs_connector_in>;
>> +};
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi index ac5ad3b..5353a20 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> @@ -506,3 +506,7 @@
>>  		 <&clkc CLKID_FCLK_DIV2>;
>>  	clock-names = "core", "clkin0", "clkin1";
>>  };
>> +
>> +&vpu {
>> +	compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
>> +};
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
>> b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts index
>> e99101a..7bd0538 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
>> @@ -117,6 +117,17 @@
>>  		clocks = <&wifi32k>;
>>  		clock-names = "ext_clock";
>>  	};
>> +
>> +	cvbs-connector {
>> +		compatible = "composite-video-connector";
>> +		label = "cvbs";
>> +
>> +		port {
>> +			cvbs_connector_in: endpoint {
>> +				remote-endpoint = <&cvbs_vdac_out>;
>> +			};
>> +		};
>> +	};
>>  };
>>
>>  &uart_AO {
>> @@ -203,3 +214,7 @@
>>  	clocks = <&clkc CLKID_FCLK_DIV4>;
>>  	clock-names = "clkin0";
>>  };
>> +
>> +&cvbs_vdac_out {
>> +	remote-endpoint = <&cvbs_connector_in>;
>> +};
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
>> b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi index 9f89b99..5c7a8fa 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
>> @@ -299,3 +299,7 @@
>>  		 <&clkc CLKID_FCLK_DIV2>;
>>  	clock-names = "core", "clkin0", "clkin1";
>>  };
>> +
>> +&vpu {
>> +	compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
>> +};
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
>> b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts index
>> d320727..5b99749 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
>> @@ -90,6 +90,17 @@
>>  		compatible = "mmc-pwrseq-emmc";
>>  		reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>;
>>  	};
>> +
>> +	cvbs-connector {
>> +		compatible = "composite-video-connector";
>> +		label = "cvbs";
>> +
>> +		port {
>> +			cvbs_connector_in: endpoint {
>> +				remote-endpoint = <&cvbs_vdac_out>;
>> +			};
>> +		};
>> +	};
>>  };
>>
>>  /* This UART is brought out to the DB9 connector */
>> @@ -167,3 +178,7 @@
>>  		max-speed = <1000>;
>>  	};
>>  };
>> +
>> +&cvbs_vdac_out {
>> +	remote-endpoint = <&cvbs_connector_in>;
>> +};
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
>> b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi index c1974bb..eb2f0c3 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
>> @@ -112,3 +112,7 @@
>>  		};
>>  	};
>>  };
>> +
>> +&vpu {
>> +	compatible = "amlogic,meson-gxm-vpu", "amlogic,meson-gx-vpu";
>> +};
> 

[PATCH v2 3/4] dt-bindings: display: add Amlogic Meson DRM Bindings

[Kevin Hilman]

Neil Armstrong <narmstrong@baylibre.com> writes:

> Hi Laurent,
> On 11/30/2016 04:58 PM, Laurent Pinchart wrote:
>> Hi Neil,
>> 
>> On Wednesday 30 Nov 2016 16:43:44 Neil Armstrong wrote:
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>>  .../bindings/display/meson/meson-drm.txt           | 101 ++++++++++++++++++
>> 
>> I forgot to mention that the file should not be named meson-drm.txt as DRM is 
>> a Linux-specific concept. You can name it meson.txt, but a better option would 
>> be amlogic,meson.txt. By the way does it really need a subdirectory ?
>
> I took example of the sun4i layout the naming, and no it does not need a subdirector..
>
> I will move it to amlogic,meson.txt, seems far better.
>

To me, amlogic,meson is redundant.  Probably should be amlogic,vpu.txt?

Kevin

[PATCH v2 3/4] dt-bindings: display: add Amlogic Meson DRM Bindings

[Neil Armstrong]

On 11/30/2016 05:21 PM, Kevin Hilman wrote:
> Neil Armstrong <narmstrong@baylibre.com> writes:
> 
>> Hi Laurent,
>> On 11/30/2016 04:58 PM, Laurent Pinchart wrote:
>>> Hi Neil,
>>>
>>> On Wednesday 30 Nov 2016 16:43:44 Neil Armstrong wrote:
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>> ---
>>>>  .../bindings/display/meson/meson-drm.txt           | 101 ++++++++++++++++++
>>>
>>> I forgot to mention that the file should not be named meson-drm.txt as DRM is 
>>> a Linux-specific concept. You can name it meson.txt, but a better option would 
>>> be amlogic,meson.txt. By the way does it really need a subdirectory ?
>>
>> I took example of the sun4i layout the naming, and no it does not need a subdirector..
>>
>> I will move it to amlogic,meson.txt, seems far better.
>>
> 
> To me, amlogic,meson is redundant.  Probably should be amlogic,vpu.txt?
> 
> Kevin
> 

Yes, seems more coherent.

Thanks,
Neil