devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Neil Armstrong <narmstrong@baylibre.com>
Cc: devicetree@vger.kernel.org, Xing.Xu@amlogic.com,
	victor.wan@amlogic.com, khilman@baylibre.com,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	jerry.cao@amlogic.com, carlo@caione.org,
	linux-amlogic@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 3/4] dt-bindings: display: add Amlogic Meson DRM Bindings
Date: Wed, 30 Nov 2016 17:56:43 +0200	[thread overview]
Message-ID: <13104644.f9boR54smD@avalon> (raw)
In-Reply-To: <1480520625-13269-4-git-send-email-narmstrong@baylibre.com>

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>;
> +		};
> +	};
> +};

-- 
Regards,

Laurent Pinchart

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

  reply	other threads:[~2016-11-30 15:56 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-30 15:43 [PATCH v2 0/4] drm: Add support for the Amlogic Video Processing Unit Neil Armstrong
2016-11-30 15:43 ` [PATCH v2 2/4] ARM64: dts: meson-gx: Add Graphic Controller nodes Neil Armstrong
     [not found]   ` <1480520625-13269-3-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
2016-11-30 16:02     ` Laurent Pinchart
2016-11-30 16:07       ` Neil Armstrong
     [not found] ` <1480520625-13269-1-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
2016-11-30 15:43   ` [PATCH v2 3/4] dt-bindings: display: add Amlogic Meson DRM Bindings Neil Armstrong
2016-11-30 15:56     ` Laurent Pinchart [this message]
2016-11-30 16:04       ` Neil Armstrong
2016-11-30 15:58     ` Laurent Pinchart
2016-11-30 16:00       ` Neil Armstrong
     [not found]         ` <2e1b16c6-eec5-8cf3-5795-d6de02f36570-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
2016-11-30 16:21           ` Kevin Hilman
     [not found]             ` <m21sxtkkg1.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
2016-11-30 16:22               ` Neil Armstrong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=13104644.f9boR54smD@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=Xing.Xu@amlogic.com \
    --cc=carlo@caione.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jerry.cao@amlogic.com \
    --cc=khilman@baylibre.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=narmstrong@baylibre.com \
    --cc=victor.wan@amlogic.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).