RE: [PATCH 06/10] dt-bindings: display: xlnx: Add ZynqMP DP subsystem bindings

2018-01-10 Thread Hyun Kwon 
Hi Rob,

Thanks for the review.

> -Original Message-
> From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf
> Of Rob Herring
> Sent: Monday, January 08, 2018 8:07 PM
> To: Hyun Kwon <hy...@xilinx.com>
> Cc: devicet...@vger.kernel.org; Michal Simek <michal.si...@xilinx.com>;
> dri-devel@lists.freedesktop.org
> Subject: Re: [PATCH 06/10] dt-bindings: display: xlnx: Add ZynqMP DP
> subsystem bindings
> 
> On Thu, Jan 04, 2018 at 06:05:55PM -0800, Hyun Kwon wrote:
> > This add a dt binding for ZynqMP DP subsystem.
> >
> > Signed-off-by: Hyun Kwon <hyun.k...@xilinx.com>
> > ---
> >  .../bindings/display/xlnx/xlnx,zynqmp-dpsub.txt| 94
> ++
> >  1 file changed, 94 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt
> >
> > diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-
> dpsub.txt b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-
> dpsub.txt
> > new file mode 100644
> > index 000..4e478ca
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-
> dpsub.txt
> > @@ -0,0 +1,94 @@
> > +Xilinx ZynqMP DisplayPort subsystem
> > +---
> > +
> > +Required properties:
> > +
> > +- compatible: Must be "xlnx,zynqmp-dpsub-1.7".
> > +
> > +- reg: Physical base address and length of the registers set for the 
> > device.
> > +- reg-names: Must be "dp", "blend", "av_buf", and "aud" to map logical
> register
> > +  partitions.
> > +
> > +- interrupts: Interrupt number.
> > +- interrupts-parent: phandle for interrupt controller.
> > +
> > +- clocks: phandles for axi, audio, non-live video, and live video clocks.
> > +  axi clock is required. Audio clock is optional. If not present, audio 
> > will
> > +  be disabled. One of non-live or live video clock should be present.
> > +- clock-names: The identification strings are required. "aclk" for axi 
> > clock.
> > +  "dp_aud_clk" for audio clock. "dp_vtc_pixel_clk_in" for non-live video
> clock.
> > +  "dp_live_video_in_clk" for live video clock (clock from programmable
> logic).
> 
> "_clk" is redundant.

This is to reflect the name of signal spelled out in the hardware spec, so I'd 
like to keep it this way unless you are still against it.

> 
> > +
> > +- phys: phandles for phy specifier.
> > +- phy-names: The identifier strings. "dp-phy" followed by index.
> > +
> > +- power-domains: phandle for the corresponding power domain
> > +
> > +- ports: crtc and encoder ports are required using DT bindings defined in
> > +  Documentation/devicetree/bindings/graph.txt.
> 
> Isn't the connection crtc->encoder?

It's bidirectional: crtc <-> encoder. Currently, as in this example, it's only 
connected between internal ports, but any of those ports can be connected with 
external ports too.

> 
> Also, crtc is pretty much a DRM term. Don't use that in bindings.
> Describe the h/w blocks.

Sure. Will fix.

> 
> > +
> > +- vid-layer, gfx-layer: Required to represent available layers
> > +
> > +Required layer properties
> > +
> > +- dmas: phandles for DMA channels as defined in
> > +  Documentation/devicetree/bindings/dma/dma.txt.
> > +- dma-names: The identifier strings are required. "graphics0" for graphics
> > +  layer. "video" followed by index for video layer
> > +
> > +Optional child node
> > +
> > +- The driver populates any child device node in this node. This can be
> used,
> > +  for example, to populate the sound device from the DisplayPort
> subsystem
> > +  driver.
> > +
> > +Example:
> > +   zynqmp_dpsub: zynqmp_dpsub@fd4a {
> 
> display-controller@...
> 
> > +   compatible = "xlnx,zynqmp-dpsub-1.7";
> > +   reg = <0x0 0xfd4a 0x0 0x1000>,
> > + <0x0 0xfd4aa000 0x0 0x1000>,
> > + <0x0 0xfd4ab000 0x0 0x1000>,
> > + <0x0 0xfd4ac000 0x0 0x1000>;
> > +   reg-names = "dp", "blend", "av_buf", "aud";
> > +   interrupts = <0 119 4>;
> > +   interrupt-parent = <>;
> > +
> > +   clock-names = "dp_apb_clk", "dp_aud_clk",
> "dp_live_video_in_clk";
> > +   clocks =

Re: [PATCH 06/10] dt-bindings: display: xlnx: Add ZynqMP DP subsystem bindings

2018-01-08 Thread Rob Herring 
On Thu, Jan 04, 2018 at 06:05:55PM -0800, Hyun Kwon wrote:
> This add a dt binding for ZynqMP DP subsystem.
> 
> Signed-off-by: Hyun Kwon 
> ---
>  .../bindings/display/xlnx/xlnx,zynqmp-dpsub.txt| 94 
> ++
>  1 file changed, 94 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt 
> b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt
> new file mode 100644
> index 000..4e478ca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt
> @@ -0,0 +1,94 @@
> +Xilinx ZynqMP DisplayPort subsystem
> +---
> +
> +Required properties:
> +
> +- compatible: Must be "xlnx,zynqmp-dpsub-1.7".
> +
> +- reg: Physical base address and length of the registers set for the device.
> +- reg-names: Must be "dp", "blend", "av_buf", and "aud" to map logical 
> register
> +  partitions.
> +
> +- interrupts: Interrupt number.
> +- interrupts-parent: phandle for interrupt controller.
> +
> +- clocks: phandles for axi, audio, non-live video, and live video clocks.
> +  axi clock is required. Audio clock is optional. If not present, audio will
> +  be disabled. One of non-live or live video clock should be present.
> +- clock-names: The identification strings are required. "aclk" for axi clock.
> +  "dp_aud_clk" for audio clock. "dp_vtc_pixel_clk_in" for non-live video 
> clock.
> +  "dp_live_video_in_clk" for live video clock (clock from programmable 
> logic).

"_clk" is redundant.

> +
> +- phys: phandles for phy specifier.
> +- phy-names: The identifier strings. "dp-phy" followed by index.
> +
> +- power-domains: phandle for the corresponding power domain
> +
> +- ports: crtc and encoder ports are required using DT bindings defined in
> +  Documentation/devicetree/bindings/graph.txt.

Isn't the connection crtc->encoder?

Also, crtc is pretty much a DRM term. Don't use that in bindings. 
Describe the h/w blocks.

> +
> +- vid-layer, gfx-layer: Required to represent available layers
> +
> +Required layer properties
> +
> +- dmas: phandles for DMA channels as defined in
> +  Documentation/devicetree/bindings/dma/dma.txt.
> +- dma-names: The identifier strings are required. "graphics0" for graphics
> +  layer. "video" followed by index for video layer
> +
> +Optional child node
> +
> +- The driver populates any child device node in this node. This can be used,
> +  for example, to populate the sound device from the DisplayPort subsystem
> +  driver.
> +
> +Example:
> + zynqmp_dpsub: zynqmp_dpsub@fd4a {

display-controller@...

> + compatible = "xlnx,zynqmp-dpsub-1.7";
> + reg = <0x0 0xfd4a 0x0 0x1000>,
> +   <0x0 0xfd4aa000 0x0 0x1000>,
> +   <0x0 0xfd4ab000 0x0 0x1000>,
> +   <0x0 0xfd4ac000 0x0 0x1000>;
> + reg-names = "dp", "blend", "av_buf", "aud";
> + interrupts = <0 119 4>;
> + interrupt-parent = <>;
> +
> + clock-names = "dp_apb_clk", "dp_aud_clk", 
> "dp_live_video_in_clk";
> + clocks = <_aclk>, < 17>, <_1>;
> +
> + phys = < PHY_TYPE_DP 0 1 2700>,
> +< PHY_TYPE_DP 1 1 2700>;
> + phy-names = "dp-phy0", "dp-phy1";
> +
> + power-domains = <_dp>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + vid-layer {
> + dma-names = "vid0", "vid1", "vid2";
> + dmas = <_dpdma 0>,
> +<_dpdma 1>,
> +<_dpdma 2>;
> + };
> +
> + gfx-layer {

These 2 nodes don't seem necessary. Just make "dmas" take 4 values and 
define where the gfx0 channel is located (index 0 or 3?).

> + dma-names = "gfx0";
> + dmas = <_dpdma 3>;
> + };
> +
> + crtc_port: port@0 {
> + reg = <0>;
> + crtc: endpoint {
> + remote-endpoint = <>;
> + };
> + };
> + port@1 {

Multiple port nodes should be under a ports node especially if you have 
other child nodes.

> + reg = <1>;
> + encoder: endpoint {
> + remote-endpoint = <>;
> + };
> + };
> + };
> +};
> +
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/10] dt-bindings: display: xlnx: Add ZynqMP DP subsystem bindings

2018-01-04 Thread Hyun Kwon 
This add a dt binding for ZynqMP DP subsystem.

Signed-off-by: Hyun Kwon 
---
 .../bindings/display/xlnx/xlnx,zynqmp-dpsub.txt| 94 ++
 1 file changed, 94 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt

diff --git 
a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt 
b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt
new file mode 100644
index 000..4e478ca
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt
@@ -0,0 +1,94 @@
+Xilinx ZynqMP DisplayPort subsystem
+---
+
+Required properties:
+
+- compatible: Must be "xlnx,zynqmp-dpsub-1.7".
+
+- reg: Physical base address and length of the registers set for the device.
+- reg-names: Must be "dp", "blend", "av_buf", and "aud" to map logical register
+  partitions.
+
+- interrupts: Interrupt number.
+- interrupts-parent: phandle for interrupt controller.
+
+- clocks: phandles for axi, audio, non-live video, and live video clocks.
+  axi clock is required. Audio clock is optional. If not present, audio will
+  be disabled. One of non-live or live video clock should be present.
+- clock-names: The identification strings are required. "aclk" for axi clock.
+  "dp_aud_clk" for audio clock. "dp_vtc_pixel_clk_in" for non-live video clock.
+  "dp_live_video_in_clk" for live video clock (clock from programmable logic).
+
+- phys: phandles for phy specifier.
+- phy-names: The identifier strings. "dp-phy" followed by index.
+
+- power-domains: phandle for the corresponding power domain
+
+- ports: crtc and encoder ports are required using DT bindings defined in
+  Documentation/devicetree/bindings/graph.txt.
+
+- vid-layer, gfx-layer: Required to represent available layers
+
+Required layer properties
+
+- dmas: phandles for DMA channels as defined in
+  Documentation/devicetree/bindings/dma/dma.txt.
+- dma-names: The identifier strings are required. "graphics0" for graphics
+  layer. "video" followed by index for video layer
+
+Optional child node
+
+- The driver populates any child device node in this node. This can be used,
+  for example, to populate the sound device from the DisplayPort subsystem
+  driver.
+
+Example:
+   zynqmp_dpsub: zynqmp_dpsub@fd4a {
+   compatible = "xlnx,zynqmp-dpsub-1.7";
+   reg = <0x0 0xfd4a 0x0 0x1000>,
+ <0x0 0xfd4aa000 0x0 0x1000>,
+ <0x0 0xfd4ab000 0x0 0x1000>,
+ <0x0 0xfd4ac000 0x0 0x1000>;
+   reg-names = "dp", "blend", "av_buf", "aud";
+   interrupts = <0 119 4>;
+   interrupt-parent = <>;
+
+   clock-names = "dp_apb_clk", "dp_aud_clk", 
"dp_live_video_in_clk";
+   clocks = <_aclk>, < 17>, <_1>;
+
+   phys = < PHY_TYPE_DP 0 1 2700>,
+  < PHY_TYPE_DP 1 1 2700>;
+   phy-names = "dp-phy0", "dp-phy1";
+
+   power-domains = <_dp>;
+
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   vid-layer {
+   dma-names = "vid0", "vid1", "vid2";
+   dmas = <_dpdma 0>,
+  <_dpdma 1>,
+  <_dpdma 2>;
+   };
+
+   gfx-layer {
+   dma-names = "gfx0";
+   dmas = <_dpdma 3>;
+   };
+
+   crtc_port: port@0 {
+   reg = <0>;
+   crtc: endpoint {
+   remote-endpoint = <>;
+   };
+   };
+   port@1 {
+   reg = <1>;
+   encoder: endpoint {
+   remote-endpoint = <>;
+   };
+   };
+   };
+};
+
-- 
2.7.4

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