Re: [PATCHv6 1/3] ARM:dt-bindings Intel FPGA Video and Image Processing Suite

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Hean Loong,

(CC'ing dri-devel again as I noticed it wasn't CC'ed anymore)

On Friday 18 Aug 2017 08:34:44 Ong, Hean Loong wrote:
> Hi Laurent,
> Thanks for the comments, I drafted a copy of the DT bindings based on
> your recommendations and inputs. I inserted the changes below the
> previous comments. 

[snip]

> Intel Video and Image Processing(VIP) Frame Buffer II bindings
> 
> Supported hardware: Intel FPGA SoC Arria10 and above with display port
> IP
> 
> The Video Frame Buffer II in Video Image Processing (VIP) suite is an IP
> core that interfaces between system memory and Avalon-ST video ports. The IP
> core can be configured to support the memory reader (from memory to Avalon-
> ST) and/or memory writer (from Avalon-ST to memory) interfaces.
> 
> Connections between the Frame Buffer II and other video IP cores in the
> system are modelled using the OF graph DT bindings. The Frame Buffer II node
> has up to two OF graph ports. When the memory writer interface is enabled,
> port 0 maps to the Avalon-ST Input (din) port. When the memory reader
> interface is enabled, port 1 maps to the Avalon-ST Output (dout) port.
> 
> More information the FPGA video IP component can be acquired from
> https://www.altera.com/content/dam/altera-www/global/en_US/pdfs\
> /literature/ug/ug_vip.pdf
> 
> New bindings:
> =============

How are the bindings "new" ? You can omit that title.

> Required properties:
> ----------------------------
> - compatible: "altr,vip-frame-buffer-2.0"
> - reg: Physical base address and length of the framebuffer controller's
> 	registers.
> - altr,max-width: The maximum width of the framebuffer in pixels.
> - altr,max-height: The maximum height of the framebuffer in pixels.
> - altr,mem-port-width = the bus width of the avalon master port 
> 	on the frame reader

You need to mention the ports here as they are mandatory. I would move the 
second paragraph from the introduction to here. You should also refer to the 
file defining the OF graph DT bindings. You can find examples in other DT 
bindings.

> Example:
> ----------------------------
>  +-----+      +-----------+      +------------+      +-----------+
>  |  D  |      | Frame     |      | DP/HDMI TX |      | DP/HDMI   |
>  |  D  |----->| Buffer II |----->| Controller |----->| Connector |
>  |  R  |      |           |      |            |      |           |
>  +-----+      +-----------+      +------------+      +-----------+
> 
> framebuffer@100000280 {
>         compatible = "altr,vip-frame-buffer-2.0";
>         reg = <0x00000001 0x00000280 0x00000040>;
>         altr,max-width = <1280>;
>         altr,max-height = <720>;
>         altr,mem-port-width = <128>;
> 
>         ports {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
> 
>                 port@1 {
>                         reg = <1>;
>                         fb_output: endpoint {
>                                 remote-endpoint = <&dp_encoder_input>;
>                         };
>                 };
>         };
> };
> 
> If there is a need to scale the Frame Buffer II IP cores in
> the pipeline, each node would have its own node, connected
> through ports and endpoints. 
> 
> hdmi-encoder@......... {
>         compatible = "altr,hdmi-tx-16.0";

This was just an example, please use the real compatible string of the HDMI 
controller (and please submit DT bindings for the HDMI controller :-)). Please 
also fill the reg property with values from a real example.

>         reg = <.....>;
>         /* Other IP-specific properties here */
> 
>         ports {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
> 
>                 port@0 {
>                         reg = <0>;
>                         hdmi_tx_input: endpoint {
>                                 remote-endpoint = <&fb_output>;
>                         };
>                 };
> 
>                 port@1 {
>                         reg = <1>;
>                         hdmi_tx_output: endpoint {
>                                 remote-endpoint = <&hdmi_conn_input>;
>                         };
>                 };
>         };
> };
> 
> hdmi-connector@0 {
>         compatible = "hdmi-connector";
>         type = "a";
> 
>         port {
>                 hdmi_conn_input: endpoint {
>                         remote-endpoint = <&hdmi_tx_output>;
>                 };
>         };
> };

-- 
Regards,

Laurent Pinchart

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