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

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

Hi Hean Loong,

On Monday, 21 August 2017 04:40:09 EEST Ong, Hean Loong wrote:
> On Fri, 2017-08-18 at 16:11 +0300, Laurent Pinchart wrote:
> > 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
> > > portIP
> > > 
> > > 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.
> 
> Noted.
> 
> > > 
> > > 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.
> 
> Noted
> 
> > > 
> > > 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.
> 
> I was trying to get overall picture to be correct before I proceed
> further. We currently do not have support for HDMI therefore I would
> omit this until a HDMI IP is available. The only values are available
> for the Display Port.

No problem. In that case the best option is to replace the HDMI encoder with a 
DisplayPort encoder in the 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