Hi Rob,
Thanks for the review.
-----Original Message----- From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of Rob Herring Sent: Monday, January 08, 2018 8:07 PM To: Hyun Kwon hyunk@xilinx.com Cc: devicetree@vger.kernel.org; Michal Simek michal.simek@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.kwon@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 0000000..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@fd4a0000 {
display-controller@...
compatible = "xlnx,zynqmp-dpsub-1.7";reg = <0x0 0xfd4a0000 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 = <&gic>;clock-names = "dp_apb_clk", "dp_aud_clk","dp_live_video_in_clk";
clocks = <&dp_aclk>, <&clkc 17>, <&si570_1>;phys = <&lane1 PHY_TYPE_DP 0 1 27000000>,<&lane0 PHY_TYPE_DP 1 1 27000000>;phy-names = "dp-phy0", "dp-phy1";power-domains = <&pd_dp>;#address-cells = <1>;#size-cells = <0>;vid-layer {dma-names = "vid0", "vid1", "vid2";dmas = <&xlnx_dpdma 0>,<&xlnx_dpdma 1>,<&xlnx_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?).
That is correct, as of now. But, in the follow up patch / internal development, each layer needs to be referenced by external device node (ex, external device connected to each layer). That's why there's a child node for each layer. I can still remove these nodes for now, and add when those are needed. Please let me know, otherwise, I'll keep these nodes.
dma-names = "gfx0";dmas = <&xlnx_dpdma 3>;};crtc_port: port@0 {reg = <0>;crtc: endpoint {remote-endpoint = <&encoder>;};};port@1 {Multiple port nodes should be under a ports node especially if you have other child nodes.
Will fix it.
Thanks, -hyun
reg = <1>;encoder: endpoint {remote-endpoint = <&crtc>;};};- };
+};
-- 2.7.4
-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@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