diff mbox

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

Message ID 1480520625-13269-4-git-send-email-narmstrong@baylibre.com (mailing list archive)
State Superseded
Headers show

Commit Message

Neil Armstrong Nov. 30, 2016, 3:43 p.m. UTC
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

Comments

Laurent Pinchart Nov. 30, 2016, 3:56 p.m. UTC | #1
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@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@0 {
> +		cvbs_vdac_out: endpoint {
> +			reg = <0>;
> +			remote-endpoint = <&tv_connector_in>;
> +		};
> +	};
> +};
Laurent Pinchart Nov. 30, 2016, 3:58 p.m. UTC | #2
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
Neil Armstrong Nov. 30, 2016, 4 p.m. UTC | #3
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
>
Neil Armstrong Nov. 30, 2016, 4:04 p.m. UTC | #4
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
Kevin Hilman Nov. 30, 2016, 4:21 p.m. UTC | #5
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
Neil Armstrong Nov. 30, 2016, 4:22 p.m. UTC | #6
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
diff mbox

Patch

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@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@0 {
+		cvbs_vdac_out: endpoint {
+			reg = <0>;
+			remote-endpoint = <&tv_connector_in>;
+		};
+	};
+};